diff options
author | Simon McVittie <smcv@collabora.com> | 2023-06-29 17:28:40 +0100 |
---|---|---|
committer | Simon McVittie <smcv@collabora.com> | 2023-08-21 13:49:31 +0000 |
commit | 8de818153b6d75f6d5592949a6bf3d9d711bcfa4 (patch) | |
tree | bfe55becfe5227870e24da5144f3068ab234faef | |
parent | 12b367daaaabab838537bfbe8164f095a6294ec3 (diff) |
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>
-rw-r--r-- | dbus/dbus-sysdeps-unix.c | 36 |
1 files changed, 29 insertions, 7 deletions
diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c index d08516ab..8438587f 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); |