From ab4a5982c195e611a03a9f970cae27d9fa78acae Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 2 Apr 2025 15:32:15 +0200 Subject: [PATCH 1/2] selinux-util: rename return parameter to ret_xyz mac_selinux_get_create_label_from_exe() didn't get this right. Clean this up, and stick to a uniform name for all three related functions. --- src/shared/selinux-util.c | 12 ++++++------ src/shared/selinux-util.h | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/shared/selinux-util.c b/src/shared/selinux-util.c index 27ed339ff8..243413bdf7 100644 --- a/src/shared/selinux-util.c +++ b/src/shared/selinux-util.c @@ -407,14 +407,14 @@ int mac_selinux_apply_fd(int fd, const char *path, const char *label) { return 0; } -int mac_selinux_get_create_label_from_exe(const char *exe, char **label) { +int mac_selinux_get_create_label_from_exe(const char *exe, char **ret_label) { #if HAVE_SELINUX _cleanup_freecon_ char *mycon = NULL, *fcon = NULL; security_class_t sclass; int r; assert(exe); - assert(label); + assert(ret_label); r = selinux_init(/* force= */ false); if (r < 0) @@ -436,14 +436,14 @@ int mac_selinux_get_create_label_from_exe(const char *exe, char **label) { if (sclass == 0) return -ENOSYS; - return RET_NERRNO(security_compute_create_raw(mycon, fcon, sclass, label)); + return RET_NERRNO(security_compute_create_raw(mycon, fcon, sclass, ret_label)); #else return -EOPNOTSUPP; #endif } -int mac_selinux_get_our_label(char **ret) { - assert(ret); +int mac_selinux_get_our_label(char **ret_label) { + assert(ret_label); #if HAVE_SELINUX int r; @@ -460,7 +460,7 @@ int mac_selinux_get_our_label(char **ret) { if (!con) return -EOPNOTSUPP; - *ret = TAKE_PTR(con); + *ret_label = TAKE_PTR(con); return 0; #else return -EOPNOTSUPP; diff --git a/src/shared/selinux-util.h b/src/shared/selinux-util.h index 3a2cf0d850..12627f94ec 100644 --- a/src/shared/selinux-util.h +++ b/src/shared/selinux-util.h @@ -35,9 +35,9 @@ int mac_selinux_fix_full(int atfd, const char *inode_path, const char *label_pat int mac_selinux_apply(const char *path, const char *label); int mac_selinux_apply_fd(int fd, const char *path, const char *label); -int mac_selinux_get_create_label_from_exe(const char *exe, char **label); -int mac_selinux_get_our_label(char **label); -int mac_selinux_get_child_mls_label(int socket_fd, const char *exe, const char *exec_label, char **label); +int mac_selinux_get_create_label_from_exe(const char *exe, char **ret_label); +int mac_selinux_get_our_label(char **ret_label); +int mac_selinux_get_child_mls_label(int socket_fd, const char *exe, const char *exec_label, char **ret_label); int mac_selinux_create_file_prepare_at(int dirfd, const char *path, mode_t mode); static inline int mac_selinux_create_file_prepare(const char *path, mode_t mode) { From ec88828430c1dd6b92c2176ac933469cc1d593a8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 2 Apr 2025 15:33:28 +0200 Subject: [PATCH 2/2] selinux-setup: modernizations Adjustments to match our current coding style, in particular ensures we always print log messages for unexpected errors. This changes our code assume mac_selinux_get_create_label_from_exe() returns a valid label on success. I checked libselinux, this is guaranteed, and is otherwise relied on in our tree, hence don't do the pointless check here. --- src/core/selinux-setup.c | 69 ++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 34 deletions(-) diff --git a/src/core/selinux-setup.c b/src/core/selinux-setup.c index 39f5edc045..f687cfcd53 100644 --- a/src/core/selinux-setup.c +++ b/src/core/selinux-setup.c @@ -10,6 +10,7 @@ #include "sd-messages.h" +#include "errno-util.h" #include "initrd-util.h" #include "log.h" #include "macro.h" @@ -26,67 +27,67 @@ static int null_log(int type, const char *fmt, ...) { #endif int mac_selinux_setup(bool *loaded_policy) { - -#if HAVE_SELINUX - int enforce = 0; - usec_t before_load, after_load; - char *con; - int r; - bool initialized; - assert(loaded_policy); - /* Turn off all of SELinux' own logging, we want to do that */ +#if HAVE_SELINUX + int r; + + /* Turn off all of SELinux' own logging, we want to do that ourselves */ selinux_set_callback(SELINUX_CB_LOG, (const union selinux_callback) { .func_log = null_log }); /* Don't load policy in the initrd if we don't appear to have it. For the real root, we check below * if we've already loaded policy, and return gracefully. */ - if (in_initrd() && access(selinux_path(), F_OK) < 0) - return 0; + if (in_initrd() && access(selinux_path(), F_OK) < 0) { + if (errno != ENOENT) + log_warning_errno(errno, "Unable to check if %s exists, assuming it does not: %m", selinux_path()); - /* Already initialized by somebody else? */ - r = getcon_raw(&con); - /* getcon_raw can return 0, and still give us a NULL pointer if /proc/self/attr/current is + return 0; + } + + bool initialized = false; + + /* Already initialized by somebody else? + * + * Note: getcon_raw() can return 0, and still give us a NULL pointer if /proc/self/attr/current is * empty. SELinux guarantees this won't happen, but that file isn't specific to SELinux, and may be * provided by some other arbitrary LSM with different semantics. */ - if (r == 0 && con) { + _cleanup_freecon_ char *con = NULL; + if (getcon_raw(&con) < 0) + log_debug_errno(errno, "getcon_raw() failed, assuming SELinux is not initialized: %m"); + else if (con) { initialized = !streq(con, "kernel"); - freecon(con); - } else - initialized = false; + log_debug("SELinux already initialized: %s", yes_no(initialized)); + } - /* Make sure we have no fds open while loading the policy and - * transitioning */ + /* Make sure we have no fds open while loading the policy and transitioning */ log_close(); /* Now load the policy */ - before_load = now(CLOCK_MONOTONIC); - r = selinux_init_load_policy(&enforce); - if (r == 0) { - _cleanup_freecon_ char *label = NULL; - + usec_t before_load = now(CLOCK_MONOTONIC); + int enforce = 0; + if (selinux_init_load_policy(&enforce) == 0) { /* NB: Apparently doesn't set useful errno! */ mac_selinux_retest(); /* Transition to the new context */ + _cleanup_freecon_ char *label = NULL; r = mac_selinux_get_create_label_from_exe(SYSTEMD_BINARY_PATH, &label); - if (r < 0 || !label) { + if (r < 0) { log_open(); - log_error("Failed to compute init label, ignoring."); + log_warning_errno(r, "Failed to compute init label, ignoring: %m"); } else { - r = setcon_raw(label); - + r = RET_NERRNO(setcon_raw(label)); log_open(); if (r < 0) - log_error("Failed to transition into init label '%s', ignoring.", label); + log_warning_errno(r, "Failed to transition into init label '%s', ignoring: %m", label); + else + log_debug("Successfully switched to calculated init label '%s'.", label); } - after_load = now(CLOCK_MONOTONIC); - + usec_t after_load = now(CLOCK_MONOTONIC); log_info("Successfully loaded SELinux policy in %s.", FORMAT_TIMESPAN(after_load - before_load, 0)); *loaded_policy = true; - } else { log_open(); @@ -96,7 +97,7 @@ int mac_selinux_setup(bool *loaded_policy) { LOG_MESSAGE("Failed to load SELinux policy :%m"), LOG_MESSAGE_ID(SD_MESSAGE_SELINUX_FAILED_STR)); - log_warning("Failed to load new SELinux policy. Continuing with old policy."); + log_notice("Failed to load new SELinux policy. Continuing with old policy."); } else log_debug("Unable to load SELinux policy. Ignoring."); }