Add UNSUPPORTED check in elf/tst-pldd.

Message ID fbb0d1e8-4f39-59ed-801c-d53e14d0320a@linux.ibm.com
State Superseded
Headers

Commit Message

Stefan Liebler Aug. 27, 2019, 10:19 a.m. UTC
  Hi,

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 is zero and otherwise marks the
test as UNSUPPORTED.

Bye
Stefan

ChangeLog:

	* elf/tst-pldd.c (do_test): Add UNSUPPORTED check.
  

Comments

Adhemerval Zanella Netto Aug. 27, 2019, 3:06 p.m. UTC | #1
On 27/08/2019 07:19, Stefan Liebler wrote:
> Hi,
> 
> 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 is zero and otherwise marks the
> test as UNSUPPORTED.
> 
> Bye
> Stefan
> 
> ChangeLog:
> 
>     * elf/tst-pldd.c (do_test): Add UNSUPPORTED check.
> 
> 20190826_tst-pldd.patch
> 
> commit 9c0b03c38bdd31618909da46b8bd4e09b5a236d2
> 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 is zero and otherwise marks the
>     test as UNSUPPORTED.
>     
>     ChangeLog:
>     
>             * elf/tst-pldd.c (do_test): Add UNSUPPORTED check.
> 
> diff --git a/elf/tst-pldd.c b/elf/tst-pldd.c
> index 6b7c94a1c0..3f211dc342 100644
> --- a/elf/tst-pldd.c
> +++ b/elf/tst-pldd.c
> @@ -52,6 +52,24 @@ in_str_list (const char *libname, const char *const strlist[])
>  static int
>  do_test (void)
>  {
> +  /* Check if all processes can be debugged with ptrace.  */
> +  {
> +    FILE *f = fopen ("/proc/sys/kernel/yama/ptrace_scope", "r");
> +    if (f != NULL)
> +      {
> +	/* If ptrace_scope exists, then it has to be 0 which means
> +	   "classic ptrace permissions".  A process can PTRACE_ATTACH
> +	   to any other process running under the same uid, as long as
> +	   it is dumpable.  Otherwise pldd will fail to attach to the
> +	   subprocess.  */
> +	int i = 99;
> +	fscanf (f, "%d", &i);
> +	fclose (f);
> +	if (i != 0)
> +	  FAIL_UNSUPPORTED ("/proc/sys/kernel/yama/ptrace_scope != 0");
> +      }
> +  }
> +

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

bool
support_can_ptrace (void)
{
  bool ret = true;

#ifdef __linux__
  /* YAMA may be not enabled.  If it is then ptrace_scope it has to be 0
     which means "classic ptrace permissions".  A process can 
     PTRACE_ATTACH to any other process running under the same uid, as
     long as it is dumpable.  Otherwise pldd will fail to attach to the
     subprocess.  */
  FILE *f = fopen ("/proc/sys/kernel/yama/ptrace_scope", "r");
  if (f == NULL)
    return true;

  int i = 99;
  TEST_COMPARE (fscanf (f, "%d", &i), 1);
  xfclose (f);
  ret = i == 0;
#endif

  return ret;
}

And I think we might eventually need to handle seccomp as well.

>    /* Create a copy of current test to check with pldd.  */
>    struct support_subprocess target = support_subprocess (target_process, NULL);
>  
>
  
Florian Weimer Aug. 27, 2019, 3:14 p.m. UTC | #2
* 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.

support_can_ptrace would be misleading because even with YAMA scope
support, tracing direct subprocesses will still work.

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.

Thanks,
Florian
  
Adhemerval Zanella Netto Aug. 27, 2019, 7:11 p.m. UTC | #3
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
>
  

Patch

commit 9c0b03c38bdd31618909da46b8bd4e09b5a236d2
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 is zero and otherwise marks the
    test as UNSUPPORTED.
    
    ChangeLog:
    
            * elf/tst-pldd.c (do_test): Add UNSUPPORTED check.

diff --git a/elf/tst-pldd.c b/elf/tst-pldd.c
index 6b7c94a1c0..3f211dc342 100644
--- a/elf/tst-pldd.c
+++ b/elf/tst-pldd.c
@@ -52,6 +52,24 @@  in_str_list (const char *libname, const char *const strlist[])
 static int
 do_test (void)
 {
+  /* Check if all processes can be debugged with ptrace.  */
+  {
+    FILE *f = fopen ("/proc/sys/kernel/yama/ptrace_scope", "r");
+    if (f != NULL)
+      {
+	/* If ptrace_scope exists, then it has to be 0 which means
+	   "classic ptrace permissions".  A process can PTRACE_ATTACH
+	   to any other process running under the same uid, as long as
+	   it is dumpable.  Otherwise pldd will fail to attach to the
+	   subprocess.  */
+	int i = 99;
+	fscanf (f, "%d", &i);
+	fclose (f);
+	if (i != 0)
+	  FAIL_UNSUPPORTED ("/proc/sys/kernel/yama/ptrace_scope != 0");
+      }
+  }
+
   /* Create a copy of current test to check with pldd.  */
   struct support_subprocess target = support_subprocess (target_process, NULL);