From ffa322b80f512d7bcbe6cc7591b0a11c0e877a66 Mon Sep 17 00:00:00 2001 From: Aurelien Jarno Date: Thu, 16 Jul 2009 09:35:27 -0400 Subject: [PATCH 01/18] Bug 19432 followup - Fix Debian/FreeBSD CMSGCRED compilation --- dbus/dbus-sysdeps-unix.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c index 1268768b..66bcf3cb 100644 --- a/dbus/dbus-sysdeps-unix.c +++ b/dbus/dbus-sysdeps-unix.c @@ -1241,7 +1241,7 @@ _dbus_read_credentials_socket (int client_fd, #elif defined(HAVE_CMSGCRED) struct cmsgcred *cred; - cred = (struct cmsgcred *) CMSG_DATA (&cmsg); + cred = (struct cmsgcred *) CMSG_DATA (&cmsg.hdr); pid_read = cred->cmcred_pid; uid_read = cred->cmcred_euid; #elif defined(LOCAL_CREDS) From 1116f210aa2bfda9ee96112b81c1f2db100dee95 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 30 Jul 2009 09:48:20 -0400 Subject: [PATCH 02/18] Bug 22805 - Fix build with -Wl,--as-needed Explicitly link dbus_convenience.la against DBUS_CLIENT_LIBS because it uses $THREAD_LIBS, just like the main library does. --- dbus/Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/dbus/Makefile.am b/dbus/Makefile.am index e966a438..597fe08d 100644 --- a/dbus/Makefile.am +++ b/dbus/Makefile.am @@ -175,6 +175,7 @@ libdbus_1_la_LIBADD= $(DBUS_CLIENT_LIBS) ## convention for internal symbols) libdbus_1_la_LDFLAGS= -export-symbols-regex "^[^_].*" -version-info $(LT_CURRENT):$(LT_REVISION):$(LT_AGE) -no-undefined @R_DYNAMIC_LDFLAG@ @PIC_LDFLAGS@ +libdbus_convenience_la_LIBADD=$(DBUS_CLIENT_LIBS) libdbus_convenience_la_LDFLAGS=@R_DYNAMIC_LDFLAG@ ## note that TESTS has special meaning (stuff to use in make check) From 43b1f91865bcaa3929c9039fb4aa1c0ee34f54e8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 31 Jul 2009 12:26:57 -0400 Subject: [PATCH 03/18] dbus-monitor: use unbuffered stdout instead of handling SIGINT The current SIGINT handling of dbus-monitor ain't making too many people happy since it defers the exit to the next msg received -- which might be quite some time away often enough. This patch replaces the SIGINT handling by simply enabling line-buffered IO for STDOUT so that even if you redirect dbus-monitor into a file no lines get accidently lost and the effect of C-c is still immediate. halfline came up with the great idea to use setvbuf here instead of fflush()ing after each printf(). (Oh and the old signal handler was broken anyway, the flag should have been of type sigatomic_t and be marked volatile) Signed-off-by: Colin Walters --- tools/dbus-monitor.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tools/dbus-monitor.c b/tools/dbus-monitor.c index 873108bc..c3681289 100644 --- a/tools/dbus-monitor.c +++ b/tools/dbus-monitor.c @@ -33,8 +33,6 @@ #include -#include - #include "dbus-print-message.h" #ifdef DBUS_WIN @@ -214,6 +212,13 @@ main (int argc, char *argv[]) int i = 0, j = 0, numFilters = 0; char **filters = NULL; + + /* Set stdout to be unbuffered; this is basically so that if people + * do dbus-monitor > file, then send SIGINT via Control-C, they + * don't lose the last chunk of messages. + */ + setvbuf (stdout, NULL, _IOLBF, 0); + for (i = 1; i < argc; i++) { char *arg = argv[i]; @@ -339,10 +344,7 @@ main (int argc, char *argv[]) exit (1); } - /* we handle SIGINT so exit() is reached and flushes stdout */ - signal (SIGINT, sigint_handler); - while (dbus_connection_read_write_dispatch(connection, -1) - && !sigint_received) + while (dbus_connection_read_write_dispatch(connection, -1)) ; exit (0); lose: From be89ffacc9051238d9b99b1b3e4fa5f67a9c7f5f Mon Sep 17 00:00:00 2001 From: Colin Watson Date: Fri, 18 Sep 2009 18:23:39 +0100 Subject: [PATCH 04/18] Fix link order: system libraries should come after libdbus-convenience libdbus-convenience may use system libraries, but not the other way round. Most platforms don't care, but on some platforms this means that system libraries need to be listed after libdbus-convenience.la on the link line. --- bus/Makefile.am | 16 ++++++++-------- test/Makefile.am | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/bus/Makefile.am b/bus/Makefile.am index 3b4f69db..ad49e6dd 100644 --- a/bus/Makefile.am +++ b/bus/Makefile.am @@ -78,8 +78,8 @@ dbus_daemon_SOURCES= \ dbus_daemon_LDADD= \ $(EFENCE) \ - $(DBUS_BUS_LIBS) \ - $(top_builddir)/dbus/libdbus-convenience.la + $(top_builddir)/dbus/libdbus-convenience.la \ + $(DBUS_BUS_LIBS) dbus_daemon_LDFLAGS=@R_DYNAMIC_LDFLAG@ @SECTION_LDFLAGS@ @PIE_LDFLAGS@ @@ -103,8 +103,8 @@ dbus_daemon_launch_helper_SOURCES= \ $(LAUNCH_HELPER_SOURCES) dbus_daemon_launch_helper_LDADD= \ - $(DBUS_LAUNCHER_LIBS) \ - $(top_builddir)/dbus/libdbus-convenience.la + $(top_builddir)/dbus/libdbus-convenience.la \ + $(DBUS_LAUNCHER_LIBS) dbus_daemon_launch_helper_LDFLAGS=@R_DYNAMIC_LDFLAG@ @SECTION_LDFLAGS@ @@ -115,8 +115,8 @@ dbus_daemon_launch_helper_test_SOURCES= \ $(LAUNCH_HELPER_SOURCES) dbus_daemon_launch_helper_test_LDADD= \ - $(DBUS_LAUNCHER_LIBS) \ - $(top_builddir)/dbus/libdbus-convenience.la + $(top_builddir)/dbus/libdbus-convenience.la \ + $(DBUS_LAUNCHER_LIBS) dbus_daemon_launch_helper_test_LDFLAGS=@R_DYNAMIC_LDFLAG@ @SECTION_LDFLAGS@ dbus_daemon_launch_helper_test_CPPFLAGS= \ @@ -129,8 +129,8 @@ bus_test_launch_helper_SOURCES= \ $(LAUNCH_HELPER_SOURCES) bus_test_launch_helper_LDADD= \ - $(DBUS_LAUNCHER_LIBS) \ - $(top_builddir)/dbus/libdbus-convenience.la + $(top_builddir)/dbus/libdbus-convenience.la \ + $(DBUS_LAUNCHER_LIBS) bus_test_launch_helper_LDFLAGS=@R_DYNAMIC_LDFLAG@ @SECTION_LDFLAGS@ bus_test_launch_helper_CPPFLAGS= \ diff --git a/test/Makefile.am b/test/Makefile.am index a7f05970..c759906a 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -61,7 +61,7 @@ test_sleep_forever_SOURCES = \ decode_gcov_SOURCES= \ decode-gcov.c -TEST_LIBS=$(DBUS_TEST_LIBS) $(top_builddir)/dbus/libdbus-convenience.la +TEST_LIBS=$(top_builddir)/dbus/libdbus-convenience.la $(DBUS_TEST_LIBS) test_service_LDADD=$(TEST_LIBS) libdbus-testutils.la test_service_LDFLAGS=@R_DYNAMIC_LDFLAG@ From 9b2c196ef36260ef455f0746f003aec6ffe6ff4c Mon Sep 17 00:00:00 2001 From: Sascha Silbe Date: Fri, 16 Oct 2009 15:20:43 -0400 Subject: [PATCH 05/18] Bug 23977 - dbus-launch --exit-with-session not killing dbus-daemon on SIGINT Handle SIGINT in the same way we handle SIGTERM. --- tools/dbus-launch.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/dbus-launch.c b/tools/dbus-launch.c index 912afba8..d3553e81 100644 --- a/tools/dbus-launch.c +++ b/tools/dbus-launch.c @@ -405,6 +405,7 @@ signal_handler (int sig) #ifdef SIGHUP case SIGHUP: #endif + case SIGINT: case SIGTERM: got_sighup = TRUE; break; @@ -429,6 +430,7 @@ kill_bus_when_session_ends (void) act.sa_flags = 0; sigaction (SIGHUP, &act, NULL); sigaction (SIGTERM, &act, NULL); + sigaction (SIGINT, &act, NULL); #ifdef DBUS_BUILD_X11 x11_init(); From 8343c971cdedfda6107b0f4f73210d88f64507c6 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Fri, 18 Dec 2009 14:29:40 -0500 Subject: [PATCH 06/18] Bug 25697 - Fix memory leak in policy reload Signed-off-by: Colin Walters --- bus/bus.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bus/bus.c b/bus/bus.c index ede6dda6..b370f92b 100644 --- a/bus/bus.c +++ b/bus/bus.c @@ -433,6 +433,8 @@ process_config_every_time (BusContext *context, /* get our limits and timeout lengths */ bus_config_parser_get_limits (parser, &context->limits); + if (context->policy) + bus_policy_unref (context->policy); context->policy = bus_config_parser_steal_policy (parser); _dbus_assert (context->policy != NULL); From 6067f88afd3b09eb4173bad48eed79bead748a60 Mon Sep 17 00:00:00 2001 From: Hendrik Buschmeier Date: Thu, 28 Jan 2010 11:22:32 +0100 Subject: [PATCH 07/18] Bug 23502 - corrected wrong verbose-output --- dbus/dbus-transport.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dbus/dbus-transport.c b/dbus/dbus-transport.c index e3ed59e8..53289ad0 100644 --- a/dbus/dbus-transport.c +++ b/dbus/dbus-transport.c @@ -651,12 +651,12 @@ auth_via_default_rules (DBusTransport *transport) if (_dbus_credentials_include(our_identity,DBUS_CREDENTIAL_WINDOWS_SID)) _dbus_verbose ("Client authorized as SID '%s'" " but our SID is '%s', disconnecting\n", - _dbus_credentials_get_windows_sid(our_identity), + _dbus_credentials_get_windows_sid(auth_identity), _dbus_credentials_get_windows_sid(our_identity)); else _dbus_verbose ("Client authorized as UID "DBUS_UID_FORMAT " but our UID is "DBUS_UID_FORMAT", disconnecting\n", - _dbus_credentials_get_unix_uid(our_identity), + _dbus_credentials_get_unix_uid(auth_identity), _dbus_credentials_get_unix_uid(our_identity)); _dbus_transport_disconnect (transport); allow = FALSE; From e2aee8bdebc0f95ff626680b9882e74442c05f98 Mon Sep 17 00:00:00 2001 From: James Westby Date: Thu, 1 Oct 2009 15:09:54 +0100 Subject: [PATCH 08/18] Correct timeout handling The timeout handling code subtracts the elapsed time from the timeout each time a message is received, which drastically reduces the timeout in circumstances such as service activation. Correct so that the timeout is never modified, and the elapsed time instead subtracted where necessary. Signed-off-by: James Westby Signed-off-by: Scott James Remnant --- dbus/dbus-connection.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index c933d7d1..5fb234dc 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -2386,7 +2386,7 @@ _dbus_connection_block_pending_call (DBusPendingCall *pending) */ _dbus_verbose ("dbus_connection_send_with_reply_and_block() waiting for more memory\n"); - _dbus_memory_pause_based_on_timeout (timeout_milliseconds); + _dbus_memory_pause_based_on_timeout (timeout_milliseconds - elapsed_milliseconds); } else { @@ -2394,7 +2394,7 @@ _dbus_connection_block_pending_call (DBusPendingCall *pending) _dbus_connection_do_iteration_unlocked (connection, DBUS_ITERATION_DO_READING | DBUS_ITERATION_BLOCK, - timeout_milliseconds); + timeout_milliseconds - elapsed_milliseconds); } goto recheck_status; @@ -2403,9 +2403,7 @@ _dbus_connection_block_pending_call (DBusPendingCall *pending) _dbus_verbose ("dbus_connection_send_with_reply_and_block(): clock set backward\n"); else if (elapsed_milliseconds < timeout_milliseconds) { - timeout_milliseconds -= elapsed_milliseconds; - _dbus_verbose ("dbus_connection_send_with_reply_and_block(): %d milliseconds remain\n", timeout_milliseconds); - _dbus_assert (timeout_milliseconds >= 0); + _dbus_verbose ("dbus_connection_send_with_reply_and_block(): %d milliseconds remain\n", timeout_milliseconds - elapsed_milliseconds); if (status == DBUS_DISPATCH_NEED_MEMORY) { @@ -2415,7 +2413,7 @@ _dbus_connection_block_pending_call (DBusPendingCall *pending) */ _dbus_verbose ("dbus_connection_send_with_reply_and_block() waiting for more memory\n"); - _dbus_memory_pause_based_on_timeout (timeout_milliseconds); + _dbus_memory_pause_based_on_timeout (timeout_milliseconds - elapsed_milliseconds); } else { @@ -2423,14 +2421,14 @@ _dbus_connection_block_pending_call (DBusPendingCall *pending) _dbus_connection_do_iteration_unlocked (connection, DBUS_ITERATION_DO_READING | DBUS_ITERATION_BLOCK, - timeout_milliseconds); + timeout_milliseconds - elapsed_milliseconds); } goto recheck_status; } _dbus_verbose ("dbus_connection_send_with_reply_and_block(): Waited %ld milliseconds and got no reply\n", - (tv_sec - start_tv_sec) * 1000 + (tv_usec - start_tv_usec) / 1000); + elapsed_milliseconds); _dbus_assert (!_dbus_pending_call_get_completed_unlocked (pending)); From 7a8fcdee22d2716f7891e030cda0e1e6536d0054 Mon Sep 17 00:00:00 2001 From: Will Thompson Date: Wed, 9 Sep 2009 20:35:13 +0100 Subject: [PATCH 09/18] Make array-printing code easier to follow Previously dbus_message_iter_get_arg_type() was called twice: once in the loop condition to update 'current_type', and once to check if the loop will run again. This patch moves updating current_type to the end of the loop body. --- tools/dbus-print-message.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tools/dbus-print-message.c b/tools/dbus-print-message.c index 335aa3dc..749fca68 100644 --- a/tools/dbus-print-message.c +++ b/tools/dbus-print-message.c @@ -186,12 +186,17 @@ print_iter (DBusMessageIter *iter, dbus_bool_t literal, int depth) dbus_message_iter_recurse (iter, &subiter); + current_type = dbus_message_iter_get_arg_type (&subiter); + printf("array [\n"); - while ((current_type = dbus_message_iter_get_arg_type (&subiter)) != DBUS_TYPE_INVALID) + while (current_type != DBUS_TYPE_INVALID) { print_iter (&subiter, literal, depth+1); + dbus_message_iter_next (&subiter); - if (dbus_message_iter_get_arg_type (&subiter) != DBUS_TYPE_INVALID) + current_type = dbus_message_iter_get_arg_type (&subiter); + + if (current_type != DBUS_TYPE_INVALID) printf (","); } indent(depth); From ce87333cac219b84ddec990b155c54ce7cc02f68 Mon Sep 17 00:00:00 2001 From: Will Thompson Date: Sat, 8 Aug 2009 14:03:46 +0100 Subject: [PATCH 10/18] Forbid zero serial numbers --- doc/dbus-specification.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/dbus-specification.xml b/doc/dbus-specification.xml index 9b22c84a..286a86e3 100644 --- a/doc/dbus-specification.xml +++ b/doc/dbus-specification.xml @@ -776,7 +776,7 @@ 2nd UINT32 The serial of this message, used as a cookie by the sender to identify the reply corresponding - to this request. + to this request. This must not be zero. From 7cf332d34ea974b9781f47f4af4efca4de71b96a Mon Sep 17 00:00:00 2001 From: Will Thompson Date: Sat, 8 Aug 2009 13:57:11 +0100 Subject: [PATCH 11/18] Include reason when reporting corrupt messages It would have been much easier to diagnose fd.o#19723 if the error message had said more than just "Message is corrupted". --- dbus/dbus-message.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c index b7b5afca..0b81806f 100644 --- a/dbus/dbus-message.c +++ b/dbus/dbus-message.c @@ -4091,7 +4091,8 @@ dbus_message_demarshal (const char *str, return msg; fail_corrupt: - dbus_set_error (error, DBUS_ERROR_INVALID_ARGS, "Message is corrupted"); + dbus_set_error (error, DBUS_ERROR_INVALID_ARGS, "Message is corrupted (%s)", + _dbus_validity_to_error_message (loader->corruption_reason)); _dbus_message_loader_unref (loader); return NULL; From 3171383ce9bac0c2718da4ce288db890d5d7bfda Mon Sep 17 00:00:00 2001 From: Will Thompson Date: Sat, 8 Aug 2009 14:29:12 +0100 Subject: [PATCH 12/18] Add an accessor for the loader's corruption reason --- dbus/dbus-message-internal.h | 2 ++ dbus/dbus-message.c | 15 +++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/dbus/dbus-message-internal.h b/dbus/dbus-message-internal.h index 7cd88d56..0134f8db 100644 --- a/dbus/dbus-message-internal.h +++ b/dbus/dbus-message-internal.h @@ -23,6 +23,7 @@ #ifndef DBUS_MESSAGE_INTERNAL_H #define DBUS_MESSAGE_INTERNAL_H +#include #include #include #include @@ -62,6 +63,7 @@ void _dbus_message_loader_putback_message_link (DBusMessageLoader DBusList *link); dbus_bool_t _dbus_message_loader_get_is_corrupted (DBusMessageLoader *loader); +DBusValidity _dbus_message_loader_get_corruption_reason (DBusMessageLoader *loader); void _dbus_message_loader_set_max_message_size (DBusMessageLoader *loader, long size); diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c index 0b81806f..272592e8 100644 --- a/dbus/dbus-message.c +++ b/dbus/dbus-message.c @@ -3784,6 +3784,21 @@ _dbus_message_loader_get_is_corrupted (DBusMessageLoader *loader) return loader->corrupted; } +/** + * Checks what kind of bad data confused the loader. + * + * @param loader the loader + * @returns why the loader is hosed, or DBUS_VALID if it isn't. + */ +DBusValidity +_dbus_message_loader_get_corruption_reason (DBusMessageLoader *loader) +{ + _dbus_assert ((loader->corrupted && loader->corruption_reason != DBUS_VALID) || + (!loader->corrupted && loader->corruption_reason == DBUS_VALID)); + + return loader->corruption_reason; +} + /** * Sets the maximum size message we allow. * From 1a33efb54b6a40d48b64c735741298afc699bbd3 Mon Sep 17 00:00:00 2001 From: Will Thompson Date: Wed, 9 Sep 2009 20:58:53 +0100 Subject: [PATCH 13/18] Print byte arrays as nicely-formatted hex. --- tools/dbus-print-message.c | 59 +++++++++++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/tools/dbus-print-message.c b/tools/dbus-print-message.c index 749fca68..f934c7d1 100644 --- a/tools/dbus-print-message.c +++ b/tools/dbus-print-message.c @@ -39,12 +39,63 @@ type_to_name (int message_type) } } +#define INDENT 3 static void indent (int depth) { while (depth-- > 0) - printf (" "); + printf (" "); /* INDENT spaces. */ +} + +static void +print_ay (DBusMessageIter *iter, int depth) +{ + int current_type; + int n = 0; + int columns; + + printf ("array of bytes [\n"); + + indent (depth + 1); + + /* Each byte takes 3 cells (two hexits, and a space), except the last one. */ + columns = (80 - ((depth + 1) * INDENT)) / 3; + + if (columns < 8) + columns = 8; + + current_type = dbus_message_iter_get_arg_type (iter); + + while (current_type != DBUS_TYPE_INVALID) + { + unsigned char val; + dbus_message_iter_get_basic (iter, &val); + printf ("%02x", val); + + n++; + + dbus_message_iter_next (iter); + current_type = dbus_message_iter_get_arg_type (iter); + + if (current_type != DBUS_TYPE_INVALID) + { + if (n == columns) + { + printf ("\n"); + indent (depth + 1); + n = 0; + } + else + { + printf (" "); + } + } + } + + printf ("\n"); + indent (depth); + printf ("]\n"); } static void @@ -188,6 +239,12 @@ print_iter (DBusMessageIter *iter, dbus_bool_t literal, int depth) current_type = dbus_message_iter_get_arg_type (&subiter); + if (current_type == DBUS_TYPE_BYTE) + { + print_ay (&subiter, depth); + break; + } + printf("array [\n"); while (current_type != DBUS_TYPE_INVALID) { From a8e620a0ff60eef983a9be95e2b27dbcdcbaf654 Mon Sep 17 00:00:00 2001 From: Will Thompson Date: Wed, 28 Oct 2009 19:40:38 +0000 Subject: [PATCH 14/18] Print all-printable-ASCII byte arrays as strings In practice, ay seems to be used mostly for binary data (in which case, hex output is fine) or for Unix file paths (because they may be non-UTF-8) and similar human-readable strings. So let's print the latter similarly to strings. --- tools/dbus-print-message.c | 75 +++++++++++++++++++++++++++++--------- 1 file changed, 58 insertions(+), 17 deletions(-) diff --git a/tools/dbus-print-message.c b/tools/dbus-print-message.c index f934c7d1..8a8e351d 100644 --- a/tools/dbus-print-message.c +++ b/tools/dbus-print-message.c @@ -21,6 +21,8 @@ */ #include "dbus-print-message.h" +#include + static const char* type_to_name (int message_type) { @@ -49,11 +51,9 @@ indent (int depth) } static void -print_ay (DBusMessageIter *iter, int depth) +print_hex (unsigned char *bytes, unsigned int len, int depth) { - int current_type; - int n = 0; - int columns; + int i, columns; printf ("array of bytes [\n"); @@ -65,26 +65,19 @@ print_ay (DBusMessageIter *iter, int depth) if (columns < 8) columns = 8; - current_type = dbus_message_iter_get_arg_type (iter); + i = 0; - while (current_type != DBUS_TYPE_INVALID) + while (i < len) { - unsigned char val; - dbus_message_iter_get_basic (iter, &val); - printf ("%02x", val); + printf ("%02x", bytes[i]); + i++; - n++; - - dbus_message_iter_next (iter); - current_type = dbus_message_iter_get_arg_type (iter); - - if (current_type != DBUS_TYPE_INVALID) + if (i != len) { - if (n == columns) + if (i % columns == 0) { printf ("\n"); indent (depth + 1); - n = 0; } else { @@ -98,6 +91,54 @@ print_ay (DBusMessageIter *iter, int depth) printf ("]\n"); } +#define DEFAULT_SIZE 100 + +static void +print_ay (DBusMessageIter *iter, int depth) +{ + /* Not using DBusString because it's not public API. It's 2009, and I'm + * manually growing a string chunk by chunk. + */ + unsigned char *bytes = malloc (DEFAULT_SIZE + 1); + unsigned int len = 0; + unsigned int max = DEFAULT_SIZE; + dbus_bool_t all_ascii = TRUE; + int current_type; + + while ((current_type = dbus_message_iter_get_arg_type (iter)) + != DBUS_TYPE_INVALID) + { + unsigned char val; + + dbus_message_iter_get_basic (iter, &val); + bytes[len] = val; + len++; + + if (val < 32 || val > 126) + all_ascii = FALSE; + + if (len == max) + { + max *= 2; + bytes = realloc (bytes, max + 1); + } + + dbus_message_iter_next (iter); + } + + if (all_ascii) + { + bytes[len] = '\0'; + printf ("array of bytes \"%s\"\n", bytes); + } + else + { + print_hex (bytes, len, depth); + } + + free (bytes); +} + static void print_iter (DBusMessageIter *iter, dbus_bool_t literal, int depth) { From b7e77c6b035f634f372c1749e61c14bde18b5d95 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 14 Dec 2009 18:12:24 -0500 Subject: [PATCH 15/18] Ignore exit code zero from activated services A variety of system components have migrated from legacy init into DBus service activation. Many of these system components "daemonize", which involves forking. The DBus activation system treated an exit as an activation failure, assuming that the child process which grabbed the DBus name didn't run first. While we're in here, also differentiate in this code path between the servicehelper (system) versus direct activation (session) paths. In the session activation path our error message mentioned a helper process which was confusing, since none was involved. Based on a patch and debugging research from Ray Strode --- bus/activation.c | 89 ++++++++++++------- configure.in | 1 + ...Bus.TestSuiteForkingEchoService.service.in | 3 + test/name-test/Makefile.am | 2 +- test/name-test/run-test.sh | 6 ++ test/name-test/test-activation-forking.py | 60 +++++++++++++ test/test-service.c | 32 ++++++- 7 files changed, 158 insertions(+), 35 deletions(-) create mode 100644 test/data/valid-service-files/org.freedesktop.DBus.TestSuiteForkingEchoService.service.in create mode 100644 test/name-test/test-activation-forking.py diff --git a/bus/activation.c b/bus/activation.c index 782ffed8..00caac27 100644 --- a/bus/activation.c +++ b/bus/activation.c @@ -1212,8 +1212,8 @@ pending_activation_failed (BusPendingActivation *pending_activation, * Depending on the exit code of the helper, set the error accordingly */ static void -handle_activation_exit_error (int exit_code, - DBusError *error) +handle_servicehelper_exit_error (int exit_code, + DBusError *error) { switch (exit_code) { @@ -1268,13 +1268,24 @@ babysitter_watch_callback (DBusWatch *watch, BusPendingActivation *pending_activation = data; dbus_bool_t retval; DBusBabysitter *babysitter; + dbus_bool_t uses_servicehelper; babysitter = pending_activation->babysitter; - + _dbus_babysitter_ref (babysitter); - + retval = dbus_watch_handle (watch, condition); + /* There are two major cases here; are we the system bus or the session? Here this + * is distinguished by whether or not we use a setuid helper launcher. With the launch helper, + * some process exit codes are meaningful, processed by handle_servicehelper_exit_error. + * + * In both cases though, just ignore when a process exits with status 0; it's possible for + * a program to (misguidedly) "daemonize", and that appears to us as an exit. This closes a race + * condition between this code and the child process claiming the bus name. + */ + uses_servicehelper = bus_context_get_servicehelper (pending_activation->activation->context) != NULL; + /* FIXME this is broken in the same way that * connection watches used to be; there should be * a separate callback for status change, instead @@ -1284,43 +1295,59 @@ babysitter_watch_callback (DBusWatch *watch, * Fixing this lets us move dbus_watch_handle * calls into dbus-mainloop.c */ - if (_dbus_babysitter_get_child_exited (babysitter)) { DBusError error; DBusHashIter iter; - + dbus_bool_t activation_failed; + int exit_code = 0; + dbus_error_init (&error); + _dbus_babysitter_set_child_exit_error (babysitter, &error); - /* refine the error code if we got an exit code */ - if (dbus_error_has_name (&error, DBUS_ERROR_SPAWN_CHILD_EXITED)) - { - int exit_code = 0; - if (_dbus_babysitter_get_child_exit_status (babysitter, &exit_code)) - { - dbus_error_free (&error); - handle_activation_exit_error (exit_code, &error); - } - } - - /* Destroy all pending activations with the same exec */ - _dbus_hash_iter_init (pending_activation->activation->pending_activations, - &iter); - while (_dbus_hash_iter_next (&iter)) + /* Explicitly check for SPAWN_CHILD_EXITED to avoid overwriting an + * exec error */ + if (dbus_error_has_name (&error, DBUS_ERROR_SPAWN_CHILD_EXITED) + && _dbus_babysitter_get_child_exit_status (babysitter, &exit_code)) { - BusPendingActivation *p = _dbus_hash_iter_get_value (&iter); - - if (p != pending_activation && strcmp (p->exec, pending_activation->exec) == 0) - pending_activation_failed (p, &error); - } - - /* Destroys the pending activation */ - pending_activation_failed (pending_activation, &error); + activation_failed = exit_code != 0; - dbus_error_free (&error); + dbus_error_free(&error); + + if (activation_failed) + { + if (uses_servicehelper) + handle_servicehelper_exit_error (exit_code, &error); + else + _dbus_babysitter_set_child_exit_error (babysitter, &error); + } + } + else + { + activation_failed = TRUE; + } + + if (activation_failed) + { + /* Destroy all pending activations with the same exec */ + _dbus_hash_iter_init (pending_activation->activation->pending_activations, + &iter); + while (_dbus_hash_iter_next (&iter)) + { + BusPendingActivation *p = _dbus_hash_iter_get_value (&iter); + + if (p != pending_activation && strcmp (p->exec, pending_activation->exec) == 0) + pending_activation_failed (p, &error); + } + + /* Destroys the pending activation */ + pending_activation_failed (pending_activation, &error); + + dbus_error_free (&error); + } } - + _dbus_babysitter_unref (babysitter); return retval; diff --git a/configure.in b/configure.in index 2e6422c1..7feb1a93 100644 --- a/configure.in +++ b/configure.in @@ -1441,6 +1441,7 @@ test/data/valid-config-files-system/debug-allow-all-pass.conf test/data/valid-config-files-system/debug-allow-all-fail.conf test/data/valid-service-files/org.freedesktop.DBus.TestSuite.PrivServer.service test/data/valid-service-files/org.freedesktop.DBus.TestSuiteEchoService.service +test/data/valid-service-files/org.freedesktop.DBus.TestSuiteForkingEchoService.service test/data/valid-service-files/org.freedesktop.DBus.TestSuiteSegfaultService.service test/data/valid-service-files/org.freedesktop.DBus.TestSuiteShellEchoServiceSuccess.service test/data/valid-service-files/org.freedesktop.DBus.TestSuiteShellEchoServiceFail.service diff --git a/test/data/valid-service-files/org.freedesktop.DBus.TestSuiteForkingEchoService.service.in b/test/data/valid-service-files/org.freedesktop.DBus.TestSuiteForkingEchoService.service.in new file mode 100644 index 00000000..49fcac39 --- /dev/null +++ b/test/data/valid-service-files/org.freedesktop.DBus.TestSuiteForkingEchoService.service.in @@ -0,0 +1,3 @@ +[D-BUS Service] +Name=org.freedesktop.DBus.TestSuiteForkingEchoService +Exec=@TEST_SERVICE_BINARY@ org.freedesktop.DBus.TestSuiteForkingEchoService fork diff --git a/test/name-test/Makefile.am b/test/name-test/Makefile.am index 1c73b877..d8e72d14 100644 --- a/test/name-test/Makefile.am +++ b/test/name-test/Makefile.am @@ -10,7 +10,7 @@ else TESTS= endif -EXTRA_DIST=run-test.sh run-test-systemserver.sh test-wait-for-echo.py +EXTRA_DIST=run-test.sh run-test-systemserver.sh test-wait-for-echo.py test-activation-forking.py if DBUS_BUILD_TESTS diff --git a/test/name-test/run-test.sh b/test/name-test/run-test.sh index fba45584..4eb24252 100755 --- a/test/name-test/run-test.sh +++ b/test/name-test/run-test.sh @@ -50,3 +50,9 @@ ${DBUS_TOP_BUILDDIR}/libtool --mode=execute $DEBUG $DBUS_TOP_BUILDDIR/test/name- echo "running test-shutdown" ${DBUS_TOP_BUILDDIR}/libtool --mode=execute $DEBUG $DBUS_TOP_BUILDDIR/test/name-test/test-shutdown || die "test-shutdown failed" + +echo "running test activation forking" +if ! python $DBUS_TOP_SRCDIR/test/name-test/test-activation-forking.py; then + echo "Failed test-activation-forking" + exit 1 +fi diff --git a/test/name-test/test-activation-forking.py b/test/name-test/test-activation-forking.py new file mode 100644 index 00000000..0d820754 --- /dev/null +++ b/test/name-test/test-activation-forking.py @@ -0,0 +1,60 @@ +#!/usr/bin/env python + +import os,sys + +try: + import gobject + import dbus + import dbus.mainloop.glib +except: + print "Failed import, aborting test" + sys.exit(0) + +dbus.mainloop.glib.DBusGMainLoop(set_as_default=True) +loop = gobject.MainLoop() + +exitcode = 0 + +bus = dbus.SessionBus() +bus_iface = dbus.Interface(bus.get_object('org.freedesktop.DBus', '/org/freedesktop/DBus'), 'org.freedesktop.DBus') + +o = bus.get_object('org.freedesktop.DBus.TestSuiteForkingEchoService', '/org/freedesktop/TestSuite') +i = dbus.Interface(o, 'org.freedesktop.TestSuite') + +# Start it up +reply = i.Echo("hello world") +print "TestSuiteForkingEchoService initial reply OK" + +def ignore(*args, **kwargs): + pass + +# Now monitor for exits, when that happens, start it up again. +# The goal here is to try to hit any race conditions in activation. +counter = 0 +def on_forking_echo_owner_changed(name, old, new): + global counter + global o + global i + if counter > 10: + print "Activated 10 times OK, TestSuiteForkingEchoService pass" + loop.quit() + return + counter += 1 + if new == '': + o = bus.get_object('org.freedesktop.DBus.TestSuiteForkingEchoService', '/org/freedesktop/TestSuite') + i = dbus.Interface(o, 'org.freedesktop.TestSuite') + i.Echo("counter %r" % counter) + i.Exit(reply_handler=ignore, error_handler=ignore) + +bus_iface.connect_to_signal('NameOwnerChanged', on_forking_echo_owner_changed, arg0='org.freedesktop.DBus.TestSuiteForkingEchoService') + +i.Exit(reply_handler=ignore, error_handler=ignore) + +def check_counter(): + if counter == 0: + print "Failed to get NameOwnerChanged for TestSuiteForkingEchoService" + sys.exit(1) +gobject.timeout_add(15000, check_counter) + +loop.run() +sys.exit(0) diff --git a/test/test-service.c b/test/test-service.c index ee0086ca..9a129aac 100644 --- a/test/test-service.c +++ b/test/test-service.c @@ -396,7 +396,33 @@ main (int argc, DBusError error; int result; DBusConnection *connection; - + const char *name; + dbus_bool_t do_fork; + + if (argc != 3) + { + name = "org.freedesktop.DBus.TestSuiteEchoService"; + do_fork = FALSE; + } + else + { + name = argv[1]; + do_fork = strcmp (argv[2], "fork") == 0; + } + + /* The bare minimum for simulating a program "daemonizing"; the intent + * is to test services which move from being legacy init scripts to + * activated services. + * https://bugzilla.redhat.com/show_bug.cgi?id=545267 + */ + if (do_fork) + { + pid_t pid = fork (); + if (pid != 0) + exit (0); + sleep (1); + } + dbus_error_init (&error); connection = dbus_bus_get (DBUS_BUS_STARTER, &error); if (connection == NULL) @@ -431,8 +457,8 @@ main (int argc, if (d != (void*) 0xdeadbeef) die ("dbus_connection_get_object_path_data() doesn't seem to work right\n"); } - - result = dbus_bus_request_name (connection, "org.freedesktop.DBus.TestSuiteEchoService", + + result = dbus_bus_request_name (connection, name, 0, &error); if (dbus_error_is_set (&error)) { From 5b2cf0d0310f5da78dc99ce55c678ca93d2c29c4 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 15 Jan 2010 16:57:39 -0500 Subject: [PATCH 16/18] Add Will Thompson and Simon McVittie to reviewers, add emails to all The reviewer list was sorely lacking actual email addresses; fix this. Also add Will and Simon. --- HACKING | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/HACKING b/HACKING index 3a981ae2..f1e9d765 100644 --- a/HACKING +++ b/HACKING @@ -320,8 +320,22 @@ rules are: No reviewer should approve a patch without these attributes, and failure on these points is grounds for reverting the patch. -The reviewer group that can approve patches: Havoc Pennington, Michael -Meeks, Alex Larsson, Zack Rusin, Joe Shaw, Mikael Hallendal, Richard -Hult, Owen Fraser-Green, Olivier Andrieu, Colin Walters, Thiago -Macieira, John Palmieri, Scott James Remnant. +The reviewer group that can approve patches: + +Havoc Pennington +Michael Meeks +Alexander Larsson +Zack Rusin +Joe Shaw +Mikael Hallendal +Richard Hult +Owen Fraser-Green +Olivier Andrieu +Colin Walters +Thiago Macieira +John Palmieri +Scott James Remnant +Will Thompson +Simon McVittie + From 0a3905d7f3b2ff43b09479863775939f9c8acad4 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 27 Jan 2010 19:38:44 -0500 Subject: [PATCH 17/18] Switch to libcap-ng, avoid linking libdbus against libcap[-ng] (Commit message written by Colin Walters ) A current Fedora goal is to convert projects to libcap-ng which more easily allows dropping Linux capabilities. For software which also links to libdbus, it's problematic to link against libcap as well. Though really, libdbus should have never linked against libcap in the first place, which is another thing this patch changes by moving the libcap-using bits out of dbus/ and into bus/. https://bugzilla.redhat.com/show_bug.cgi?id=518541 --- bus/selinux.c | 118 ++++++++++++++++++++++++-- bus/selinux.h | 2 + configure.in | 5 +- dbus/dbus-sysdeps-util-unix.c | 154 ---------------------------------- dbus/dbus-sysdeps.h | 2 - 5 files changed, 117 insertions(+), 164 deletions(-) diff --git a/bus/selinux.c b/bus/selinux.c index df9a00b1..456723ac 100644 --- a/bus/selinux.c +++ b/bus/selinux.c @@ -22,6 +22,7 @@ */ #include #include +#include #include "selinux.h" #include "services.h" #include "policy.h" @@ -44,7 +45,9 @@ #include #include #include +#include #ifdef HAVE_LIBAUDIT +#include #include #endif /* HAVE_LIBAUDIT */ #endif /* HAVE_SELINUX */ @@ -143,13 +146,17 @@ log_callback (const char *fmt, ...) #ifdef HAVE_LIBAUDIT if (audit_fd >= 0) { - char buf[PATH_MAX*2]; + capng_get_caps_process(); + if (capng_have_capability(CAPNG_EFFECTIVE, CAP_AUDIT_WRITE)) + { + char buf[PATH_MAX*2]; - /* FIXME: need to change this to show real user */ - vsnprintf(buf, sizeof(buf), fmt, ap); - audit_log_user_avc_message(audit_fd, AUDIT_USER_AVC, buf, NULL, NULL, + /* FIXME: need to change this to show real user */ + vsnprintf(buf, sizeof(buf), fmt, ap); + audit_log_user_avc_message(audit_fd, AUDIT_USER_AVC, buf, NULL, NULL, NULL, getuid()); - return; + return; + } } #endif /* HAVE_LIBAUDIT */ @@ -1010,3 +1017,104 @@ bus_selinux_shutdown (void) #endif /* HAVE_SELINUX */ } +/** + * Changes the user and group the bus is running as. + * + * @param user the user to become + * @param error return location for errors + * @returns #FALSE on failure + */ +dbus_bool_t +_dbus_change_to_daemon_user (const char *user, + DBusError *error) +{ + dbus_uid_t uid; + dbus_gid_t gid; + DBusString u; + + _dbus_string_init_const (&u, user); + + if (!_dbus_get_user_id_and_primary_group (&u, &uid, &gid)) + { + dbus_set_error (error, DBUS_ERROR_FAILED, + "User '%s' does not appear to exist?", + user); + return FALSE; + } + +#ifdef HAVE_LIBAUDIT + /* If we were root */ + if (_dbus_geteuid () == 0) + { + int rc; + + capng_clear (CAPNG_SELECT_BOTH); + capng_update (CAPNG_ADD, CAPNG_EFFECTIVE | CAPNG_PERMITTED, + CAP_AUDIT_WRITE); + rc = capng_change_id (uid, gid, 0); + if (rc) + { + switch (rc) { + default: + dbus_set_error (error, DBUS_ERROR_FAILED, + "Failed to drop capabilities: %s\n", + _dbus_strerror (errno)); + break; + case -4: + dbus_set_error (error, _dbus_error_from_errno (errno), + "Failed to set GID to %lu: %s", gid, + _dbus_strerror (errno)); + break; + case -5: + _dbus_warn ("Failed to drop supplementary groups: %s\n", + _dbus_strerror (errno)); + break; + case -6: + dbus_set_error (error, _dbus_error_from_errno (errno), + "Failed to set UID to %lu: %s", uid, + _dbus_strerror (errno)); + break; + case -7: + dbus_set_error (error, _dbus_error_from_errno (errno), + "Failed to unset keep-capabilities: %s\n", + _dbus_strerror (errno)); + break; + } + return FALSE; + } + } +#else + /* setgroups() only works if we are a privileged process, + * so we don't return error on failure; the only possible + * failure is that we don't have perms to do it. + * + * not sure this is right, maybe if setuid() + * is going to work then setgroups() should also work. + */ + if (setgroups (0, NULL) < 0) + _dbus_warn ("Failed to drop supplementary groups: %s\n", + _dbus_strerror (errno)); + + /* Set GID first, or the setuid may remove our permission + * to change the GID + */ + if (setgid (gid) < 0) + { + dbus_set_error (error, _dbus_error_from_errno (errno), + "Failed to set GID to %lu: %s", gid, + _dbus_strerror (errno)); + return FALSE; + } + + if (setuid (uid) < 0) + { + dbus_set_error (error, _dbus_error_from_errno (errno), + "Failed to set UID to %lu: %s", uid, + _dbus_strerror (errno)); + return FALSE; + } +#endif /* !HAVE_LIBAUDIT */ + + return TRUE; +} + diff --git a/bus/selinux.h b/bus/selinux.h index 3bab36de..f208fbeb 100644 --- a/bus/selinux.h +++ b/bus/selinux.h @@ -68,5 +68,7 @@ BusSELinuxID* bus_selinux_init_connection_id (DBusConnection *connection, void bus_selinux_audit_init(void); +dbus_bool_t _dbus_change_to_daemon_user (const char *user, + DBusError *error); #endif /* BUS_SELINUX_H */ diff --git a/configure.in b/configure.in index 7feb1a93..cb16eec4 100644 --- a/configure.in +++ b/configure.in @@ -851,7 +851,7 @@ else AC_CHECK_LIB(audit, audit_log_user_avc_message, have_libaudit=yes, have_libaudit=no) if test x$have_libaudit = xyes ; then - AC_CHECK_LIB(cap, cap_set_proc, + AC_CHECK_LIB(cap-ng, capng_clear, have_libaudit=yes, have_libaudit=no) fi fi @@ -859,8 +859,7 @@ fi AM_CONDITIONAL(HAVE_LIBAUDIT, test x$have_libaudit = xyes) if test x$have_libaudit = xyes ; then - SELINUX_LIBS="$SELINUX_LIBS -laudit" - LIBS="-lcap $LIBS" + SELINUX_LIBS="$SELINUX_LIBS -laudit -lcap-ng" AC_DEFINE(HAVE_LIBAUDIT,1,[audit daemon SELinux support]) fi diff --git a/dbus/dbus-sysdeps-util-unix.c b/dbus/dbus-sysdeps-util-unix.c index 83f74fe2..27cdbb01 100644 --- a/dbus/dbus-sysdeps-util-unix.c +++ b/dbus/dbus-sysdeps-util-unix.c @@ -44,11 +44,6 @@ #include #include #include -#ifdef HAVE_LIBAUDIT -#include -#include -#include -#endif /* HAVE_LIBAUDIT */ #ifdef HAVE_SYS_SYSLIMITS_H #include @@ -308,155 +303,6 @@ _dbus_verify_daemon_user (const char *user) return _dbus_get_user_id_and_primary_group (&u, NULL, NULL); } -/** - * Changes the user and group the bus is running as. - * - * @param user the user to become - * @param error return location for errors - * @returns #FALSE on failure - */ -dbus_bool_t -_dbus_change_to_daemon_user (const char *user, - DBusError *error) -{ - dbus_uid_t uid; - dbus_gid_t gid; - DBusString u; -#ifdef HAVE_LIBAUDIT - dbus_bool_t we_were_root; - cap_t new_caps; -#endif - - _dbus_string_init_const (&u, user); - - if (!_dbus_get_user_id_and_primary_group (&u, &uid, &gid)) - { - dbus_set_error (error, DBUS_ERROR_FAILED, - "User '%s' does not appear to exist?", - user); - return FALSE; - } - -#ifdef HAVE_LIBAUDIT - we_were_root = _dbus_geteuid () == 0; - new_caps = NULL; - /* have a tmp set of caps that we use to transition to the usr/grp dbus should - * run as ... doesn't really help. But keeps people happy. - */ - - if (we_were_root) - { - cap_value_t new_cap_list[] = { CAP_AUDIT_WRITE }; - cap_value_t tmp_cap_list[] = { CAP_AUDIT_WRITE, CAP_SETUID, CAP_SETGID }; - cap_t tmp_caps = cap_init(); - - if (!tmp_caps || !(new_caps = cap_init ())) - { - dbus_set_error (error, DBUS_ERROR_FAILED, - "Failed to initialize drop of capabilities: %s\n", - _dbus_strerror (errno)); - - if (tmp_caps) - cap_free (tmp_caps); - - return FALSE; - } - - /* assume these work... */ - cap_set_flag (new_caps, CAP_PERMITTED, 1, new_cap_list, CAP_SET); - cap_set_flag (new_caps, CAP_EFFECTIVE, 1, new_cap_list, CAP_SET); - cap_set_flag (tmp_caps, CAP_PERMITTED, 3, tmp_cap_list, CAP_SET); - cap_set_flag (tmp_caps, CAP_EFFECTIVE, 3, tmp_cap_list, CAP_SET); - - if (prctl (PR_SET_KEEPCAPS, 1, 0, 0, 0) == -1) - { - dbus_set_error (error, _dbus_error_from_errno (errno), - "Failed to set keep-capabilities: %s\n", - _dbus_strerror (errno)); - cap_free (tmp_caps); - goto fail; - } - - if (cap_set_proc (tmp_caps) == -1) - { - dbus_set_error (error, DBUS_ERROR_FAILED, - "Failed to drop capabilities: %s\n", - _dbus_strerror (errno)); - cap_free (tmp_caps); - goto fail; - } - cap_free (tmp_caps); - } -#endif /* HAVE_LIBAUDIT */ - - /* setgroups() only works if we are a privileged process, - * so we don't return error on failure; the only possible - * failure is that we don't have perms to do it. - * - * not sure this is right, maybe if setuid() - * is going to work then setgroups() should also work. - */ - if (setgroups (0, NULL) < 0) - _dbus_warn ("Failed to drop supplementary groups: %s\n", - _dbus_strerror (errno)); - - /* Set GID first, or the setuid may remove our permission - * to change the GID - */ - if (setgid (gid) < 0) - { - dbus_set_error (error, _dbus_error_from_errno (errno), - "Failed to set GID to %lu: %s", gid, - _dbus_strerror (errno)); - goto fail; - } - - if (setuid (uid) < 0) - { - dbus_set_error (error, _dbus_error_from_errno (errno), - "Failed to set UID to %lu: %s", uid, - _dbus_strerror (errno)); - goto fail; - } - -#ifdef HAVE_LIBAUDIT - if (we_were_root) - { - if (cap_set_proc (new_caps)) - { - dbus_set_error (error, DBUS_ERROR_FAILED, - "Failed to drop capabilities: %s\n", - _dbus_strerror (errno)); - goto fail; - } - cap_free (new_caps); - - /* should always work, if it did above */ - if (prctl (PR_SET_KEEPCAPS, 0, 0, 0, 0) == -1) - { - dbus_set_error (error, _dbus_error_from_errno (errno), - "Failed to unset keep-capabilities: %s\n", - _dbus_strerror (errno)); - return FALSE; - } - } -#endif - - return TRUE; - - fail: -#ifdef HAVE_LIBAUDIT - if (!we_were_root) - { - /* should always work, if it did above */ - prctl (PR_SET_KEEPCAPS, 0, 0, 0, 0); - cap_free (new_caps); - } -#endif - - return FALSE; -} - void _dbus_init_system_log (void) { diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h index 30e7dff2..b154f016 100644 --- a/dbus/dbus-sysdeps.h +++ b/dbus/dbus-sysdeps.h @@ -418,8 +418,6 @@ dbus_bool_t _dbus_become_daemon (const DBusString *pidfile, dbus_bool_t keep_umask); dbus_bool_t _dbus_verify_daemon_user (const char *user); -dbus_bool_t _dbus_change_to_daemon_user (const char *user, - DBusError *error); dbus_bool_t _dbus_write_pid_to_file_and_pipe (const DBusString *pidfile, DBusPipe *print_pid_pipe, From b93476ce07acce83ff3b396616bb8a0eaf719916 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 28 Jan 2010 15:04:14 -0500 Subject: [PATCH 18/18] Don't drop pending activations when reloading configuration The reload handling for activation simply dropped all knowledge of pending activations, which was clearly wrong. Refactor things so that reload only reloads directories, server address etc. Based on a patch originally from Matthias Clasen --- bus/activation.c | 104 +++++++++++++++++++++++++++++------------------ bus/activation.h | 4 ++ bus/bus.c | 21 ++++++---- 3 files changed, 80 insertions(+), 49 deletions(-) diff --git a/bus/activation.c b/bus/activation.c index 00caac27..0a28df16 100644 --- a/bus/activation.c +++ b/bus/activation.c @@ -735,74 +735,56 @@ out: return retval; } -BusActivation* -bus_activation_new (BusContext *context, - const DBusString *address, - DBusList **directories, - DBusError *error) +dbus_bool_t +bus_activation_reload (BusActivation *activation, + const DBusString *address, + DBusList **directories, + DBusError *error) { - BusActivation *activation; DBusList *link; char *dir; - - _DBUS_ASSERT_ERROR_IS_CLEAR (error); - - activation = dbus_new0 (BusActivation, 1); - if (activation == NULL) - { - BUS_SET_OOM (error); - return NULL; - } - - activation->refcount = 1; - activation->context = context; - activation->n_pending_activations = 0; - + + if (activation->server_address != NULL) + dbus_free (activation->server_address); if (!_dbus_string_copy_data (address, &activation->server_address)) { BUS_SET_OOM (error); goto failed; } - + + if (activation->entries != NULL) + _dbus_hash_table_unref (activation->entries); activation->entries = _dbus_hash_table_new (DBUS_HASH_STRING, NULL, (DBusFreeFunction)bus_activation_entry_unref); if (activation->entries == NULL) - { - BUS_SET_OOM (error); - goto failed; - } - - activation->pending_activations = _dbus_hash_table_new (DBUS_HASH_STRING, NULL, - (DBusFreeFunction)bus_pending_activation_unref); - - if (activation->pending_activations == NULL) { BUS_SET_OOM (error); goto failed; } + if (activation->directories != NULL) + _dbus_hash_table_unref (activation->directories); activation->directories = _dbus_hash_table_new (DBUS_HASH_STRING, NULL, (DBusFreeFunction)bus_service_directory_unref); - - if (activation->directories == NULL) + + if (activation->directories == NULL) { BUS_SET_OOM (error); goto failed; } - - /* Load service files */ + link = _dbus_list_get_first_link (directories); while (link != NULL) { BusServiceDirectory *s_dir; - + dir = _dbus_strdup ((const char *) link->data); if (!dir) { BUS_SET_OOM (error); goto failed; } - + s_dir = dbus_new0 (BusServiceDirectory, 1); if (!s_dir) { @@ -813,7 +795,7 @@ bus_activation_new (BusContext *context, s_dir->refcount = 1; s_dir->dir_c = dir; - + s_dir->entries = _dbus_hash_table_new (DBUS_HASH_STRING, NULL, (DBusFreeFunction)bus_activation_entry_unref); @@ -833,8 +815,8 @@ bus_activation_new (BusContext *context, /* only fail on OOM, it is ok if we can't read the directory */ if (!update_directory (activation, s_dir, error)) - { - if (dbus_error_has_name (error, DBUS_ERROR_NO_MEMORY)) + { + if (dbus_error_has_name (error, DBUS_ERROR_NO_MEMORY)) goto failed; else dbus_error_free (error); @@ -843,10 +825,52 @@ bus_activation_new (BusContext *context, link = _dbus_list_get_next_link (directories, link); } + return TRUE; + failed: + return FALSE; +} + +BusActivation* +bus_activation_new (BusContext *context, + const DBusString *address, + DBusList **directories, + DBusError *error) +{ + BusActivation *activation; + DBusList *link; + char *dir; + + _DBUS_ASSERT_ERROR_IS_CLEAR (error); + + activation = dbus_new0 (BusActivation, 1); + if (activation == NULL) + { + BUS_SET_OOM (error); + return NULL; + } + + activation->refcount = 1; + activation->context = context; + activation->n_pending_activations = 0; + + if (!bus_activation_reload (activation, address, directories, error)) + goto failed; + + /* Initialize this hash table once, we don't want to lose pending + * activations on reload. */ + activation->pending_activations = _dbus_hash_table_new (DBUS_HASH_STRING, NULL, + (DBusFreeFunction)bus_pending_activation_unref); + + if (activation->pending_activations == NULL) + { + BUS_SET_OOM (error); + goto failed; + } + activation->environment = _dbus_hash_table_new (DBUS_HASH_STRING, (DBusFreeFunction) dbus_free, (DBusFreeFunction) dbus_free); - + if (activation->environment == NULL) { BUS_SET_OOM (error); diff --git a/bus/activation.h b/bus/activation.h index 2dff812a..03bfed28 100644 --- a/bus/activation.h +++ b/bus/activation.h @@ -32,6 +32,10 @@ BusActivation* bus_activation_new (BusContext *context, const DBusString *address, DBusList **directories, DBusError *error); +dbus_bool_t bus_activation_reload (BusActivation *activation, + const DBusString *address, + DBusList **directories, + DBusError *error); BusActivation* bus_activation_ref (BusActivation *activation); void bus_activation_unref (BusActivation *activation); diff --git a/bus/bus.c b/bus/bus.c index b370f92b..81db7d69 100644 --- a/bus/bus.c +++ b/bus/bus.c @@ -498,21 +498,24 @@ process_config_every_time (BusContext *context, dbus_free(context->servicehelper); context->servicehelper = s; } - + /* Create activation subsystem */ - new_activation = bus_activation_new (context, &full_address, - dirs, error); - if (new_activation == NULL) + if (context->activation) + { + if (!bus_activation_reload (context->activation, &full_address, dirs, error)) + goto failed; + } + else + { + context->activation = bus_activation_new (context, &full_address, dirs, error); + } + + if (context->activation == NULL) { _DBUS_ASSERT_ERROR_IS_SET (error); goto failed; } - if (is_reload) - bus_activation_unref (context->activation); - - context->activation = new_activation; - /* Drop existing conf-dir watches (if applicable) */ if (is_reload)