Bug#708243: pu: package systemtap/1.7-1+deb7u1
Package: release.debian.org
Severity: normal
Tags: wheezy
User: release.debian.org@packages.debian.org
Usertags: pu
systemtap currently only works as root. This regression occured during
the freeze when linux 3.2.32-1 migrated to testing in 2012-11-03 and
changed the permissions of /sys/kernel/debug to 0700. The issue got
unnoticed since I was mostly testing experimental kernels at such a late
stage of the freeze. You can see further details in the bug report at
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=706817 .
I would like to upload a stable update to address this issue. Upstream
has a patch that opens a specific debugfs directory in the privileged
staprun binary and passes a file descriptor so that the unprivileged
part can communicate with the kernel module.
I have tested the patch on both amd64 and i386. From the bug report you
can also see that author of the original upstream patch has briefly gone
through the patch to verify that I did not make any obvious mistakes
while backporting it.
Please let me know if you need any further information.
-Timo
diff -Nru systemtap-1.7/debian/changelog systemtap-1.7/debian/changelog
--- systemtap-1.7/debian/changelog 2012-03-14 14:45:25.000000000 +0200
+++ systemtap-1.7/debian/changelog 2013-05-14 10:26:06.000000000 +0300
@@ -1,3 +1,13 @@
+systemtap (1.7-1+deb7u1) stable; urgency=low
+
+ * Backport upstream commit c5f7c84bf1dcc515 (PR14245:
+ support /sys/kernel/debug mounted 0700) to cope with
+ new debugfs permissions introduced by linux 3.2.29-1
+ (Closes: #706817):
+ - PR14245-support-sys-kernel-debug-mounted-0700.patch
+
+ -- Timo Juhani Lindfors <timo.lindfors@iki.fi> Mon, 13 May 2013 20:59:26 +0000
+
systemtap (1.7-1) unstable; urgency=low
* Explicitly list supported architectures of systemtap-sdt-dev.
diff -Nru systemtap-1.7/debian/patches/PR14245-support-sys-kernel-debug-mounted-0700.patch systemtap-1.7/debian/patches/PR14245-support-sys-kernel-debug-mounted-0700.patch
--- systemtap-1.7/debian/patches/PR14245-support-sys-kernel-debug-mounted-0700.patch 1970-01-01 02:00:00.000000000 +0200
+++ systemtap-1.7/debian/patches/PR14245-support-sys-kernel-debug-mounted-0700.patch 2013-05-14 10:25:52.000000000 +0300
@@ -0,0 +1,270 @@
+Bug: http://sourceware.org/bugzilla/show_bug.cgi?id=14245
+Bug-Debian: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=706817
+Origin: backport, http://sourceware.org/git/gitweb.cgi?p=systemtap.git;a=commit;h=c5f7c84bf1dcc515465fe3a0d41dfb545adfc683
+Author: Timo Juhani Lindfors <timo.lindfors@iki.fi>
+From c5f7c84bf1dcc515465fe3a0d41dfb545adfc683 Mon Sep 17 00:00:00 2001
+From: "Frank Ch. Eigler" <fche@redhat.com>
+Date: Wed, 10 Oct 2012 18:10:40 -0400
+Subject: [PATCH] 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.
+---
+ staprun/common.c | 18 ++++++++++++++-
+ staprun/ctl.c | 58 +++++++++++++++++++++++++++++++++++++++++++----
+ staprun/mainloop.c | 2 ++
+ staprun/relay.c | 21 +++++++++++------
+ staprun/staprun.c | 33 +++++++++++++++++++++++++++
+ staprun/staprun.h | 1 +
+ staprun/staprun_funcs.c | 16 +++----------
+ 7 files changed, 124 insertions(+), 25 deletions(-)
+
+--- a/runtime/staprun/common.c
++++ b/runtime/staprun/common.c
+@@ -35,6 +35,7 @@
+ int fnum_max;
+ int remote_id;
+ const char *remote_uri;
++int relay_basedir_fd;
+
+ /* module variables */
+ char *modname = NULL;
+@@ -128,8 +129,9 @@
+ 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:")) != EOF) {
++ while ((c = getopt(argc, argv, "ALu::vb:t:dc:o:x:S:DwRr:F:")) != EOF) {
+ switch (c) {
+ case 'u':
+ need_uprobes = 1;
+@@ -175,6 +177,13 @@
+ 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;
+@@ -305,6 +314,7 @@
+ " When the number of output files reaches N, it\n"
+ " switches to the first output file. You can omit\n"
+ " the second argument.\n\n"
++ "-F fd Specifies file descriptor for module relay directory\n"
+ "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"));
+ {
+--- a/runtime/staprun/ctl.c
++++ b/runtime/staprun/ctl.c
+@@ -12,33 +12,74 @@
+
+ #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 (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. */
++
+ 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. */
++ 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. */
++ }
++ 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;
+ }
+
+ control_channel = open(buf, O_RDWR);
+ dbug(2, "Opened %s (%d)\n", buf, control_channel);
+
++ if (relay_basedir_fd >= 0) {
++ if (faccessat (relay_basedir_fd, CTL_CHANNEL_NAME, R_OK|W_OK, 0) == 0)
++ goto out;
++ }
+ /* It's actually safe to do this check before the open(),
+ * as the file we're trying to access connot be modified
+ * by a typical user.
+ */
+ 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)
+--- a/runtime/staprun/relay.c
++++ b/runtime/staprun/relay.c
+@@ -242,10 +242,12 @@
+ 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;
+ }
+@@ -258,10 +260,14 @@
+ 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;
++ if (relay_basedir_fd >= 0)
++ relay_fd[i] = openat(relay_basedir_fd, buf, O_RDONLY | O_NONBLOCK);
++ 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;
+ }
+--- a/runtime/staprun/staprun.c
++++ b/runtime/staprun/staprun.c
+@@ -376,6 +376,17 @@
+
+ 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);
+
+@@ -417,6 +428,25 @@
+ 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. */
++ 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;
++ }
++ }
++
+ /* Run stapio */
+ if (run_as (1, getuid(), getgid(), argv[0], argv) < 0) {
+ perror(argv[0]);
+--- a/runtime/staprun/staprun.h
++++ b/runtime/staprun/staprun.h
+@@ -215,6 +215,7 @@
+ extern int fnum_max;
+ extern int remote_id;
+ extern const char *remote_uri;
++extern int relay_basedir_fd;
+
+ /* getopt variables */
+ extern char *optarg;
+--- a/runtime/staprun/staprun_funcs.c
++++ b/runtime/staprun/staprun_funcs.c
+@@ -181,6 +181,7 @@
+ 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);
diff -Nru systemtap-1.7/debian/patches/series systemtap-1.7/debian/patches/series
--- systemtap-1.7/debian/patches/series 2012-03-14 14:17:32.000000000 +0200
+++ systemtap-1.7/debian/patches/series 2013-05-14 10:26:06.000000000 +0300
@@ -4,3 +4,4 @@
spelling-fixes2.diff
manpage-paths1.diff
fix.CVE-2012-0875.diff
+PR14245-support-sys-kernel-debug-mounted-0700.patch
Reply to: