Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 16 Aug 2022 07:44:48 -0400
From:   Brian Foster <bfoster@redhat.com>
To:     Alexey Dobriyan <adobriyan@gmail.com>
Cc:     akpm@linux-foundation.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] proc: save LOC in vsyscall test
Message-ID: <YvuDMGHcNtjhhMuq@bfoster>
References: <YvoWzAn5dlhF75xa@localhost.localdomain>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YvoWzAn5dlhF75xa@localhost.localdomain>
Precedence: bulk

On Mon, Aug 15, 2022 at 12:50:04PM +0300, Alexey Dobriyan wrote:
> From: Brian Foster <bfoster@redhat.com>
> 
> Do one fork in vsyscall detection code and let SIGSEGV handler exit and
> carry information to the parent saving LOC.
> 
> 	[
> 		redo original patch,
> 		delete unnecessary variables,
> 		minimise code changes.
> 					--adobriyan
> 	]
> 
> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> ---

LGTM and passes my quick tests, thanks!

FWIW:

Reviewed-by: Brian Foster <bfoster@redhat.com>

> 
>  tools/testing/selftests/proc/proc-pid-vm.c |   56 ++++++++---------------------
>  1 file changed, 16 insertions(+), 40 deletions(-)
> 
> --- a/tools/testing/selftests/proc/proc-pid-vm.c
> +++ b/tools/testing/selftests/proc/proc-pid-vm.c
> @@ -213,22 +213,22 @@ static int make_exe(const uint8_t *payload, size_t len)
>  
>  /*
>   * 0: vsyscall VMA doesn't exist	vsyscall=none
> - * 1: vsyscall VMA is r-xp		vsyscall=emulate
> - * 2: vsyscall VMA is --xp		vsyscall=xonly
> + * 1: vsyscall VMA is --xp		vsyscall=xonly
> + * 2: vsyscall VMA is r-xp		vsyscall=emulate
>   */
> -static int g_vsyscall;
> +static volatile int g_vsyscall;
>  static const char *str_vsyscall;
>  
>  static const char str_vsyscall_0[] = "";
>  static const char str_vsyscall_1[] =
> -"ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0                  [vsyscall]\n";
> -static const char str_vsyscall_2[] =
>  "ffffffffff600000-ffffffffff601000 --xp 00000000 00:00 0                  [vsyscall]\n";
> +static const char str_vsyscall_2[] =
> +"ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0                  [vsyscall]\n";
>  
>  #ifdef __x86_64__
>  static void sigaction_SIGSEGV(int _, siginfo_t *__, void *___)
>  {
> -	_exit(1);
> +	_exit(g_vsyscall);
>  }
>  
>  /*
> @@ -255,6 +255,7 @@ static void vsyscall(void)
>  		act.sa_sigaction = sigaction_SIGSEGV;
>  		(void)sigaction(SIGSEGV, &act, NULL);
>  
> +		g_vsyscall = 0;
>  		/* gettimeofday(NULL, NULL); */
>  		asm volatile (
>  			"call %P0"
> @@ -262,45 +263,20 @@ static void vsyscall(void)
>  			: "i" (0xffffffffff600000), "D" (NULL), "S" (NULL)
>  			: "rax", "rcx", "r11"
>  		);
> -		exit(0);
> -	}
> -	waitpid(pid, &wstatus, 0);
> -	if (WIFEXITED(wstatus) && WEXITSTATUS(wstatus) == 0) {
> -		/* vsyscall page exists and is executable. */
> -	} else {
> -		/* vsyscall page doesn't exist. */
> -		g_vsyscall = 0;
> -		return;
> -	}
> -
> -	pid = fork();
> -	if (pid < 0) {
> -		fprintf(stderr, "fork, errno %d\n", errno);
> -		exit(1);
> -	}
> -	if (pid == 0) {
> -		struct rlimit rlim = {0, 0};
> -		(void)setrlimit(RLIMIT_CORE, &rlim);
> -
> -		/* Hide "segfault at ffffffffff600000" messages. */
> -		struct sigaction act;
> -		memset(&act, 0, sizeof(struct sigaction));
> -		act.sa_flags = SA_SIGINFO;
> -		act.sa_sigaction = sigaction_SIGSEGV;
> -		(void)sigaction(SIGSEGV, &act, NULL);
>  
> +		g_vsyscall = 1;
>  		*(volatile int *)0xffffffffff600000UL;
> -		exit(0);
> +
> +		g_vsyscall = 2;
> +		exit(g_vsyscall);
>  	}
>  	waitpid(pid, &wstatus, 0);
> -	if (WIFEXITED(wstatus) && WEXITSTATUS(wstatus) == 0) {
> -		/* vsyscall page is readable and executable. */
> -		g_vsyscall = 1;
> -		return;
> +	if (WIFEXITED(wstatus)) {
> +		g_vsyscall = WEXITSTATUS(wstatus);
> +	} else {
> +		fprintf(stderr, "error: wstatus %08x\n", wstatus);
> +		exit(1);
>  	}
> -
> -	/* vsyscall page is executable but unreadable. */
> -	g_vsyscall = 2;
>  }
>  
>  int main(void)
>