From 39b52d0fd015382a5089f57b4559a97340c7d4fc Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 24 Sep 2020 19:35:57 +0200 Subject: [PATCH 1/3] shared: add nm_utils_uid_to_name()/nm_utils_name_to_uid() helpers These are inspired by systemd. We should replace our calls to getpwuid() and getpwnam() with their thread safe variants. We run possibly multiple threads (e.g. helper threads from GDBus and GResolver). It's hard to be sure that they don't also access the functions. --- shared/nm-glib-aux/nm-shared-utils.c | 79 ++++++++++++++++++++++++++++ shared/nm-glib-aux/nm-shared-utils.h | 3 ++ 2 files changed, 82 insertions(+) diff --git a/shared/nm-glib-aux/nm-shared-utils.c b/shared/nm-glib-aux/nm-shared-utils.c index 57c1caeb6b..38a680928e 100644 --- a/shared/nm-glib-aux/nm-shared-utils.c +++ b/shared/nm-glib-aux/nm-shared-utils.c @@ -7,6 +7,7 @@ #include "nm-shared-utils.h" +#include #include #include #include @@ -5636,3 +5637,81 @@ nm_utils_is_specific_hostname (const char *name) return TRUE; } + +/*****************************************************************************/ + +/* taken from systemd's uid_to_name(). */ +char * +nm_utils_uid_to_name (uid_t uid) +{ + gs_free char *buf_heap = NULL; + char buf_stack[4096]; + gsize bufsize; + char *buf; + + bufsize = sizeof (buf_stack); + buf = buf_stack; + + for (;;) { + struct passwd pwbuf; + struct passwd *pw = NULL; + int r; + + r = getpwuid_r (uid, &pwbuf, buf, bufsize, &pw); + if ( r == 0 + && pw) + return nm_strdup_not_empty (pw->pw_name); + + if (r != ERANGE) + return NULL; + + if (bufsize > G_MAXSIZE / 2u) + return NULL; + + bufsize *= 2u; + g_free (buf_heap); + buf_heap = g_malloc (bufsize); + buf = buf_heap; + } +} + +/* taken from systemd's nss_user_record_by_name() */ +gboolean +nm_utils_name_to_uid (const char *name, uid_t *out_uid) +{ + gs_free char *buf_heap = NULL; + char buf_stack[4096]; + gsize bufsize; + char *buf; + + if (!name) + return nm_assert_unreachable_val (FALSE); + + bufsize = sizeof (buf_stack); + buf = buf_stack; + + for (;;) { + struct passwd *result; + struct passwd pwd; + int r; + + r = getpwnam_r (name, &pwd, buf, bufsize, &result); + if (r == 0) { + if (!result) + return FALSE; + NM_SET_OUT (out_uid, pwd.pw_uid); + return TRUE; + } + + if (r != ERANGE) + return FALSE; + + if (bufsize > G_MAXSIZE / 2u) + return FALSE; + + bufsize *= 2u; + g_free (buf_heap); + buf_heap = g_malloc (bufsize); + buf = buf_heap; + } +} diff --git a/shared/nm-glib-aux/nm-shared-utils.h b/shared/nm-glib-aux/nm-shared-utils.h index 28a519ae44..f4caf3d74c 100644 --- a/shared/nm-glib-aux/nm-shared-utils.h +++ b/shared/nm-glib-aux/nm-shared-utils.h @@ -2339,4 +2339,7 @@ gboolean nm_utils_is_localhost (const char *name); gboolean nm_utils_is_specific_hostname (const char *name); +char *nm_utils_uid_to_name (uid_t uid); +gboolean nm_utils_name_to_uid (const char *name, uid_t *out_uid); + #endif /* __NM_SHARED_UTILS_H__ */ From 714955c3b5fb63f8b0043ada65d232a7b8d4fff8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 24 Sep 2020 19:49:25 +0200 Subject: [PATCH 2/3] all: use nm_utils_uid_to_name() instead of getpwuid() We shouldn't use non-threadsafe API. We don't really know what other threads (e.g. GDBus' helper thread or GResolver) are doing. --- clients/common/nm-polkit-listener.c | 31 ++++++++++++----------------- src/nm-auth-utils.c | 5 +++-- src/nm-session-monitor.c | 22 -------------------- src/nm-session-monitor.h | 1 - src/settings/nm-agent-manager.c | 16 +++++++-------- src/settings/nm-secret-agent.c | 9 +++------ 6 files changed, 26 insertions(+), 58 deletions(-) diff --git a/clients/common/nm-polkit-listener.c b/clients/common/nm-polkit-listener.c index ace205e809..a694cf49bf 100644 --- a/clients/common/nm-polkit-listener.c +++ b/clients/common/nm-polkit-listener.c @@ -164,20 +164,14 @@ auth_request_complete (AuthRequest *request, gboolean success) static gboolean uid_to_name (uid_t uid, - const char **out_name) + gboolean *out_cached, + char **out_name) { - const struct passwd *passwd; - - if (*out_name) - return TRUE; - - passwd = getpwuid (uid); - if ( !passwd - || !passwd->pw_name) - return FALSE; - - *out_name = passwd->pw_name; - return TRUE; + if (!*out_cached) { + *out_cached = TRUE; + *out_name = nm_utils_uid_to_name (uid); + } + return !!(*out_name); } static char * @@ -205,17 +199,18 @@ choose_identity (GVariant *identities) v = g_variant_lookup_value (details, "uid", G_VARIANT_TYPE_UINT32); if (v) { guint32 uid = g_variant_get_uint32 (v); - const char *u = NULL; + gs_free char *u = NULL; + gboolean cached = FALSE; if (user) { - if (!uid_to_name (uid, &u)) + if (!uid_to_name (uid, &cached, &u)) continue; if (nm_streq (u, user)) - return g_strdup (user); + return g_steal_pointer (&u); } if ( !username_root && uid == 0) { - if (!uid_to_name (uid, &u)) + if (!uid_to_name (uid, &cached, &u)) continue; username_root = g_strdup (u); if (!user) @@ -223,7 +218,7 @@ choose_identity (GVariant *identities) } if ( !username_root && !username_first) { - if (!uid_to_name (uid, &u)) + if (!uid_to_name (uid, &cached, &u)) continue; username_first = g_strdup (u); } diff --git a/src/nm-auth-utils.c b/src/nm-auth-utils.c index d4728a7f33..80dcaaf113 100644 --- a/src/nm-auth-utils.c +++ b/src/nm-auth-utils.c @@ -618,7 +618,7 @@ nm_auth_is_subject_in_acl (NMConnection *connection, char **out_error_desc) { NMSettingConnection *s_con; - const char *user = NULL; + gs_free char *user = NULL; gulong uid; g_return_val_if_fail (connection, FALSE); @@ -635,7 +635,8 @@ nm_auth_is_subject_in_acl (NMConnection *connection, if (0 == uid) return TRUE; - if (!nm_session_monitor_uid_to_user (uid, &user)) { + user = nm_utils_uid_to_name (uid); + if (!user) { NM_SET_OUT (out_error_desc, g_strdup_printf ("Could not determine username for uid %lu", uid)); return FALSE; diff --git a/src/nm-session-monitor.c b/src/nm-session-monitor.c index 6fdc57ebc5..6e2d95a219 100644 --- a/src/nm-session-monitor.c +++ b/src/nm-session-monitor.c @@ -280,28 +280,6 @@ ck_finalize (NMSessionMonitor *monitor) NM_DEFINE_SINGLETON_GETTER (NMSessionMonitor, nm_session_monitor_get, NM_TYPE_SESSION_MONITOR); -/** - * nm_session_monitor_uid_to_user: - * @uid: UID. - * @out_user: Return location for user name. - * - * Translates a UID to a user name. - */ -gboolean -nm_session_monitor_uid_to_user (uid_t uid, const char **out_user) -{ - struct passwd *pw = getpwuid (uid); - - g_assert (out_user); - - if (!pw) - return FALSE; - - *out_user = pw->pw_name; - - return TRUE; -} - /** * nm_session_monitor_user_to_uid: * @user: User naee. diff --git a/src/nm-session-monitor.h b/src/nm-session-monitor.h index 64f9194203..7ea1e14a91 100644 --- a/src/nm-session-monitor.h +++ b/src/nm-session-monitor.h @@ -23,7 +23,6 @@ GType nm_session_monitor_get_type (void) G_GNUC_CONST; NMSessionMonitor *nm_session_monitor_get (void); -gboolean nm_session_monitor_uid_to_user (uid_t uid, const char **out_user); gboolean nm_session_monitor_user_to_uid (const char *user, uid_t *out_uid); gboolean nm_session_monitor_session_exists (NMSessionMonitor *self, uid_t uid, diff --git a/src/settings/nm-agent-manager.c b/src/settings/nm-agent-manager.c index a3b99e94e7..40d43775c2 100644 --- a/src/settings/nm-agent-manager.c +++ b/src/settings/nm-agent-manager.c @@ -869,8 +869,7 @@ _con_get_request_done (NMSecretAgent *agent, Request *req = user_data; GVariant *setting_secrets; const char *agent_dbus_owner; - struct passwd *pw; - char *agent_uname = NULL; + gs_free char *agent_name = NULL; g_return_if_fail (call_id == req->current_call_id); g_return_if_fail (agent == req->current); @@ -921,17 +920,16 @@ _con_get_request_done (NMSecretAgent *agent, _LOGD (agent, "agent returned secrets for request "LOG_REQ_FMT, LOG_REQ_ARG (req)); - /* Get the agent's username */ - pw = getpwuid (nm_secret_agent_get_owner_uid (agent)); - if (pw && strlen (pw->pw_name)) { + agent_name = nm_utils_uid_to_name (nm_secret_agent_get_owner_uid (agent)); + if ( agent_name + && !g_utf8_validate (agent_name, -1, NULL)) { /* Needs to be UTF-8 valid since it may be pushed through D-Bus */ - if (g_utf8_validate (pw->pw_name, -1, NULL)) - agent_uname = g_strdup (pw->pw_name); + nm_clear_g_free (&agent_name); } agent_dbus_owner = nm_secret_agent_get_dbus_owner (agent); - req_complete (req, secrets, agent_dbus_owner, agent_uname, NULL); - g_free (agent_uname); + + req_complete (req, secrets, agent_dbus_owner, agent_name, NULL); } static void diff --git a/src/settings/nm-secret-agent.c b/src/settings/nm-secret-agent.c index d7172c6392..367e7c43c6 100644 --- a/src/settings/nm-secret-agent.c +++ b/src/settings/nm-secret-agent.c @@ -695,8 +695,7 @@ nm_secret_agent_new (GDBusMethodInvocation *context, NMSecretAgent *self; NMSecretAgentPrivate *priv; const char *dbus_owner; - struct passwd *pw; - char *owner_username = NULL; + gs_free char *owner_username = NULL; char *description = NULL; char buf_subject[64]; char buf_caps[150]; @@ -715,9 +714,7 @@ nm_secret_agent_new (GDBusMethodInvocation *context, uid = nm_auth_subject_get_unix_process_uid (subject); - pw = getpwuid (uid); - if (pw && pw->pw_name && pw->pw_name[0]) - owner_username = g_strdup (pw->pw_name); + owner_username = nm_utils_uid_to_name (uid); dbus_owner = nm_auth_subject_get_unix_process_dbus_sender (subject); @@ -735,7 +732,7 @@ nm_secret_agent_new (GDBusMethodInvocation *context, _capabilities_to_string (capabilities, buf_caps, sizeof (buf_caps))); priv->identifier = g_strdup (identifier); - priv->owner_username = owner_username; + priv->owner_username = g_steal_pointer (&owner_username); priv->dbus_owner = g_strdup (dbus_owner); priv->description = description; priv->capabilities = capabilities; From 99abd397d03145c50ec21cf4c71e2f23ee716098 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 24 Sep 2020 20:15:22 +0200 Subject: [PATCH 3/3] all: use nm_utils_name_to_uid() instead of getpwnam() --- src/nm-session-monitor.c | 22 ---------------------- src/nm-session-monitor.h | 1 - src/settings/nm-settings-connection.c | 2 +- 3 files changed, 1 insertion(+), 24 deletions(-) diff --git a/src/nm-session-monitor.c b/src/nm-session-monitor.c index 6e2d95a219..9fa9215e7a 100644 --- a/src/nm-session-monitor.c +++ b/src/nm-session-monitor.c @@ -280,28 +280,6 @@ ck_finalize (NMSessionMonitor *monitor) NM_DEFINE_SINGLETON_GETTER (NMSessionMonitor, nm_session_monitor_get, NM_TYPE_SESSION_MONITOR); -/** - * nm_session_monitor_user_to_uid: - * @user: User naee. - * @out_uid: Return location for UID. - * - * Translates a user name to a UID. - */ -gboolean -nm_session_monitor_user_to_uid (const char *user, uid_t *out_uid) -{ - struct passwd *pw = getpwnam (user); - - g_assert (out_uid); - - if (!pw) - return FALSE; - - *out_uid = pw->pw_uid; - - return TRUE; -} - /** * nm_session_monitor_session_exists: * @self: the session monitor diff --git a/src/nm-session-monitor.h b/src/nm-session-monitor.h index 7ea1e14a91..e2ff8fbab2 100644 --- a/src/nm-session-monitor.h +++ b/src/nm-session-monitor.h @@ -23,7 +23,6 @@ GType nm_session_monitor_get_type (void) G_GNUC_CONST; NMSessionMonitor *nm_session_monitor_get (void); -gboolean nm_session_monitor_user_to_uid (const char *user, uid_t *out_uid); gboolean nm_session_monitor_session_exists (NMSessionMonitor *self, uid_t uid, gboolean active); diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index ca46de681d..fe93ea91c6 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -357,7 +357,7 @@ nm_settings_connection_check_visibility (NMSettingsConnection *self, if (!nm_setting_connection_get_permission (s_con, i, NULL, &user, NULL)) continue; - if (!nm_session_monitor_user_to_uid (user, &uid)) + if (!nm_utils_name_to_uid (user, &uid)) continue; if (!nm_session_monitor_session_exists (session_monitor, uid, FALSE)) continue;