userdb cache expiry

Tim Dijkstra newsuser at famdijkstra.org
Thu Sep 21 06:41:30 PDT 2006


On Thu, 21 Sep 2006 12:53:15 +0200
Tim Dijkstra <newsuser at famdijkstra.org> wrote:
 
> I think it would be better to change _dbus_user_database_get_groups to
> _dbus_groups_from_uid (just like _dbus_homedir_from_username) and do the 
> database handling inside it. That way calling functions do not have know 
> about the database at all.

I prepared a patch that does this. Because _dbus_groups_from_uid no
longer needs a reference to a DBusUserDatabase, I removed that from the
BusContext. Unless I'm missing something, it was weird to have it in
there anyway. There were two caches in use simultaneously, the
'system_db' used internally and the one in the BusContext. Only that one
got flushed on config_reload, btw; that could account for some weirdness
I've been seeing...

With this change DBusUserDatabase is entirely private to
dbus-userdb{-util,}.c so I also made all the functions that access it
directly private in the dbus-userdb.h header file.

This is only compile and 'make check' tested btw. I think this (or
something similar) should go in independent of shutting off caching
altogether.

grts Tim

Index: bus/bus.c
===================================================================
RCS file: /cvs/dbus/dbus/bus/bus.c,v
retrieving revision 1.71
diff -u -r1.71 bus.c
--- bus/bus.c	16 Sep 2006 15:34:34 -0000	1.71
+++ bus/bus.c	21 Sep 2006 13:35:09 -0000
@@ -34,6 +34,7 @@
 #include <dbus/dbus-list.h>
 #include <dbus/dbus-hash.h>
 #include <dbus/dbus-internals.h>
+#include <dbus/dbus-userdb.h>
 
 struct BusContext
 {
@@ -50,7 +51,6 @@
   BusRegistry *registry;
   BusPolicy *policy;
   BusMatchmaker *matchmaker;
-  DBusUserDatabase *user_database;
   BusLimits limits;
   unsigned int fork : 1;
 };
@@ -596,13 +596,6 @@
   if (!dbus_server_allocate_data_slot (&server_data_slot))
     _dbus_assert_not_reached ("second ref of server data slot failed");
 
-  context->user_database = _dbus_user_database_new ();
-  if (context->user_database == NULL)
-    {
-      BUS_SET_OOM (error);
-      goto failed;
-    }
-  
   /* Note that we don't know whether the print_addr_fd is
    * one of the sockets we're using to listen on, or some
    * other random thing. But I think the answer is "don't do
@@ -800,8 +793,7 @@
   dbus_bool_t ret;
 
   /* Flush the user database cache */
-  _dbus_user_database_flush(context->user_database);
-
+  //FIXME: we should flush the system database here, right?
   ret = FALSE;
   _dbus_string_init_const (&config_file, context->config_file);
   parser = bus_config_load (&config_file, TRUE, NULL, error);
@@ -951,10 +943,6 @@
 
           dbus_free (context->pidfile); 
 	}
-
-      if (context->user_database != NULL)
-	_dbus_user_database_unref (context->user_database);
-      
       dbus_free (context);
 
       dbus_server_free_data_slot (&server_data_slot);
@@ -1004,18 +992,11 @@
   return context->loop;
 }
 
-DBusUserDatabase*
-bus_context_get_user_database (BusContext *context)
-{
-  return context->user_database;
-}
-
 dbus_bool_t
 bus_context_allow_user (BusContext   *context,
                         unsigned long uid)
 {
   return bus_policy_allow_user (context->policy,
-                                context->user_database,
                                 uid);
 }
 
Index: bus/bus.h
===================================================================
RCS file: /cvs/dbus/dbus/bus/bus.h,v
retrieving revision 1.27
diff -u -r1.27 bus.h
--- bus/bus.h	22 Nov 2005 20:37:00 -0000	1.27
+++ bus/bus.h	21 Sep 2006 13:35:09 -0000
@@ -29,7 +29,6 @@
 #include <dbus/dbus.h>
 #include <dbus/dbus-string.h>
 #include <dbus/dbus-mainloop.h>
-#include <dbus/dbus-userdb.h>
 
 typedef struct BusActivation    BusActivation;
 typedef struct BusConnections   BusConnections;
@@ -86,8 +85,6 @@
 BusActivation*    bus_context_get_activation                     (BusContext       *context);
 BusMatchmaker*    bus_context_get_matchmaker                     (BusContext       *context);
 DBusLoop*         bus_context_get_loop                           (BusContext       *context);
