This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]