* [PATCH] Add UNSUPPORTED check in elf/tst-pldd. @ 2019-08-27 10:19 Stefan Liebler 2019-08-27 15:06 ` Adhemerval Zanella 0 siblings, 1 reply; 22+ messages in thread From: Stefan Liebler @ 2019-08-27 10:19 UTC (permalink / raw) To: GNU C Library [-- Attachment #1: Type: text/plain, Size: 402 bytes --] Hi, the testcase forks a child process and runs pldd with PID of this child. On systems where /proc/sys/kernel/yama/ptrace_scope differs from zero, pldd will fail with /usr/bin/pldd: cannot attach to process 3: Operation not permitted This patch checks if ptrace_scope is zero and otherwise marks the test as UNSUPPORTED. Bye Stefan ChangeLog: * elf/tst-pldd.c (do_test): Add UNSUPPORTED check. [-- Attachment #2: 20190826_tst-pldd.patch --] [-- Type: text/x-patch, Size: 1581 bytes --] commit 9c0b03c38bdd31618909da46b8bd4e09b5a236d2 Author: Stefan Liebler <stli@linux.ibm.com> Date: Mon Aug 26 15:45:07 2019 +0200 Add UNSUPPORTED check in elf/tst-pldd. The testcase forks a child process and runs pldd with PID of this child. On systems where /proc/sys/kernel/yama/ptrace_scope differs from zero, pldd will fail with /usr/bin/pldd: cannot attach to process 3: Operation not permitted This patch checks if ptrace_scope is zero and otherwise marks the test as UNSUPPORTED. ChangeLog: * elf/tst-pldd.c (do_test): Add UNSUPPORTED check. diff --git a/elf/tst-pldd.c b/elf/tst-pldd.c index 6b7c94a1c0..3f211dc342 100644 --- a/elf/tst-pldd.c +++ b/elf/tst-pldd.c @@ -52,6 +52,24 @@ in_str_list (const char *libname, const char *const strlist[]) static int do_test (void) { + /* Check if all processes can be debugged with ptrace. */ + { + FILE *f = fopen ("/proc/sys/kernel/yama/ptrace_scope", "r"); + if (f != NULL) + { + /* If ptrace_scope exists, then it has to be 0 which means + "classic ptrace permissions". A process can PTRACE_ATTACH + to any other process running under the same uid, as long as + it is dumpable. Otherwise pldd will fail to attach to the + subprocess. */ + int i = 99; + fscanf (f, "%d", &i); + fclose (f); + if (i != 0) + FAIL_UNSUPPORTED ("/proc/sys/kernel/yama/ptrace_scope != 0"); + } + } + /* Create a copy of current test to check with pldd. */ struct support_subprocess target = support_subprocess (target_process, NULL); ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add UNSUPPORTED check in elf/tst-pldd. 2019-08-27 10:19 [PATCH] Add UNSUPPORTED check in elf/tst-pldd Stefan Liebler @ 2019-08-27 15:06 ` Adhemerval Zanella 2019-08-27 15:14 ` Florian Weimer 0 siblings, 1 reply; 22+ messages in thread From: Adhemerval Zanella @ 2019-08-27 15:06 UTC (permalink / raw) To: libc-alpha On 27/08/2019 07:19, Stefan Liebler wrote: > Hi, > > the testcase forks a child process and runs pldd with PID of > this child. On systems where /proc/sys/kernel/yama/ptrace_scope > differs from zero, pldd will fail with > /usr/bin/pldd: cannot attach to process 3: Operation not permitted > > This patch checks if ptrace_scope is zero and otherwise marks the > test as UNSUPPORTED. > > Bye > Stefan > > ChangeLog: > >     * elf/tst-pldd.c (do_test): Add UNSUPPORTED check. > > 20190826_tst-pldd.patch > > commit 9c0b03c38bdd31618909da46b8bd4e09b5a236d2 > Author: Stefan Liebler <stli@linux.ibm.com> > Date: Mon Aug 26 15:45:07 2019 +0200 > > Add UNSUPPORTED check in elf/tst-pldd. > > The testcase forks a child process and runs pldd with PID of > this child. On systems where /proc/sys/kernel/yama/ptrace_scope > differs from zero, pldd will fail with > /usr/bin/pldd: cannot attach to process 3: Operation not permitted > > This patch checks if ptrace_scope is zero and otherwise marks the > test as UNSUPPORTED. > > ChangeLog: > > * elf/tst-pldd.c (do_test): Add UNSUPPORTED check. > > diff --git a/elf/tst-pldd.c b/elf/tst-pldd.c > index 6b7c94a1c0..3f211dc342 100644 > --- a/elf/tst-pldd.c > +++ b/elf/tst-pldd.c > @@ -52,6 +52,24 @@ in_str_list (const char *libname, const char *const strlist[]) > static int > do_test (void) > { > + /* Check if all processes can be debugged with ptrace. */ > + { > + FILE *f = fopen ("/proc/sys/kernel/yama/ptrace_scope", "r"); > + if (f != NULL) > + { > + /* If ptrace_scope exists, then it has to be 0 which means > + "classic ptrace permissions". A process can PTRACE_ATTACH > + to any other process running under the same uid, as long as > + it is dumpable. Otherwise pldd will fail to attach to the > + subprocess. */ > + int i = 99; > + fscanf (f, "%d", &i); > + fclose (f); > + if (i != 0) > + FAIL_UNSUPPORTED ("/proc/sys/kernel/yama/ptrace_scope != 0"); > + } > + } > + This is a Linuxism and I think we should create a 'support_can_ptrace' similar to 'support_can_chroot'. The logic to detect it seems correct, I would just check fscanf returned to value and use xfclose. It would be something like bool support_can_ptrace (void) { bool ret = true; #ifdef __linux__ /* YAMA may be not enabled. If it is then ptrace_scope it has to be 0 which means "classic ptrace permissions". A process can PTRACE_ATTACH to any other process running under the same uid, as long as it is dumpable. Otherwise pldd will fail to attach to the subprocess. */ FILE *f = fopen ("/proc/sys/kernel/yama/ptrace_scope", "r"); if (f == NULL) return true; int i = 99; TEST_COMPARE (fscanf (f, "%d", &i), 1); xfclose (f); ret = i == 0; #endif return ret; } And I think we might eventually need to handle seccomp as well. > /* Create a copy of current test to check with pldd. */ > struct support_subprocess target = support_subprocess (target_process, NULL); > > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add UNSUPPORTED check in elf/tst-pldd. 2019-08-27 15:06 ` Adhemerval Zanella @ 2019-08-27 15:14 ` Florian Weimer 2019-08-27 19:11 ` Adhemerval Zanella 0 siblings, 1 reply; 22+ messages in thread From: Florian Weimer @ 2019-08-27 15:14 UTC (permalink / raw) To: Adhemerval Zanella; +Cc: libc-alpha * Adhemerval Zanella: > This is a Linuxism and I think we should create a 'support_can_ptrace' > similar to 'support_can_chroot'. The logic to detect it seems > correct, I would just check fscanf returned to value and use xfclose. > It would be something like The test does the right thing if the path does not exist, so I'm not sure if it is necessary. support_can_ptrace would be misleading because even with YAMA scope support, tracing direct subprocesses will still work. I fact, I think we might be able to get this test to work even with fairly restrictive YAMA scopes, by proper ordering of forks and using execve to run tst-pldd. Thanks, Florian ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add UNSUPPORTED check in elf/tst-pldd. 2019-08-27 15:14 ` Florian Weimer @ 2019-08-27 19:11 ` Adhemerval Zanella 2019-08-28 9:06 ` Stefan Liebler 0 siblings, 1 reply; 22+ messages in thread From: Adhemerval Zanella @ 2019-08-27 19:11 UTC (permalink / raw) To: Florian Weimer; +Cc: libc-alpha On 27/08/2019 12:14, Florian Weimer wrote: > * Adhemerval Zanella: > >> This is a Linuxism and I think we should create a 'support_can_ptrace' >> similar to 'support_can_chroot'. The logic to detect it seems >> correct, I would just check fscanf returned to value and use xfclose. >> It would be something like > > The test does the right thing if the path does not exist, so I'm not > sure if it is necessary. Even though, a check for a Linux specific path should be restricted to Linux builds only. > > support_can_ptrace would be misleading because even with YAMA scope > support, tracing direct subprocesses will still work. Indeed, a better prototype would be indeed: /* Return the current YAMA mode set on the machine (0 to 3) or -1 if YAMA is not supported. */ int support_ptrace_scope (void); > > I fact, I think we might be able to get this test to work even with > fairly restrictive YAMA scopes, by proper ordering of forks and using > execve to run tst-pldd. The problem with ptrace_scope 1 is tst-pldd will either make its fork helper process a pldd descendant or pass pldd pid to the forked process so it can call prctl (PR_SET_PTRACER, ...). In either case I am not really sure it is possible without change pldd process itself (since it does not have an interactive mode), nor if the complexity to support this specific scenarios pays off. The ptrace_scope 2 is even more tricky since it requires CAP_SYS_PTRACE. In any case I think by restricting the test to run on ptrace_scope 0 is a fair assumption. > > Thanks, > Florian > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add UNSUPPORTED check in elf/tst-pldd. 2019-08-27 19:11 ` Adhemerval Zanella @ 2019-08-28 9:06 ` Stefan Liebler 2019-08-28 9:24 ` Florian Weimer 2019-08-28 12:19 ` Adhemerval Zanella 0 siblings, 2 replies; 22+ messages in thread From: Stefan Liebler @ 2019-08-28 9:06 UTC (permalink / raw) To: libc-alpha [-- Attachment #1: Type: text/plain, Size: 2100 bytes --] On 8/27/19 9:11 PM, Adhemerval Zanella wrote: > > > On 27/08/2019 12:14, Florian Weimer wrote: >> * Adhemerval Zanella: >> >>> This is a Linuxism and I think we should create a 'support_can_ptrace' >>> similar to 'support_can_chroot'. The logic to detect it seems >>> correct, I would just check fscanf returned to value and use xfclose. >>> It would be something like >> >> The test does the right thing if the path does not exist, so I'm not >> sure if it is necessary. > > Even though, a check for a Linux specific path should be restricted to > Linux builds only. > >> >> support_can_ptrace would be misleading because even with YAMA scope >> support, tracing direct subprocesses will still work. > > Indeed, a better prototype would be indeed: > > /* Return the current YAMA mode set on the machine (0 to 3) or -1 > if YAMA is not supported. */ > int support_ptrace_scope (void); > >> >> I fact, I think we might be able to get this test to work even with >> fairly restrictive YAMA scopes, by proper ordering of forks and using >> execve to run tst-pldd. > > The problem with ptrace_scope 1 is tst-pldd will either make its fork > helper process a pldd descendant or pass pldd pid to the forked process > so it can call prctl (PR_SET_PTRACER, ...). In either case I am not > really sure it is possible without change pldd process itself (since > it does not have an interactive mode), nor if the complexity to support > this specific scenarios pays off. > > The ptrace_scope 2 is even more tricky since it requires CAP_SYS_PTRACE. > > In any case I think by restricting the test to run on ptrace_scope 0 > is a fair assumption. > >> >> Thanks, >> Florian >> Please have a look at the adjusted the patch. I've introduced new support functions. And if ptrace_scope is 1 "restricted ptrace", I've just call prctl (PR_SET_PTRACER, PR_SET_PTRACER_ANY,...) on the target process. This way the target process does not need to know the pid of pldd and pldd is allowed to attach to the target process. If ptrace_scope is 2 or 3, the test is marked as UNSUPPORTED. Thanks. Stefan [-- Attachment #2: 20190828_tst-pldd.patch --] [-- Type: text/x-patch, Size: 5872 bytes --] commit 7eba88e4f44df6f4a6d174e566b1796f2abc490c Author: Stefan Liebler <stli@linux.ibm.com> Date: Mon Aug 26 15:45:07 2019 +0200 Add UNSUPPORTED check in elf/tst-pldd. The testcase forks a child process and runs pldd with PID of this child. On systems where /proc/sys/kernel/yama/ptrace_scope differs from zero, pldd will fail with /usr/bin/pldd: cannot attach to process 3: Operation not permitted This patch checks if ptrace_scope exists, is zero "classic ptrace permissions" or one "restricted ptrace". In case of "restricted ptrace", we can effectively disable the restriction by using prctl (PR_SET_PTRACER_ANY). If ptrace_scope exists and has a higher restriction, then the test is marked as UNSUPPORTED. ChangeLog: * elf/tst-pldd.c (do_test): Add UNSUPPORTED check. (target_process): Disable restricted ptrace. * support/Makefile (libsupport-routines): Add support_ptrace. * support/ptrace.h: New file. * support/support_ptrace.c: Likewise. diff --git a/elf/tst-pldd.c b/elf/tst-pldd.c index 6b7c94a1c0..728272d177 100644 --- a/elf/tst-pldd.c +++ b/elf/tst-pldd.c @@ -30,10 +30,20 @@ #include <support/capture_subprocess.h> #include <support/check.h> #include <support/support.h> +#include <support/ptrace.h> + +static int ptrace_scope; static void target_process (void *arg) { + if (ptrace_scope == 1) + { + /* YAMA is configured to "restricted ptrace". + Disable the restriction for this subprocess. */ + support_ptrace_process_set_ptracer_any (); + } + pause (); } @@ -52,6 +62,11 @@ in_str_list (const char *libname, const char *const strlist[]) static int do_test (void) { + /* Check if our subprocess can be debugged with ptrace. */ + ptrace_scope = support_ptrace_scope (); + if (ptrace_scope >= 2) + FAIL_UNSUPPORTED ("/proc/sys/kernel/yama/ptrace_scope >= 2"); + /* Create a copy of current test to check with pldd. */ struct support_subprocess target = support_subprocess (target_process, NULL); diff --git a/support/Makefile b/support/Makefile index ab66913a02..34d14eb1bb 100644 --- a/support/Makefile +++ b/support/Makefile @@ -56,6 +56,7 @@ libsupport-routines = \ support_format_hostent \ support_format_netent \ support_isolate_in_subprocess \ + support_ptrace \ support_openpty \ support_paths \ support_quote_blob \ diff --git a/support/ptrace.h b/support/ptrace.h new file mode 100644 index 0000000000..82f79ff25c --- /dev/null +++ b/support/ptrace.h @@ -0,0 +1,36 @@ +/* Support functions handling ptrace_scope. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#ifndef SUPPORT_PTRACE_H +#define SUPPORT_PTRACE_H + +#include <sys/cdefs.h> + +__BEGIN_DECLS + +/* Return the current YAMA mode set on the machine (0 to 3) or -1 + if YAMA is not supported. */ +int support_ptrace_scope (void); + +/* Effectively disables YAMA restriction for the calling process + if support_ptrace_scope returns 1 "restricted ptrace". */ +void support_ptrace_process_set_ptracer_any (void); + +__END_DECLS + +#endif diff --git a/support/support_ptrace.c b/support/support_ptrace.c new file mode 100644 index 0000000000..e9410384b5 --- /dev/null +++ b/support/support_ptrace.c @@ -0,0 +1,58 @@ +/* Support functions handling ptrace_scope. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#include <support/check.h> +#include <support/xstdio.h> +#include <support/ptrace.h> +#include <sys/prctl.h> + +int +support_ptrace_scope (void) +{ + int ptrace_scope = -1; + +#ifdef __linux__ + /* YAMA may be not enabled. Otherwise it contains a value from 0 to 3: + - 0 classic ptrace permissions + - 1 restricted ptrace + - 2 admin-only attach + - 3 no attach */ + FILE *f = fopen ("/proc/sys/kernel/yama/ptrace_scope", "r"); + if (f != NULL) + { + TEST_COMPARE (fscanf (f, "%d", &ptrace_scope), 1); + xfclose (f); + } +#endif + + return ptrace_scope; +} + +void +support_ptrace_process_set_ptracer_any (void) +{ +#ifdef __linux__ + int ret = prctl (PR_SET_PTRACER, PR_SET_PTRACER_ANY, 0, 0, 0); + if (ret != 0) + FAIL_EXIT1 ("Failed to disable YAMA restriction. (prctl returned %d: %m)", + ret); +#else + FAIL_UNSUPPORTED ("prctl (PR_SET_PTRACER, PR_SET_PTRACER_ANY, ...) " + "not supported!"); +#endif +} ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add UNSUPPORTED check in elf/tst-pldd. 2019-08-28 9:06 ` Stefan Liebler @ 2019-08-28 9:24 ` Florian Weimer 2019-08-28 14:42 ` Stefan Liebler 2019-08-28 12:19 ` Adhemerval Zanella 1 sibling, 1 reply; 22+ messages in thread From: Florian Weimer @ 2019-08-28 9:24 UTC (permalink / raw) To: Stefan Liebler; +Cc: libc-alpha * Stefan Liebler: > static void > target_process (void *arg) > { > + if (ptrace_scope == 1) > + { > + /* YAMA is configured to "restricted ptrace". > + Disable the restriction for this subprocess. */ > + support_ptrace_process_set_ptracer_any (); > + } > + > pause (); > } I think this has a race condition if pldd attaches to the process before the support_ptrace_process_set_ptracer_any call. I have no idea how hard it is in practice to hit this race. It should be possible to use a process-shared barrier or some other form of synchronization to avoid this issue. Thanks, Florian ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add UNSUPPORTED check in elf/tst-pldd. 2019-08-28 9:24 ` Florian Weimer @ 2019-08-28 14:42 ` Stefan Liebler 2019-08-29 8:47 ` Florian Weimer 0 siblings, 1 reply; 22+ messages in thread From: Stefan Liebler @ 2019-08-28 14:42 UTC (permalink / raw) To: Florian Weimer; +Cc: libc-alpha, Adhemerval Zanella [-- Attachment #1: Type: text/plain, Size: 998 bytes --] On 8/28/19 11:24 AM, Florian Weimer wrote: > * Stefan Liebler: > >> static void >> target_process (void *arg) >> { >> + if (ptrace_scope == 1) >> + { >> + /* YAMA is configured to "restricted ptrace". >> + Disable the restriction for this subprocess. */ >> + support_ptrace_process_set_ptracer_any (); >> + } >> + >> pause (); >> } > > I think this has a race condition if pldd attaches to the process before > the support_ptrace_process_set_ptracer_any call. I have no idea how > hard it is in practice to hit this race. It should be possible to use a > process-shared barrier or some other form of synchronization to avoid > this issue. > > Thanks, > Florian > I've added a synchronization with stdatomic.h on a shared memory mapping. I've not used pthread* functions as I don't want to link against libpthread.so. Then further adjustments are needed. Or should I just restrict the test ptrace_scope 0 as Adhemerval has proposed in his post? Bye. Stefan [-- Attachment #2: 20190828_1600_tst-pldd.patch --] [-- Type: text/x-patch, Size: 6851 bytes --] commit e2aed097192c423dcf1882cb1c39567676ef2255 Author: Stefan Liebler <stli@linux.ibm.com> Date: Mon Aug 26 15:45:07 2019 +0200 Add UNSUPPORTED check in elf/tst-pldd. The testcase forks a child process and runs pldd with PID of this child. On systems where /proc/sys/kernel/yama/ptrace_scope differs from zero, pldd will fail with /usr/bin/pldd: cannot attach to process 3: Operation not permitted This patch checks if ptrace_scope exists, is zero "classic ptrace permissions" or one "restricted ptrace". In case of "restricted ptrace", we can effectively disable the restriction by using prctl (PR_SET_PTRACER_ANY). If ptrace_scope exists and has a higher restriction, then the test is marked as UNSUPPORTED. ChangeLog: * elf/tst-pldd.c (ptrace_scope, target_barrier): New variable. (do_test): Add UNSUPPORTED check. (target_process): Disable restricted ptrace. * support/Makefile (libsupport-routines): Add support_ptrace. * support/ptrace.h: New file. * support/support_ptrace.c: Likewise. diff --git a/elf/tst-pldd.c b/elf/tst-pldd.c index 6b7c94a1c0..130d1f51db 100644 --- a/elf/tst-pldd.c +++ b/elf/tst-pldd.c @@ -30,10 +30,27 @@ #include <support/capture_subprocess.h> #include <support/check.h> #include <support/support.h> +#include <support/ptrace.h> +#include <support/xunistd.h> +#include <sys/mman.h> +#include <stdatomic.h> + +static int ptrace_scope; +static atomic_int *target_barrier; static void target_process (void *arg) { + if (ptrace_scope == 1) + { + /* YAMA is configured to "restricted ptrace". + Disable the restriction for this subprocess. */ + support_ptrace_process_set_ptracer_any (); + } + + /* Ensure that pldd is called after ptrace restriction has been disabled. */ + *target_barrier = 1; + pause (); } @@ -52,9 +69,24 @@ in_str_list (const char *libname, const char *const strlist[]) static int do_test (void) { + /* Check if our subprocess can be debugged with ptrace. */ + ptrace_scope = support_ptrace_scope (); + if (ptrace_scope >= 2) + FAIL_UNSUPPORTED ("/proc/sys/kernel/yama/ptrace_scope >= 2"); + + target_barrier = (atomic_int *) xmmap (NULL, sizeof (int), PROT_READ | PROT_WRITE, + MAP_SHARED | MAP_ANONYMOUS, -1); + atomic_init (target_barrier, 0); + /* Create a copy of current test to check with pldd. */ struct support_subprocess target = support_subprocess (target_process, NULL); + /* Ensure that pldd is called after ptrace restriction has been disabled in + target_process. Do not use pthread synchronization functions here as the + test below expects not to be linked against libpthread. */ + while (*target_barrier == 0) + ; + /* Run 'pldd' on test subprocess. */ struct support_capture_subprocess pldd; { @@ -129,6 +161,7 @@ do_test (void) support_capture_subprocess_free (&pldd); support_process_terminate (&target); + xmunmap (target_barrier, sizeof (int)); return 0; } diff --git a/support/Makefile b/support/Makefile index ab66913a02..34d14eb1bb 100644 --- a/support/Makefile +++ b/support/Makefile @@ -56,6 +56,7 @@ libsupport-routines = \ support_format_hostent \ support_format_netent \ support_isolate_in_subprocess \ + support_ptrace \ support_openpty \ support_paths \ support_quote_blob \ diff --git a/support/ptrace.h b/support/ptrace.h new file mode 100644 index 0000000000..82f79ff25c --- /dev/null +++ b/support/ptrace.h @@ -0,0 +1,36 @@ +/* Support functions handling ptrace_scope. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#ifndef SUPPORT_PTRACE_H +#define SUPPORT_PTRACE_H + +#include <sys/cdefs.h> + +__BEGIN_DECLS + +/* Return the current YAMA mode set on the machine (0 to 3) or -1 + if YAMA is not supported. */ +int support_ptrace_scope (void); + +/* Effectively disables YAMA restriction for the calling process + if support_ptrace_scope returns 1 "restricted ptrace". */ +void support_ptrace_process_set_ptracer_any (void); + +__END_DECLS + +#endif diff --git a/support/support_ptrace.c b/support/support_ptrace.c new file mode 100644 index 0000000000..e9410384b5 --- /dev/null +++ b/support/support_ptrace.c @@ -0,0 +1,58 @@ +/* Support functions handling ptrace_scope. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#include <support/check.h> +#include <support/xstdio.h> +#include <support/ptrace.h> +#include <sys/prctl.h> + +int +support_ptrace_scope (void) +{ + int ptrace_scope = -1; + +#ifdef __linux__ + /* YAMA may be not enabled. Otherwise it contains a value from 0 to 3: + - 0 classic ptrace permissions + - 1 restricted ptrace + - 2 admin-only attach + - 3 no attach */ + FILE *f = fopen ("/proc/sys/kernel/yama/ptrace_scope", "r"); + if (f != NULL) + { + TEST_COMPARE (fscanf (f, "%d", &ptrace_scope), 1); + xfclose (f); + } +#endif + + return ptrace_scope; +} + +void +support_ptrace_process_set_ptracer_any (void) +{ +#ifdef __linux__ + int ret = prctl (PR_SET_PTRACER, PR_SET_PTRACER_ANY, 0, 0, 0); + if (ret != 0) + FAIL_EXIT1 ("Failed to disable YAMA restriction. (prctl returned %d: %m)", + ret); +#else + FAIL_UNSUPPORTED ("prctl (PR_SET_PTRACER, PR_SET_PTRACER_ANY, ...) " + "not supported!"); +#endif +} ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add UNSUPPORTED check in elf/tst-pldd. 2019-08-28 14:42 ` Stefan Liebler @ 2019-08-29 8:47 ` Florian Weimer 2019-09-02 15:28 ` Stefan Liebler 2019-09-02 19:37 ` Adhemerval Zanella 0 siblings, 2 replies; 22+ messages in thread From: Florian Weimer @ 2019-08-29 8:47 UTC (permalink / raw) To: Stefan Liebler; +Cc: libc-alpha, Adhemerval Zanella * Stefan Liebler: > On 8/28/19 11:24 AM, Florian Weimer wrote: >> * Stefan Liebler: >> >>> static void >>> target_process (void *arg) >>> { >>> + if (ptrace_scope == 1) >>> + { >>> + /* YAMA is configured to "restricted ptrace". >>> + Disable the restriction for this subprocess. */ >>> + support_ptrace_process_set_ptracer_any (); >>> + } >>> + >>> pause (); >>> } >> >> I think this has a race condition if pldd attaches to the process before >> the support_ptrace_process_set_ptracer_any call. I have no idea how >> hard it is in practice to hit this race. It should be possible to use a >> process-shared barrier or some other form of synchronization to avoid >> this issue. >> >> Thanks, >> Florian >> > > I've added a synchronization with stdatomic.h on a shared memory mapping. > I've not used pthread* functions as I don't want to link against > libpthread.so. Then further adjustments are needed. > > Or should I just restrict the test ptrace_scope 0 as Adhemerval has > proposed in his post? Is it possible to create a process tree like this? parent (performs output checks) subprocess 1 (becomes pldd via execve) subprocess 2 If you execve pldd from subprocess 1, wouldn't subprocess 2 in its ptrace scope for ptrace_scope < 2? Thanks, Florian ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add UNSUPPORTED check in elf/tst-pldd. 2019-08-29 8:47 ` Florian Weimer @ 2019-09-02 15:28 ` Stefan Liebler 2019-09-17 13:31 ` Adhemerval Zanella 2019-09-02 19:37 ` Adhemerval Zanella 1 sibling, 1 reply; 22+ messages in thread From: Stefan Liebler @ 2019-09-02 15:28 UTC (permalink / raw) To: Florian Weimer; +Cc: libc-alpha, Adhemerval Zanella [-- Attachment #1: Type: text/plain, Size: 1838 bytes --] On 8/29/19 10:47 AM, Florian Weimer wrote: > * Stefan Liebler: > >> On 8/28/19 11:24 AM, Florian Weimer wrote: >>> * Stefan Liebler: >>> >>>> static void >>>> target_process (void *arg) >>>> { >>>> + if (ptrace_scope == 1) >>>> + { >>>> + /* YAMA is configured to "restricted ptrace". >>>> + Disable the restriction for this subprocess. */ >>>> + support_ptrace_process_set_ptracer_any (); >>>> + } >>>> + >>>> pause (); >>>> } >>> >>> I think this has a race condition if pldd attaches to the process before >>> the support_ptrace_process_set_ptracer_any call. I have no idea how >>> hard it is in practice to hit this race. It should be possible to use a >>> process-shared barrier or some other form of synchronization to avoid >>> this issue. >>> >>> Thanks, >>> Florian >>> >> >> I've added a synchronization with stdatomic.h on a shared memory mapping. >> I've not used pthread* functions as I don't want to link against >> libpthread.so. Then further adjustments are needed. >> >> Or should I just restrict the test ptrace_scope 0 as Adhemerval has >> proposed in his post? > > Is it possible to create a process tree like this? > > > parent (performs output checks) > subprocess 1 (becomes pldd via execve) > subprocess 2 > > If you execve pldd from subprocess 1, wouldn't subprocess 2 in its > ptrace scope for ptrace_scope < 2? Yes, this is possible. I've rearranged the subprocesses. See attached patch. Now we have a new function pldd_process which forks target_process, stores the pid of target_prcess to a shared memory mapping as do_test needs to know this pid. Afterwards it execve to pldd which successfully ptrace target_process in case of "restricted ptrace". Please review the usage of support-subprocess-functions. Bye, Stefan > > Thanks, > Florian > [-- Attachment #2: 20190902_tst-pldd.patch --] [-- Type: text/x-patch, Size: 8660 bytes --] commit ad51263d51d12ce6ca2ce9304efe5ba05b3912b1 Author: Stefan Liebler <stli@linux.ibm.com> Date: Mon Aug 26 15:45:07 2019 +0200 Add UNSUPPORTED check in elf/tst-pldd. The testcase forks a child process and runs pldd with PID of this child. On systems where /proc/sys/kernel/yama/ptrace_scope differs from zero, pldd will fail with /usr/bin/pldd: cannot attach to process 3: Operation not permitted This patch checks if ptrace_scope exists, is zero "classic ptrace permissions" or one "restricted ptrace". If ptrace_scope exists and has a higher restriction, then the test is marked as UNSUPPORTED. The case "restricted ptrace" is handled by rearranging the processes involved during the test. Now we have the following process tree: -parent: do_test (performs output checks) --subprocess 1: pldd_process (becomes pldd via execve) ---subprocess 2: target_process (ptraced via pldd) ChangeLog: * elf/tst-pldd.c (do_test): Add UNSUPPORTED check. Rearrange subprocesses. (pldd_process): New function. * support/Makefile (libsupport-routines): Add support_ptrace. * support/ptrace.h: New file. * support/support_ptrace.c: Likewise. diff --git a/elf/tst-pldd.c b/elf/tst-pldd.c index 6b7c94a1c0..eb1b9cb5f5 100644 --- a/elf/tst-pldd.c +++ b/elf/tst-pldd.c @@ -30,6 +30,12 @@ #include <support/capture_subprocess.h> #include <support/check.h> #include <support/support.h> +#include <support/ptrace.h> +#include <support/xunistd.h> +#include <sys/mman.h> +#include <errno.h> +#include <signal.h> + static void target_process (void *arg) @@ -37,6 +43,34 @@ target_process (void *arg) pause (); } +static void +pldd_process (void *arg) +{ + pid_t *target_pid_ptr = (pid_t *) arg; + + /* Create a copy of current test to check with pldd. As the + target_process is a child of this pldd_process, pldd is also able + to attach to target_process if YAMA is configured to 1 = + "restricted ptrace". */ + struct support_subprocess target = support_subprocess (target_process, NULL); + + /* Store the pid of target-process as do_test needs it in order to + e.g. terminate it at end of the test. */ + *target_pid_ptr = target.pid; + + /* Three digits per byte plus null terminator. */ + char pid[3 * sizeof (uint32_t) + 1]; + snprintf (pid, array_length (pid), "%d", target.pid); + + char *prog = xasprintf ("%s/pldd", support_bindir_prefix); + + /* Run pldd and use the pid of target_process as argument. */ + execve (prog, (char *const []) { (char *) prog, pid, NULL }, + (char *const []) { NULL }); + + FAIL_EXIT1 ("Returned from execve: errno=%d=%m\n", errno); +} + /* The test runs in a container because pldd does not support tracing a binary started by the loader iself (as with testrun.sh). */ @@ -52,25 +86,22 @@ in_str_list (const char *libname, const char *const strlist[]) static int do_test (void) { - /* Create a copy of current test to check with pldd. */ - struct support_subprocess target = support_subprocess (target_process, NULL); - - /* Run 'pldd' on test subprocess. */ - struct support_capture_subprocess pldd; + /* Check if our subprocess can be debugged with ptrace. */ { - /* Three digits per byte plus null terminator. */ - char pid[3 * sizeof (uint32_t) + 1]; - snprintf (pid, array_length (pid), "%d", target.pid); - - char *prog = xasprintf ("%s/pldd", support_bindir_prefix); - - pldd = support_capture_subprogram (prog, - (char *const []) { (char *) prog, pid, NULL }); + int ptrace_scope = support_ptrace_scope (); + if (ptrace_scope >= 2) + FAIL_UNSUPPORTED ("/proc/sys/kernel/yama/ptrace_scope >= 2"); + } - support_capture_subprocess_check (&pldd, "pldd", 0, sc_allow_stdout); + pid_t *target_pid_ptr = (pid_t *) xmmap (NULL, sizeof (pid_t), + PROT_READ | PROT_WRITE, + MAP_SHARED | MAP_ANONYMOUS, -1); - free (prog); - } + /* Run 'pldd' on test subprocess which will be created in pldd_process. + The pid of the subprocess will be written to target_pid_ptr. */ + struct support_capture_subprocess pldd; + pldd = support_capture_subprocess (pldd_process, target_pid_ptr); + support_capture_subprocess_check (&pldd, "pldd", 0, sc_allow_stdout); /* Check 'pldd' output. The test is expected to be linked against only loader and libc. */ @@ -85,7 +116,7 @@ do_test (void) /* First line is in the form of <pid>: <full path of executable> */ TEST_COMPARE (fscanf (out, "%u: " STRINPUT (512), &pid, buffer), 2); - TEST_COMPARE (pid, target.pid); + TEST_COMPARE (pid, *target_pid_ptr); TEST_COMPARE (strcmp (basename (buffer), "tst-pldd"), 0); /* It expects only one loader and libc loaded by the program. */ @@ -93,7 +124,7 @@ do_test (void) while (fgets (buffer, array_length (buffer), out) != NULL) { /* Ignore vDSO. */ - if (buffer[0] != '/') + if (buffer[0] != '/') continue; /* Remove newline so baseline (buffer) can compare against the @@ -128,7 +159,9 @@ do_test (void) } support_capture_subprocess_free (&pldd); - support_process_terminate (&target); + if (kill (*target_pid_ptr, SIGKILL) != 0) + FAIL_EXIT1 ("Unable to kill target_process: errno=%d=%m\n", errno); + xmunmap (target_pid_ptr, sizeof (pid_t)); return 0; } diff --git a/support/Makefile b/support/Makefile index ab66913a02..34d14eb1bb 100644 --- a/support/Makefile +++ b/support/Makefile @@ -56,6 +56,7 @@ libsupport-routines = \ support_format_hostent \ support_format_netent \ support_isolate_in_subprocess \ + support_ptrace \ support_openpty \ support_paths \ support_quote_blob \ diff --git a/support/ptrace.h b/support/ptrace.h new file mode 100644 index 0000000000..90006a6b75 --- /dev/null +++ b/support/ptrace.h @@ -0,0 +1,32 @@ +/* Support functions handling ptrace_scope. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#ifndef SUPPORT_PTRACE_H +#define SUPPORT_PTRACE_H + +#include <sys/cdefs.h> + +__BEGIN_DECLS + +/* Return the current YAMA mode set on the machine (0 to 3) or -1 + if YAMA is not supported. */ +int support_ptrace_scope (void); + +__END_DECLS + +#endif diff --git a/support/support_ptrace.c b/support/support_ptrace.c new file mode 100644 index 0000000000..2084e5a189 --- /dev/null +++ b/support/support_ptrace.c @@ -0,0 +1,44 @@ +/* Support functions handling ptrace_scope. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#include <support/check.h> +#include <support/xstdio.h> +#include <support/ptrace.h> +#include <sys/prctl.h> + +int +support_ptrace_scope (void) +{ + int ptrace_scope = -1; + +#ifdef __linux__ + /* YAMA may be not enabled. Otherwise it contains a value from 0 to 3: + - 0 classic ptrace permissions + - 1 restricted ptrace + - 2 admin-only attach + - 3 no attach */ + FILE *f = fopen ("/proc/sys/kernel/yama/ptrace_scope", "r"); + if (f != NULL) + { + TEST_COMPARE (fscanf (f, "%d", &ptrace_scope), 1); + xfclose (f); + } +#endif + + return ptrace_scope; +} ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add UNSUPPORTED check in elf/tst-pldd. 2019-09-02 15:28 ` Stefan Liebler @ 2019-09-17 13:31 ` Adhemerval Zanella 2019-09-17 15:17 ` Stefan Liebler 0 siblings, 1 reply; 22+ messages in thread From: Adhemerval Zanella @ 2019-09-17 13:31 UTC (permalink / raw) To: Stefan Liebler, Florian Weimer; +Cc: libc-alpha On 02/09/2019 12:28, Stefan Liebler wrote: > On 8/29/19 10:47 AM, Florian Weimer wrote: >> * Stefan Liebler: >> >>> On 8/28/19 11:24 AM, Florian Weimer wrote: >>>> * Stefan Liebler: >>>> >>>>>   static void >>>>>   target_process (void *arg) >>>>>   { >>>>> + if (ptrace_scope == 1) >>>>> +   { >>>>> +     /* YAMA is configured to "restricted ptrace". >>>>> +    Disable the restriction for this subprocess. */ >>>>> +     support_ptrace_process_set_ptracer_any (); >>>>> +   } >>>>> + >>>>>     pause (); >>>>>   } >>>> >>>> I think this has a race condition if pldd attaches to the process before >>>> the support_ptrace_process_set_ptracer_any call. I have no idea how >>>> hard it is in practice to hit this race. It should be possible to use a >>>> process-shared barrier or some other form of synchronization to avoid >>>> this issue. >>>> >>>> Thanks, >>>> Florian >>>> >>> >>> I've added a synchronization with stdatomic.h on a shared memory mapping. >>> I've not used pthread* functions as I don't want to link against >>> libpthread.so. Then further adjustments are needed. >>> >>> Or should I just restrict the test ptrace_scope 0 as Adhemerval has >>> proposed in his post? >> >> Is it possible to create a process tree like this? >> >> >>   parent (performs output checks) >>     subprocess 1 (becomes pldd via execve) >>       subprocess 2 >> >> If you execve pldd from subprocess 1, wouldn't subprocess 2 in its >> ptrace scope for ptrace_scope < 2? > Yes, this is possible. > I've rearranged the subprocesses. See attached patch. > Now we have a new function pldd_process which forks target_process, > stores the pid of target_prcess to a shared memory mapping as do_test needs to know this pid. > > Afterwards it execve to pldd which successfully ptrace target_process in case of "restricted ptrace". > > Please review the usage of support-subprocess-functions. > > Bye, > Stefan >> >> Thanks, >> Florian >> > > > 20190902_tst-pldd.patch > > commit ad51263d51d12ce6ca2ce9304efe5ba05b3912b1 > Author: Stefan Liebler <stli@linux.ibm.com> > Date: Mon Aug 26 15:45:07 2019 +0200 > > Add UNSUPPORTED check in elf/tst-pldd. > > The testcase forks a child process and runs pldd with PID of > this child. On systems where /proc/sys/kernel/yama/ptrace_scope > differs from zero, pldd will fail with > /usr/bin/pldd: cannot attach to process 3: Operation not permitted > > This patch checks if ptrace_scope exists, is zero "classic ptrace permissions" > or one "restricted ptrace". If ptrace_scope exists and has a higher > restriction, then the test is marked as UNSUPPORTED. > > The case "restricted ptrace" is handled by rearranging the processes involved > during the test. Now we have the following process tree: > -parent: do_test (performs output checks) > --subprocess 1: pldd_process (becomes pldd via execve) > ---subprocess 2: target_process (ptraced via pldd) > > ChangeLog: > > * elf/tst-pldd.c (do_test): Add UNSUPPORTED check. > Rearrange subprocesses. > (pldd_process): New function. > * support/Makefile (libsupport-routines): Add support_ptrace. > * support/ptrace.h: New file. > * support/support_ptrace.c: Likewise. LGTM with just a change below, thanks. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > > diff --git a/elf/tst-pldd.c b/elf/tst-pldd.c > index 6b7c94a1c0..eb1b9cb5f5 100644 > --- a/elf/tst-pldd.c > +++ b/elf/tst-pldd.c > @@ -30,6 +30,12 @@ > #include <support/capture_subprocess.h> > #include <support/check.h> > #include <support/support.h> > +#include <support/ptrace.h> > +#include <support/xunistd.h> > +#include <sys/mman.h> > +#include <errno.h> > +#include <signal.h> > + > > static void > target_process (void *arg) > @@ -37,6 +43,34 @@ target_process (void *arg) > pause (); > } > > +static void > +pldd_process (void *arg) > +{ > + pid_t *target_pid_ptr = (pid_t *) arg; > + > + /* Create a copy of current test to check with pldd. As the > + target_process is a child of this pldd_process, pldd is also able > + to attach to target_process if YAMA is configured to 1 = > + "restricted ptrace". */ > + struct support_subprocess target = support_subprocess (target_process, NULL); > + > + /* Store the pid of target-process as do_test needs it in order to > + e.g. terminate it at end of the test. */ > + *target_pid_ptr = target.pid; > + > + /* Three digits per byte plus null terminator. */ > + char pid[3 * sizeof (uint32_t) + 1]; > + snprintf (pid, array_length (pid), "%d", target.pid); > + > + char *prog = xasprintf ("%s/pldd", support_bindir_prefix); > + > + /* Run pldd and use the pid of target_process as argument. */ > + execve (prog, (char *const []) { (char *) prog, pid, NULL }, > + (char *const []) { NULL }); > + > + FAIL_EXIT1 ("Returned from execve: errno=%d=%m\n", errno); > +} > + Ok. > /* The test runs in a container because pldd does not support tracing > a binary started by the loader iself (as with testrun.sh). */ > > @@ -52,25 +86,22 @@ in_str_list (const char *libname, const char *const strlist[]) > static int > do_test (void) > { > - /* Create a copy of current test to check with pldd. */ > - struct support_subprocess target = support_subprocess (target_process, NULL); > - > - /* Run 'pldd' on test subprocess. */ > - struct support_capture_subprocess pldd; > + /* Check if our subprocess can be debugged with ptrace. */ > { > - /* Three digits per byte plus null terminator. */ > - char pid[3 * sizeof (uint32_t) + 1]; > - snprintf (pid, array_length (pid), "%d", target.pid); > - > - char *prog = xasprintf ("%s/pldd", support_bindir_prefix); > - > - pldd = support_capture_subprogram (prog, > - (char *const []) { (char *) prog, pid, NULL }); > + int ptrace_scope = support_ptrace_scope (); > + if (ptrace_scope >= 2) > + FAIL_UNSUPPORTED ("/proc/sys/kernel/yama/ptrace_scope >= 2"); > + } > Ok. > - support_capture_subprocess_check (&pldd, "pldd", 0, sc_allow_stdout); > + pid_t *target_pid_ptr = (pid_t *) xmmap (NULL, sizeof (pid_t), > + PROT_READ | PROT_WRITE, > + MAP_SHARED | MAP_ANONYMOUS, -1); > > - free (prog); > - } Ok. > + /* Run 'pldd' on test subprocess which will be created in pldd_process. > + The pid of the subprocess will be written to target_pid_ptr. */ > + struct support_capture_subprocess pldd; > + pldd = support_capture_subprocess (pldd_process, target_pid_ptr); > + support_capture_subprocess_check (&pldd, "pldd", 0, sc_allow_stdout); Ok, this is will updated by the pldd_process. > > /* Check 'pldd' output. The test is expected to be linked against only > loader and libc. */ > @@ -85,7 +116,7 @@ do_test (void) > /* First line is in the form of <pid>: <full path of executable> */ > TEST_COMPARE (fscanf (out, "%u: " STRINPUT (512), &pid, buffer), 2); > > - TEST_COMPARE (pid, target.pid); > + TEST_COMPARE (pid, *target_pid_ptr); > TEST_COMPARE (strcmp (basename (buffer), "tst-pldd"), 0); > > /* It expects only one loader and libc loaded by the program. */ > @@ -93,7 +124,7 @@ do_test (void) > while (fgets (buffer, array_length (buffer), out) != NULL) > { > /* Ignore vDSO. */ > - if (buffer[0] != '/') > + if (buffer[0] != '/') > continue; > Ok. > /* Remove newline so baseline (buffer) can compare against the > @@ -128,7 +159,9 @@ do_test (void) > } > > support_capture_subprocess_free (&pldd); > - support_process_terminate (&target); > + if (kill (*target_pid_ptr, SIGKILL) != 0) > + FAIL_EXIT1 ("Unable to kill target_process: errno=%d=%m\n", errno); > + xmunmap (target_pid_ptr, sizeof (pid_t)); > > return 0; > } Ok. > diff --git a/support/Makefile b/support/Makefile > index ab66913a02..34d14eb1bb 100644 > --- a/support/Makefile > +++ b/support/Makefile > @@ -56,6 +56,7 @@ libsupport-routines = \ > support_format_hostent \ > support_format_netent \ > support_isolate_in_subprocess \ > + support_ptrace \ > support_openpty \ > support_paths \ > support_quote_blob \ Ok. > diff --git a/support/ptrace.h b/support/ptrace.h > new file mode 100644 > index 0000000000..90006a6b75 > --- /dev/null > +++ b/support/ptrace.h > @@ -0,0 +1,32 @@ > +/* Support functions handling ptrace_scope. > + Copyright (C) 2019 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <http://www.gnu.org/licenses/>. */ > + > +#ifndef SUPPORT_PTRACE_H > +#define SUPPORT_PTRACE_H > + > +#include <sys/cdefs.h> > + > +__BEGIN_DECLS > + > +/* Return the current YAMA mode set on the machine (0 to 3) or -1 > + if YAMA is not supported. */ > +int support_ptrace_scope (void); > + > +__END_DECLS > + > +#endif I think it should named xptrace.h. > diff --git a/support/support_ptrace.c b/support/support_ptrace.c > new file mode 100644 > index 0000000000..2084e5a189 > --- /dev/null > +++ b/support/support_ptrace.c > @@ -0,0 +1,44 @@ > +/* Support functions handling ptrace_scope. > + Copyright (C) 2019 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <http://www.gnu.org/licenses/>. */ > + > +#include <support/check.h> > +#include <support/xstdio.h> > +#include <support/ptrace.h> > +#include <sys/prctl.h> > + > +int > +support_ptrace_scope (void) > +{ > + int ptrace_scope = -1; > + > +#ifdef __linux__ > + /* YAMA may be not enabled. Otherwise it contains a value from 0 to 3: > + - 0 classic ptrace permissions > + - 1 restricted ptrace > + - 2 admin-only attach > + - 3 no attach */ > + FILE *f = fopen ("/proc/sys/kernel/yama/ptrace_scope", "r"); > + if (f != NULL) > + { > + TEST_COMPARE (fscanf (f, "%d", &ptrace_scope), 1); > + xfclose (f); > + } > +#endif > + > + return ptrace_scope; > +} > Ok. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add UNSUPPORTED check in elf/tst-pldd. 2019-09-17 13:31 ` Adhemerval Zanella @ 2019-09-17 15:17 ` Stefan Liebler 2019-09-18 10:45 ` Stefan Liebler 0 siblings, 1 reply; 22+ messages in thread From: Stefan Liebler @ 2019-09-17 15:17 UTC (permalink / raw) To: Adhemerval Zanella, Florian Weimer; +Cc: libc-alpha On 9/17/19 3:31 PM, Adhemerval Zanella wrote: > > > On 02/09/2019 12:28, Stefan Liebler wrote: >> On 8/29/19 10:47 AM, Florian Weimer wrote: >>> * Stefan Liebler: >>> >>>> On 8/28/19 11:24 AM, Florian Weimer wrote: >>>>> * Stefan Liebler: >>>>> >>>>>>   static void >>>>>>   target_process (void *arg) >>>>>>   { >>>>>> + if (ptrace_scope == 1) >>>>>> +   { >>>>>> +     /* YAMA is configured to "restricted ptrace". >>>>>> +    Disable the restriction for this subprocess. */ >>>>>> +     support_ptrace_process_set_ptracer_any (); >>>>>> +   } >>>>>> + >>>>>>     pause (); >>>>>>   } >>>>> >>>>> I think this has a race condition if pldd attaches to the process before >>>>> the support_ptrace_process_set_ptracer_any call. I have no idea how >>>>> hard it is in practice to hit this race. It should be possible to use a >>>>> process-shared barrier or some other form of synchronization to avoid >>>>> this issue. >>>>> >>>>> Thanks, >>>>> Florian >>>>> >>>> >>>> I've added a synchronization with stdatomic.h on a shared memory mapping. >>>> I've not used pthread* functions as I don't want to link against >>>> libpthread.so. Then further adjustments are needed. >>>> >>>> Or should I just restrict the test ptrace_scope 0 as Adhemerval has >>>> proposed in his post? >>> >>> Is it possible to create a process tree like this? >>> >>> >>>   parent (performs output checks) >>>     subprocess 1 (becomes pldd via execve) >>>       subprocess 2 >>> >>> If you execve pldd from subprocess 1, wouldn't subprocess 2 in its >>> ptrace scope for ptrace_scope < 2? >> Yes, this is possible. >> I've rearranged the subprocesses. See attached patch. >> Now we have a new function pldd_process which forks target_process, >> stores the pid of target_prcess to a shared memory mapping as do_test needs to know this pid. >> >> Afterwards it execve to pldd which successfully ptrace target_process in case of "restricted ptrace". >> >> Please review the usage of support-subprocess-functions. >> >> Bye, >> Stefan >>> >>> Thanks, >>> Florian >>> >> >> >> 20190902_tst-pldd.patch >> >> commit ad51263d51d12ce6ca2ce9304efe5ba05b3912b1 >> Author: Stefan Liebler <stli@linux.ibm.com> >> Date: Mon Aug 26 15:45:07 2019 +0200 >> >> Add UNSUPPORTED check in elf/tst-pldd. >> >> The testcase forks a child process and runs pldd with PID of >> this child. On systems where /proc/sys/kernel/yama/ptrace_scope >> differs from zero, pldd will fail with >> /usr/bin/pldd: cannot attach to process 3: Operation not permitted >> >> This patch checks if ptrace_scope exists, is zero "classic ptrace permissions" >> or one "restricted ptrace". If ptrace_scope exists and has a higher >> restriction, then the test is marked as UNSUPPORTED. >> >> The case "restricted ptrace" is handled by rearranging the processes involved >> during the test. Now we have the following process tree: >> -parent: do_test (performs output checks) >> --subprocess 1: pldd_process (becomes pldd via execve) >> ---subprocess 2: target_process (ptraced via pldd) >> >> ChangeLog: >> >> * elf/tst-pldd.c (do_test): Add UNSUPPORTED check. >> Rearrange subprocesses. >> (pldd_process): New function. >> * support/Makefile (libsupport-routines): Add support_ptrace. >> * support/ptrace.h: New file. >> * support/support_ptrace.c: Likewise. > > LGTM with just a change below, thanks. > > Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > ... >> diff --git a/support/ptrace.h b/support/ptrace.h >> new file mode 100644 >> index 0000000000..90006a6b75 >> --- /dev/null >> +++ b/support/ptrace.h >> @@ -0,0 +1,32 @@ >> +/* Support functions handling ptrace_scope. >> + Copyright (C) 2019 Free Software Foundation, Inc. >> + This file is part of the GNU C Library. >> + >> + The GNU C Library is free software; you can redistribute it and/or >> + modify it under the terms of the GNU Lesser General Public >> + License as published by the Free Software Foundation; either >> + version 2.1 of the License, or (at your option) any later version. >> + >> + The GNU C Library is distributed in the hope that it will be useful, >> + but WITHOUT ANY WARRANTY; without even the implied warranty of >> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> + Lesser General Public License for more details. >> + >> + You should have received a copy of the GNU Lesser General Public >> + License along with the GNU C Library; if not, see >> + <http://www.gnu.org/licenses/>. */ >> + >> +#ifndef SUPPORT_PTRACE_H >> +#define SUPPORT_PTRACE_H >> + >> +#include <sys/cdefs.h> >> + >> +__BEGIN_DECLS >> + >> +/* Return the current YAMA mode set on the machine (0 to 3) or -1 >> + if YAMA is not supported. */ >> +int support_ptrace_scope (void); >> + >> +__END_DECLS >> + >> +#endif > > I think it should named xptrace.h. > ... Okay. Then I will rename it to support/xptrace.h and if nobody opposes, I'll commit it tomorrow. Thanks for the review. Stefan ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add UNSUPPORTED check in elf/tst-pldd. 2019-09-17 15:17 ` Stefan Liebler @ 2019-09-18 10:45 ` Stefan Liebler 2019-09-18 15:18 ` Joseph Myers 0 siblings, 1 reply; 22+ messages in thread From: Stefan Liebler @ 2019-09-18 10:45 UTC (permalink / raw) To: libc-alpha On 9/17/19 5:17 PM, Stefan Liebler wrote: > On 9/17/19 3:31 PM, Adhemerval Zanella wrote: >> >> >> On 02/09/2019 12:28, Stefan Liebler wrote: >>> On 8/29/19 10:47 AM, Florian Weimer wrote: >>>> * Stefan Liebler: >>>> >>>>> On 8/28/19 11:24 AM, Florian Weimer wrote: >>>>>> * Stefan Liebler: >>>>>> >>>>>>>    static void >>>>>>>    target_process (void *arg) >>>>>>>    { >>>>>>> + if (ptrace_scope == 1) >>>>>>> +   { >>>>>>> +     /* YAMA is configured to "restricted ptrace". >>>>>>> +    Disable the restriction for this subprocess. */ >>>>>>> +     support_ptrace_process_set_ptracer_any (); >>>>>>> +   } >>>>>>> + >>>>>>>      pause (); >>>>>>>    } >>>>>> >>>>>> I think this has a race condition if pldd attaches to the process >>>>>> before >>>>>> the support_ptrace_process_set_ptracer_any call. I have no idea how >>>>>> hard it is in practice to hit this race. It should be possible to >>>>>> use a >>>>>> process-shared barrier or some other form of synchronization to avoid >>>>>> this issue. >>>>>> >>>>>> Thanks, >>>>>> Florian >>>>>> >>>>> >>>>> I've added a synchronization with stdatomic.h on a shared memory >>>>> mapping. >>>>> I've not used pthread* functions as I don't want to link against >>>>> libpthread.so. Then further adjustments are needed. >>>>> >>>>> Or should I just restrict the test ptrace_scope 0 as Adhemerval has >>>>> proposed in his post? >>>> >>>> Is it possible to create a process tree like this? >>>> >>>> >>>>    parent (performs output checks) >>>>      subprocess 1 (becomes pldd via execve) >>>>        subprocess 2 >>>> >>>> If you execve pldd from subprocess 1, wouldn't subprocess 2 in its >>>> ptrace scope for ptrace_scope < 2? >>> Yes, this is possible. >>> I've rearranged the subprocesses. See attached patch. >>> Now we have a new function pldd_process which forks target_process, >>> stores the pid of target_prcess to a shared memory mapping as do_test >>> needs to know this pid. >>> >>> Afterwards it execve to pldd which successfully ptrace target_process >>> in case of "restricted ptrace". >>> >>> Please review the usage of support-subprocess-functions. >>> >>> Bye, >>> Stefan >>>> >>>> Thanks, >>>> Florian >>>> >>> >>> >>> 20190902_tst-pldd.patch >>> >>> commit ad51263d51d12ce6ca2ce9304efe5ba05b3912b1 >>> Author: Stefan Liebler <stli@linux.ibm.com> >>> Date:  Mon Aug 26 15:45:07 2019 +0200 >>> >>>     Add UNSUPPORTED check in elf/tst-pldd. >>>     The testcase forks a child process and runs pldd with PID of >>>     this child. On systems where /proc/sys/kernel/yama/ptrace_scope >>>     differs from zero, pldd will fail with >>>     /usr/bin/pldd: cannot attach to process 3: Operation not permitted >>>     This patch checks if ptrace_scope exists, is zero "classic >>> ptrace permissions" >>>     or one "restricted ptrace". If ptrace_scope exists and has a >>> higher >>>     restriction, then the test is marked as UNSUPPORTED. >>>     The case "restricted ptrace" is handled by rearranging the >>> processes involved >>>     during the test. Now we have the following process tree: >>>     -parent: do_test (performs output checks) >>>     --subprocess 1: pldd_process (becomes pldd via execve) >>>     ---subprocess 2: target_process (ptraced via pldd) >>>     ChangeLog: >>>             * elf/tst-pldd.c (do_test): Add UNSUPPORTED check. >>>             Rearrange subprocesses. >>>             (pldd_process): New function. >>>             * support/Makefile (libsupport-routines): Add >>> support_ptrace. >>>             * support/ptrace.h: New file. >>>             * support/support_ptrace.c: Likewise. >> >> LGTM with just a change below, thanks. >> >> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> >> > ... >>> diff --git a/support/ptrace.h b/support/ptrace.h >>> new file mode 100644 >>> index 0000000000..90006a6b75 >>> --- /dev/null >>> +++ b/support/ptrace.h >>> @@ -0,0 +1,32 @@ >>> +/* Support functions handling ptrace_scope. >>> +  Copyright (C) 2019 Free Software Foundation, Inc. >>> +  This file is part of the GNU C Library. >>> + >>> +  The GNU C Library is free software; you can redistribute it and/or >>> +  modify it under the terms of the GNU Lesser General Public >>> +  License as published by the Free Software Foundation; either >>> +  version 2.1 of the License, or (at your option) any later version. >>> + >>> +  The GNU C Library is distributed in the hope that it will be useful, >>> +  but WITHOUT ANY WARRANTY; without even the implied warranty of >>> +  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >>> +  Lesser General Public License for more details. >>> + >>> +  You should have received a copy of the GNU Lesser General Public >>> +  License along with the GNU C Library; if not, see >>> +  <http://www.gnu.org/licenses/>. */ >>> + >>> +#ifndef SUPPORT_PTRACE_H >>> +#define SUPPORT_PTRACE_H >>> + >>> +#include <sys/cdefs.h> >>> + >>> +__BEGIN_DECLS >>> + >>> +/* Return the current YAMA mode set on the machine (0 to 3) or -1 >>> +  if YAMA is not supported. */ >>> +int support_ptrace_scope (void); >>> + >>> +__END_DECLS >>> + >>> +#endif >> >> I think it should named xptrace.h. >> > ... > Okay. Then I will rename it to support/xptrace.h and if nobody opposes, > I'll commit it tomorrow. > > Thanks for the review. > Stefan > Committed with xptrace.h instead of ptrace.h. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add UNSUPPORTED check in elf/tst-pldd. 2019-09-18 10:45 ` Stefan Liebler @ 2019-09-18 15:18 ` Joseph Myers 2019-09-19 10:28 ` Stefan Liebler 0 siblings, 1 reply; 22+ messages in thread From: Joseph Myers @ 2019-09-18 15:18 UTC (permalink / raw) To: Stefan Liebler; +Cc: libc-alpha This has broken the build for i686-gnu. In file included from support_ptrace.c:22: ../include/sys/prctl.h:2:15: fatal error: sys/prctl.h: No such file or directory #include_next <sys/prctl.h> ^~~~~~~~~~~~~ -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add UNSUPPORTED check in elf/tst-pldd. 2019-09-18 15:18 ` Joseph Myers @ 2019-09-19 10:28 ` Stefan Liebler 0 siblings, 0 replies; 22+ messages in thread From: Stefan Liebler @ 2019-09-19 10:28 UTC (permalink / raw) To: Joseph Myers; +Cc: libc-alpha [-- Attachment #1: Type: text/plain, Size: 448 bytes --] On 9/18/19 5:17 PM, Joseph Myers wrote: > This has broken the build for i686-gnu. > > In file included from support_ptrace.c:22: > ../include/sys/prctl.h:2:15: fatal error: sys/prctl.h: No such file or directory > #include_next <sys/prctl.h> > ^~~~~~~~~~~~~ > Sorry for that. This include of sys/prctl.h is a leftover from a previous patch version. I've just committed the attached patch which just removes it. Thanks. Stefan [-- Attachment #2: 20190919_tst-pldd_fix.patch --] [-- Type: text/x-patch, Size: 876 bytes --] commit 0e055c90cadf2577daaaa2bb8fcb2d0542403d8f Author: Stefan Liebler <stli@linux.ibm.com> Date: Thu Sep 19 11:00:55 2019 +0200 Fix building support_ptrace.c on i686-gnu. On i686-gnu the build is broken: In file included from support_ptrace.c:22: ../include/sys/prctl.h:2:15: fatal error: sys/prctl.h: No such file or directory #include_next <sys/prctl.h> This patch just removes the unused prctl.h inclusion. ChangeLog: * support/support_ptrace.c: Remove inclusion of sys/prctl.h. diff --git a/support/support_ptrace.c b/support/support_ptrace.c index 616b08cff3..a733adf2c8 100644 --- a/support/support_ptrace.c +++ b/support/support_ptrace.c @@ -19,7 +19,6 @@ #include <support/check.h> #include <support/xstdio.h> #include <support/xptrace.h> -#include <sys/prctl.h> int support_ptrace_scope (void) ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add UNSUPPORTED check in elf/tst-pldd. 2019-08-29 8:47 ` Florian Weimer 2019-09-02 15:28 ` Stefan Liebler @ 2019-09-02 19:37 ` Adhemerval Zanella 2019-09-03 6:30 ` Stefan Liebler 1 sibling, 1 reply; 22+ messages in thread From: Adhemerval Zanella @ 2019-09-02 19:37 UTC (permalink / raw) To: Florian Weimer, Stefan Liebler; +Cc: libc-alpha On 29/08/2019 05:47, Florian Weimer wrote: > * Stefan Liebler: > >> On 8/28/19 11:24 AM, Florian Weimer wrote: >>> * Stefan Liebler: >>> >>>> static void >>>> target_process (void *arg) >>>> { >>>> + if (ptrace_scope == 1) >>>> + { >>>> + /* YAMA is configured to "restricted ptrace". >>>> + Disable the restriction for this subprocess. */ >>>> + support_ptrace_process_set_ptracer_any (); >>>> + } >>>> + >>>> pause (); >>>> } >>> >>> I think this has a race condition if pldd attaches to the process before >>> the support_ptrace_process_set_ptracer_any call. I have no idea how >>> hard it is in practice to hit this race. It should be possible to use a >>> process-shared barrier or some other form of synchronization to avoid >>> this issue. >>> >>> Thanks, >>> Florian >>> >> >> I've added a synchronization with stdatomic.h on a shared memory mapping. >> I've not used pthread* functions as I don't want to link against >> libpthread.so. Then further adjustments are needed. >> >> Or should I just restrict the test ptrace_scope 0 as Adhemerval has >> proposed in his post? > > Is it possible to create a process tree like this? > > > parent (performs output checks) > subprocess 1 (becomes pldd via execve) > subprocess 2 > > If you execve pldd from subprocess 1, wouldn't subprocess 2 in its > ptrace scope for ptrace_scope < 2? Do we really need that ad-hoc support on tst-pldd to make it support ptrace_scope 1? I don't oppose the support Stefan has added on latest iteration to make it work, but this is a lot of code to support a very specific scenario... ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add UNSUPPORTED check in elf/tst-pldd. 2019-09-02 19:37 ` Adhemerval Zanella @ 2019-09-03 6:30 ` Stefan Liebler 2019-09-03 13:34 ` Adhemerval Zanella 0 siblings, 1 reply; 22+ messages in thread From: Stefan Liebler @ 2019-09-03 6:30 UTC (permalink / raw) To: Adhemerval Zanella, Florian Weimer; +Cc: libc-alpha On 9/2/19 9:37 PM, Adhemerval Zanella wrote: > > > On 29/08/2019 05:47, Florian Weimer wrote: >> * Stefan Liebler: >> >>> On 8/28/19 11:24 AM, Florian Weimer wrote: >>>> * Stefan Liebler: >>>> >>>>> static void >>>>> target_process (void *arg) >>>>> { >>>>> + if (ptrace_scope == 1) >>>>> + { >>>>> + /* YAMA is configured to "restricted ptrace". >>>>> + Disable the restriction for this subprocess. */ >>>>> + support_ptrace_process_set_ptracer_any (); >>>>> + } >>>>> + >>>>> pause (); >>>>> } >>>> >>>> I think this has a race condition if pldd attaches to the process before >>>> the support_ptrace_process_set_ptracer_any call. I have no idea how >>>> hard it is in practice to hit this race. It should be possible to use a >>>> process-shared barrier or some other form of synchronization to avoid >>>> this issue. >>>> >>>> Thanks, >>>> Florian >>>> >>> >>> I've added a synchronization with stdatomic.h on a shared memory mapping. >>> I've not used pthread* functions as I don't want to link against >>> libpthread.so. Then further adjustments are needed. >>> >>> Or should I just restrict the test ptrace_scope 0 as Adhemerval has >>> proposed in his post? >> >> Is it possible to create a process tree like this? >> >> >> parent (performs output checks) >> subprocess 1 (becomes pldd via execve) >> subprocess 2 >> >> If you execve pldd from subprocess 1, wouldn't subprocess 2 in its >> ptrace scope for ptrace_scope < 2? > > Do we really need that ad-hoc support on tst-pldd to make it support > ptrace_scope 1? > > I don't oppose the support Stefan has added on latest iteration to > make it work, but this is a lot of code to support a very specific > scenario... > As there are systems where ptrace_scope is configured to 1 by default, we should adjust the testcase as the FAIL is misleading. (I've just recognized that Steve Ellcey had also seen this FAIL: https://www.sourceware.org/ml/libc-alpha/2019-07/msg00618.html) The minimum change should be to detect ptrace_scope == 1 and mark the test as UNSUPPORTED. Or we change a bit more and let the test also run in this scenario. (Either by support_ptrace_process_set_ptracer_any or adjusting the subprocess-tree) Bye Stefan ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add UNSUPPORTED check in elf/tst-pldd. 2019-09-03 6:30 ` Stefan Liebler @ 2019-09-03 13:34 ` Adhemerval Zanella 2019-09-06 3:21 ` Carlos O'Donell 0 siblings, 1 reply; 22+ messages in thread From: Adhemerval Zanella @ 2019-09-03 13:34 UTC (permalink / raw) To: Stefan Liebler, Florian Weimer; +Cc: libc-alpha On 03/09/2019 03:30, Stefan Liebler wrote: > On 9/2/19 9:37 PM, Adhemerval Zanella wrote: >> >> >> On 29/08/2019 05:47, Florian Weimer wrote: >>> * Stefan Liebler: >>> >>>> On 8/28/19 11:24 AM, Florian Weimer wrote: >>>>> * Stefan Liebler: >>>>> >>>>>>   static void >>>>>>   target_process (void *arg) >>>>>>   { >>>>>> + if (ptrace_scope == 1) >>>>>> +   { >>>>>> +     /* YAMA is configured to "restricted ptrace". >>>>>> +    Disable the restriction for this subprocess. */ >>>>>> +     support_ptrace_process_set_ptracer_any (); >>>>>> +   } >>>>>> + >>>>>>     pause (); >>>>>>   } >>>>> >>>>> I think this has a race condition if pldd attaches to the process before >>>>> the support_ptrace_process_set_ptracer_any call. I have no idea how >>>>> hard it is in practice to hit this race. It should be possible to use a >>>>> process-shared barrier or some other form of synchronization to avoid >>>>> this issue. >>>>> >>>>> Thanks, >>>>> Florian >>>>> >>>> >>>> I've added a synchronization with stdatomic.h on a shared memory mapping. >>>> I've not used pthread* functions as I don't want to link against >>>> libpthread.so. Then further adjustments are needed. >>>> >>>> Or should I just restrict the test ptrace_scope 0 as Adhemerval has >>>> proposed in his post? >>> >>> Is it possible to create a process tree like this? >>> >>> >>>   parent (performs output checks) >>>     subprocess 1 (becomes pldd via execve) >>>       subprocess 2 >>> >>> If you execve pldd from subprocess 1, wouldn't subprocess 2 in its >>> ptrace scope for ptrace_scope < 2? >> >> Do we really need that ad-hoc support on tst-pldd to make it support >> ptrace_scope 1? >> >> I don't oppose the support Stefan has added on latest iteration to >> make it work, but this is a lot of code to support a very specific >> scenario... >> > As there are systems where ptrace_scope is configured to 1 by default, we should adjust the testcase as the FAIL is misleading. > (I've just recognized that Steve Ellcey had also seen this FAIL: https://www.sourceware.org/ml/libc-alpha/2019-07/msg00618.html) > > The minimum change should be to detect ptrace_scope == 1 and mark the test as UNSUPPORTED. Or we change a bit more and let the test also run in this scenario. (Either by support_ptrace_process_set_ptracer_any or adjusting the subprocess-tree) Yes, my initial suggestion was just to make it as UNSUPPORTED for ptrace_scope >= 1. But I do not oppose adjusting it to run on ptrace_scope 1, it is just that the required hackery lead to make it somewhat as complex than the test itself. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add UNSUPPORTED check in elf/tst-pldd. 2019-09-03 13:34 ` Adhemerval Zanella @ 2019-09-06 3:21 ` Carlos O'Donell 2019-09-10 8:46 ` Stefan Liebler 0 siblings, 1 reply; 22+ messages in thread From: Carlos O'Donell @ 2019-09-06 3:21 UTC (permalink / raw) To: Adhemerval Zanella, Stefan Liebler, Florian Weimer; +Cc: libc-alpha On 9/3/19 9:34 AM, Adhemerval Zanella wrote: > Yes, my initial suggestion was just to make it as UNSUPPORTED for > ptrace_scope >= 1. But I do not oppose adjusting it to run on > ptrace_scope 1, it is just that the required hackery lead to make it > somewhat as complex than the test itself. The flip side of the coin is that the more "UNSUPPORTED" results we add *implies* there is "one valid way" to setup a glibc test run and we don't clearly document how to turn all the "UNSUPPORTED" entries into supported tests? Stefan's code can at least be refactored into support/ if we need to do the same thing again in another test. -- Cheers, Carlos. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add UNSUPPORTED check in elf/tst-pldd. 2019-09-06 3:21 ` Carlos O'Donell @ 2019-09-10 8:46 ` Stefan Liebler 2019-09-10 13:33 ` Adhemerval Zanella 0 siblings, 1 reply; 22+ messages in thread From: Stefan Liebler @ 2019-09-10 8:46 UTC (permalink / raw) To: Carlos O'Donell, Adhemerval Zanella, Florian Weimer; +Cc: libc-alpha On 9/6/19 5:21 AM, Carlos O'Donell wrote: > On 9/3/19 9:34 AM, Adhemerval Zanella wrote: >> Yes, my initial suggestion was just to make it as UNSUPPORTED for >> ptrace_scope >= 1. But I do not oppose adjusting it to run on >> ptrace_scope 1, it is just that the required hackery lead to make it >> somewhat as complex than the test itself. > > The flip side of the coin is that the more "UNSUPPORTED" results we > add *implies* there is "one valid way" to setup a glibc test run > and we don't clearly document how to turn all the "UNSUPPORTED" > entries into supported tests? > > Stefan's code can at least be refactored into support/ if we need > to do the same thing again in another test. > PING. As I have already posted multiple versions of the patch, how to proceed? 1) UNSUPPORTED if support_ptrace_scope() >= 2; Support support_ptrace_scope() == 1 by adjusting the process tree; (see https://www.sourceware.org/ml/libc-alpha/2019-09/msg00024.html) 2) UNSUPPORTED if support_ptrace_scope() >= 2; Support support_ptrace_scope() == 1 by calling support_ptrace_process_set_ptracer_any(); (see https://www.sourceware.org/ml/libc-alpha/2019-08/msg00722.html) 3) UNSUPPORTED if support_ptrace_scope() != 0 (patch would use support_ptrace_scope() of one of the patches above in order to trigger FAIL_UNSUPPORTED) Bye, Stefan ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add UNSUPPORTED check in elf/tst-pldd. 2019-09-10 8:46 ` Stefan Liebler @ 2019-09-10 13:33 ` Adhemerval Zanella 2019-09-11 7:05 ` Stefan Liebler 0 siblings, 1 reply; 22+ messages in thread From: Adhemerval Zanella @ 2019-09-10 13:33 UTC (permalink / raw) To: Stefan Liebler, Carlos O'Donell, Florian Weimer; +Cc: libc-alpha On 10/09/2019 05:46, Stefan Liebler wrote: > On 9/6/19 5:21 AM, Carlos O'Donell wrote: >> On 9/3/19 9:34 AM, Adhemerval Zanella wrote: >>> Yes, my initial suggestion was just to make it as UNSUPPORTED for >>> ptrace_scope >= 1. But I do not oppose adjusting it to run on >>> ptrace_scope 1, it is just that the required hackery lead to make it >>> somewhat as complex than the test itself. >> >> The flip side of the coin is that the more "UNSUPPORTED" results we >> add *implies* there is "one valid way" to setup a glibc test run >> and we don't clearly document how to turn all the "UNSUPPORTED" >> entries into supported tests? >> >> Stefan's code can at least be refactored into support/ if we need >> to do the same thing again in another test. >> > > PING. > > As I have already posted multiple versions of the patch, how to proceed? > 1) UNSUPPORTED if support_ptrace_scope() >= 2; > Support support_ptrace_scope() == 1 > by adjusting the process tree; > (see https://www.sourceware.org/ml/libc-alpha/2019-09/msg00024.html) > > 2) UNSUPPORTED if support_ptrace_scope() >= 2; > Support support_ptrace_scope() == 1 > by calling support_ptrace_process_set_ptracer_any(); > (see https://www.sourceware.org/ml/libc-alpha/2019-08/msg00722.html) > > 3) UNSUPPORTED if support_ptrace_scope() != 0 > (patch would use support_ptrace_scope() of one of the patches above in order to trigger FAIL_UNSUPPORTED) My view is although 2) is way complex that I would like, I think it should the more complete solution. Does still need review or is it ready to land? ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add UNSUPPORTED check in elf/tst-pldd. 2019-09-10 13:33 ` Adhemerval Zanella @ 2019-09-11 7:05 ` Stefan Liebler 0 siblings, 0 replies; 22+ messages in thread From: Stefan Liebler @ 2019-09-11 7:05 UTC (permalink / raw) To: Adhemerval Zanella, Carlos O'Donell, Florian Weimer; +Cc: libc-alpha On 9/10/19 3:32 PM, Adhemerval Zanella wrote: > > > On 10/09/2019 05:46, Stefan Liebler wrote: >> On 9/6/19 5:21 AM, Carlos O'Donell wrote: >>> On 9/3/19 9:34 AM, Adhemerval Zanella wrote: >>>> Yes, my initial suggestion was just to make it as UNSUPPORTED for >>>> ptrace_scope >= 1. But I do not oppose adjusting it to run on >>>> ptrace_scope 1, it is just that the required hackery lead to make it >>>> somewhat as complex than the test itself. >>> >>> The flip side of the coin is that the more "UNSUPPORTED" results we >>> add *implies* there is "one valid way" to setup a glibc test run >>> and we don't clearly document how to turn all the "UNSUPPORTED" >>> entries into supported tests? >>> >>> Stefan's code can at least be refactored into support/ if we need >>> to do the same thing again in another test. >>> >> >> PING. >> >> As I have already posted multiple versions of the patch, how to proceed? >> 1) UNSUPPORTED if support_ptrace_scope() >= 2; >> Support support_ptrace_scope() == 1 >> by adjusting the process tree; >> (see https://www.sourceware.org/ml/libc-alpha/2019-09/msg00024.html) >> >> 2) UNSUPPORTED if support_ptrace_scope() >= 2; >> Support support_ptrace_scope() == 1 >> by calling support_ptrace_process_set_ptracer_any(); >> (see https://www.sourceware.org/ml/libc-alpha/2019-08/msg00722.html) >> >> 3) UNSUPPORTED if support_ptrace_scope() != 0 >> (patch would use support_ptrace_scope() of one of the patches above in order to trigger FAIL_UNSUPPORTED) > > My view is although 2) is way complex that I would like, I think it should > the more complete solution. Does still need review or is it ready to land? > You've already reviewed the support part on a previous version of "patch 2)" (the support part was not changed in the latest version; see https://www.sourceware.org/ml/libc-alpha/2019-08/msg00703.html). But it needs review for the synchronization part between the target_process and do_test in tst-pldd.c. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add UNSUPPORTED check in elf/tst-pldd. 2019-08-28 9:06 ` Stefan Liebler 2019-08-28 9:24 ` Florian Weimer @ 2019-08-28 12:19 ` Adhemerval Zanella 1 sibling, 0 replies; 22+ messages in thread From: Adhemerval Zanella @ 2019-08-28 12:19 UTC (permalink / raw) To: libc-alpha On 28/08/2019 06:06, Stefan Liebler wrote: > On 8/27/19 9:11 PM, Adhemerval Zanella wrote: >> >> >> On 27/08/2019 12:14, Florian Weimer wrote: >>> * Adhemerval Zanella: >>> >>>> This is a Linuxism and I think we should create a 'support_can_ptrace' >>>> similar to 'support_can_chroot'. The logic to detect it seems >>>> correct, I would just check fscanf returned to value and use xfclose. >>>> It would be something like >>> >>> The test does the right thing if the path does not exist, so I'm not >>> sure if it is necessary. >> >> Even though, a check for a Linux specific path should be restricted to >> Linux builds only. >> >>> >>> support_can_ptrace would be misleading because even with YAMA scope >>> support, tracing direct subprocesses will still work. >> >> Indeed, a better prototype would be indeed: >> >> /* Return the current YAMA mode set on the machine (0 to 3) or -1 >>    if YAMA is not supported. */ >> int support_ptrace_scope (void); >> >>> >>> I fact, I think we might be able to get this test to work even with >>> fairly restrictive YAMA scopes, by proper ordering of forks and using >>> execve to run tst-pldd. >> >> The problem with ptrace_scope 1 is tst-pldd will either make its fork >> helper process a pldd descendant or pass pldd pid to the forked process >> so it can call prctl (PR_SET_PTRACER, ...). In either case I am not >> really sure it is possible without change pldd process itself (since >> it does not have an interactive mode), nor if the complexity to support >> this specific scenarios pays off. >> >> The ptrace_scope 2 is even more tricky since it requires CAP_SYS_PTRACE. >> >> In any case I think by restricting the test to run on ptrace_scope 0 >> is a fair assumption. >> >>> >>> Thanks, >>> Florian >>> > > Please have a look at the adjusted the patch. > > I've introduced new support functions. > And if ptrace_scope is 1 "restricted ptrace", I've just call prctl (PR_SET_PTRACER, PR_SET_PTRACER_ANY,...) on the target process. > This way the target process does not need to know the pid of pldd and pldd is allowed to attach to the target process. > > If ptrace_scope is 2 or 3, the test is marked as UNSUPPORTED. > > Thanks. > Stefan > > 20190828_tst-pldd.patch > > commit 7eba88e4f44df6f4a6d174e566b1796f2abc490c > Author: Stefan Liebler <stli@linux.ibm.com> > Date: Mon Aug 26 15:45:07 2019 +0200 > > Add UNSUPPORTED check in elf/tst-pldd. > > The testcase forks a child process and runs pldd with PID of > this child. On systems where /proc/sys/kernel/yama/ptrace_scope > differs from zero, pldd will fail with > /usr/bin/pldd: cannot attach to process 3: Operation not permitted > > This patch checks if ptrace_scope exists, is zero "classic ptrace permissions" > or one "restricted ptrace". In case of "restricted ptrace", we can > effectively disable the restriction by using prctl (PR_SET_PTRACER_ANY). > > If ptrace_scope exists and has a higher restriction, then the test > is marked as UNSUPPORTED. > > ChangeLog: > > * elf/tst-pldd.c (do_test): Add UNSUPPORTED check. > (target_process): Disable restricted ptrace. > * support/Makefile (libsupport-routines): Add support_ptrace. > * support/ptrace.h: New file. > * support/support_ptrace.c: Likewise. LGTM with a change below. > > diff --git a/elf/tst-pldd.c b/elf/tst-pldd.c > index 6b7c94a1c0..728272d177 100644 > --- a/elf/tst-pldd.c > +++ b/elf/tst-pldd.c > @@ -30,10 +30,20 @@ > #include <support/capture_subprocess.h> > #include <support/check.h> > #include <support/support.h> > +#include <support/ptrace.h> > + > +static int ptrace_scope; > > static void > target_process (void *arg) > { > + if (ptrace_scope == 1) > + { > + /* YAMA is configured to "restricted ptrace". > + Disable the restriction for this subprocess. */ > + support_ptrace_process_set_ptracer_any (); > + } > + > pause (); > } > > @@ -52,6 +62,11 @@ in_str_list (const char *libname, const char *const strlist[]) > static int > do_test (void) > { > + /* Check if our subprocess can be debugged with ptrace. */ > + ptrace_scope = support_ptrace_scope (); > + if (ptrace_scope >= 2) > + FAIL_UNSUPPORTED ("/proc/sys/kernel/yama/ptrace_scope >= 2"); > + > /* Create a copy of current test to check with pldd. */ > struct support_subprocess target = support_subprocess (target_process, NULL); > I think we can restrict the test to ptrace_scope 0 for now, since as Florian has brought we will need some synchronization with parent after the support_ptrace_process_set_ptracer_any call. > diff --git a/support/Makefile b/support/Makefile > index ab66913a02..34d14eb1bb 100644 > --- a/support/Makefile > +++ b/support/Makefile > @@ -56,6 +56,7 @@ libsupport-routines = \ > support_format_hostent \ > support_format_netent \ > support_isolate_in_subprocess \ > + support_ptrace \ > support_openpty \ > support_paths \ > support_quote_blob \ Ok. > diff --git a/support/ptrace.h b/support/ptrace.h > new file mode 100644 > index 0000000000..82f79ff25c > --- /dev/null > +++ b/support/ptrace.h > @@ -0,0 +1,36 @@ > +/* Support functions handling ptrace_scope. > + Copyright (C) 2019 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <http://www.gnu.org/licenses/>. */ > + > +#ifndef SUPPORT_PTRACE_H > +#define SUPPORT_PTRACE_H > + > +#include <sys/cdefs.h> > + > +__BEGIN_DECLS > + > +/* Return the current YAMA mode set on the machine (0 to 3) or -1 > + if YAMA is not supported. */ > +int support_ptrace_scope (void); > + > +/* Effectively disables YAMA restriction for the calling process > + if support_ptrace_scope returns 1 "restricted ptrace". */ > +void support_ptrace_process_set_ptracer_any (void); > + > +__END_DECLS > + > +#endif Ok. > diff --git a/support/support_ptrace.c b/support/support_ptrace.c > new file mode 100644 > index 0000000000..e9410384b5 > --- /dev/null > +++ b/support/support_ptrace.c > @@ -0,0 +1,58 @@ > +/* Support functions handling ptrace_scope. > + Copyright (C) 2019 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <http://www.gnu.org/licenses/>. */ > + > +#include <support/check.h> > +#include <support/xstdio.h> > +#include <support/ptrace.h> > +#include <sys/prctl.h> > + > +int > +support_ptrace_scope (void) > +{ > + int ptrace_scope = -1; > + > +#ifdef __linux__ > + /* YAMA may be not enabled. Otherwise it contains a value from 0 to 3: > + - 0 classic ptrace permissions > + - 1 restricted ptrace > + - 2 admin-only attach > + - 3 no attach */ > + FILE *f = fopen ("/proc/sys/kernel/yama/ptrace_scope", "r"); > + if (f != NULL) > + { > + TEST_COMPARE (fscanf (f, "%d", &ptrace_scope), 1); > + xfclose (f); > + } > +#endif > + > + return ptrace_scope; > +} > + > +void > +support_ptrace_process_set_ptracer_any (void) > +{ > +#ifdef __linux__ > + int ret = prctl (PR_SET_PTRACER, PR_SET_PTRACER_ANY, 0, 0, 0); > + if (ret != 0) > + FAIL_EXIT1 ("Failed to disable YAMA restriction. (prctl returned %d: %m)", > + ret); > +#else > + FAIL_UNSUPPORTED ("prctl (PR_SET_PTRACER, PR_SET_PTRACER_ANY, ...) " > + "not supported!"); > +#endif > +} > Ok. ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2019-09-19 10:28 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-27 10:19 [PATCH] Add UNSUPPORTED check in elf/tst-pldd Stefan Liebler 2019-08-27 15:06 ` Adhemerval Zanella 2019-08-27 15:14 ` Florian Weimer 2019-08-27 19:11 ` Adhemerval Zanella 2019-08-28 9:06 ` Stefan Liebler 2019-08-28 9:24 ` Florian Weimer 2019-08-28 14:42 ` Stefan Liebler 2019-08-29 8:47 ` Florian Weimer 2019-09-02 15:28 ` Stefan Liebler 2019-09-17 13:31 ` Adhemerval Zanella 2019-09-17 15:17 ` Stefan Liebler 2019-09-18 10:45 ` Stefan Liebler 2019-09-18 15:18 ` Joseph Myers 2019-09-19 10:28 ` Stefan Liebler 2019-09-02 19:37 ` Adhemerval Zanella 2019-09-03 6:30 ` Stefan Liebler 2019-09-03 13:34 ` Adhemerval Zanella 2019-09-06 3:21 ` Carlos O'Donell 2019-09-10 8:46 ` Stefan Liebler 2019-09-10 13:33 ` Adhemerval Zanella 2019-09-11 7:05 ` Stefan Liebler 2019-08-28 12:19 ` Adhemerval Zanella
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).