2022-08-15 10:35:26

by Alexey Dobriyan

[permalink] [raw]
Subject: [PATCH] proc: save LOC in vsyscall test

From: Brian Foster <[email protected]>

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 <[email protected]>
---

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)


2022-08-16 12:57:54

by Brian Foster

[permalink] [raw]
Subject: Re: [PATCH] proc: save LOC in vsyscall test

On Mon, Aug 15, 2022 at 12:50:04PM +0300, Alexey Dobriyan wrote:
> From: Brian Foster <[email protected]>
>
> 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 <[email protected]>
> ---

LGTM and passes my quick tests, thanks!

FWIW:

Reviewed-by: Brian Foster <[email protected]>

>
> 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)
>

2022-08-17 18:37:29

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] proc: save LOC in vsyscall test

On Tue, 16 Aug 2022 07:44:48 -0400 Brian Foster <[email protected]> wrote:

> On Mon, Aug 15, 2022 at 12:50:04PM +0300, Alexey Dobriyan wrote:
> > From: Brian Foster <[email protected]>
> >
> > 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 <[email protected]>
> > ---
>
> LGTM and passes my quick tests, thanks!
>
> FWIW:
>
> Reviewed-by: Brian Foster <[email protected]>

It would be appropriate for us to include your Signed-off-by. Please
send one along?

2022-08-17 18:45:16

by Brian Foster

[permalink] [raw]
Subject: Re: [PATCH] proc: save LOC in vsyscall test

On Wed, Aug 17, 2022 at 11:28:02AM -0700, Andrew Morton wrote:
> On Tue, 16 Aug 2022 07:44:48 -0400 Brian Foster <[email protected]> wrote:
>
> > On Mon, Aug 15, 2022 at 12:50:04PM +0300, Alexey Dobriyan wrote:
> > > From: Brian Foster <[email protected]>
> > >
> > > 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 <[email protected]>
> > > ---
> >
> > LGTM and passes my quick tests, thanks!
> >
> > FWIW:
> >
> > Reviewed-by: Brian Foster <[email protected]>
>
> It would be appropriate for us to include your Signed-off-by. Please
> send one along?
>

Sure, feel free to use whichever tags make the most sense:

Signed-off-by: Brian Foster <[email protected]>