[PATCH] Add UNSUPPORTED check in elf/tst-pldd.

[Stefan Liebler]

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