-DBusUserDatabase* bus_context_get_user_database                  (BusContext       *context);
-
 dbus_bool_t       bus_context_allow_user                         (BusContext       *context,
                                                                   unsigned long     uid);
 BusPolicy*        bus_context_get_policy                         (BusContext       *context);
Index: bus/connection.c
===================================================================
RCS file: /cvs/dbus/dbus/bus/connection.c,v
retrieving revision 1.64
diff -u -r1.64 connection.c
--- bus/connection.c	11 Sep 2006 17:41:21 -0000	1.64
+++ bus/connection.c	21 Sep 2006 13:35:10 -0000
@@ -31,6 +31,7 @@
 #include <dbus/dbus-list.h>
 #include <dbus/dbus-hash.h>
 #include <dbus/dbus-timeout.h>
+#include <dbus/dbus-userdb.h>
 
 static void bus_connection_remove_transactions (DBusConnection *connection);
 
@@ -776,24 +777,18 @@
 {
   BusConnectionData *d;
   unsigned long uid;
-  DBusUserDatabase *user_database;
   
   d = BUS_CONNECTION_DATA (connection);
 
   _dbus_assert (d != NULL);
 
-  user_database = bus_context_get_user_database (d->connections->context);
-  
   *groups = NULL;
   *n_groups = 0;
 
   if (dbus_connection_get_unix_user (connection, &uid))
     {
-      if (!_dbus_user_database_get_groups (user_database,
-                                           uid, groups, n_groups,
-                                           error))
+      if (!_dbus_groups_from_uid (uid, groups, n_groups))
         {
-          _DBUS_ASSERT_ERROR_IS_SET (error);
           _dbus_verbose ("Did not get any groups for UID %lu\n",
                          uid);
           return FALSE;
Index: bus/policy.c
===================================================================
RCS file: /cvs/dbus/dbus/bus/policy.c,v
retrieving revision 1.23
diff -u -r1.23 policy.c
--- bus/policy.c	29 Aug 2005 20:19:19 -0000	1.23
+++ bus/policy.c	21 Sep 2006 13:35:11 -0000
@@ -28,6 +28,7 @@
 #include <dbus/dbus-list.h>
 #include <dbus/dbus-hash.h>
 #include <dbus/dbus-internals.h>
+#include <dbus/dbus-userdb.h>
 
 BusPolicyRule*
 bus_policy_rule_new (BusPolicyRuleType type,
@@ -438,7 +439,6 @@
 
 dbus_bool_t
 bus_policy_allow_user (BusPolicy        *policy,
-                       DBusUserDatabase *user_database,
                        unsigned long     uid)
 {
   dbus_bool_t allowed;
@@ -446,8 +446,7 @@
   int n_group_ids;
 
   /* On OOM or error we always reject the user */
-  if (!_dbus_user_database_get_groups (user_database,
-                                       uid, &group_ids, &n_group_ids, NULL))
+  if (!_dbus_groups_from_uid (uid, &group_ids, &n_group_ids))
     {
       _dbus_verbose ("Did not get any groups for UID %lu\n",
                      uid);
Index: bus/policy.h
===================================================================
RCS file: /cvs/dbus/dbus/bus/policy.h,v
retrieving revision 1.16
diff -u -r1.16 policy.h
--- bus/policy.h	25 Aug 2004 22:11:49 -0000	1.16
+++ bus/policy.h	21 Sep 2006 13:35:11 -0000
@@ -113,7 +113,6 @@
                                                    DBusConnection   *connection,
                                                    DBusError        *error);
 dbus_bool_t      bus_policy_allow_user            (BusPolicy        *policy,
-                                                   DBusUserDatabase *user_database,
                                                    unsigned long     uid);
 dbus_bool_t      bus_policy_append_default_rule   (BusPolicy        *policy,
                                                    BusPolicyRule    *rule);
Index: dbus/dbus-userdb-util.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-userdb-util.c,v
retrieving revision 1.7
diff -u -r1.7 dbus-userdb-util.c
--- dbus/dbus-userdb-util.c	7 Jun 2006 19:59:11 -0000	1.7
+++ dbus/dbus-userdb-util.c	21 Sep 2006 13:35:11 -0000
@@ -86,6 +86,8 @@
       return FALSE;
     }
 
+  /* TPTD: this should be cache-safe, we've locked the DB and
+    _dbus_user_at_console doesn't pass it on. */
   info = _dbus_user_database_lookup (db, uid, NULL, error);
 
   if (info == NULL)
@@ -347,45 +349,51 @@
 
 
 /**
- * Gets all groups for a particular user. Returns #FALSE
+ * Gets all groups  corresponding to the given UID. Returns #FALSE
  * if no memory, or user isn't known, but always initializes
- * group_ids to a NULL array. Sets error to the reason
- * for returning #FALSE.
+ * group_ids to a NULL array. 
  *
- * @param db the user database object
- * @param uid the user ID
+ * @param uid the UID
  * @param group_ids return location for array of group IDs
  * @param n_group_ids return location for length of returned array
- * @param error return location for error
- * @returns #TRUE on success
+ * @returns #TRUE if the UID existed and we got some credentials
  */
+//FIXME: This doesn't set an error when memory runs out,
+// but then again, neither does _dbus_homedir_from_username, etc
 dbus_bool_t
-_dbus_user_database_get_groups (DBusUserDatabase  *db,
-                                dbus_uid_t         uid,
-                                dbus_gid_t       **group_ids,
-                                int               *n_group_ids,
-                                DBusError         *error)
+_dbus_groups_from_uid (dbus_uid_t         uid,
+                       dbus_gid_t       **group_ids,
+                       int               *n_group_ids)
 {
-  DBusUserInfo *info;
-  
-  _DBUS_ASSERT_ERROR_IS_CLEAR (error);
-
+  DBusUserDatabase *db;
+  const DBusUserInfo *info;
   *group_ids = NULL;
   *n_group_ids = 0;
-  
-  info = _dbus_user_database_lookup (db, uid, NULL, error);
-  if (info == NULL)
+
+  _dbus_user_database_lock_system ();
+
+  db = _dbus_user_database_get_system ();
+  if (db == NULL)
+    {
+      _dbus_user_database_unlock_system ();
+      return FALSE;
+    }
+
+  if (!_dbus_user_database_get_uid (db, uid,
+                                    &info, NULL))
     {
-      _DBUS_ASSERT_ERROR_IS_SET (error);
+      _dbus_user_database_unlock_system ();
       return FALSE;
     }
 
+  _dbus_assert (info->uid == uid);
+  
   if (info->n_group_ids > 0)
     {
       *group_ids = dbus_new (dbus_gid_t, info->n_group_ids);
       if (*group_ids == NULL)
         {
-          dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL);
+	  _dbus_user_database_unlock_system ();
           return FALSE;
         }
 
@@ -394,9 +402,9 @@
       memcpy (*group_ids, info->group_ids, info->n_group_ids * sizeof (dbus_gid_t));
     }
 
+  _dbus_user_database_unlock_system ();
   return TRUE;
 }
-
 /** @} */
 
 #ifdef DBUS_BUILD_TESTS
Index: dbus/dbus-userdb.h
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-userdb.h,v
retrieving revision 1.11
diff -u -r1.11 dbus-userdb.h
--- dbus/dbus-userdb.h	6 Mar 2006 19:06:45 -0000	1.11
+++ dbus/dbus-userdb.h	21 Sep 2006 13:35:11 -0000
@@ -47,17 +47,11 @@
 
 };
 
-#endif /* DBUS_USERDB_INCLUDES_PRIVATE */
 
 DBusUserDatabase* _dbus_user_database_new           (void);
 DBusUserDatabase* _dbus_user_database_ref           (DBusUserDatabase     *db);
 void              _dbus_user_database_flush         (DBusUserDatabase     *db);
 void              _dbus_user_database_unref         (DBusUserDatabase     *db);
-dbus_bool_t       _dbus_user_database_get_groups    (DBusUserDatabase     *db,
-                                                     dbus_uid_t            uid,
-                                                     dbus_gid_t          **group_ids,
-                                                     int                  *n_group_ids,
-                                                     DBusError            *error);
 dbus_bool_t       _dbus_user_database_get_uid       (DBusUserDatabase     *db,
                                                      dbus_uid_t            uid,
                                                      const DBusUserInfo  **info,
@@ -75,7 +69,6 @@
                                                      const DBusGroupInfo **info,
                                                      DBusError            *error);
 
-#ifdef DBUS_USERDB_INCLUDES_PRIVATE
 DBusUserInfo*  _dbus_user_database_lookup       (DBusUserDatabase *db,
                                                  dbus_uid_t        uid,
                                                  const DBusString *username,
@@ -104,6 +97,9 @@
                                                  DBusCredentials   *credentials);
 dbus_bool_t _dbus_credentials_from_uid          (dbus_uid_t         user_id,
                                                  DBusCredentials   *credentials);
+dbus_bool_t _dbus_groups_from_uid		(dbus_uid_t            uid,
+                                                 dbus_gid_t          **group_ids,
+                                                 int                  *n_group_ids);
 dbus_bool_t _dbus_is_console_user               (dbus_uid_t         uid,
                                                  DBusError         *error);
 




More information about the dbus mailing list