From 5f4c9c27d8507fc079e7dd0d448cab097491c85f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 22 Dec 2022 10:28:05 +0100 Subject: [PATCH 1/3] core: tighten validation checks in SwitchRoot() dbus call --- src/core/dbus-manager.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 9e529d6375..ed91e9a1cf 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -1680,9 +1680,9 @@ static int method_switch_root(sd_bus_message *message, void *userdata, sd_bus_er if (r < 0) return r; - if (isempty(root)) + if (!path_is_valid(root)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, - "New root directory may not be the empty string."); + "New root directory must be a valid path."); if (!path_is_absolute(root)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "New root path '%s' is not absolute.", root); @@ -1704,6 +1704,10 @@ static int method_switch_root(sd_bus_message *message, void *userdata, sd_bus_er } else { _cleanup_free_ char *chased = NULL; + if (!path_is_valid(init)) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "Path to init binary '%s' is not a valid path.", init); + if (!path_is_absolute(init)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Path to init binary '%s' not absolute.", init); From e10086ac01627a19d3cb32fcd6fc42c12fb2584f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 22 Dec 2022 10:30:02 +0100 Subject: [PATCH 2/3] core: use chase_symlinks_and_access() where appropriate --- src/core/dbus-manager.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index ed91e9a1cf..6109ab6ce0 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -1702,8 +1702,6 @@ static int method_switch_root(sd_bus_message *message, void *userdata, sd_bus_er "Specified switch root path '%s' does not seem to be an OS tree. os-release file is missing.", root); } else { - _cleanup_free_ char *chased = NULL; - if (!path_is_valid(init)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Path to init binary '%s' is not a valid path.", init); @@ -1712,19 +1710,13 @@ static int method_switch_root(sd_bus_message *message, void *userdata, sd_bus_er return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Path to init binary '%s' not absolute.", init); - r = chase_symlinks(init, root, CHASE_PREFIX_ROOT|CHASE_TRAIL_SLASH, &chased, NULL); + r = chase_symlinks_and_access(init, root, CHASE_PREFIX_ROOT, X_OK, NULL, NULL); + if (r == -EACCES) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "Init binary %s is not executable.", init); if (r < 0) return sd_bus_error_set_errnof(error, r, "Could not resolve init executable %s: %m", init); - - if (laccess(chased, X_OK) < 0) { - if (errno == EACCES) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, - "Init binary %s is not executable.", init); - - return sd_bus_error_set_errnof(error, r, - "Could not check whether init binary %s is executable: %m", init); - } } rt = strdup(root); From 457bbbce7b8d27e7fe0eab2b35662472a21a3e3c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 23 Dec 2022 18:27:33 +0100 Subject: [PATCH 3/3] systemctl: rework how we detect if init is systemd --- src/systemctl/systemctl-switch-root.c | 36 ++++++++++++++++++++++----- 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/src/systemctl/systemctl-switch-root.c b/src/systemctl/systemctl-switch-root.c index 8a7749be91..7fa8b26f59 100644 --- a/src/systemctl/systemctl-switch-root.c +++ b/src/systemctl/systemctl-switch-root.c @@ -3,14 +3,34 @@ #include "argv-util.h" #include "bus-error.h" #include "bus-locator.h" +#include "chase-symlinks.h" #include "parse-util.h" #include "path-util.h" #include "proc-cmdline.h" #include "signal-util.h" #include "stat-util.h" +#include "systemctl.h" #include "systemctl-switch-root.h" #include "systemctl-util.h" -#include "systemctl.h" + +static int same_file_in_root( + const char *root, + const char *a, + const char *b) { + + struct stat sta, stb; + int r; + + r = chase_symlinks_and_stat(a, root, CHASE_PREFIX_ROOT, NULL, &sta, NULL); + if (r < 0) + return r; + + r = chase_symlinks_and_stat(b, root, CHASE_PREFIX_ROOT, NULL, &stb, NULL); + if (r < 0) + return r; + + return stat_inode_same(&sta, &stb); +} int verb_switch_root(int argc, char *argv[], void *userdata) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; @@ -26,6 +46,10 @@ int verb_switch_root(int argc, char *argv[], void *userdata) { return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Wrong number of arguments."); root = argv[1]; + if (!path_is_valid(root)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid root path: %s", root); + if (!path_is_absolute(root)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Root path is not absolute: %s", root); if (argc >= 3) init = argv[2]; @@ -39,13 +63,13 @@ int verb_switch_root(int argc, char *argv[], void *userdata) { init = empty_to_null(init); if (init) { - const char *root_systemd_path = NULL, *root_init_path = NULL; - - root_systemd_path = prefix_roota(root, "/" SYSTEMD_BINARY_PATH); - root_init_path = prefix_roota(root, init); + if (!path_is_valid(init)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid path to init binary: %s", init); + if (!path_is_absolute(init)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Path to init binary is not absolute: %s", init); /* If the passed init is actually the same as the systemd binary, then let's suppress it. */ - if (files_same(root_init_path, root_systemd_path, 0) > 0) + if (same_file_in_root(root, SYSTEMD_BINARY_PATH, init) > 0) init = NULL; }