Add UNSUPPORTED check in elf/tst-pldd.

Message ID 21f1131e-5756-3c56-d41f-a37f172c48e8@linux.ibm.com
State Committed
Headers

Commit Message

Stefan Liebler Sept. 2, 2019, 3:28 p.m. UTC
  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
>
  

Comments

Adhemerval Zanella Netto Sept. 17, 2019, 1:31 p.m. UTC | #1
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.
  
Stefan Liebler Sept. 17, 2019, 3:17 p.m. UTC | #2
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
  
Stefan Liebler Sept. 18, 2019, 10:45 a.m. UTC | #3
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.
  
Joseph Myers Sept. 18, 2019, 3:17 p.m. UTC | #4
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>
               ^~~~~~~~~~~~~
  

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.

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;
+}