PR14245: support /sys/kernel/debug mounted 0700

This is done by staprun passing a file descriptor for the
/sys/kernel/debug/systemtap/stap_MODULE directory from staprun
(running setuid) to stapio (running unprivileged, previously unable to
traverse to that path itself). This FD passing is done with a new
option -F<fd> for stapio (though by accident staprun also accepts (and
rejects) this option).

Since openat(2) is relatively recent, autoconf macros are used to back
down to graceful failure on older kernels, and to hide the new code.
New staprun always uses -F<fd> to stapio, even if permissions on
/sys/kernel/debug do not require it.

* staprun/common.c (relay_basedir_fd): New variable.
  (parse_args): Parse new -F: option.
  (usage): Document it.
* staprun/staprun.h: Corresponding changes.
* staprun/ctl.c (init_ctl_channel): Reorganize to try an incoming
  relay_basedir_fd first (with a faccessat cross-user check) first.
  Try to compute a relay_basedir_fd if not already set.
* staprun/mainloop.c (read_buffer_info): Note ignoring of this PR facility on
  RHEL4-era old_transport.
* staprun/relayfs.c (init_relayfs): Attempt to open relay_fd[] using
  relay_basedir_fd if specified.
* staprun/stapio.c: Top secret.
* staprun/staprun.c (main): Don't allow staprun itself to take -F, for it
  could be misused by a very bad person (tm).  However, arrange to pass
  it to stapio, if we have incidentally discovered a good relay_basedir_fd.
* staprun/staprun_funcs.c (mountfs): Drop access_debugfs() check at this
  point, as init_ctl_channel() will do the check later.

diff --git a/staprun/common.c b/staprun/common.c
index ba59c884843f3784d5e7e26409e45a68159de76c..f6acb6fae32d839f5087b63eac70e42078156f68 100644 (file)
--- a/staprun/common.c
+++ b/staprun/common.c
@@ -38,6 +38,7 @@ off_t fsize_max;
 int fnum_max;
 int remote_id;
 const char *remote_uri;
+int relay_basedir_fd;
 
 /* module variables */
 char *modname = NULL;
@@ -132,8 +133,13 @@ void parse_args(int argc, char **argv)
        fnum_max = 0;
         remote_id = -1;
         remote_uri = NULL;
+        relay_basedir_fd = -1;
 
