From c2e925279f91726e4e9d19ea2defed76e173ce1e Mon Sep 17 00:00:00 2001 From: Laurent Bigonville Date: Sat, 3 Mar 2018 13:15:17 +0100 Subject: [PATCH] Stop using avc_init() which is deprecated Stop using avc_init() and use avc_open() instead. With this commit dbus-daemon will stop using a thread to monitor the avc netlink and will poll it instead. https://bugs.freedesktop.org/show_bug.cgi?id=92831 --- bus/bus.c | 2 +- bus/selinux.c | 194 ++++++++++++++++++++++++++------------------------------ bus/selinux.h | 2 +- bus/test-main.c | 6 -- bus/test.c | 7 ++ 5 files changed, 98 insertions(+), 113 deletions(-) diff --git a/bus/bus.c b/bus/bus.c index 9fd9820b..b9b32d82 100644 --- a/bus/bus.c +++ b/bus/bus.c @@ -995,7 +995,7 @@ bus_context_new (const DBusString *config_file, */ bus_audit_init (context); - if (!bus_selinux_full_init ()) + if (!bus_selinux_full_init (context)) { bus_context_log (context, DBUS_SYSTEM_LOG_ERROR, "SELinux enabled but D-Bus initialization failed; " diff --git a/bus/selinux.c b/bus/selinux.c index d09afb4b..3345c889 100644 --- a/bus/selinux.c +++ b/bus/selinux.c @@ -49,6 +49,7 @@ #include #include #include +#include #endif /* HAVE_SELINUX */ #ifdef HAVE_LIBAUDIT #include @@ -64,45 +65,20 @@ static dbus_bool_t selinux_enabled = FALSE; /* Store an avc_entry_ref to speed AVC decisions. */ static struct avc_entry_ref aeref; +/* Store the avc netlink fd. */ +static int avc_netlink_fd = -1; + +/* Watch to listen for SELinux status changes via netlink. */ +static DBusWatch *avc_netlink_watch_obj = NULL; +static DBusLoop *avc_netlink_loop_obj = NULL; + /* Store the SID of the bus itself to use as the default. */ static security_id_t bus_sid = SECSID_WILD; -/* Thread to listen for SELinux status changes via netlink. */ -static pthread_t avc_notify_thread; - /* Prototypes for AVC callback functions. */ -static void log_callback (const char *fmt, ...) _DBUS_GNUC_PRINTF (1, 2); -static void log_audit_callback (void *data, security_class_t class, char *buf, size_t bufleft); -static void *avc_create_thread (void (*run) (void)); -static void avc_stop_thread (void *thread); -static void *avc_alloc_lock (void); -static void avc_get_lock (void *lock); -static void avc_release_lock (void *lock); -static void avc_free_lock (void *lock); - -/* AVC callback structures for use in avc_init. */ -static const struct avc_memory_callback mem_cb = -{ - .func_malloc = dbus_malloc, - .func_free = dbus_free -}; -static const struct avc_log_callback log_cb = -{ - .func_log = log_callback, - .func_audit = log_audit_callback -}; -static const struct avc_thread_callback thread_cb = -{ - .func_create_thread = avc_create_thread, - .func_stop_thread = avc_stop_thread -}; -static const struct avc_lock_callback lock_cb = -{ - .func_alloc_lock = avc_alloc_lock, - .func_get_lock = avc_get_lock, - .func_release_lock = avc_release_lock, - .func_free_lock = avc_free_lock -}; +static int log_callback (int type, const char *fmt, ...) _DBUS_GNUC_PRINTF (2, 3); +static int log_audit_callback (void *data, security_class_t class, char *buf, size_t bufleft); + #endif /* HAVE_SELINUX */ /** @@ -115,8 +91,8 @@ static const struct avc_lock_callback lock_cb = */ #ifdef HAVE_SELINUX -static void -log_callback (const char *fmt, ...) +static int +log_callback (int type, const char *fmt, ...) { va_list ap; #ifdef HAVE_LIBAUDIT @@ -150,6 +126,8 @@ log_callback (const char *fmt, ...) out: #endif va_end(ap); + + return 0; } /** @@ -170,7 +148,7 @@ policy_reload_callback (u_int32_t event, security_id_t ssid, /** * Log any auxiliary data */ -static void +static int log_audit_callback (void *data, security_class_t class, char *buf, size_t bufleft) { DBusString *audmsg = data; @@ -188,73 +166,20 @@ log_audit_callback (void *data, security_class_t class, char *buf, size_t buflef if (bufleft > (size_t) _dbus_string_get_length(&s)) _dbus_string_copy_to_buffer_with_nul (&s, buf, bufleft); } -} - -/** - * Create thread to notify the AVC of enforcing and policy reload - * changes via netlink. - * - * @param run the thread run function - * @return pointer to the thread - */ -static void * -avc_create_thread (void (*run) (void)) -{ - int rc; - - rc = pthread_create (&avc_notify_thread, NULL, (void *(*) (void *)) run, NULL); - if (rc != 0) - { - _dbus_warn ("Failed to start AVC thread: %s", _dbus_strerror (rc)); - exit (1); - } - return &avc_notify_thread; -} -/* Stop AVC netlink thread. */ -static void -avc_stop_thread (void *thread) -{ - pthread_cancel (*(pthread_t *) thread); + return 0; } -/* Allocate a new AVC lock. */ -static void * -avc_alloc_lock (void) +static dbus_bool_t +handle_avc_netlink_watch (DBusWatch *passed_watch, unsigned int flags, void *data) { - pthread_mutex_t *avc_mutex; - - avc_mutex = dbus_new (pthread_mutex_t, 1); - if (avc_mutex == NULL) + if (avc_netlink_check_nb() < 0) { - _dbus_warn ("Could not create mutex: %s", _dbus_strerror (errno)); - exit (1); + _dbus_warn ("Failed to check the netlink socket for pending messages and process them: %s", _dbus_strerror(errno)); + return FALSE; } - pthread_mutex_init (avc_mutex, NULL); - return avc_mutex; -} - -/* Acquire an AVC lock. */ -static void -avc_get_lock (void *lock) -{ - pthread_mutex_lock (lock); -} - -/* Release an AVC lock. */ -static void -avc_release_lock (void *lock) -{ - pthread_mutex_unlock (lock); -} - -/* Free an AVC lock. */ -static void -avc_free_lock (void *lock) -{ - pthread_mutex_destroy (lock); - dbus_free (lock); + return TRUE; } #endif /* HAVE_SELINUX */ @@ -335,7 +260,7 @@ static struct security_class_mapping dbus_map[] = { * logging callbacks. */ dbus_bool_t -bus_selinux_full_init (void) +bus_selinux_full_init (BusContext *context) { #ifdef HAVE_SELINUX char *bus_context; @@ -358,9 +283,9 @@ bus_selinux_full_init (void) } avc_entry_ref_init (&aeref); - if (avc_init ("avc", &mem_cb, &log_cb, &thread_cb, &lock_cb) < 0) + if (avc_open (NULL, 0) < 0) { - _dbus_warn ("Failed to start Access Vector Cache (AVC)."); + _dbus_warn ("Failed to start Access Vector Cache (AVC): %s", _dbus_strerror (errno)); return FALSE; } else @@ -368,15 +293,38 @@ bus_selinux_full_init (void) _dbus_verbose ("Access Vector Cache (AVC) started.\n"); } + avc_netlink_fd = avc_netlink_acquire_fd(); + if (avc_netlink_fd < 0) + { + _dbus_warn ("Cannot acquire avc netlink fd"); + goto error; + } + + _dbus_fd_set_close_on_exec (avc_netlink_fd); + + avc_netlink_loop_obj = bus_context_get_loop (context); + _dbus_loop_ref (avc_netlink_loop_obj); + + avc_netlink_watch_obj = _dbus_watch_new (avc_netlink_fd, DBUS_WATCH_READABLE, TRUE, + handle_avc_netlink_watch, NULL, NULL); + + if (!_dbus_loop_add_watch (avc_netlink_loop_obj, avc_netlink_watch_obj)) + { + _dbus_warn ("Unable to add reload watch to main loop"); + goto error; + } + if (avc_add_callback (policy_reload_callback, AVC_CALLBACK_RESET, NULL, NULL, 0, 0) < 0) { _dbus_warn ("Failed to add policy reload callback: %s", _dbus_strerror (errno)); - avc_destroy (); - return FALSE; + goto error; } + selinux_set_callback (SELINUX_CB_AUDIT, (union selinux_callback) log_audit_callback); + selinux_set_callback (SELINUX_CB_LOG, (union selinux_callback) log_callback); + bus_context = NULL; bus_sid = SECSID_WILD; @@ -384,7 +332,7 @@ bus_selinux_full_init (void) { _dbus_verbose ("Error getting context of bus: %s\n", _dbus_strerror (errno)); - return FALSE; + goto error; } if (avc_context_to_sid (bus_context, &bus_sid) < 0) @@ -392,10 +340,28 @@ bus_selinux_full_init (void) _dbus_verbose ("Error getting SID from bus context: %s\n", _dbus_strerror (errno)); freecon (bus_context); - return FALSE; + goto error; } freecon (bus_context); + + return TRUE; + +error: + if (avc_netlink_watch_obj) { + _dbus_loop_remove_watch (avc_netlink_loop_obj, avc_netlink_watch_obj); + _dbus_watch_invalidate (avc_netlink_watch_obj); + _dbus_clear_watch (&avc_netlink_watch_obj); + } + if (avc_netlink_loop_obj) { + _dbus_clear_loop (&avc_netlink_loop_obj); + } + if (avc_netlink_fd >= 0) { + avc_netlink_release_fd (); + avc_netlink_fd = -1; + } + avc_destroy (); + return FALSE; #endif /* HAVE_SELINUX */ return TRUE; @@ -976,6 +942,24 @@ bus_selinux_shutdown (void) _dbus_verbose ("AVC shutdown\n"); + avc_netlink_loop_obj = NULL; + if (avc_netlink_watch_obj) + { + _dbus_loop_remove_watch (avc_netlink_loop_obj, avc_netlink_watch_obj); + _dbus_watch_invalidate (avc_netlink_watch_obj); + _dbus_clear_watch (&avc_netlink_watch_obj); + } + if (avc_netlink_loop_obj) + { + _dbus_clear_loop (&avc_netlink_loop_obj); + } + + if (avc_netlink_fd >= -1) + { + avc_netlink_release_fd (); + avc_netlink_fd = -1; + } + if (bus_sid != SECSID_WILD) { bus_sid = SECSID_WILD; diff --git a/bus/selinux.h b/bus/selinux.h index a0383cdd..ae233b60 100644 --- a/bus/selinux.h +++ b/bus/selinux.h @@ -28,7 +28,7 @@ #include "services.h" dbus_bool_t bus_selinux_pre_init (void); -dbus_bool_t bus_selinux_full_init(void); +dbus_bool_t bus_selinux_full_init(BusContext *context); void bus_selinux_shutdown (void); dbus_bool_t bus_selinux_enabled (void); diff --git a/bus/test-main.c b/bus/test-main.c index 400ea423..ba73a1b4 100644 --- a/bus/test-main.c +++ b/bus/test-main.c @@ -47,12 +47,6 @@ static DBusString test_data_dir; static void test_pre_hook (void) { - - if (_dbus_getenv ("DBUS_TEST_SELINUX") - && (!bus_selinux_pre_init () - || !bus_selinux_full_init ())) - _dbus_test_fatal ("Could not init selinux support"); - initial_fds = _dbus_check_fdleaks_enter (); } diff --git a/bus/test.c b/bus/test.c index 76960a30..23245ac9 100644 --- a/bus/test.c +++ b/bus/test.c @@ -28,6 +28,8 @@ #include #include #include +#include +#include "selinux.h" /* The "debug client" watch/timeout handlers don't dispatch messages, * as we manually pull them in order to verify them. This is why they @@ -307,6 +309,11 @@ bus_context_new_test (const DBusString *test_data_dir, return NULL; } + if (_dbus_getenv ("DBUS_TEST_SELINUX") + && (!bus_selinux_pre_init () + || !bus_selinux_full_init (context))) + _dbus_test_fatal ("Could not init selinux support"); + _dbus_string_free (&config_file); return context; -- 2.16.2