Add UNSUPPORTED check in elf/tst-pldd.

  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
  

* 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
  

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.