diff --git a/backtrace-gdb.patch b/backtrace-gdb.patch new file mode 100644 index 0000000..d1de0e3 --- /dev/null +++ b/backtrace-gdb.patch @@ -0,0 +1,205 @@ +From 8eab929c763e3a951794968f1a7d2f99a83c4f9d Mon Sep 17 00:00:00 2001 +From: Stef Walter +Date: Thu, 27 Aug 2015 09:37:10 +0200 +Subject: [PATCH 1/2] common: Produce better backtraces with gdb when possible + +Note that this may require this sysctl to be set on the system: + +/proc/sys/kernel/yama/ptrace_scope = 0 + +https://wiki.ubuntu.com/SecurityTeam/Roadmap/KernelHardening#ptrace%20Protection + +Reviewed-by: Peter +--- + .travis.yml | 2 +- + src/common/cockpittest.c | 155 ++++++++++++++++++++++++++++++++++++++++++++++- + tools/cockpit.spec | 1 + + 3 files changed, 154 insertions(+), 4 deletions(-) + +diff --git a/src/common/cockpittest.c b/src/common/cockpittest.c +index 5501681..bf20b11 100644 +--- a/src/common/cockpittest.c ++++ b/src/common/cockpittest.c +@@ -39,6 +39,10 @@ + #include + #include + ++#include ++#include ++#include ++ + /* + * HACK: We can't yet use g_test_expect_message() and friends. + * They were pretty broken until GLib 2.40 if you have any debug +@@ -466,20 +470,165 @@ _cockpit_assert_bytes_eq_msg (const char *domain, + expect, exp_len); + } + ++/* ++ * This gdb code only works if /proc/sys/kernel/yama/ptrace_scope is set to zero ++ * See: https://wiki.ubuntu.com/SecurityTeam/Roadmap/KernelHardening#ptrace%20Protection ++ */ ++ ++static gboolean stack_trace_done = FALSE; ++ ++static void ++stack_trace_sigchld (int signum) ++{ ++ stack_trace_done = TRUE; ++} ++ ++static void ++stack_trace (char **args) ++{ ++ pid_t pid; ++ int in_fd[2]; ++ int out_fd[2]; ++ fd_set fdset; ++ fd_set readset; ++ struct timeval tv; ++ int sel, idx, state; ++ char buffer[256]; ++ char c; ++ ++ stack_trace_done = FALSE; ++ signal (SIGCHLD, stack_trace_sigchld); ++ ++ if ((pipe (in_fd) == -1) || (pipe (out_fd) == -1)) ++ { ++ perror ("unable to open pipe"); ++ _exit (0); ++ } ++ ++ pid = fork (); ++ if (pid == 0) ++ { ++ /* Save stderr for printing failure below */ ++ int old_err = dup (2); ++ fcntl (old_err, F_SETFD, fcntl (old_err, F_GETFD) | FD_CLOEXEC); ++ ++ close (0); dup (in_fd[0]); /* set the stdin to the in pipe */ ++ close (1); dup (out_fd[1]); /* set the stdout to the out pipe */ ++ ++ execvp (args[0], args); /* exec gdb */ ++ ++ /* Print failure to original stderr */ ++ perror ("exec gdb failed"); ++ _exit (0); ++ } ++ else if (pid == (pid_t) -1) ++ { ++ perror ("unable to fork"); ++ _exit (0); ++ } ++ ++ FD_ZERO (&fdset); ++ FD_SET (out_fd[0], &fdset); ++ ++ write (in_fd[1], "backtrace\n", 10); ++ write (in_fd[1], "quit\n", 5); ++ ++ idx = 0; ++ state = 0; ++ ++ while (1) ++ { ++ readset = fdset; ++ tv.tv_sec = 1; ++ tv.tv_usec = 0; ++ ++ sel = select (FD_SETSIZE, &readset, NULL, NULL, &tv); ++ if (sel == -1) ++ break; ++ ++ if ((sel > 0) && (FD_ISSET (out_fd[0], &readset))) ++ { ++ if (read (out_fd[0], &c, 1)) ++ { ++ switch (state) ++ { ++ case 0: ++ if (c == '#') ++ { ++ state = 1; ++ idx = 0; ++ buffer[idx++] = c; ++ } ++ break; ++ case 1: ++ buffer[idx++] = c; ++ if ((c == '\n') || (c == '\r')) ++ { ++ buffer[idx] = 0; ++ fprintf (stderr, "%s", buffer); ++ state = 0; ++ idx = 0; ++ } ++ break; ++ default: ++ break; ++ } ++ } ++ } ++ else if (stack_trace_done) ++ break; ++ } ++ ++ close (in_fd[0]); ++ close (in_fd[1]); ++ close (out_fd[0]); ++ close (out_fd[1]); ++ _exit (0); ++} ++ ++static void ++gdb_stack_trace (void) ++{ ++ pid_t pid; ++ gchar buf[16]; ++ gchar *args[4] = { "gdb", "-p", buf, NULL }; ++ int status; ++ ++ sprintf (buf, "%u", (guint) getpid ()); ++ ++ pid = fork (); ++ if (pid == 0) ++ { ++ stack_trace (args); ++ _exit (0); ++ } ++ else if (pid == (pid_t) -1) ++ { ++ perror ("unable to fork gdb"); ++ return; ++ } ++ ++ waitpid (pid, &status, 0); ++} ++ + void + cockpit_test_signal_backtrace (int sig) + { + void *array[16]; + size_t size; + +- // get void*'s for all entries on the stack ++ signal (sig, SIG_DFL); ++ ++ /* Try to trace with gdb first */ ++ gdb_stack_trace (); ++ ++ /* In case above didn't work, print raw stack trace */ + size = backtrace (array, G_N_ELEMENTS (array)); + +- // print out all the frames to stderr ++ /* print out all the frames to stderr */ + fprintf (stderr, "Error: signal %s:\n", strsignal (sig)); + backtrace_symbols_fd (array, size, STDERR_FILENO); + +- signal (sig, SIG_DFL); + raise (sig); + } + +-- +2.4.3 + diff --git a/backtrace-webservice.patch b/backtrace-webservice.patch new file mode 100644 index 0000000..7b9f93b --- /dev/null +++ b/backtrace-webservice.patch @@ -0,0 +1,72 @@ +From 1d480347c730596c23a8dcb769e6e46895862c68 Mon Sep 17 00:00:00 2001 +From: Stef Walter +Date: Thu, 27 Aug 2015 09:38:55 +0200 +Subject: [PATCH 2/2] ws: Add backtrace handler to test-webservice to debug + issues + +Closes #2633 +Reviewed-by: Peter +--- + src/common/cockpittest.c | 24 +++++++++++++++--------- + src/ws/test-webservice.c | 3 +++ + 2 files changed, 18 insertions(+), 9 deletions(-) + +diff --git a/src/common/cockpittest.c b/src/common/cockpittest.c +index bf20b11..2aeb0ad 100644 +--- a/src/common/cockpittest.c ++++ b/src/common/cockpittest.c +@@ -512,13 +512,15 @@ stack_trace (char **args) + int old_err = dup (2); + fcntl (old_err, F_SETFD, fcntl (old_err, F_GETFD) | FD_CLOEXEC); + +- close (0); dup (in_fd[0]); /* set the stdin to the in pipe */ +- close (1); dup (out_fd[1]); /* set the stdout to the out pipe */ +- +- execvp (args[0], args); /* exec gdb */ +- +- /* Print failure to original stderr */ +- perror ("exec gdb failed"); ++ if (dup2 (in_fd[0], 0) < 0 || dup2 (out_fd[1], 1) < 0) ++ { ++ perror ("dup fds failed"); ++ } ++ else ++ { ++ execvp (args[0], args); ++ perror ("exec gdb failed"); ++ } + _exit (0); + } + else if (pid == (pid_t) -1) +@@ -530,8 +532,12 @@ stack_trace (char **args) + FD_ZERO (&fdset); + FD_SET (out_fd[0], &fdset); + +- write (in_fd[1], "backtrace\n", 10); +- write (in_fd[1], "quit\n", 5); ++ if (write (in_fd[1], "backtrace\n", 10) != 10 || ++ write (in_fd[1], "quit\n", 5) != 5) ++ { ++ perror ("unable to send commands to gdb"); ++ _exit (0); ++ } + + idx = 0; + state = 0; +diff --git a/src/ws/test-webservice.c b/src/ws/test-webservice.c +index 0cb96b2..3a2dca8 100644 +--- a/src/ws/test-webservice.c ++++ b/src/ws/test-webservice.c +@@ -2028,6 +2028,9 @@ main (int argc, + */ + g_timeout_add_seconds (1, on_hack_raise_sigchld, NULL); + ++ /* Try to debug crashing during tests */ ++ signal (SIGSEGV, cockpit_test_signal_backtrace); ++ + /* We don't want to test the ping functionality in these tests */ + cockpit_ws_ping_interval = G_MAXUINT; + +-- +2.4.3 + diff --git a/cockpit.spec b/cockpit.spec index 2d89ca3..c1d17c0 100644 --- a/cockpit.spec +++ b/cockpit.spec @@ -57,6 +57,10 @@ Source0: cockpit-%{version}.tar.gz Source0: https://github.com/cockpit-project/cockpit/releases/download/%{version}/cockpit-%{version}.tar.bz2 %endif Source1: cockpit.pam +Patch0: test-use-after-free.patch +Patch1: relax-test-fs.patch +Patch10: backtrace-gdb.patch +Patch11: backtrace-webservice.patch BuildRequires: pkgconfig(gio-unix-2.0) BuildRequires: pkgconfig(json-glib-1.0) @@ -76,6 +80,9 @@ BuildRequires: dbus-devel BuildRequires: glib-networking BuildRequires: sed +BuildRequires: gdb +BuildRequires: valgrind + BuildRequires: glib2-devel >= 2.37.4 BuildRequires: systemd-devel BuildRequires: polkit @@ -179,6 +186,10 @@ The Cockpit Web Service listens on the network, and authenticates users. %if 0%{?fedora} == 20 sed -i s/unconfined_service_t/unconfined_t/g src/ws/test-server.service.in %endif +%patch0 -p1 +%patch1 -p1 +%patch10 -p1 +%patch11 -p1 %build %if %{defined gitcommit} @@ -263,11 +274,7 @@ cat subscriptions.list docker.list >> shell.list %endif # Only strip out debug info in non wip builds -%if %{defined gitcommit} %define find_debug_info %{nil} -%else -%define find_debug_info %{_rpmconfigdir}/find-debuginfo.sh %{?_missing_build_ids_terminate_build:--strict-build-id} %{?_include_minidebuginfo:-m} %{?_find_debuginfo_dwz_opts} %{?_find_debuginfo_opts} "%{_builddir}/%{?buildsubdir}" -%endif # Redefine how debug info is built to slip in our extra debug files %define __debug_install_post \ diff --git a/relax-test-fs.patch b/relax-test-fs.patch new file mode 100644 index 0000000..163f1bc --- /dev/null +++ b/relax-test-fs.patch @@ -0,0 +1,36 @@ +From 969caaa104a7567192c23bb1cd2964e3c2bf686e Mon Sep 17 00:00:00 2001 +From: Stef Walter +Date: Wed, 2 Sep 2015 12:15:40 +0200 +Subject: [PATCH] bridge: Fix test-fs to account for different glib2 behavior + +Different versions of glib2 send the GFileMonitor events in +different order, some see "created" first, others "deleted" first. + +So relax the test a bit to account for this. +--- + src/bridge/test-fs.c | 9 +++++++++ + 1 file changed, 9 insertions(+) + +diff --git a/src/bridge/test-fs.c b/src/bridge/test-fs.c +index 32c66c8..fe2b947 100644 +--- a/src/bridge/test-fs.c ++++ b/src/bridge/test-fs.c +@@ -664,6 +664,15 @@ test_watch_simple (TestCase *tc, + tag = cockpit_get_file_tag (tc->test_path); + + event = recv_json (tc); ++ g_assert (event != NULL); ++ ++ /* Account for different behavior by different glib2 versions */ ++ if (g_str_equal (json_object_get_string_member (event, "event"), "deleted")) ++ { ++ json_object_unref (event); ++ event = recv_json (tc); ++ } ++ + g_assert_cmpstr (json_object_get_string_member (event, "event"), ==, "created"); + g_assert_cmpstr (json_object_get_string_member (event, "path"), ==, tc->test_path); + g_assert_cmpstr (json_object_get_string_member (event, "tag"), ==, tag); +-- +2.4.3 + diff --git a/test-use-after-free.patch b/test-use-after-free.patch new file mode 100644 index 0000000..bfbddee --- /dev/null +++ b/test-use-after-free.patch @@ -0,0 +1,36 @@ +From fe2156eda6b42436dbfe82d1402ecfabc1c2ffd4 Mon Sep 17 00:00:00 2001 +From: Stef Walter +Date: Wed, 2 Sep 2015 13:26:16 +0200 +Subject: [PATCH] common: Hold reference during processing of messages + +This prevents use-after-free in tests, where a message triggers +the test completion. +--- + src/common/cockpitpipetransport.c | 4 ++++ + 1 file changed, 4 insertions(+) + +diff --git a/src/common/cockpitpipetransport.c b/src/common/cockpitpipetransport.c +index af9a8e6..743ec3f 100644 +--- a/src/common/cockpitpipetransport.c ++++ b/src/common/cockpitpipetransport.c +@@ -81,6 +81,8 @@ on_pipe_read_transport (CockpitPipe *pipe, + guint32 i, size; + gchar *data; + ++ g_object_ref (self); ++ + for (;;) + { + size = 0; +@@ -135,6 +137,8 @@ on_pipe_read_transport (CockpitPipe *pipe, + cockpit_pipe_close (pipe, "internal-error"); + } + } ++ ++ g_object_unref (self); + } + + static void +-- +2.4.3 +