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
This commit is contained in:
committed by
Lennart Poettering
parent
db0c0f5e00
commit
a2dd39b4cb
+18
-5
@@ -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@");
|
||||
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user