From: Zbigniew Jędrzejewski-Szmek Date: Tue, 4 Oct 2022 12:19:12 +0000 (+0200) Subject: shared/pam-util: add pam_syslog_errno() wrapper that sets errno X-Git-Tag: v252-rc2~70^2~22 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=b1eff892bb7c4ba2fac2b1fdf54d86534aaf2d08;p=thirdparty%2Fsystemd.git shared/pam-util: add pam_syslog_errno() wrapper that sets errno So far our pam code was using strerror_safe(). But that's not a good approach, because strerror_safe() is not thread-safe, and the pam code is "library code" that should be thread-safe. In fact, the whole effort to use strerror() is unnecessary, because pam_syslog() is documented to support %m. The implementation in linux-pam simply uses vasprintf(). If we use %m too, we get rid of the issue. The wrapper sets errno temporarily from the argument. Apparently some PAM consumers run multiple PAM stacks in threads, so we should avoid non-thread-safe code. The new helper returns PAM_BUF_ERR for ENOMEM, and PAM_SERVICE_ERR in other cases. This may change the returned code in some cases, but I think a) it doesn't matter much, b) it's probably for the better. E.g. we might now return PAM_SERVICE_ERR if the dbus message is borked, and PAM_SERVICE_ERR seems appropriate. --- diff --git a/src/shared/pam-util.c b/src/shared/pam-util.c index 621e7fe802e..d9fee22cc60 100644 --- a/src/shared/pam-util.c +++ b/src/shared/pam-util.c @@ -9,22 +9,16 @@ #include "macro.h" #include "pam-util.h" -int pam_log_oom(pam_handle_t *handle) { - /* This is like log_oom(), but uses PAM logging */ - pam_syslog(handle, LOG_ERR, "Out of memory."); - return PAM_BUF_ERR; -} +int pam_syslog_errno(pam_handle_t *handle, int level, int error, const char *format, ...) { + va_list ap; -int pam_bus_log_create_error(pam_handle_t *handle, int r) { - /* This is like bus_log_create_error(), but uses PAM logging */ - pam_syslog(handle, LOG_ERR, "Failed to create bus message: %s", strerror_safe(r)); - return PAM_BUF_ERR; -} + LOCAL_ERRNO(error); -int pam_bus_log_parse_error(pam_handle_t *handle, int r) { - /* This is like bus_log_parse_error(), but uses PAM logging */ - pam_syslog(handle, LOG_ERR, "Failed to parse bus message: %s", strerror_safe(r)); - return PAM_BUF_ERR; + va_start(ap, format); + pam_vsyslog(handle, LOG_ERR, format, ap); + va_end(ap); + + return error == -ENOMEM ? PAM_BUF_ERR : PAM_SERVICE_ERR; } static void cleanup_system_bus(pam_handle_t *handle, void *data, int error_status) { @@ -50,10 +44,8 @@ int pam_acquire_bus_connection(pam_handle_t *handle, sd_bus **ret) { } r = sd_bus_open_system(&bus); - if (r < 0) { - pam_syslog(handle, LOG_ERR, "Failed to connect to system bus: %s", strerror_safe(r)); - return PAM_SERVICE_ERR; - } + 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); if (r != PAM_SUCCESS) { diff --git a/src/shared/pam-util.h b/src/shared/pam-util.h index 41f1835b3c7..4a97d2d132a 100644 --- a/src/shared/pam-util.h +++ b/src/shared/pam-util.h @@ -5,9 +5,22 @@ #include "sd-bus.h" -int pam_log_oom(pam_handle_t *handle); -int pam_bus_log_create_error(pam_handle_t *handle, int r); -int pam_bus_log_parse_error(pam_handle_t *handle, int r); +int pam_syslog_errno(pam_handle_t *handle, int level, int error, const char *format, ...) _printf_(4,5); + +static inline int pam_log_oom(pam_handle_t *handle) { + /* This is like log_oom(), but uses PAM logging */ + return pam_syslog_errno(handle, LOG_ERR, ENOMEM, "Out of memory."); +} + +static inline int pam_bus_log_create_error(pam_handle_t *handle, int r) { + /* This is like bus_log_create_error(), but uses PAM logging */ + return pam_syslog_errno(handle, LOG_ERR, r, "Failed to create bus message: %m"); +} + +static inline int pam_bus_log_parse_error(pam_handle_t *handle, int r) { + /* This is like bus_log_parse_error(), but uses PAM logging */ + 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);