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

Wed Sep 18 10:45:00 GMT 2019

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.