commit:     ba41b3b01c573a4f942605142a5a0d2f08b4c799
Author:     Mike Frysinger <vapier <AT> gentoo <DOT> org>
AuthorDate: Mon Oct 18 22:06:39 2021 +0000
Commit:     Mike Frysinger <vapier <AT> gentoo <DOT> org>
CommitDate: Wed Nov  3 00:05:25 2021 +0000
URL:        https://gitweb.gentoo.org/proj/sandbox.git/commit/?id=ba41b3b0

libsandbox: fix ptracing children

The ptrace logic was largely built around the assumption of execing a
single static binary and that's it.  But there's nothing stopping it
from also forking & creating children.  Today, that means children do
not get tracked for problems.

One major known issue is that the sandbox env is frozen upon launch.
So once we switch to ptrace mode, it's not possible for traced code
to disable sandboxing or otherwise reconfigure it.  Currently that
shouldn't be a big deal as we assume the main execution environment
(i.e. bash) is dynamic, and that's where the env will be tweaked,
but we'll have to address this before we can deploy ptrace more.

Signed-off-by: Mike Frysinger <vapier <AT> gentoo.org>

 libsandbox/trace.c             | 73 +++++++++++++++++++++++++++++++++++++-----
 libsbutil/sb_efuncs.c          |  1 +
 libsbutil/sbutil.h             |  9 ++++++
 tests/fork-follow_static_tst.c |  1 +
 tests/fork-follow_tst.c        | 34 ++++++++++++++++++++
 tests/local.mk                 |  2 ++
 tests/script-17.sh             | 17 ++++++++++
 tests/script.at                |  3 +-
 8 files changed, 131 insertions(+), 9 deletions(-)

diff --git a/libsandbox/trace.c b/libsandbox/trace.c
index 4ae58aa..0434f96 100644
--- a/libsandbox/trace.c
+++ b/libsandbox/trace.c
@@ -29,7 +29,7 @@ static long _do_ptrace(sb_ptrace_req_t request, const char 
*srequest, void *addr
 # define SBDEBUG 0
 #endif
 #define __sb_debug(fmt, args...) do { if (SBDEBUG) sb_eraw(fmt, ## args); } 
while (0)
-#define _sb_debug(fmt, args...)  do { if (SBDEBUG) sb_ewarn("TRACE 
(pid=%i):%s: " fmt, getpid(), __func__, ## args); } while (0)
+#define _sb_debug(fmt, args...)  do { if (SBDEBUG) sb_ewarn("TRACE 
(pid=%i<%i):%s: " fmt, getpid(), trace_pid, __func__, ## args); } while (0)
 #define sb_debug(fmt, args...)   _sb_debug(fmt "\n", ## args)
 
 #include "trace/os.c"
@@ -397,6 +397,19 @@ static bool trace_check_syscall(const struct syscall_entry 
*se, void *regs)
        return ret;
 }
 
