Attachment 139865 Details for Bug 92831 – Stop using avc_init() which is deprecated

[patch] Stop using avc_init() which is deprecated

Stop-using-avcinit-which-is-deprecated.patch (text/plain), 12.14 KB, created by Laurent Bigonville on 2018-05-30 16:13:30 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Laurent Bigonville

Created: 2018-05-30 16:13:30 UTC

Size: 12.14 KB

patch

obsolete

>From 3798e38d1ac6d5ccf1718648292ba7cd282038c1 Mon Sep 17 00:00:00 2001
>From: Laurent Bigonville <bigon@bigon.be>
>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       |  15 ++--
> bus/selinux.c   | 218 ++++++++++++++++++++++++------------------------
> bus/selinux.h   |   2 +-
> bus/test-main.c |   6 --
> bus/test.c      |   9 ++
> 5 files changed, 127 insertions(+), 123 deletions(-)
>
>diff --git a/bus/bus.c b/bus/bus.c
>index 9fd9820b..e3e26912 100644
>--- a/bus/bus.c
>+++ b/bus/bus.c
>@@ -995,12 +995,10 @@ bus_context_new (const DBusString *config_file,
>    */
>   bus_audit_init (context);
> 
>-  if (!bus_selinux_full_init ())
>+  if (!bus_selinux_full_init (context, error))
>     {
>-      bus_context_log (context, DBUS_SYSTEM_LOG_ERROR,
>-                       "SELinux enabled but D-Bus initialization failed; "
>-                       "check system log");
>-      exit (1);
>+      _DBUS_ASSERT_ERROR_IS_SET (error);
>+      goto failed;
>     }
> 
>   if (!bus_apparmor_full_init (error))
>@@ -1009,6 +1007,13 @@ bus_context_new (const DBusString *config_file,
>       goto failed;
>     }
> 
>+  if (bus_selinux_enabled ())
>+    {
>+      if (context->syslog)
>+        bus_context_log (context, DBUS_SYSTEM_LOG_INFO,
>+                         "SELinux support is enabled\n");
>+    }
>+
>   if (bus_apparmor_enabled ())
>     {
>       /* Only print AppArmor mediation message when syslog support is enabled */
>diff --git a/bus/selinux.c b/bus/selinux.c
>index d09afb4b..5aef0902 100644
>--- a/bus/selinux.c
>+++ b/bus/selinux.c
>@@ -49,6 +49,7 @@
> #include <stdarg.h>
> #include <stdio.h>
> #include <grp.h>
>+#include <dbus/dbus-watch.h>
> #endif /* HAVE_SELINUX */
> #ifdef HAVE_LIBAUDIT
> #include <libaudit.h>
>@@ -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, DBusError *error)
> {
> #ifdef HAVE_SELINUX
>   char *bus_context;
>@@ -358,9 +283,11 @@ 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_set_error (error, DBUS_ERROR_FAILED,
>+                      "Failed to start Access Vector Cache (AVC): %s",
>+                      _dbus_strerror (errno));
>       return FALSE;
>     }
>   else
>@@ -368,34 +295,89 @@ 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_set_error (error, DBUS_ERROR_FAILED,
>+                      "Cannot acquire AVC netlink fd: %s",
>+                      _dbus_strerror (errno));
>+       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 (avc_netlink_watch_obj == NULL)
>+    {
>+      BUS_SET_OOM (error);
>+      goto error;
>+    }
>+
>+  if (!_dbus_loop_add_watch (avc_netlink_loop_obj, avc_netlink_watch_obj))
>+    {
>+      _dbus_watch_invalidate (avc_netlink_watch_obj);
>+      _dbus_clear_watch (&avc_netlink_watch_obj);
>+      avc_netlink_watch_obj = NULL;
>+      BUS_SET_OOM (error);
>+      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;
>+      dbus_set_error (error, DBUS_ERROR_FAILED,
>+                      "Failed to add policy reload callback: %s",
>+                      _dbus_strerror (errno));
>+      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;
> 
>   if (getcon (&bus_context) < 0)
>     {
>-      _dbus_verbose ("Error getting context of bus: %s\n",
>-                     _dbus_strerror (errno));
>-      return FALSE;
>+      dbus_set_error (error, DBUS_ERROR_FAILED,
>+                      "Error getting context of bus: %s",
>+                      _dbus_strerror (errno));
>+      goto error;
>     }
>       
>   if (avc_context_to_sid (bus_context, &bus_sid) < 0)
>     {
>-      _dbus_verbose ("Error getting SID from bus context: %s\n",
>-                     _dbus_strerror (errno));
>+      dbus_set_error (error, DBUS_ERROR_FAILED,
>+                      "Error getting SID from bus context: %s",
>+                      _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);
>+    }
>+  _dbus_clear_loop (&avc_netlink_loop_obj);
>+  if (avc_netlink_fd >= 0)
>+    {
>+      avc_netlink_release_fd ();
>+      avc_netlink_fd = -1;
>+    }
>+  avc_destroy ();
>+  _DBUS_ASSERT_ERROR_IS_SET (error);
>+  return FALSE;
>   
> #endif /* HAVE_SELINUX */
>   return TRUE;
>@@ -976,6 +958,20 @@ bus_selinux_shutdown (void)
> 
>   _dbus_verbose ("AVC shutdown\n");
> 
>+  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);
>+    }
>+  _dbus_clear_loop (&avc_netlink_loop_obj);
>+
>+  if (avc_netlink_fd >= 0)
>+    {
>+      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..53de1a84 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, DBusError *error);
> 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..730cd64a 100644
>--- a/bus/test.c
>+++ b/bus/test.c
>@@ -28,6 +28,8 @@
> #include <dbus/dbus-internals.h>
> #include <dbus/dbus-list.h>
> #include <dbus/dbus-sysdeps.h>
>+#include <dbus/dbus-test-tap.h>
>+#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,13 @@ 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, &error)))
>+    _dbus_test_fatal ("Could not init selinux support");
>+
>+  dbus_error_free (&error);
>+
>   _dbus_string_free (&config_file);
> 
>   return context;
>-- 
>2.17.1

Actions: View | Splinter Review

Attachments on bug 92831: 137773 | 137794 | 137801 | 137802 | 137803 | 138031 | 139865 | 139866

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.