From: Simon McVittie <smcv@collabora.com>
Date: Thu, 29 Jun 2023 16:28:40 +0000 (+0100)
Subject: sysdeps: Improve error reporting for looking up a user
X-Git-Tag: dbus-1.15.8~3^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8de818153b6d75f6d5592949a6bf3d9d711bcfa4;p=thirdparty%2Fdbus.git

sysdeps: Improve error reporting for looking up a user

Our implementation always assumed that both code paths set errno, but
according to their API documentation, getpwnam_r and getpwuid_r actually
don't: they *return* a code from the same pseudo-enum as errno. They
also return 0 (but with a NULL struct passwd) if the user is not found,
which these APIs don't count as an error (but we do).

Similarly, in the legacy getpwnam/getpwuid code path, it is unspecified
whether looking up a nonexistent user will set errno or not.

Having retrieved an errno-like error code, we might as well use it in
the human-readable message and not just the machine-readable code,
because the human-readable message is what ends up in the system log.

Helps: https://gitlab.freedesktop.org/dbus/dbus/-/issues/343
Signed-off-by: Simon McVittie <smcv@collabora.com>
---

diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c
index d08516ab5..8438587f8 100644
--- a/dbus/dbus-sysdeps-unix.c
+++ b/dbus/dbus-sysdeps-unix.c
@@ -2730,8 +2730,8 @@ fill_user_info (DBusUserInfo       *info,
   {
     struct passwd *p;
     char *buf = NULL;
-#ifdef HAVE_GETPWNAM_R
     int result;
+#ifdef HAVE_GETPWNAM_R
     size_t buflen;
     struct passwd p_str;
 
@@ -2774,16 +2774,32 @@ fill_user_info (DBusUserInfo       *info,
           }
       }
 
+    /* There are three possibilities:
+     * - an error: result is a nonzero error code, p should be NULL
+     * - name or uid not found: result is 0, p is NULL
+     * - success: result is 0, p should be &p_str
+     *
+     * Ensure that in all failure cases, p is set to NULL, matching the
+     * getpwuid/getpwnam interface. */
     if (result != 0 || p != &p_str)
       p = NULL;
+
 #else /* ! HAVE_GETPWNAM_R */
     /* I guess we're screwed on thread safety here */
 #warning getpwnam_r() not available, please report this to the dbus maintainers with details of your OS
 
+    /* It is unspecified whether "failed to find" counts as an error,
+     * or whether it's reported as p == NULL without touching errno.
+     * Reset errno so we can distinguish. */
+    errno = 0;
+
     if (uid != DBUS_UID_UNSET)
       p = getpwuid (uid);
     else
       p = getpwnam (username_c);
+
+    /* Always initialized, but only meaningful if p is NULL */
+    result = errno;
 #endif  /* ! HAVE_GETPWNAM_R */
 
     if (p != NULL)
@@ -2798,15 +2814,21 @@ fill_user_info (DBusUserInfo       *info,
     else
       {
         DBusError local_error = DBUS_ERROR_INIT;
+        const char *error_str;
+
+        if (result == 0)
+          error_str = "not found";
+        else
+          error_str = _dbus_strerror (result);
 
         if (uid != DBUS_UID_UNSET)
-          dbus_set_error (&local_error, _dbus_error_from_errno (errno),
-                          "User ID " DBUS_UID_FORMAT " unknown or no memory to allocate password entry",
-                          uid);
+          dbus_set_error (&local_error, _dbus_error_from_errno (result),
+                          "Looking up user ID " DBUS_UID_FORMAT ": %s",
+                          uid, error_str);
         else
-          dbus_set_error (&local_error, _dbus_error_from_errno (errno),
-                          "User \"%s\" unknown or no memory to allocate password entry\n",
-                          username_c ? username_c : "???");
+          dbus_set_error (&local_error, _dbus_error_from_errno (result),
+                          "Looking up user \"%s\": %s",
+                          username_c ? username_c : "???", error_str);
 
         _dbus_verbose ("%s", local_error.message);
         dbus_move_error (&local_error, error);