Subject: Re: [PATCH] selftests/powerpc: Add a test of sigreturn vs VDSO

From: Nathan Lynch <[email-masked]>
Date: Fri, 06 Mar 2020 12:25:18 -0600

Michael Ellerman <[email-masked]> writes:

> +static int search_proc_maps(char *needle, unsigned long *low, unsigned long *high)

                               ^^ const?
                               
> +{
> +	unsigned long start, end;
> +	static char buf[4096];
> +	char name[128];
> +	FILE *f;
> +	int rc = -1;
> +
> +	f = fopen("/proc/self/maps", "r");
> +	if (!f) {
> +		perror("fopen");
> +		return -1;
> +	}
> +
> +	while (fgets(buf, sizeof(buf), f)) {
> +		rc = sscanf(buf, "%lx-%lx %*c%*c%*c%*c %*x %*d:%*d %*d %127s\n",
> +			    &start, &end, name);

I suspect it doesn't matter in practice for this particular test, but
since this looks like a generally useful function that could gain users
in the future: does this spuriously fail if the matching line straddles
a 4096-byte boundary? Maybe fscanf(3) should be used instead?

> +		if (rc == 2)
> +			continue;
> +
> +		if (rc != 3) {
> +			printf("sscanf errored\n");
> +			rc = -1;
> +			break;
> +		}
> +
> +		if (strstr(name, needle)) {
> +			*low = start;
> +			*high = end - 1;
> +			rc = 0;
> +			break;
> +		}
> +	}
> +
> +	fclose(f);
> +
> +	return rc;
> +}
> +
> +static volatile sig_atomic_t took_signal = 0;
> +
> +static void sigusr1_handler(int sig)
> +{
> +	took_signal++;
> +}
> +
> +int test_sigreturn_vdso(void)
> +{
> +	unsigned long low, high, size;
> +	struct sigaction act;
> +	char *p;
> +
> +	act.sa_handler = sigusr1_handler;
> +	act.sa_flags = 0;
> +	sigemptyset(&act.sa_mask);
> +
> +	assert(sigaction(SIGUSR1, &act, NULL) == 0);
> +
> +	// Confirm the VDSO is mapped, and work out where it is
> +	assert(search_proc_maps("[vdso]", &low, &high) == 0);
> +	size = high - low + 1;
> +	printf("VDSO is at 0x%lx-0x%lx (%lu bytes)\n", low, high, size);
> +
> +	kill(getpid(), SIGUSR1);
> +	assert(took_signal == 1);
> +	printf("Signal delivered OK with VDSO mapped\n");

I haven't looked at the test harness in detail but this should be
reliable if the program is a single thread - lgtm.