+static void trace_init_tracee(void)
+{
+       do_ptrace(PTRACE_SETOPTIONS, NULL, (void *)(uintptr_t)(
+               PTRACE_O_EXITKILL |
+               PTRACE_O_TRACECLONE |
+               PTRACE_O_TRACEEXEC |
+               PTRACE_O_TRACEEXIT |
+               PTRACE_O_TRACEFORK |
+               PTRACE_O_TRACEVFORK |
+               PTRACE_O_TRACESYSGOOD
+       ));
+}
+
 static void trace_loop(void)
 {
        trace_regs regs;
@@ -471,6 +484,56 @@ static void trace_loop(void)
                        __sb_debug(" exit event!\n");
                        continue;
 
+               case PTRACE_EVENT_CLONE:
+               case PTRACE_EVENT_FORK:
+               case PTRACE_EVENT_VFORK: {
+                       /* The tracee is forking, so fork a new tracer to 
handle it. */
+                       long newpid;
+                       do_ptrace(PTRACE_GETEVENTMSG, NULL, &newpid);
+                       sb_debug("following forking event %i; pid=%li %i\n",
+                                event, newpid, before_syscall);
+
+                       /* Pipe for synchronizing detach & attach events. */
+                       int fds[2];
+                       ret = pipe(fds);
+                       sb_assert(ret == 0);
+                       if (fork() == 0) {
+                               /* New tracer needs to take control of new 
tracee. */
+                               char ch;
+                               close(fds[1]);
+                               RETRY_EINTR(read(fds[0], &ch, 1));
+                               close(fds[0]);
+                               trace_pid = newpid;
+ retry_attach:
+                               ret = do_ptrace(PTRACE_ATTACH, NULL, NULL);
+                               if (ret) {
+                                       if (errno == EPERM)
+                                               goto retry_attach;
+                                       sb_ebort("ISE:PTRACE_ATTACH %s", 
strerror(errno));
+                               }
+                               trace_init_tracee();
+                               before_syscall = true;
+                               continue;
+                       } else {
+                               /* Existing tracer needs to release new tracee. 
*/
+ retry_detach:
+                               ret = ptrace(PTRACE_DETACH, newpid, NULL, (void 
*)SIGSTOP);
+                               if (ret) {
+                                       if (errno == ESRCH) {
+                                               /* The kernel might not have 
the proc ready yet. */
+                                               struct timespec ts = {0, 500 * 
1000 /* 0.5 millisec */};
+                                               nanosleep(&ts, NULL);
+                                               goto retry_detach;
+                                       }
+                                       sb_ebort("ISE:PTRACE_DETACH %s", 
strerror(errno));
+                               }
+                               close(fds[0]);
+                               RETRY_EINTR(write(fds[1], "", 1));
+                               close(fds[1]);
+                       }
+                       continue;
+               }
+
                default:
                        sb_ebort("ISE: unhandle ptrace signal %s (%i) event 
%u\n",
                                 strsig(sig), sig, event);
@@ -524,13 +587,7 @@ void trace_main(void)
        } else if (trace_pid) {
                sb_debug("parent waiting for child (pid=%i) to signal", 
trace_pid);
                waitpid(trace_pid, NULL, 0);
-               do_ptrace(PTRACE_SETOPTIONS, NULL,
-                       (void *)(uintptr_t)(
-                               PTRACE_O_EXITKILL |
-                               PTRACE_O_TRACEEXEC |
-                               PTRACE_O_TRACEEXIT |
-                               PTRACE_O_TRACESYSGOOD
-                       ));
+               trace_init_tracee();
                sb_close_all_fds();
                trace_loop();
                sb_ebort("ISE: child should have quit, as should we\n");

diff --git a/libsbutil/sb_efuncs.c b/libsbutil/sb_efuncs.c
index 7ded90d..1283784 100644
--- a/libsbutil/sb_efuncs.c
+++ b/libsbutil/sb_efuncs.c
@@ -52,6 +52,7 @@ static void sb_vefunc(const char *prog, const char *color, 
const char *format, v
                sb_fdprintf(fd, " %s*%s ", color, COLOR_NORMAL);
        sb_vfdprintf(fd, format, args);
 
+       fsync(fd);
        if (opened)
                close(fd);
 }

diff --git a/libsbutil/sbutil.h b/libsbutil/sbutil.h
index d81543b..267f717 100644
--- a/libsbutil/sbutil.h
+++ b/libsbutil/sbutil.h
@@ -169,6 +169,15 @@ char *__xstrndup(const char *str, size_t size, const char 
*file, const char *fun
 #define restore_errno() errno = old_errno;
 #define saved_errno     old_errno
 
+#define RETRY_EINTR(call) \
+({ \
+       long result; \
+       do { \
+               result = (call); \
+       } while (result == -1 && errno == EINTR); \
+       result; \
+})
+
 #include "gnulib/canonicalize.h"
 
 #endif /* __SBUTIL_H__ */

diff --git a/tests/fork-follow_static_tst.c b/tests/fork-follow_static_tst.c
new file mode 100644
index 0000000..363384e
--- /dev/null
+++ b/tests/fork-follow_static_tst.c
@@ -0,0 +1 @@
+#include "fork-follow_tst.c"

diff --git a/tests/fork-follow_tst.c b/tests/fork-follow_tst.c
new file mode 100644
index 0000000..2e3bb95
--- /dev/null
+++ b/tests/fork-follow_tst.c
@@ -0,0 +1,34 @@
+/*
+ * Make sure violations in children are caught.
+ */
+
+#include "tests.h"
+
+int main(int argc, char *argv[])
+{
+       if (argc != 3) {
+               printf("usage: %s <number forks> <path to remove>\n", argv[0]);
+               exit(1);
+       }
+
+       int i, forks = atoi(argv[1]);
+       const char *path = argv[2];
+
+       for (i = 0; i < forks; ++i) {
+               pid_t pid = fork();
+               if (pid < 0)
+                       errp("unable to fork");
+
+               if (pid > 0) {
+                       /* parent -- wait for child */
+                       int status;
+                       if (waitpid(pid, &status, 0) == pid)
+                               exit(WEXITSTATUS(status));
+                       errp("waitpid failed");
+               }
+               /* child -- keep looping */
+       }
+
+       /* final child -- try to create the path */
+       exit(creat(path, 0666) < 0 ? 0 : 1);
+}

diff --git a/tests/local.mk b/tests/local.mk
index 86a8a65..046cf6f 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -87,6 +87,8 @@ check_PROGRAMS += \
        %D%/utimes-0 \
        %D%/vfork-0 \
        \
+       %D%/fork-follow_tst \
+       %D%/fork-follow_static_tst \
        %D%/getcwd-gnulib_tst \
        %D%/libsigsegv_tst \
        %D%/malloc_hooked_tst \

diff --git a/tests/script-17.sh b/tests/script-17.sh
new file mode 100755
index 0000000..a8a8f51
--- /dev/null
+++ b/tests/script-17.sh
@@ -0,0 +1,17 @@
+#!/bin/sh
+# Make sure forked children are caught.  Historically, dynamic worked fine, but
+# static missed forks.
+[ "${at_xfail}" = "yes" ] && exit 77 # see script-0
+
+# Setup scratch path.
+mkdir subdir
+adddeny "${PWD}/subdir"
+
+for child in 0 1 2 3 4 5 ; do
+       fork-follow_tst ${child} subdir/dyn${child} || exit $?
+done
+for child in 0 1 2 3 4 5 ; do
+       fork-follow_static_tst ${child} subdir/static${child} || exit $?
+done
+
+exit 0

diff --git a/tests/script.at b/tests/script.at
index f1119ef..037d27e 100644
--- a/tests/script.at
+++ b/tests/script.at
@@ -13,4 +13,5 @@ SB_CHECK(12)
 SB_CHECK(13)
 SB_CHECK(14)
 SB_CHECK(15)
-SB_CHECK(16)
\ No newline at end of file
+SB_CHECK(16)
+SB_CHECK(17)

Reply via email to