From 90fe96b1875350f86a4a773d4a0a22009950dd4d Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 2 Feb 2010 12:37:17 -0500 Subject: [PATCH 1/7] Fix inotify shutdown We were incorrectly passing NULL for a DBusList when the usage expected is a pointer to a NULL DBusList pointer. Also during dbus_shutdown we need to actually close the inotify fd, and remove our watch. Move the shutdown handler out of bus.c and into inotify where we can do all of this cleanly. --- bus/bus.c | 8 --- bus/dir-watch-inotify.c | 128 +++++++++++++++++++++++++--------------- 2 files changed, 82 insertions(+), 54 deletions(-) diff --git a/bus/bus.c b/bus/bus.c index bfd398e6..8150df24 100644 --- a/bus/bus.c +++ b/bus/bus.c @@ -551,12 +551,6 @@ process_config_postinit (BusContext *context, return TRUE; } -static void -bus_shutdown_all_directory_watches (void *data) -{ - bus_set_watched_dirs ((BusContext *) data, NULL); -} - BusContext* bus_context_new (const DBusString *config_file, ForceForkSetting force_fork, @@ -588,8 +582,6 @@ bus_context_new (const DBusString *config_file, _dbus_generate_uuid (&context->uuid); - _dbus_register_shutdown_func (bus_shutdown_all_directory_watches, context); - if (!_dbus_string_copy_data (config_file, &context->config_file)) { BUS_SET_OOM (error); diff --git a/bus/dir-watch-inotify.c b/bus/dir-watch-inotify.c index f87a6347..bb71394c 100644 --- a/bus/dir-watch-inotify.c +++ b/bus/dir-watch-inotify.c @@ -92,59 +92,16 @@ _handle_inotify_watch (DBusWatch *passed_watch, unsigned int flags, void *data) return TRUE; } -static int -_init_inotify (BusContext *context) -{ - int ret = 0; +#include - if (inotify_fd == -1) { -#ifdef HAVE_INOTIFY_INIT1 - inotify_fd = inotify_init1 (IN_CLOEXEC); -#else - inotify_fd = inotify_init (); -#endif - if (inotify_fd <= 0) { - _dbus_warn ("Cannot initialize inotify\n"); - goto out; - } - loop = bus_context_get_loop (context); - - watch = _dbus_watch_new (inotify_fd, DBUS_WATCH_READABLE, TRUE, - _handle_inotify_watch, NULL, NULL); - - if (watch == NULL) - { - _dbus_warn ("Unable to create inotify watch\n"); - goto out; - } - - if (!_dbus_loop_add_watch (loop, watch, _inotify_watch_callback, - NULL, NULL)) - { - _dbus_warn ("Unable to add reload watch to main loop"); - _dbus_watch_unref (watch); - watch = NULL; - goto out; - } - } - - ret = 1; - -out: - return ret; -} - -void -bus_set_watched_dirs (BusContext *context, DBusList **directories) +static void +_set_watched_dirs_internal (DBusList **directories) { int new_wds[MAX_DIRS_TO_WATCH]; char *new_dirs[MAX_DIRS_TO_WATCH]; DBusList *link; int i, j, wd; - if (!_init_inotify (context)) - goto out; - for (i = 0; i < MAX_DIRS_TO_WATCH; i++) { new_wds[i] = -1; @@ -226,3 +183,82 @@ bus_set_watched_dirs (BusContext *context, DBusList **directories) out:; } + +#include +static void +_shutdown_inotify (void *data) +{ + DBusList *empty = NULL; + + if (inotify_fd == -1) + return; + + _set_watched_dirs_internal (&empty); + + close (inotify_fd); + inotify_fd = -1; + if (watch != NULL) + { + _dbus_loop_remove_watch (loop, watch, _inotify_watch_callback, NULL); + _dbus_watch_unref (watch); + _dbus_loop_unref (loop); + } + watch = NULL; + loop = NULL; +} + +static int +_init_inotify (BusContext *context) +{ + int ret = 0; + + if (inotify_fd == -1) + { +#ifdef HAVE_INOTIFY_INIT1 + inotify_fd = inotify_init1 (IN_CLOEXEC); +#else + inotify_fd = inotify_init (); +#endif + if (inotify_fd <= 0) + { + _dbus_warn ("Cannot initialize inotify\n"); + goto out; + } + loop = bus_context_get_loop (context); + _dbus_loop_ref (loop); + + watch = _dbus_watch_new (inotify_fd, DBUS_WATCH_READABLE, TRUE, + _handle_inotify_watch, NULL, NULL); + + if (watch == NULL) + { + _dbus_warn ("Unable to create inotify watch\n"); + goto out; + } + + if (!_dbus_loop_add_watch (loop, watch, _inotify_watch_callback, + NULL, NULL)) + { + _dbus_warn ("Unable to add reload watch to main loop"); + _dbus_watch_unref (watch); + watch = NULL; + goto out; + } + + _dbus_register_shutdown_func (_shutdown_inotify, NULL); + } + + ret = 1; + +out: + return ret; +} + +void +bus_set_watched_dirs (BusContext *context, DBusList **directories) +{ + if (!_init_inotify (context)) + return; + + _set_watched_dirs_internal (directories); +} From 3dac125d61ebc4f614a1723580043e2f1c811f59 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 2 Feb 2010 14:57:47 -0500 Subject: [PATCH 2/7] Fix compilation in --disable-selinux case _dbus_change_to_daemon_user moved into selinux.c for the --with-selinux (and audit) case because that's where all of the relevant libcap headers were being used. However in the --disable-selinux case this didn't compile and wasn't very clean. If we don't have libaudit, use the legacy direct setgid/setuid bits we had before in dbus-sysdeps-util-unix.c. --- bus/selinux.c | 35 ++------------------ bus/selinux.h | 2 -- dbus/dbus-sysdeps-util-unix.c | 62 +++++++++++++++++++++++++++++++++++ dbus/dbus-sysdeps.h | 3 ++ 4 files changed, 68 insertions(+), 34 deletions(-) diff --git a/bus/selinux.c b/bus/selinux.c index 456723ac..e61efc5d 100644 --- a/bus/selinux.c +++ b/bus/selinux.c @@ -1017,6 +1017,8 @@ bus_selinux_shutdown (void) #endif /* HAVE_SELINUX */ } +/* The !HAVE_LIBAUDIT case lives in dbus-sysdeps-util-unix.c */ +#ifdef HAVE_LIBAUDIT /** * Changes the user and group the bus is running as. * @@ -1042,7 +1044,6 @@ _dbus_change_to_daemon_user (const char *user, return FALSE; } -#ifdef HAVE_LIBAUDIT /* If we were root */ if (_dbus_geteuid () == 0) { @@ -1083,38 +1084,8 @@ _dbus_change_to_daemon_user (const char *user, 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; } +#endif diff --git a/bus/selinux.h b/bus/selinux.h index f208fbeb..3bab36de 100644 --- a/bus/selinux.h +++ b/bus/selinux.h @@ -68,7 +68,5 @@ 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/dbus/dbus-sysdeps-util-unix.c b/dbus/dbus-sysdeps-util-unix.c index 27cdbb01..74e8d88f 100644 --- a/dbus/dbus-sysdeps-util-unix.c +++ b/dbus/dbus-sysdeps-util-unix.c @@ -303,6 +303,68 @@ _dbus_verify_daemon_user (const char *user) return _dbus_get_user_id_and_primary_group (&u, NULL, NULL); } + +/* The HAVE_LIBAUDIT case lives in selinux.c */ +#ifndef HAVE_LIBAUDIT +/** + * 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; + } + + /* 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; + } + + return TRUE; +} +#endif /* !HAVE_LIBAUDIT */ + void _dbus_init_system_log (void) { diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h index b154f016..80f0ba26 100644 --- a/dbus/dbus-sysdeps.h +++ b/dbus/dbus-sysdeps.h @@ -512,6 +512,9 @@ unsigned long _dbus_pid_for_log (void); */ dbus_pid_t _dbus_getpid (void); +dbus_bool_t _dbus_change_to_daemon_user (const char *user, + DBusError *error); + void _dbus_flush_caches (void); /** @} */ From 65be3cd5d94debee32eb05a14cf9fef9d6c92bdc Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 3 Feb 2010 12:13:38 -0500 Subject: [PATCH 3/7] Release 1.2.20 --- configure.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.in b/configure.in index 3c8626ec..ccc9c7fe 100644 --- a/configure.in +++ b/configure.in @@ -3,7 +3,7 @@ AC_PREREQ(2.52) m4_define([dbus_major_version], [1]) m4_define([dbus_minor_version], [2]) -m4_define([dbus_micro_version], [18]) +m4_define([dbus_micro_version], [20]) m4_define([dbus_version], [dbus_major_version.dbus_minor_version.dbus_micro_version]) AC_INIT(dbus, [dbus_version]) From b429488739a5d7a49275b3d7f4628c295069dd87 Mon Sep 17 00:00:00 2001 From: Cyril Brulebois Date: Mon, 8 Feb 2010 12:21:35 -0500 Subject: [PATCH 4/7] Fix compilation of kqueue file monitoring on FreeBSD --- bus/dir-watch-kqueue.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/bus/dir-watch-kqueue.c b/bus/dir-watch-kqueue.c index 7c18a3c9..e7b0e2c5 100644 --- a/bus/dir-watch-kqueue.c +++ b/bus/dir-watch-kqueue.c @@ -139,17 +139,18 @@ out: } void -bus_set_watched_dir (BusContext *context, DBusList **directories) +bus_set_watched_dirs (BusContext *context, DBusList **directories) { int new_fds[MAX_DIRS_TO_WATCH]; char *new_dirs[MAX_DIRS_TO_WATCH]; DBusList *link; - int i, f, fd; + int i, j, f, fd; + struct kevent ev; if (!_init_kqueue (context)) goto out; - for (i = 0; i < MAX_DIRS_TO_WATCH; i++) { + for (i = 0; i < MAX_DIRS_TO_WATCH; i++) { new_fds[i] = -1; new_dirs[i] = NULL; @@ -213,7 +214,7 @@ bus_set_watched_dir (BusContext *context, DBusList **directories) NOTE_DELETE | NOTE_EXTEND | NOTE_WRITE | NOTE_RENAME, 0, 0); if (kevent (kq, &ev, 1, NULL, 0, NULL) == -1) { - _dbus_warn ("Cannot setup a kevent for '%s'; error '%s'\n", dir, _dbus_strerror (errno)); + _dbus_warn ("Cannot setup a kevent for '%s'; error '%s'\n", new_dirs[i], _dbus_strerror (errno)); close (fd); goto out; } From 8ca10fd4ded0383e597b17ec315b955515e19a78 Mon Sep 17 00:00:00 2001 From: Brian Cameron Date: Thu, 18 Feb 2010 10:38:42 -0500 Subject: [PATCH 5/7] Fix dummy file monitoring backend compilation https://bugs.freedesktop.org/show_bug.cgi?id=26421 --- bus/dir-watch-default.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/bus/dir-watch-default.c b/bus/dir-watch-default.c index 6f8f1e99..8e457eb6 100644 --- a/bus/dir-watch-default.c +++ b/bus/dir-watch-default.c @@ -29,12 +29,12 @@ /* NoOp */ -void -bus_drop_all_directory_watches (void) -{ -} - void bus_watch_directory (const char *dir, BusContext *context) { } + +void +bus_set_watched_dirs (BusContext *context, DBusList **directories) +{ +} From 9c90fcd2dc4b1b7d818a35ef43d4686052902f59 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 18 Feb 2010 15:33:28 -0500 Subject: [PATCH 6/7] Monitor service directories for changes It's not expected to have to manually SIGHUP the bus after installing a new .service file. Since our directory monitoring is already set up to queue a full reload which includes service activation, simply monitor the servicedirs too. https://bugs.freedesktop.org/show_bug.cgi?id=23846 --- bus/bus.c | 44 +++++++++++++++++++++++++++++++++++++++-- bus/dir-watch-inotify.c | 14 +++++++++++-- bus/dir-watch-kqueue.c | 17 ++++++++++++---- 3 files changed, 67 insertions(+), 8 deletions(-) diff --git a/bus/bus.c b/bus/bus.c index 8150df24..6495ae7f 100644 --- a/bus/bus.c +++ b/bus/bus.c @@ -528,12 +528,40 @@ process_config_every_time (BusContext *context, return retval; } +static dbus_bool_t +list_concat_new (DBusList **a, + DBusList **b, + DBusList **result) +{ + DBusList *link; + + *result = NULL; + + link = _dbus_list_get_first_link (a); + for (link = _dbus_list_get_first_link (a); link; link = _dbus_list_get_next_link (a, link)) + { + if (!_dbus_list_append (result, link->data)) + goto oom; + } + for (link = _dbus_list_get_first_link (b); link; link = _dbus_list_get_next_link (b, link)) + { + if (!_dbus_list_append (result, link->data)) + goto oom; + } + + return TRUE; +oom: + _dbus_list_clear (result); + return FALSE; +} + static dbus_bool_t process_config_postinit (BusContext *context, BusConfigParser *parser, DBusError *error) { DBusHashTable *service_context_table; + DBusList *watched_dirs = NULL; service_context_table = bus_config_parser_steal_service_context_table (parser); if (!bus_registry_set_service_context_table (context->registry, @@ -545,8 +573,20 @@ process_config_postinit (BusContext *context, _dbus_hash_table_unref (service_context_table); - /* Watch all conf directories */ - bus_set_watched_dirs (context, bus_config_parser_get_conf_dirs (parser)); + /* We need to monitor both the configuration directories and directories + * containing .service files. + */ + if (!list_concat_new (bus_config_parser_get_conf_dirs (parser), + bus_config_parser_get_service_dirs (parser), + &watched_dirs)) + { + BUS_SET_OOM (error); + return FALSE; + } + + bus_set_watched_dirs (context, &watched_dirs); + + _dbus_list_clear (&watched_dirs); return TRUE; } diff --git a/bus/dir-watch-inotify.c b/bus/dir-watch-inotify.c index bb71394c..c98e6fcb 100644 --- a/bus/dir-watch-inotify.c +++ b/bus/dir-watch-inotify.c @@ -156,8 +156,18 @@ _set_watched_dirs_internal (DBusList **directories) wd = inotify_add_watch (inotify_fd, new_dirs[i], IN_CLOSE_WRITE | IN_DELETE | IN_MOVED_TO | IN_MOVED_FROM); if (wd < 0) { - _dbus_warn ("Cannot setup inotify for '%s'; error '%s'\n", new_dirs[i], _dbus_strerror (errno)); - goto out; + /* Not all service directories need to exist. */ + if (errno != ENOENT) + { + _dbus_warn ("Cannot setup inotify for '%s'; error '%s'\n", new_dirs[i], _dbus_strerror (errno)); + goto out; + } + else + { + new_wds[i] = -1; + new_dirs[i] = NULL; + continue; + } } new_wds[i] = wd; new_dirs[i] = _dbus_strdup (new_dirs[i]); diff --git a/bus/dir-watch-kqueue.c b/bus/dir-watch-kqueue.c index e7b0e2c5..4a01b748 100644 --- a/bus/dir-watch-kqueue.c +++ b/bus/dir-watch-kqueue.c @@ -204,11 +204,20 @@ bus_set_watched_dirs (BusContext *context, DBusList **directories) * we may need to sleep. */ fd = open (new_dirs[i], O_RDONLY); - if (fd < 0) + if (fd < 0) { - _dbus_warn ("Cannot open directory '%s'; error '%s'\n", new_dirs[i], _dbus_strerror (errno)); - goto out; - } + if (errno != ENOENT) + { + _dbus_warn ("Cannot open directory '%s'; error '%s'\n", new_dirs[i], _dbus_strerror (errno)); + goto out; + } + else + { + new_fds[i] = -1; + new_dirs[i] = NULL; + continue; + } + } EV_SET (&ev, fd, EVFILT_VNODE, EV_ADD | EV_ENABLE | EV_CLEAR, NOTE_DELETE | NOTE_EXTEND | NOTE_WRITE | NOTE_RENAME, 0, 0); From fbeb13517ef667b8ed4136bcb9e52ff9924419c1 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 19 Feb 2010 16:34:47 -0500 Subject: [PATCH 7/7] [dbus-string] Sync up UNICODE_VALID with glib, add documentation See https://bugzilla.gnome.org/show_bug.cgi?id=107427 for rationale behind the first change. The documentation was derived from an IRC conversation with Behdad Esfahbod. --- dbus/dbus-string.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/dbus/dbus-string.c b/dbus/dbus-string.c index b7a8b992..6da46d1a 100644 --- a/dbus/dbus-string.c +++ b/dbus/dbus-string.c @@ -1799,7 +1799,18 @@ _dbus_string_split_on_byte (DBusString *source, } /** - * Check whether a unicode char is in a valid range. + * Check whether a Unicode (5.2) char is in a valid range. + * + * The first check comes from the Unicode guarantee to never encode + * a point above 0x0010ffff, since UTF-16 couldn't represent it. + * + * The second check covers surrogate pairs (category Cs). + * + * The last two checks cover "Noncharacter": defined as: + * "A code point that is permanently reserved for + * internal use, and that should never be interchanged. In + * Unicode 3.1, these consist of the values U+nFFFE and U+nFFFF + * (where n is from 0 to 10_16) and the values U+FDD0..U+FDEF." * * @param Char the character */ @@ -1807,7 +1818,7 @@ _dbus_string_split_on_byte (DBusString *source, ((Char) < 0x110000 && \ (((Char) & 0xFFFFF800) != 0xD800) && \ ((Char) < 0xFDD0 || (Char) > 0xFDEF) && \ - ((Char) & 0xFFFF) != 0xFFFF) + ((Char) & 0xFFFE) != 0xFFFE) #ifdef DBUS_BUILD_TESTS /**