Add UNSUPPORTED check in elf/tst-pldd.
Commit Message
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
Comments
* 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
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...
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
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.
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.
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
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?
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.
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.
@@ -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;
}
@@ -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 \
new file mode 100644
@@ -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
new file mode 100644
@@ -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
+}