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 e28c95f787e6732424676d0b9a0a02b6d29f1b39..6a3e656035fea4c2355f09334dfba5b63c24da2b 100644 (file)
--- 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 65d09886bee0e101ec5daebf17b694c50772c9fb..3a009d8e37adfe53864c61053b2b899183a0d0d5 100644 (file)
--- 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 9d74e08a2be23a2c6f825f9d9ef32223fff1d4e4..bf576c482ef7f15fdf3415f82339699d0938ece5 100644 (file)
--- 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 1a17ea18c5c9a5be0d4f832e1859ae6fe6f3a049..d9c906aaad05d6e799fed0c82011f437e1016cde 100644 (file)
--- 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);