From 5caa3167ff98a29c1ef9bbe9c4fddf6aa1cd8888 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 11 Dec 2017 22:04:46 +0100 Subject: [PATCH 1/5] efi: rework find_esp() error propagation/logging a bit This renames find_esp() to find_esp_and_warn() and tries to normalize its behaviour: 1. Change the error that is returned when we can't find the ESP to ENOKEY (from ENOENT). This way the error code can only mean one thing: that our search loop didn't find a good candidate. 2. Really log about all errors, except for ENOKEY and EACCES, and document the letter cases. 3. Normalize parameters to the call: separate out the path parameter in two: an input path and an output path. That way the memory management is clear: we will access the input parameter only for reading, and only write out the output parameter, using malloc() memory. Before the calling convention were quire surprising for internal API code, as the path parameter had to be malloc() memory and might and might not have changed. 4. Rename bootctl's find_esp_warn() to acquire_esp(), and make it a simple wrapper around find_esp_warn(), that basically just adds the friendly logging for the ENOKEY case. This rework removes double logging in a number of error cases, as we no longer log here in anything but ENOKEY, and leave that entirely to find_esp_warn(). 5. find_esp_and_warn() now takes a bool flag parameter "unprivileged_mode", which disables logging in the EACCES case, and skips privileged validation of the path. This makes the function less magic, and doesn't hide this internal silencing automatism from the caller anymore. With all that in place "bootctl list" and "bootctl status" work properly (or as good as they can) when I invoke the tools whithout privileges on my system where /boot is not world-readable --- src/boot/bootctl.c | 55 +++++++++++++++++++++-------- src/shared/bootspec.c | 73 +++++++++++++++++++++++++-------------- src/shared/bootspec.h | 3 +- src/systemctl/systemctl.c | 19 +++++----- 4 files changed, 99 insertions(+), 51 deletions(-) diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c index e733e206e4..52207661cf 100644 --- a/src/boot/bootctl.c +++ b/src/boot/bootctl.c @@ -61,19 +61,33 @@ static char *arg_path = NULL; static bool arg_print_path = false; static bool arg_touch_variables = true; -static int find_esp_and_warn(uint32_t *part, uint64_t *pstart, uint64_t *psize, sd_id128_t *uuid) { +static int acquire_esp( + bool unprivileged_mode, + uint32_t *ret_part, + uint64_t *ret_pstart, + uint64_t *ret_psize, + sd_id128_t *ret_uuid) { + + char *np; int r; - r = find_esp(&arg_path, part, pstart, psize, uuid); - if (r == -ENOENT) + /* Find the ESP, and log about errors. Note that find_esp_and_warn() will log in all error cases on its own, + * except for ENOKEY (which is good, we want to show our own message in that case, suggesting use of --path=) + * and EACCESS (only when we request unprivileged mode; in this case we simply eat up the error here, so that + * --list and --status work too, without noise about this). */ + + r = find_esp_and_warn(arg_path, unprivileged_mode, &np, ret_part, ret_pstart, ret_psize, ret_uuid); + if (r == -ENOKEY) return log_error_errno(r, - "Couldn't find EFI system partition. It is recommended to mount it to /boot.\n" + "Couldn't find EFI system partition. It is recommended to mount it to /boot or /efi.\n" "Alternatively, use --path= to specify path to mount point."); - else if (r < 0) - return log_error_errno(r, - "Couldn't find EFI system partition: %m"); + if (r < 0) + return r; + + free_and_replace(arg_path, np); log_debug("Using EFI System Partition at %s.", arg_path); + return 0; } @@ -925,9 +939,12 @@ static int verb_status(int argc, char *argv[], void *userdata) { sd_id128_t uuid = SD_ID128_NULL; int r, k; - r = find_esp_and_warn(NULL, NULL, NULL, &uuid); + r = acquire_esp(geteuid() != 0, NULL, NULL, NULL, &uuid); if (arg_print_path) { + if (r == -EACCES) /* If we couldn't acquire the ESP path, log about access errors (which is the only + * error the find_esp_and_warn() won't log on its own) */ + return log_error_errno(r, "Failed to determine ESP: %m"); if (r < 0) return r; @@ -935,6 +952,9 @@ static int verb_status(int argc, char *argv[], void *userdata) { return 0; } + r = 0; /* If we couldn't determine the path, then don't consider that a problem from here on, just show what we + * can show */ + if (is_efi_boot()) { _cleanup_free_ char *fw_type = NULL, *fw_info = NULL, *loader = NULL, *loader_path = NULL; sd_id128_t loader_part_uuid = SD_ID128_NULL; @@ -997,13 +1017,18 @@ static int verb_status(int argc, char *argv[], void *userdata) { } static int verb_list(int argc, char *argv[], void *userdata) { - sd_id128_t uuid = SD_ID128_NULL; - int r; - unsigned n; - _cleanup_(boot_config_free) BootConfig config = {}; + sd_id128_t uuid = SD_ID128_NULL; + unsigned n; + int r; - r = find_esp_and_warn(NULL, NULL, NULL, &uuid); + /* If we lack privileges we invoke find_esp_and_warn() in "unprivileged mode" here, which does two things: turn + * off logging about access errors and turn off potentially privileged device probing. Here we're interested in + * the latter but not the former, hence request the mode, and log about EACCES. */ + + r = acquire_esp(geteuid() != 0, NULL, NULL, NULL, &uuid); + if (r == -EACCES) /* We really need the ESP path for this call, hence also log about access errors */ + return log_error_errno(r, "Failed to determine ESP: %m"); if (r < 0) return r; @@ -1071,7 +1096,7 @@ static int verb_install(int argc, char *argv[], void *userdata) { if (r < 0) return r; - r = find_esp_and_warn(&part, &pstart, &psize, &uuid); + r = acquire_esp(false, &part, &pstart, &psize, &uuid); if (r < 0) return r; @@ -1106,7 +1131,7 @@ static int verb_remove(int argc, char *argv[], void *userdata) { if (r < 0) return r; - r = find_esp_and_warn(NULL, NULL, NULL, &uuid); + r = acquire_esp(false, NULL, NULL, NULL, &uuid); if (r < 0) return r; diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index dea3c5212a..acd9711839 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -413,8 +413,9 @@ int boot_entries_load_config(const char *esp_path, BootConfig *config) { /********************************************************************************/ static int verify_esp( - bool searching, const char *p, + bool searching, + bool unprivileged_mode, uint32_t *ret_part, uint64_t *ret_pstart, uint64_t *ret_psize, @@ -430,21 +431,19 @@ static int verify_esp( struct statfs sfs; sd_id128_t uuid = SD_ID128_NULL; uint32_t part = 0; - bool quiet; int r; assert(p); - /* Non-root user can only check the status, so if an error occured in the following, - * it does not cause any issues. Let's silence the error messages. */ - quiet = geteuid() != 0; + /* Non-root user can only check the status, so if an error occured in the following, it does not cause any + * issues. Let's also, silence the error messages. */ if (statfs(p, &sfs) < 0) { /* If we are searching for the mount point, don't generate a log message if we can't find the path */ if (errno == ENOENT && searching) return -ENOENT; - return log_full_errno(quiet && errno == EACCES ? LOG_DEBUG : LOG_ERR, errno, + return log_full_errno(unprivileged_mode && errno == EACCES ? LOG_DEBUG : LOG_ERR, errno, "Failed to check file system type of \"%s\": %m", p); } @@ -457,7 +456,7 @@ static int verify_esp( } if (stat(p, &st) < 0) - return log_full_errno(quiet && errno == EACCES ? LOG_DEBUG : LOG_ERR, errno, + return log_full_errno(unprivileged_mode && errno == EACCES ? LOG_DEBUG : LOG_ERR, errno, "Failed to determine block device node of \"%s\": %m", p); if (major(st.st_dev) == 0) { @@ -468,7 +467,7 @@ static int verify_esp( t2 = strjoina(p, "/.."); r = stat(t2, &st2); if (r < 0) - return log_full_errno(quiet && errno == EACCES ? LOG_DEBUG : LOG_ERR, errno, + return log_full_errno(unprivileged_mode && errno == EACCES ? LOG_DEBUG : LOG_ERR, errno, "Failed to determine block device node of parent of \"%s\": %m", p); if (st.st_dev == st2.st_dev) { @@ -478,7 +477,7 @@ static int verify_esp( /* In a container we don't have access to block devices, skip this part of the verification, we trust the * container manager set everything up correctly on its own. Also skip the following verification for non-root user. */ - if (detect_container() > 0 || geteuid() != 0) + if (detect_container() > 0 || unprivileged_mode) goto finish; #if HAVE_BLKID @@ -579,29 +578,53 @@ finish: return 0; } -int find_esp(char **path, - uint32_t *part, uint64_t *pstart, uint64_t *psize, sd_id128_t *uuid) { +int find_esp_and_warn( + const char *path, + bool unprivileged_mode, + char **ret_path, + uint32_t *ret_part, + uint64_t *ret_pstart, + uint64_t *ret_psize, + sd_id128_t *ret_uuid) { - const char *p; int r; - if (*path) - return verify_esp(false, *path, part, pstart, psize, uuid); + /* This logs about all errors except: + * + * -ENOKEY → when we can't find the partition + * -EACCESS → when unprivileged_mode is true, and we can't access something + */ - FOREACH_STRING(p, "/efi", "/boot", "/boot/efi") { - - r = verify_esp(true, p, part, pstart, psize, uuid); - if (IN_SET(r, -ENOENT, -EADDRNOTAVAIL)) /* This one is not it */ - continue; + if (path) { + r = verify_esp(path, false, unprivileged_mode, ret_part, ret_pstart, ret_psize, ret_uuid); if (r < 0) return r; - *path = strdup(p); - if (!*path) - return log_oom(); - - return 0; + goto found; } - return -ENOENT; + FOREACH_STRING(path, "/efi", "/boot", "/boot/efi") { + + r = verify_esp(path, true, unprivileged_mode, ret_part, ret_pstart, ret_psize, ret_uuid); + if (r >= 0) + goto found; + if (!IN_SET(r, -ENOENT, -EADDRNOTAVAIL)) /* This one is not it */ + return r; + } + + /* No logging here */ + return -ENOKEY; + +found: + if (ret_path) { + char *c; + + c = strdup(path); + if (!c) + return log_oom(); + + *ret_path = c; + } + + return 0; } diff --git a/src/shared/bootspec.h b/src/shared/bootspec.h index fb8c10a69a..d9c641bf08 100644 --- a/src/shared/bootspec.h +++ b/src/shared/bootspec.h @@ -61,5 +61,4 @@ static inline const char* boot_entry_title(const BootEntry *entry) { return entry->show_title ?: entry->title ?: entry->filename; } -int find_esp(char **path, - uint32_t *part, uint64_t *pstart, uint64_t *psize, sd_id128_t *uuid); +int find_esp_and_warn(const char *path, bool unprivileged_mode, char **ret_path, uint32_t *ret_part, uint64_t *ret_pstart, uint64_t *ret_psize, sd_id128_t *ret_uuid); diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 33fbaf9950..0a39a7e26c 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -3497,7 +3497,7 @@ static int prepare_firmware_setup(void) { static int load_kexec_kernel(void) { _cleanup_(boot_config_free) BootConfig config = {}; - _cleanup_free_ char *kernel = NULL, *initrd = NULL, *options = NULL; + _cleanup_free_ char *where = NULL, *kernel = NULL, *initrd = NULL, *options = NULL; const BootEntry *e; pid_t pid; int r; @@ -3507,14 +3507,15 @@ static int load_kexec_kernel(void) { return 0; } - r = find_esp(&arg_esp_path, NULL, NULL, NULL, NULL); - if (r < 0) - return log_error_errno(r, "Cannot find the ESP partition mount point: %m"); + r = find_esp_and_warn(arg_esp_path, false, &where, NULL, NULL, NULL, NULL); + if (r == -ENOKEY) /* find_esp_and_warn() doesn't warn about this case */ + return log_error_errno(r, "Cannot find the ESP partition mount point."); + if (r < 0) /* But it logs about all these cases, hence don't log here again */ + return r; - r = boot_entries_load_config(arg_esp_path, &config); + r = boot_entries_load_config(where, &config); if (r < 0) - return log_error_errno(r, "Failed to load bootspec config from \"%s/loader\": %m", - arg_esp_path); + return log_error_errno(r, "Failed to load bootspec config from \"%s/loader\": %m", where); if (config.default_entry < 0) { log_error("No entry suitable as default, refusing to guess."); @@ -3527,9 +3528,9 @@ static int load_kexec_kernel(void) { return -EINVAL; } - kernel = path_join(NULL, arg_esp_path, e->kernel); + kernel = path_join(NULL, where, e->kernel); if (!strv_isempty(e->initrd)) - initrd = path_join(NULL, arg_esp_path, *e->initrd); + initrd = path_join(NULL, where, *e->initrd); options = strv_join(e->options, " "); if (!options) return log_oom(); From ecec2a5d7a6b5259beffba02771945bc46ddfd06 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 11 Dec 2017 22:15:03 +0100 Subject: [PATCH 2/5] bootctl: don't trip up in "bootctl status" when we can't find the ESP because of lack of privilges On my system the boot and EFI partitions are protected, hence "bootctl status" can't find the ESP, and then the tool continues with arg_path == NULL, which it really should not. Handle these cases, and simply suppress all output that needs arg_path. --- src/boot/bootctl.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c index 52207661cf..a5700992d4 100644 --- a/src/boot/bootctl.c +++ b/src/boot/bootctl.c @@ -999,9 +999,11 @@ static int verb_status(int argc, char *argv[], void *userdata) { } else printf("System:\n Not booted with EFI\n\n"); - k = status_binaries(arg_path, uuid); - if (k < 0) - r = k; + if (arg_path) { + k = status_binaries(arg_path, uuid); + if (k < 0) + r = k; + } if (is_efi_boot()) { k = status_variables(); @@ -1009,9 +1011,11 @@ static int verb_status(int argc, char *argv[], void *userdata) { r = k; } - k = status_entries(arg_path, uuid); - if (k < 0) - r = k; + if (arg_path) { + k = status_entries(arg_path, uuid); + if (k < 0) + r = k; + } return r; } From 4fe2ba0e2598e70a102fee85d4832ec3d20b5097 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 11 Dec 2017 22:22:04 +0100 Subject: [PATCH 3/5] bootspec: sprinkle some argument assert()s all over the place The previous commit fixed a NULL parameter issue, let's check for such, to make it easier to find issues like this. --- src/shared/bootspec.c | 34 +++++++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index acd9711839..c0a10417d8 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -36,8 +36,9 @@ #include "virt.h" void boot_entry_free(BootEntry *entry) { - free(entry->filename); + assert(entry); + free(entry->filename); free(entry->title); free(entry->show_title); free(entry->version); @@ -56,6 +57,9 @@ int boot_entry_load(const char *path, BootEntry *entry) { _cleanup_(boot_entry_free) BootEntry tmp = {}; int r; + assert(path); + assert(entry); + f = fopen(path, "re"); if (!f) return log_error_errno(errno, "Failed to open \"%s\": %m", path); @@ -123,6 +127,8 @@ int boot_entry_load(const char *path, BootEntry *entry) { void boot_config_free(BootConfig *config) { unsigned i; + assert(config); + free(config->default_pattern); free(config->timeout); free(config->editor); @@ -140,6 +146,9 @@ int boot_loader_read_conf(const char *path, BootConfig *config) { unsigned line = 1; int r; + assert(path); + assert(config); + f = fopen(path, "re"); if (!f) return log_error_errno(errno, "Failed to open \"%s\": %m", path); @@ -251,14 +260,17 @@ static int boot_entry_compare(const void *a, const void *b) { return str_verscmp(aa->filename, bb->filename); } -int boot_entries_find(const char *dir, BootEntry **entries, size_t *n_entries) { +int boot_entries_find(const char *dir, BootEntry **ret_entries, size_t *ret_n_entries) { _cleanup_strv_free_ char **files = NULL; char **f; int r; - BootEntry *array = NULL; size_t n_allocated = 0, n = 0; + assert(dir); + assert(ret_entries); + assert(ret_n_entries); + r = conf_files_list(&files, ".conf", NULL, 0, dir, NULL); if (r < 0) return log_error_errno(r, "Failed to list files in \"%s\": %m", dir); @@ -276,8 +288,9 @@ int boot_entries_find(const char *dir, BootEntry **entries, size_t *n_entries) { qsort_safe(array, n, sizeof(BootEntry), boot_entry_compare); - *entries = array; - *n_entries = n; + *ret_entries = array; + *ret_n_entries = n; + return 0; } @@ -285,6 +298,9 @@ static bool find_nonunique(BootEntry *entries, size_t n_entries, bool *arr) { unsigned i, j; bool non_unique = false; + assert(entries || n_entries == 0); + assert(arr || n_entries == 0); + for (i = 0; i < n_entries; i++) arr[i] = false; @@ -303,6 +319,8 @@ static int boot_entries_uniquify(BootEntry *entries, size_t n_entries) { int r; bool arr[n_entries]; + assert(entries || n_entries == 0); + /* Find _all_ non-unique titles */ if (!find_nonunique(entries, n_entries, arr)) return 0; @@ -349,6 +367,8 @@ static int boot_entries_uniquify(BootEntry *entries, size_t n_entries) { static int boot_entries_select_default(const BootConfig *config) { int i; + assert(config); + if (config->entry_oneshot) for (i = config->n_entries - 1; i >= 0; i--) if (streq(config->entry_oneshot, config->entries[i].filename)) { @@ -377,6 +397,7 @@ static int boot_entries_select_default(const BootConfig *config) { log_debug("Found default: last entry \"%s\"", config->entries[config->n_entries - 1].filename); else log_debug("Found no default boot entry :("); + return config->n_entries - 1; /* -1 means "no default" */ } @@ -384,6 +405,9 @@ int boot_entries_load_config(const char *esp_path, BootConfig *config) { const char *p; int r; + assert(esp_path); + assert(config); + p = strjoina(esp_path, "/loader/loader.conf"); r = boot_loader_read_conf(p, config); if (r < 0) From fba868fa710a216adade59c2683d8bee20c7da4b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 11 Dec 2017 23:00:57 +0100 Subject: [PATCH 4/5] tree-wide: unify logging of "Must be root" message Let's unify this in one call, generalizing must_be_root() from bootctl.c. --- src/basic/process-util.c | 9 +++++++++ src/basic/process-util.h | 2 ++ src/boot/bootctl.c | 9 --------- src/nspawn/nspawn.c | 7 +++---- src/systemctl/systemctl.c | 16 ++++++++-------- src/udev/udevadm-control.c | 4 +--- src/udev/udevd.c | 5 ++--- 7 files changed, 25 insertions(+), 27 deletions(-) diff --git a/src/basic/process-util.c b/src/basic/process-util.c index b3d96cdb0f..5f001494f0 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -1053,6 +1053,15 @@ pid_t getpid_cached(void) { } } +int must_be_root(void) { + + if (geteuid() == 0) + return 0; + + log_error("Need to be root."); + return -EPERM; +} + static const char *const ioprio_class_table[] = { [IOPRIO_CLASS_NONE] = "none", [IOPRIO_CLASS_RT] = "realtime", diff --git a/src/basic/process-util.h b/src/basic/process-util.h index d99fb3d6b8..39c194df0d 100644 --- a/src/basic/process-util.h +++ b/src/basic/process-util.h @@ -138,3 +138,5 @@ static inline bool pid_is_valid(pid_t p) { int ioprio_parse_priority(const char *s, int *ret); pid_t getpid_cached(void); + +int must_be_root(void); diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c index a5700992d4..b685b3c791 100644 --- a/src/boot/bootctl.c +++ b/src/boot/bootctl.c @@ -925,15 +925,6 @@ static void read_loader_efi_var(const char *name, char **var) { log_warning_errno(r, "Failed to read EFI variable %s: %m", name); } -static int must_be_root(void) { - - if (geteuid() == 0) - return 0; - - log_error("Need to be root."); - return -EPERM; -} - static int verb_status(int argc, char *argv[], void *userdata) { sd_id128_t uuid = SD_ID128_NULL; diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index f217def92d..51663e28dc 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -3783,11 +3783,10 @@ int main(int argc, char *argv[]) { if (r <= 0) goto finish; - if (geteuid() != 0) { - log_error("Need to be root."); - r = -EPERM; + r = must_be_root(); + if (r < 0) goto finish; - } + r = determine_names(); if (r < 0) goto finish; diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 0a39a7e26c..4f92659fcd 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -2074,10 +2074,9 @@ static int list_machines(int argc, char *argv[], void *userdata) { sd_bus *bus; int r; - if (geteuid() != 0) { - log_error("Must be root."); - return -EPERM; - } + r = must_be_root(); + if (r < 0) + return r; r = acquire_bus(BUS_MANAGER, &bus); if (r < 0) @@ -3603,9 +3602,10 @@ static int start_special(int argc, char *argv[], void *userdata) { if (r < 0) return r; - if (arg_force >= 2 && geteuid() != 0) { - log_error("Must be root."); - return -EPERM; + if (arg_force >= 2) { + r = must_be_root(); + if (r < 0) + return r; } r = prepare_firmware_setup(); @@ -8638,7 +8638,7 @@ static int halt_main(void) { if (geteuid() != 0) { if (arg_dry_run || arg_force > 0) { - log_error("Must be root."); + (void) must_be_root(); return -EPERM; } diff --git a/src/udev/udevadm-control.c b/src/udev/udevadm-control.c index ab9237ff78..d80d61583d 100644 --- a/src/udev/udevadm-control.c +++ b/src/udev/udevadm-control.c @@ -63,10 +63,8 @@ static int adm_control(struct udev *udev, int argc, char *argv[]) { {} }; - if (getuid() != 0) { - log_error("root privileges required"); + if (must_be_root() < 0) return 1; - } uctrl = udev_ctrl_new(udev); if (uctrl == NULL) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 38ced53676..1644935ff9 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -1670,10 +1670,9 @@ int main(int argc, char *argv[]) { log_set_max_level(LOG_DEBUG); } - if (getuid() != 0) { - r = log_error_errno(EPERM, "root privileges required"); + r = must_be_root(); + if (r < 0) goto exit; - } if (arg_children_max == 0) { cpu_set_t cpu_set; From 0c63eb7138306c988a12e5366ba291700db7b03f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 11 Dec 2017 23:10:11 +0100 Subject: [PATCH 5/5] verbs: add a new VERB_MUSTBEROOT flag Given that we regularly have verbs that require privileges, let's just make this a flag of the verb. --- src/basic/verbs.c | 8 +++++++- src/basic/verbs.h | 10 +++++++--- src/boot/bootctl.c | 20 ++++++-------------- src/systemctl/systemctl.c | 6 +----- 4 files changed, 21 insertions(+), 23 deletions(-) diff --git a/src/basic/verbs.c b/src/basic/verbs.c index cc1bd7e323..cb42e6dd08 100644 --- a/src/basic/verbs.c +++ b/src/basic/verbs.c @@ -33,7 +33,7 @@ int dispatch_verb(int argc, char *argv[], const Verb verbs[], void *userdata) { const Verb *verb; const char *name; unsigned i; - int left; + int left, r; assert(verbs); assert(verbs[0].dispatch); @@ -89,6 +89,12 @@ int dispatch_verb(int argc, char *argv[], const Verb verbs[], void *userdata) { return 0; } + if (verb->flags & VERB_MUSTBEROOT) { + r = must_be_root(); + if (r < 0) + return r; + } + if (name) return verb->dispatch(left, argv + optind, userdata); else { diff --git a/src/basic/verbs.h b/src/basic/verbs.h index ed62f4d07b..5f44a18f8e 100644 --- a/src/basic/verbs.h +++ b/src/basic/verbs.h @@ -21,13 +21,17 @@ ***/ #define VERB_ANY ((unsigned) -1) -#define VERB_DEFAULT 1U -#define VERB_NOCHROOT 2U + +typedef enum VerbFlags { + VERB_DEFAULT = 1 << 0, + VERB_NOCHROOT = 1 << 1, + VERB_MUSTBEROOT = 1 << 2, +} VerbFlags; typedef struct { const char *verb; unsigned min_args, max_args; - unsigned flags; + VerbFlags flags; int (* const dispatch)(int argc, char *argv[], void *userdata); } Verb; diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c index b685b3c791..468a655118 100644 --- a/src/boot/bootctl.c +++ b/src/boot/bootctl.c @@ -1087,10 +1087,6 @@ static int verb_install(int argc, char *argv[], void *userdata) { bool install; int r; - r = must_be_root(); - if (r < 0) - return r; - r = acquire_esp(false, &part, &pstart, &psize, &uuid); if (r < 0) return r; @@ -1122,10 +1118,6 @@ static int verb_remove(int argc, char *argv[], void *userdata) { sd_id128_t uuid = SD_ID128_NULL; int r; - r = must_be_root(); - if (r < 0) - return r; - r = acquire_esp(false, NULL, NULL, NULL, &uuid); if (r < 0) return r; @@ -1146,12 +1138,12 @@ static int verb_remove(int argc, char *argv[], void *userdata) { static int bootctl_main(int argc, char *argv[]) { static const Verb verbs[] = { - { "help", VERB_ANY, VERB_ANY, 0, help }, - { "status", VERB_ANY, 1, VERB_DEFAULT, verb_status }, - { "list", VERB_ANY, 1, 0, verb_list }, - { "install", VERB_ANY, 1, 0, verb_install }, - { "update", VERB_ANY, 1, 0, verb_install }, - { "remove", VERB_ANY, 1, 0, verb_remove }, + { "help", VERB_ANY, VERB_ANY, 0, help }, + { "status", VERB_ANY, 1, VERB_DEFAULT, verb_status }, + { "list", VERB_ANY, 1, 0, verb_list }, + { "install", VERB_ANY, 1, VERB_MUSTBEROOT, verb_install }, + { "update", VERB_ANY, 1, VERB_MUSTBEROOT, verb_install }, + { "remove", VERB_ANY, 1, VERB_MUSTBEROOT, verb_remove }, {} }; diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 4f92659fcd..ef76af817c 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -2074,10 +2074,6 @@ static int list_machines(int argc, char *argv[], void *userdata) { sd_bus *bus; int r; - r = must_be_root(); - if (r < 0) - return r; - r = acquire_bus(BUS_MANAGER, &bus); if (r < 0) return r; @@ -8408,7 +8404,7 @@ static int systemctl_main(int argc, char *argv[]) { { "list-sockets", VERB_ANY, VERB_ANY, VERB_NOCHROOT, list_sockets }, { "list-timers", VERB_ANY, VERB_ANY, VERB_NOCHROOT, list_timers }, { "list-jobs", VERB_ANY, VERB_ANY, VERB_NOCHROOT, list_jobs }, - { "list-machines", VERB_ANY, VERB_ANY, VERB_NOCHROOT, list_machines }, + { "list-machines", VERB_ANY, VERB_ANY, VERB_NOCHROOT|VERB_MUSTBEROOT, list_machines }, { "clear-jobs", VERB_ANY, 1, VERB_NOCHROOT, trivial_method }, { "cancel", VERB_ANY, VERB_ANY, VERB_NOCHROOT, cancel_job }, { "start", 2, VERB_ANY, VERB_NOCHROOT, start_unit },