-       while ((c = getopt(argc, argv, "ALu::vb:t:dc:o:x:S:DwRr:VT:")) != EOF) {
+       while ((c = getopt(argc, argv, "ALu::vb:t:dc:o:x:S:DwRr:VT:"
+#ifdef HAVE_OPENAT
+                           "F:"
+#endif
+                        )) != EOF) {
                switch (c) {
                case 'u':
                        need_uprobes = 1;
@@ -179,6 +185,13 @@ void parse_args(int argc, char **argv)
                case 'D':
                        daemon_mode = 1;
                        break;
+               case 'F':
+                       relay_basedir_fd = atoi(optarg);
+                       if (relay_basedir_fd < 0) {
+                               err(_("Invalid file descriptor option '%s'.\n"), optarg);
+                               usage(argv[0]);
+                       }
+                       break;
                case 'S':
                        fsize_max = strtoul(optarg, &s, 10);
                        fsize_max <<= 20;
@@ -327,6 +340,9 @@ void usage(char *prog)
         "-T timeout      Specifies upper limit on amount of time reader thread\n"
         "                will wait for new full trace buffer. Value should be an\n"
         "                integer >= 1, which is timeout value in ms. Default 200ms.\n\n"
+#ifdef HAVE_OPENAT
+        "-F fd           Specifies file descriptor for module relay directory\n"
+#endif
        "MODULE can be either a module name or a module path.  If a\n"
        "module name is used, it is searched in the following directory:\n"));
         {

diff --git a/staprun/ctl.c b/staprun/ctl.c
index 9cc87ea66c9a6ee012177fb9b760ac809a0c6589..aa07b96597eee7895a078d93eb86dfd25340cb8a 100644 (file)
--- a/staprun/ctl.c
+++ b/staprun/ctl.c
@@ -7,23 +7,63 @@
  * Public License (GPL); either version 2, or (at your option) any
  * later version.
  *
- * Copyright (C) 2007 Red Hat Inc.
+ * Copyright (C) 2012 Red Hat Inc.
  */
 
 #include "staprun.h"
 
+#define CTL_CHANNEL_NAME ".cmd"
+
 int init_ctl_channel(const char *name, int verb)
 {
        char buf[PATH_MAX];
        struct statfs st;
        int old_transport = 0;
 
+        if (0) goto out; /* just to defeat gcc warnings */
+
+#ifdef HAVE_OPENAT
+        if (relay_basedir_fd >= 0) {
+                strncpy(buf, CTL_CHANNEL_NAME, PATH_MAX);
+                control_channel = openat(relay_basedir_fd, CTL_CHANNEL_NAME, O_RDWR);
+                dbug(2, "Opened %s (%d)\n", CTL_CHANNEL_NAME, control_channel);
+
+                /* NB: Extra real-id access check as below */
+                if (faccessat(relay_basedir_fd, CTL_CHANNEL_NAME, R_OK|W_OK, 0) != 0){
+                        close(control_channel);
+                        return -5;
+                }
+                if (control_channel >= 0)
+                        goto out; /* It's OK to bypass the [f]access[at] check below,
+                                     since this would only occur the *second* time 
+                                     staprun tries this gig, or within unprivileged stapio. */
+        }
+        /* PR14245, NB: we fall through to /sys ... /proc searching,
+           in case the relay_basedir_fd option wasn't given (i.e., for
+           early in staprun), or if errors out for some reason. */
+#endif
+
        if (statfs("/sys/kernel/debug", &st) == 0 && (int)st.f_type == (int)DEBUGFS_MAGIC) {
-               if (sprintf_chk(buf, "/sys/kernel/debug/systemtap/%s/.cmd", name))
+                /* PR14245: allow subsequent operations, and if
+                   necessary, staprun->stapio forks, to reuse an fd for 
+                   directory lookups (even if some parent directories have
+                   perms 0700. */
+#ifdef HAVE_OPENAT
+                if (! sprintf_chk(buf, "/sys/kernel/debug/systemtap/%s", name)) {
+                        relay_basedir_fd = open (buf, O_DIRECTORY | O_RDONLY);
+                        /* If this fails, we don't much care; the
+                           negative return value will just keep us
+                           looking up by name again next time. */
+                        /* NB: we don't plan to close this fd, so that we can pass
+                           it across staprun->stapio fork/execs. */
+                }
+#endif
+               if (sprintf_chk(buf, "/sys/kernel/debug/systemtap/%s/%s", 
+                                name, CTL_CHANNEL_NAME))
                        return -1;
        } else {
                old_transport = 1;
-               if (sprintf_chk(buf, "/proc/systemtap/%s/.cmd", name))
+               if (sprintf_chk(buf, "/proc/systemtap/%s/%s", name, CTL_CHANNEL_NAME))
                        return -2;
        }
 
@@ -33,18 +73,28 @@ int init_ctl_channel(const char *name, int verb)
        /* NB: Even if open() succeeded with effective-UID permissions, we
         * need the access() check to make sure real-UID permissions are also
         * sufficient.  When we run under the setuid staprun, effective and
-        * real UID may not be the same.
+        * real UID may not be the same.  Specifically, we want to prevent 
+         * a local stapusr from trying to attach to a different stapusr's module.
         *
         * The access() is done *after* open() to avoid any TOCTOU-style race
         * condition.  We believe it's probably safe either way, as the file
         * we're trying to access connot be modified by a typical user, but
         * better safe than sorry.
         */
+#ifdef HAVE_OPENAT
+        if (relay_basedir_fd >= 0) {
+                if (faccessat (relay_basedir_fd, CTL_CHANNEL_NAME, R_OK|W_OK, 0) == 0)
+                        goto out;
+        }
+#endif
        if (access(buf, R_OK|W_OK) != 0){
                close(control_channel);
+               err("ERROR: no access to debugfs; try \"chmod 0755 %s\" as root\n",
+                     DEBUGFSDIR);
                return -5;
        }
 
+out:
        if (control_channel < 0) {
                if (verb) {
                        if (attach_mod && errno == ENOENT)

diff --git a/staprun/mainloop.c b/staprun/mainloop.c
index b8c39d2150c3adc16ffe5aa671dedcddfe588e18..e68efb30713e6708f483ac843e76770be23061e6 100644 (file)
--- a/staprun/mainloop.c
+++ b/staprun/mainloop.c
@@ -329,6 +329,8 @@ static void read_buffer_info(void)
   struct statfs st;
   int fd, len, ret;
 
+  /* NB: we don't have to worry about PR14245 on old_transport aka
+     rhel4; no HAVE_OPENAT, and thus no -F fd option. */
   if (!use_old_transport)
     return;
 

diff --git a/staprun/relay.c b/staprun/relay.c
index d5c64b7d25630da7292c66d7a4c0440acd749b1b..864a8f9a10cb2b326f8665724f02c3eabdae97a1 100644 (file)
--- a/staprun/relay.c
+++ b/staprun/relay.c
@@ -7,7 +7,7 @@
  * Public License (GPL); either version 2, or (at your option) any
  * later version.
  *
- * Copyright (C) 2007 Red Hat Inc.
+ * Copyright (C) 2007-2012 Red Hat Inc.
  */
 
 #include "staprun.h"
@@ -247,14 +247,15 @@ int init_relayfs(void)
        relay_fd[0] = 0;
        out_fd[0] = 0;
 
-       if (statfs("/sys/kernel/debug", &st) == 0
+        if (relay_basedir_fd >= 0)
+                strcpy(relay_filebase, "\0");
+       else if (statfs("/sys/kernel/debug", &st) == 0
            && (int) st.f_type == (int) DEBUGFS_MAGIC) {
                if (sprintf_chk(relay_filebase,
-                               "/sys/kernel/debug/systemtap/%s",
+                               "/sys/kernel/debug/systemtap/%s/",
                                modname))
                        return -1;
-       }
-       else {
+       } else {
                err("Cannot find relayfs or debugfs mount point.\n");
                return -1;
        }
@@ -263,10 +264,16 @@ int init_relayfs(void)
                bulkmode = 1;
 
        for (i = 0; i < NR_CPUS; i++) {
-               if (sprintf_chk(buf, "%s/trace%d", relay_filebase, i))
+               if (sprintf_chk(buf, "%strace%d", relay_filebase, i))
                        return -1;
                dbug(2, "attempting to open %s\n", buf);
-               relay_fd[i] = open(buf, O_RDONLY | O_NONBLOCK);
+                relay_fd[i] = -1;
+#ifdef HAVE_OPENAT
+                if (relay_basedir_fd >= 0)
+                        relay_fd[i] = openat(relay_basedir_fd, buf, O_RDONLY | O_NONBLOCK);
+#endif
+                if (relay_fd[i] < 0)
+                        relay_fd[i] = open(buf, O_RDONLY | O_NONBLOCK);
                if (relay_fd[i] < 0 || set_clexec(relay_fd[i]) < 0)
                        break;
        }

diff --git a/staprun/staprun.c b/staprun/staprun.c
index e02a2dd1f04c1b68138cd0fbd8db5dfcc0617b85..dd3b1f5a4b990967309c66558e749b0fcb819d85 100644 (file)
--- a/staprun/staprun.c
+++ b/staprun/staprun.c
@@ -204,6 +204,7 @@ static void remove_all_modules(void)
        struct dirent *d;
        DIR *moddir;
 
+        /* NB: nothing to do with PR14245 */
        if (statfs("/sys/kernel/debug", &st) == 0 && (int)st.f_type == (int)DEBUGFS_MAGIC)
                base = "/sys/kernel/debug/systemtap";
        else
@@ -386,6 +387,17 @@ int main(int argc, char **argv)
 
        parse_args(argc, argv);
 
+        /* PR14245, For security reasons, preclude "staprun -F fd".
+           The -F option is only for stapio, but the overzealous quest
+           for commonality doesn't let us express that nicer. */
+        if (relay_basedir_fd >= 0) {
+                err(_("ERROR: relay basedir -F option is invalid for staprun\n"));
+                exit(1);
+        }
+        /* NB: later on, some of our own code may set relay_basedir_fd, for
+           passing onto stapio - or for our own reuse.  That's OK.  */
+
+
        if (buffer_size)
                dbug(2, "Using a buffer of %u MB.\n", buffer_size);
 
@@ -427,6 +439,27 @@ int main(int argc, char **argv)
        if(rename_mod)
                argv[mod_optind] = modname;
 
+        /* PR14245: pass -F fd to stapio. Unfortunately, this requires
+           us to extend argv[], with all the C fun that entails. */
+#ifdef HAVE_OPENAT
+        if (relay_basedir_fd >= 0) {
+                char ** new_argv = calloc(sizeof(char *),argc+1);
+                const int new_Foption_size = 10; /* -FNNNNN */
+                char * new_Foption = malloc(new_Foption_size);
+                int i;
+
+                if (new_argv && new_Foption) {
+                        snprintf (new_Foption, new_Foption_size, "-F%d", relay_basedir_fd);
+                        for (i=0; argv[i] != NULL; i++)
+                                new_argv[i] = argv[i];
+                        new_argv[i++] = new_Foption; /* overwrite the NULL */
+                        new_argv[i++] = NULL; /* ensconce a new NULL */
+
+                        argv = new_argv;
+                }
+        }
+#endif
+
        /* Run stapio */
        if (run_as (1, getuid(), getgid(), argv[0], argv) < 0) {
                perror(argv[0]);

diff --git a/staprun/staprun.h b/staprun/staprun.h
index 821b485397df483249c1d0f09bcaacc39ebe11ea..39cbcaeefeb4732758d7e65ee2c8a58a46deee45 100644 (file)
--- a/staprun/staprun.h
+++ b/staprun/staprun.h
@@ -231,6 +231,7 @@ extern off_t fsize_max;
 extern int fnum_max;
 extern int remote_id;
 extern const char *remote_uri;
+extern int relay_basedir_fd;
 
 /* getopt variables */
 extern char *optarg;

diff --git a/staprun/staprun_funcs.c b/staprun/staprun_funcs.c
index d80913dd0c64749366d60193746f86fa60f4d99f..4bc691b841a79be8f15a7b3f8f9d8bb104280f26 100644 (file)
--- a/staprun/staprun_funcs.c
+++ b/staprun/staprun_funcs.c
@@ -182,6 +182,7 @@ int insert_module(
         if (getenv ("SYSTEMTAP_SYNC") != NULL)
                 sync();
 
+        dbug(2,"Module %s inserted from file %s\n", modname, module_realpath);
        PROBE1(staprun, insert__module, (char*)module_realpath);
        /* Actually insert the module */
        ret = init_module(module_file, sbuf.st_size, opts);
@@ -311,17 +312,6 @@ rename_module(void* module_file, const __off_t st_size)
 #endif
 }
 
-static int
-access_debugfs(void)
-{
-       /* We need to make sure that debugfs is accessible by the real UID, or
-        * else we won't be able to reach the .ctl path within. (PR14244) */
-       int rc = access(DEBUGFSDIR, X_OK);
-       if (rc < 0)
-               err("ERROR: no access to debugfs; try \"chmod 0755 %s\" as root\n",
-                               DEBUGFSDIR);
-       return rc;
-}
 
 int mountfs(void)
 {
@@ -332,7 +322,7 @@ int mountfs(void)
        /* If the debugfs dir is already mounted correctly, we're done. */
        if (statfs(DEBUGFSDIR, &st) == 0
            && (int) st.f_type == (int) DEBUGFS_MAGIC)
-               return access_debugfs();
+               return 0;
 
        /* If DEBUGFSDIR exists (and is a directory), try to mount
         * DEBUGFSDIR. */
@@ -341,7 +331,7 @@ int mountfs(void)
                /* If we can mount the debugfs dir correctly, we're done. */
                rc = mount ("debugfs", DEBUGFSDIR, "debugfs", 0, NULL);
                if (rc == 0)
-                       return access_debugfs();
+                       return 0;
                /* If we got ENODEV, that means that debugfs isn't
                 * supported, so we'll need try try relayfs.  If we
                 * didn't get ENODEV, we got a real error. */