From: Luca Boccassi Date: Sun, 16 Apr 2023 13:55:09 +0000 (+0100) Subject: pam: cache sd-bus separately per module X-Git-Tag: v254-rc1~659 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=a2dd39b4cb19f4ed4599422e635cc019dcae6ada;p=thirdparty%2Fsystemd.git pam: cache sd-bus separately per module sd-bus connection is cached by the two pam modules globally, but this can lead to issues due to hashmaps (used by sd-bus) using a global static variable for the shared hash key, which is different per module as both modules are loaded in the same process. This happens because the sd-bus object is create in one module, but used in the other, so global state does not match. Use a different pam cache identifier for the sd-bus pointer, so that each module uses a different sd-bus connection as a workaround. Fixes https://github.com/systemd/systemd/issues/27216 Fixes https://github.com/systemd/systemd/issues/17266 --- diff --git a/src/home/pam_systemd_home.c b/src/home/pam_systemd_home.c index e28c95f787e..6a3e656035f 100644 --- a/src/home/pam_systemd_home.c +++ b/src/home/pam_systemd_home.c @@ -140,7 +140,7 @@ static int acquire_user_record( _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_free_ char *generic_field = NULL, *json_copy = NULL; - r = pam_acquire_bus_connection(handle, &bus); + r = pam_acquire_bus_connection(handle, "pam-systemd-home", &bus); if (r != PAM_SUCCESS) return r; @@ -513,7 +513,7 @@ static int acquire_home( if (r == PAM_SUCCESS && PTR_TO_FD(home_fd_ptr) >= 0) return PAM_SUCCESS; - r = pam_acquire_bus_connection(handle, &bus); + r = pam_acquire_bus_connection(handle, "pam-systemd-home", &bus); if (r != PAM_SUCCESS) return r; @@ -745,7 +745,7 @@ success: /* Let's release the D-Bus connection, after all the session might live quite a long time, and we are * not going to process the bus connection in that time, so let's better close before the daemon * kicks us off because we are not processing anything. */ - (void) pam_release_bus_connection(handle); + (void) pam_release_bus_connection(handle, "pam-systemd-home"); return PAM_SUCCESS; } @@ -785,7 +785,7 @@ _public_ PAM_EXTERN int pam_sm_close_session( if (r != PAM_SUCCESS) return r; - r = pam_acquire_bus_connection(handle, &bus); + r = pam_acquire_bus_connection(handle, "pam-systemd-home", &bus); if (r != PAM_SUCCESS) return r; @@ -944,7 +944,7 @@ _public_ PAM_EXTERN int pam_sm_chauthtok( if (debug) pam_syslog(handle, LOG_DEBUG, "pam-systemd-homed account management"); - r = pam_acquire_bus_connection(handle, &bus); + r = pam_acquire_bus_connection(handle, "pam-systemd-home", &bus); if (r != PAM_SUCCESS) return r; diff --git a/src/login/pam_systemd.c b/src/login/pam_systemd.c index 65d09886bee..3a009d8e37a 100644 --- a/src/login/pam_systemd.c +++ b/src/login/pam_systemd.c @@ -948,7 +948,7 @@ _public_ PAM_EXTERN int pam_sm_open_session( /* Talk to logind over the message bus */ - r = pam_acquire_bus_connection(handle, &bus); + r = pam_acquire_bus_connection(handle, "pam-systemd", &bus); if (r != PAM_SUCCESS) return r; @@ -1115,7 +1115,7 @@ success: /* Let's release the D-Bus connection, after all the session might live quite a long time, and we are * not going to use the bus connection in that time, so let's better close before the daemon kicks us * off because we are not processing anything. */ - (void) pam_release_bus_connection(handle); + (void) pam_release_bus_connection(handle, "pam-systemd"); return PAM_SUCCESS; } @@ -1159,7 +1159,7 @@ _public_ PAM_EXTERN int pam_sm_close_session( /* Before we go and close the FIFO we need to tell logind that this is a clean session * shutdown, so that it doesn't just go and slaughter us immediately after closing the fd */ - r = pam_acquire_bus_connection(handle, &bus); + r = pam_acquire_bus_connection(handle, "pam-systemd", &bus); if (r != PAM_SUCCESS) return r; diff --git a/src/shared/pam-util.c b/src/shared/pam-util.c index 9d74e08a2be..bf576c482ef 100644 --- a/src/shared/pam-util.c +++ b/src/shared/pam-util.c @@ -53,15 +53,21 @@ static void cleanup_system_bus(pam_handle_t *handle, void *data, int error_statu sd_bus_flush_close_unref(data); } -int pam_acquire_bus_connection(pam_handle_t *handle, sd_bus **ret) { +int pam_acquire_bus_connection(pam_handle_t *handle, const char *module_name, sd_bus **ret) { _cleanup_(sd_bus_unrefp) sd_bus *bus = NULL; + _cleanup_free_ char *cache_id = NULL; int r; assert(handle); + assert(module_name); assert(ret); + cache_id = strjoin("system-bus-", module_name); + if (!cache_id) + return pam_log_oom(handle); + /* We cache the bus connection so that we can share it between the session and the authentication hooks */ - r = pam_get_data(handle, "systemd-system-bus", (const void**) &bus); + r = pam_get_data(handle, cache_id, (const void**) &bus); if (r == PAM_SUCCESS && bus) { *ret = sd_bus_ref(TAKE_PTR(bus)); /* Increase the reference counter, so that the PAM data stays valid */ return PAM_SUCCESS; @@ -73,7 +79,7 @@ int pam_acquire_bus_connection(pam_handle_t *handle, sd_bus **ret) { if (r < 0) return pam_syslog_errno(handle, LOG_ERR, r, "Failed to connect to system bus: %m"); - r = pam_set_data(handle, "systemd-system-bus", bus, cleanup_system_bus); + r = pam_set_data(handle, cache_id, bus, cleanup_system_bus); if (r != PAM_SUCCESS) return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to set PAM bus data: @PAMERR@"); @@ -83,10 +89,17 @@ int pam_acquire_bus_connection(pam_handle_t *handle, sd_bus **ret) { return PAM_SUCCESS; } -int pam_release_bus_connection(pam_handle_t *handle) { +int pam_release_bus_connection(pam_handle_t *handle, const char *module_name) { + _cleanup_free_ char *cache_id = NULL; int r; - r = pam_set_data(handle, "systemd-system-bus", NULL, NULL); + assert(module_name); + + cache_id = strjoin("system-bus-", module_name); + if (!cache_id) + return pam_log_oom(handle); + + r = pam_set_data(handle, cache_id, NULL, NULL); if (r != PAM_SUCCESS) return pam_syslog_pam_error(handle, LOG_ERR, r, "Failed to release PAM user record data: @PAMERR@"); diff --git a/src/shared/pam-util.h b/src/shared/pam-util.h index 1a17ea18c5c..d9c906aaad0 100644 --- a/src/shared/pam-util.h +++ b/src/shared/pam-util.h @@ -24,7 +24,9 @@ static inline int pam_bus_log_parse_error(pam_handle_t *handle, int r) { return pam_syslog_errno(handle, LOG_ERR, r, "Failed to parse bus message: %m"); } -int pam_acquire_bus_connection(pam_handle_t *handle, sd_bus **ret); -int pam_release_bus_connection(pam_handle_t *handle); +/* Use a different module name per different PAM module. They are all loaded in the same namespace, and this + * helps avoid a clash in the internal data structures of sd-bus. It will be used as key for cache items. */ +int pam_acquire_bus_connection(pam_handle_t *handle, const char *module_name, sd_bus **ret); +int pam_release_bus_connection(pam_handle_t *handle, const char *module_name); void pam_cleanup_free(pam_handle_t *handle, void *data, int error_status);