From 782671bc8f580835a917ac712fe6dc0d17205bd5 Mon Sep 17 00:00:00 2001 From: Maanya Goenka Date: Wed, 30 Jun 2021 09:28:19 -0700 Subject: [PATCH 1/5] systemd-analyze: validate root argument --- src/analyze/analyze.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/analyze/analyze.c b/src/analyze/analyze.c index e7600934ea..970ed34f06 100644 --- a/src/analyze/analyze.c +++ b/src/analyze/analyze.c @@ -86,12 +86,13 @@ static const char *arg_host = NULL; static UnitFileScope arg_scope = UNIT_FILE_SYSTEM; static bool arg_man = true; static bool arg_generators = false; -static const char *arg_root = NULL; +static char *arg_root = NULL; static unsigned arg_iterations = 1; static usec_t arg_base_time = USEC_INFINITY; STATIC_DESTRUCTOR_REGISTER(arg_dot_from_patterns, strv_freep); STATIC_DESTRUCTOR_REGISTER(arg_dot_to_patterns, strv_freep); +STATIC_DESTRUCTOR_REGISTER(arg_root, freep); typedef struct BootTimes { usec_t firmware_time; @@ -2281,7 +2282,9 @@ static int parse_argv(int argc, char *argv[]) { return version(); case ARG_ROOT: - arg_root = optarg; + r = parse_path_argument(optarg, /* suppress_root= */ true, &arg_root); + if (r < 0) + return r; break; case ARG_SYSTEM: From 2a7cf953e1f5372ccbabed02208de989d396ae51 Mon Sep 17 00:00:00 2001 From: Maanya Goenka Date: Wed, 30 Jun 2021 09:57:54 -0700 Subject: [PATCH 2/5] systemd-analyze: add --root option for 'verify' verb and allow path parsing ------------------------------------------------------------------------------- Example Run: foobar.service created below is a service unit file that has a non-existing key-value pairing (foo = bar) and is thus, syntactically invalid. maanya-goenka@debian:~/systemd (img-support)$ cat <img/usr/lib/systemd/system/foobar.service > [Unit] > foo = bar > > [Service] > ExecStart = /opt/script0.sh > EOF The failure to create foobar.service because of the recursive dependency searching and verification has been addressed in a different PR: systemd-analyze: add option to return an error value when unit verification fails #20233 maanya-goenka@debian:~/systemd (img-support)$ sudo build/systemd-analyze verify --root=img/ foobar.service /home/maanya-goenka/systemd/img/usr/lib/systemd/system/foobar.service:2: Unknown key name 'foo' in section 'Unit', ignoring. foobar.service: Failed to create foobar.service/start: Unit sysinit.target not found. --- man/systemd-analyze.xml | 4 ++-- shell-completion/bash/systemd-analyze | 2 +- shell-completion/zsh/_systemd-analyze | 1 + src/analyze/analyze-condition.c | 2 +- src/analyze/analyze-verify.c | 4 ++-- src/analyze/analyze-verify.h | 2 +- src/analyze/analyze.c | 6 +++--- src/core/main.c | 2 +- src/core/manager.c | 4 ++-- src/core/manager.h | 2 +- src/test/test-bpf-firewall.c | 2 +- src/test/test-bpf-foreign-programs.c | 2 +- src/test/test-cgroup-mask.c | 2 +- src/test/test-cgroup-unit-default.c | 2 +- src/test/test-engine.c | 2 +- src/test/test-execute.c | 2 +- src/test/test-load-fragment.c | 4 ++-- src/test/test-path.c | 2 +- src/test/test-sched-prio.c | 2 +- src/test/test-socket-bind.c | 2 +- src/test/test-watch-pid.c | 2 +- 21 files changed, 27 insertions(+), 26 deletions(-) diff --git a/man/systemd-analyze.xml b/man/systemd-analyze.xml index 4da066e05c..21e2e928cf 100644 --- a/man/systemd-analyze.xml +++ b/man/systemd-analyze.xml @@ -747,8 +747,8 @@ Service b@0.service not loaded, b.socket cannot be started. - With cat-files, show config files underneath - the specified root path PATH. + With cat-files and verify, + operate on files underneath the specified root path PATH. diff --git a/shell-completion/bash/systemd-analyze b/shell-completion/bash/systemd-analyze index 36fcf432ff..e0a9ef5d15 100644 --- a/shell-completion/bash/systemd-analyze +++ b/shell-completion/bash/systemd-analyze @@ -125,7 +125,7 @@ _systemd_analyze() { elif __contains_word "$verb" ${VERBS[VERIFY]}; then if [[ $cur = -* ]]; then - comps='--help --version --system --user --global --man=no --generators=yes' + comps='--help --version --system --user --global --man=no --generators=yes --root' else comps=$( compgen -A file -- "$cur" ) compopt -o filenames diff --git a/shell-completion/zsh/_systemd-analyze b/shell-completion/zsh/_systemd-analyze index ce8e6162e8..921b6cb27d 100644 --- a/shell-completion/zsh/_systemd-analyze +++ b/shell-completion/zsh/_systemd-analyze @@ -87,6 +87,7 @@ _arguments \ '--system[Operate on system systemd instance]' \ '--user[Operate on user systemd instance]' \ '--global[Show global user instance config]' \ + '--root=[Add support for root argument]:PATH' \ '--no-pager[Do not pipe output into a pager]' \ '--man=[Do (not) check for existence of man pages]:boolean:(1 0)' \ '--order[When generating graph for dot, show only order]' \ diff --git a/src/analyze/analyze-condition.c b/src/analyze/analyze-condition.c index 241c188ed6..09870b95ec 100644 --- a/src/analyze/analyze-condition.c +++ b/src/analyze/analyze-condition.c @@ -83,7 +83,7 @@ int verify_conditions(char **lines, UnitFileScope scope) { return log_error_errno(r, "Failed to initialize manager: %m"); log_debug("Starting manager..."); - r = manager_startup(m, NULL, NULL); + r = manager_startup(m, /* serialization= */ NULL, /* fds= */ NULL, /* root= */ NULL); if (r < 0) return r; diff --git a/src/analyze/analyze-verify.c b/src/analyze/analyze-verify.c index bb5bdf998a..4fcec2fcdc 100644 --- a/src/analyze/analyze-verify.c +++ b/src/analyze/analyze-verify.c @@ -218,7 +218,7 @@ static int verify_unit(Unit *u, bool check_man) { return r; } -int verify_units(char **filenames, UnitFileScope scope, bool check_man, bool run_generators) { +int verify_units(char **filenames, UnitFileScope scope, bool check_man, bool run_generators, const char *root) { const ManagerTestRunFlags flags = MANAGER_TEST_RUN_MINIMAL | MANAGER_TEST_RUN_ENV_GENERATORS | @@ -246,7 +246,7 @@ int verify_units(char **filenames, UnitFileScope scope, bool check_man, bool run log_debug("Starting manager..."); - r = manager_startup(m, NULL, NULL); + r = manager_startup(m, /* serialization= */ NULL, /* fds= */ NULL, root); if (r < 0) return r; diff --git a/src/analyze/analyze-verify.h b/src/analyze/analyze-verify.h index 43bfbcbc8c..b547ca6b8d 100644 --- a/src/analyze/analyze-verify.h +++ b/src/analyze/analyze-verify.h @@ -7,4 +7,4 @@ #include "path-lookup.h" int verify_executable(Unit *u, const ExecCommand *exec); -int verify_units(char **filenames, UnitFileScope scope, bool check_man, bool run_generators); +int verify_units(char **filenames, UnitFileScope scope, bool check_man, bool run_generators, const char *root); diff --git a/src/analyze/analyze.c b/src/analyze/analyze.c index 970ed34f06..f5ce2c3ad3 100644 --- a/src/analyze/analyze.c +++ b/src/analyze/analyze.c @@ -2142,7 +2142,7 @@ static int do_condition(int argc, char *argv[], void *userdata) { } static int do_verify(int argc, char *argv[], void *userdata) { - return verify_units(strv_skip(argv, 1), arg_scope, arg_man, arg_generators); + return verify_units(strv_skip(argv, 1), arg_scope, arg_man, arg_generators, arg_root); } static int do_security(int argc, char *argv[], void *userdata) { @@ -2381,9 +2381,9 @@ static int parse_argv(int argc, char *argv[]) { return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Option --user is not supported for cat-config right now."); - if (arg_root && !streq_ptr(argv[optind], "cat-config")) + if (arg_root && !STRPTR_IN_SET(argv[optind], "cat-config", "verify")) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Option --root is only supported for cat-config right now."); + "Option --root is only supported for cat-config and verify right now."); return 1; /* work to do */ } diff --git a/src/core/main.c b/src/core/main.c index eb24245fb3..0914f92a2e 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -2908,7 +2908,7 @@ int main(int argc, char *argv[]) { before_startup = now(CLOCK_MONOTONIC); - r = manager_startup(m, arg_serialization, fds); + r = manager_startup(m, arg_serialization, fds, /* root= */ NULL); if (r < 0) { error_message = "Failed to start up manager"; goto finish; diff --git a/src/core/manager.c b/src/core/manager.c index 24dfe9fc06..aab230e114 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -1727,7 +1727,7 @@ void manager_reloading_stopp(Manager **m) { } } -int manager_startup(Manager *m, FILE *serialization, FDSet *fds) { +int manager_startup(Manager *m, FILE *serialization, FDSet *fds, const char *root) { int r; assert(m); @@ -1736,7 +1736,7 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) { * but we should not touch the real generator directories. */ r = lookup_paths_init(&m->lookup_paths, m->unit_file_scope, MANAGER_IS_TEST_RUN(m) ? LOOKUP_PATHS_TEMPORARY_GENERATED : 0, - NULL); + root); if (r < 0) return log_error_errno(r, "Failed to initialize path lookup table: %m"); diff --git a/src/core/manager.h b/src/core/manager.h index 284ea42a9d..4ce4368474 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -470,7 +470,7 @@ int manager_new(UnitFileScope scope, ManagerTestRunFlags test_run_flags, Manager Manager* manager_free(Manager *m); DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free); -int manager_startup(Manager *m, FILE *serialization, FDSet *fds); +int manager_startup(Manager *m, FILE *serialization, FDSet *fds, const char *root); Job *manager_get_job(Manager *m, uint32_t id); Unit *manager_get_unit(Manager *m, const char *name); diff --git a/src/test/test-bpf-firewall.c b/src/test/test-bpf-firewall.c index b29c0d7844..8b7d46bee3 100644 --- a/src/test/test-bpf-firewall.c +++ b/src/test/test-bpf-firewall.c @@ -97,7 +97,7 @@ int main(int argc, char *argv[]) { /* The simple tests succeeded. Now let's try full unit-based use-case. */ assert_se(manager_new(UNIT_FILE_USER, MANAGER_TEST_RUN_BASIC, &m) >= 0); - assert_se(manager_startup(m, NULL, NULL) >= 0); + assert_se(manager_startup(m, NULL, NULL, NULL) >= 0); assert_se(u = unit_new(m, sizeof(Service))); assert_se(unit_add_name(u, "foo.service") == 0); diff --git a/src/test/test-bpf-foreign-programs.c b/src/test/test-bpf-foreign-programs.c index a6f8eb6f4a..bbf3916872 100644 --- a/src/test/test-bpf-foreign-programs.c +++ b/src/test/test-bpf-foreign-programs.c @@ -304,7 +304,7 @@ int main(int argc, char *argv[]) { assert_se(runtime_dir = setup_fake_runtime_dir()); assert_se(manager_new(UNIT_FILE_USER, MANAGER_TEST_RUN_BASIC, &m) >= 0); - assert_se(manager_startup(m, NULL, NULL) >= 0); + assert_se(manager_startup(m, NULL, NULL, NULL) >= 0); assert_se(test_bpf_cgroup_programs(m, "single_prog.service", single_prog, ELEMENTSOF(single_prog)) >= 0); diff --git a/src/test/test-cgroup-mask.c b/src/test/test-cgroup-mask.c index 19e159b9ff..184b393dd5 100644 --- a/src/test/test-cgroup-mask.c +++ b/src/test/test-cgroup-mask.c @@ -60,7 +60,7 @@ static int test_cgroup_mask(void) { m->default_tasks_accounting = false; m->default_tasks_max = TASKS_MAX_UNSET; - assert_se(manager_startup(m, NULL, NULL) >= 0); + assert_se(manager_startup(m, NULL, NULL, NULL) >= 0); /* Load units and verify hierarchy. */ assert_se(manager_load_startable_unit_or_warn(m, "parent.slice", NULL, &parent) >= 0); diff --git a/src/test/test-cgroup-unit-default.c b/src/test/test-cgroup-unit-default.c index 225d138e41..0fae2f64cb 100644 --- a/src/test/test-cgroup-unit-default.c +++ b/src/test/test-cgroup-unit-default.c @@ -33,7 +33,7 @@ static int test_default_memory_low(void) { } assert_se(r >= 0); - assert_se(manager_startup(m, NULL, NULL) >= 0); + assert_se(manager_startup(m, NULL, NULL, NULL) >= 0); /* dml.slice has DefaultMemoryLow=50. Beyond that, individual subhierarchies look like this: * diff --git a/src/test/test-engine.c b/src/test/test-engine.c index 6dc16193d3..880af36fb5 100644 --- a/src/test/test-engine.c +++ b/src/test/test-engine.c @@ -95,7 +95,7 @@ int main(int argc, char *argv[]) { if (manager_errno_skip_test(r)) return log_tests_skipped_errno(r, "manager_new"); assert_se(r >= 0); - assert_se(manager_startup(m, NULL, NULL) >= 0); + assert_se(manager_startup(m, NULL, NULL, NULL) >= 0); printf("Load1:\n"); assert_se(manager_load_startable_unit_or_warn(m, "a.service", NULL, &a) >= 0); diff --git a/src/test/test-execute.c b/src/test/test-execute.c index 125e0bbf4f..a0481f1194 100644 --- a/src/test/test-execute.c +++ b/src/test/test-execute.c @@ -844,7 +844,7 @@ static int run_tests(UnitFileScope scope, const test_entry tests[], char **patte if (manager_errno_skip_test(r)) return log_tests_skipped_errno(r, "manager_new"); assert_se(r >= 0); - assert_se(manager_startup(m, NULL, NULL) >= 0); + assert_se(manager_startup(m, NULL, NULL, NULL) >= 0); for (const test_entry *test = tests; test->f; test++) if (strv_fnmatch_or_empty(patterns, test->name, FNM_NOESCAPE)) diff --git a/src/test/test-load-fragment.c b/src/test/test-load-fragment.c index b41a8abf7b..e0ba99199c 100644 --- a/src/test/test-load-fragment.c +++ b/src/test/test-load-fragment.c @@ -104,7 +104,7 @@ static void test_config_parse_exec(void) { } assert_se(r >= 0); - assert_se(manager_startup(m, NULL, NULL) >= 0); + assert_se(manager_startup(m, NULL, NULL, NULL) >= 0); assert_se(u = unit_new(m, sizeof(Service))); @@ -448,7 +448,7 @@ static void test_config_parse_log_extra_fields(void) { } assert_se(r >= 0); - assert_se(manager_startup(m, NULL, NULL) >= 0); + assert_se(manager_startup(m, NULL, NULL, NULL) >= 0); assert_se(u = unit_new(m, sizeof(Service))); diff --git a/src/test/test-path.c b/src/test/test-path.c index 490fb136a7..04cb4fa37c 100644 --- a/src/test/test-path.c +++ b/src/test/test-path.c @@ -38,7 +38,7 @@ static int setup_test(Manager **m) { if (manager_errno_skip_test(r)) return log_tests_skipped_errno(r, "manager_new"); assert_se(r >= 0); - assert_se(manager_startup(tmp, NULL, NULL) >= 0); + assert_se(manager_startup(tmp, NULL, NULL, NULL) >= 0); STRV_FOREACH(test_path, tests_path) { _cleanup_free_ char *p = NULL; diff --git a/src/test/test-sched-prio.c b/src/test/test-sched-prio.c index 1f125b1d1e..35f7be491a 100644 --- a/src/test/test-sched-prio.c +++ b/src/test/test-sched-prio.c @@ -34,7 +34,7 @@ int main(int argc, char *argv[]) { if (manager_errno_skip_test(r)) return log_tests_skipped_errno(r, "manager_new"); assert_se(r >= 0); - assert_se(manager_startup(m, NULL, NULL) >= 0); + assert_se(manager_startup(m, NULL, NULL, NULL) >= 0); /* load idle ok */ assert_se(manager_load_startable_unit_or_warn(m, "sched_idle_ok.service", NULL, &idle_ok) >= 0); diff --git a/src/test/test-socket-bind.c b/src/test/test-socket-bind.c index 989172eee3..ecad86baeb 100644 --- a/src/test/test-socket-bind.c +++ b/src/test/test-socket-bind.c @@ -138,7 +138,7 @@ int main(int argc, char *argv[]) { assert_se(runtime_dir = setup_fake_runtime_dir()); assert_se(manager_new(UNIT_FILE_USER, MANAGER_TEST_RUN_BASIC, &m) >= 0); - assert_se(manager_startup(m, NULL, NULL) >= 0); + assert_se(manager_startup(m, NULL, NULL, NULL) >= 0); assert_se(test_socket_bind(m, "socket_bind_test.service", netcat_path, "2000", STRV_MAKE("2000"), STRV_MAKE("any")) >= 0); assert_se(test_socket_bind(m, "socket_bind_test.service", netcat_path, "2000", STRV_MAKE("ipv6:2001-2002"), STRV_MAKE("any")) >= 0); diff --git a/src/test/test-watch-pid.c b/src/test/test-watch-pid.c index 4afc46f10f..885ed802d4 100644 --- a/src/test/test-watch-pid.c +++ b/src/test/test-watch-pid.c @@ -27,7 +27,7 @@ int main(int argc, char *argv[]) { assert_se(runtime_dir = setup_fake_runtime_dir()); assert_se(manager_new(UNIT_FILE_USER, MANAGER_TEST_RUN_BASIC, &m) >= 0); - assert_se(manager_startup(m, NULL, NULL) >= 0); + assert_se(manager_startup(m, NULL, NULL, NULL) >= 0); assert_se(a = unit_new(m, sizeof(Service))); assert_se(unit_add_name(a, "a.service") >= 0); From e5ea5c3a17c6b8916c35abb665fd926d2d9ff41b Mon Sep 17 00:00:00 2001 From: Maanya Goenka Date: Wed, 30 Jun 2021 10:02:51 -0700 Subject: [PATCH 3/5] systemd-analyze: support discrete images for 'verify' verb Adding --image parameter for verify verb using the dissect image functionality ----------------------------------------------------------------------------------- Example Run: I created a unit service file testrun.service with an invalid key-value pairing (foo = bar) and a squashfs image run.raw to test the code. maanya-goenka@debian:~/systemd (img-support)$ cat <img/usr/lib/systemd/system/testrun.service > [Unit] > foo = bar > > [Service] > ExecStart = /opt/script0.sh > EOF maanya-goenka@debian:~/systemd (img-support)$ mksquashfs img/ run.raw Parallel mksquashfs: Using 4 processors Creating 4.0 filesystem on run.raw, block size 131072. [==============================================================================================================================|] 6/6 100% Exportable Squashfs 4.0 filesystem, gzip compressed, data block size 131072 compressed data, compressed metadata, compressed fragments, compressed xattrs duplicates are removed Filesystem size 0.60 Kbytes (0.00 Mbytes) 52.32% of uncompressed filesystem size (1.14 Kbytes) Inode table size 166 bytes (0.16 Kbytes) 43.01% of uncompressed inode table size (386 bytes) Directory table size 153 bytes (0.15 Kbytes) 58.40% of uncompressed directory table size (262 bytes) Number of duplicate files found 1 Number of inodes 12 Number of files 6 Number of fragments 1 Number of symbolic links 0 Number of device nodes 0 Number of fifo nodes 0 Number of socket nodes 0 Number of directories 6 Number of ids (unique uids + gids) 1 Number of uids 1 maanya-goenka (1000) Number of gids 1 maanya-goenka (1000) maanya-goenka@debian:~/systemd (img-support)$ sudo build/systemd-analyze verify --image=run.raw testrun.service /tmp/.#systemd-analyzec71c7297a936b91c/usr/lib/systemd/system/testrun.service:2: Unknown key name 'foo' in section 'Unit', ignoring. testrun.service: Failed to create testrun.service/start: Unit sysinit.target not found. The 'Unit sysinit.target not found' error that we see here is due to recursive dependency searching during unit loading and has been addressed in a different PR: systemd-analyze: add option to return an error value when unit verification fails #20233 --- man/systemd-analyze.xml | 7 +++++ shell-completion/bash/systemd-analyze | 2 +- shell-completion/zsh/_systemd-analyze | 1 + src/analyze/analyze.c | 42 +++++++++++++++++++++++++-- 4 files changed, 49 insertions(+), 3 deletions(-) diff --git a/man/systemd-analyze.xml b/man/systemd-analyze.xml index 21e2e928cf..dc93ac4e72 100644 --- a/man/systemd-analyze.xml +++ b/man/systemd-analyze.xml @@ -751,6 +751,13 @@ Service b@0.service not loaded, b.socket cannot be started. operate on files underneath the specified root path PATH. + + + + With cat-files and verify, + operate on files inside the specified image path PATH. + + diff --git a/shell-completion/bash/systemd-analyze b/shell-completion/bash/systemd-analyze index e0a9ef5d15..872518add0 100644 --- a/shell-completion/bash/systemd-analyze +++ b/shell-completion/bash/systemd-analyze @@ -125,7 +125,7 @@ _systemd_analyze() { elif __contains_word "$verb" ${VERBS[VERIFY]}; then if [[ $cur = -* ]]; then - comps='--help --version --system --user --global --man=no --generators=yes --root' + comps='--help --version --system --user --global --man=no --generators=yes --root --image' else comps=$( compgen -A file -- "$cur" ) compopt -o filenames diff --git a/shell-completion/zsh/_systemd-analyze b/shell-completion/zsh/_systemd-analyze index 921b6cb27d..88a09d84b9 100644 --- a/shell-completion/zsh/_systemd-analyze +++ b/shell-completion/zsh/_systemd-analyze @@ -88,6 +88,7 @@ _arguments \ '--user[Operate on user systemd instance]' \ '--global[Show global user instance config]' \ '--root=[Add support for root argument]:PATH' \ + '--image=[Add support for discrete images]:PATH' \ '--no-pager[Do not pipe output into a pager]' \ '--man=[Do (not) check for existence of man pages]:boolean:(1 0)' \ '--order[When generating graph for dot, show only order]' \ diff --git a/src/analyze/analyze.c b/src/analyze/analyze.c index f5ce2c3ad3..8d637ff8de 100644 --- a/src/analyze/analyze.c +++ b/src/analyze/analyze.c @@ -34,6 +34,7 @@ #include "locale-util.h" #include "log.h" #include "main-func.h" +#include "mount-util.h" #include "nulstr-util.h" #include "pager.h" #include "parse-argument.h" @@ -87,12 +88,14 @@ static UnitFileScope arg_scope = UNIT_FILE_SYSTEM; static bool arg_man = true; static bool arg_generators = false; static char *arg_root = NULL; +static char *arg_image = NULL; static unsigned arg_iterations = 1; static usec_t arg_base_time = USEC_INFINITY; STATIC_DESTRUCTOR_REGISTER(arg_dot_from_patterns, strv_freep); STATIC_DESTRUCTOR_REGISTER(arg_dot_to_patterns, strv_freep); STATIC_DESTRUCTOR_REGISTER(arg_root, freep); +STATIC_DESTRUCTOR_REGISTER(arg_image, freep); typedef struct BootTimes { usec_t firmware_time; @@ -2232,6 +2235,7 @@ static int parse_argv(int argc, char *argv[]) { ARG_ORDER, ARG_REQUIRE, ARG_ROOT, + ARG_IMAGE, ARG_SYSTEM, ARG_USER, ARG_GLOBAL, @@ -2251,6 +2255,7 @@ static int parse_argv(int argc, char *argv[]) { { "order", no_argument, NULL, ARG_ORDER }, { "require", no_argument, NULL, ARG_REQUIRE }, { "root", required_argument, NULL, ARG_ROOT }, + { "image", required_argument, NULL, ARG_IMAGE }, { "system", no_argument, NULL, ARG_SYSTEM }, { "user", no_argument, NULL, ARG_USER }, { "global", no_argument, NULL, ARG_GLOBAL }, @@ -2287,6 +2292,12 @@ static int parse_argv(int argc, char *argv[]) { return r; break; + case ARG_IMAGE: + r = parse_path_argument(optarg, /* suppress_root= */ false, &arg_image); + if (r < 0) + return r; + break; + case ARG_SYSTEM: arg_scope = UNIT_FILE_SYSTEM; break; @@ -2381,14 +2392,21 @@ static int parse_argv(int argc, char *argv[]) { return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Option --user is not supported for cat-config right now."); - if (arg_root && !STRPTR_IN_SET(argv[optind], "cat-config", "verify")) + if ((arg_root || arg_image) && !STRPTR_IN_SET(argv[optind], "cat-config", "verify")) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Option --root is only supported for cat-config and verify right now."); + "Options --root= and --image= are only supported for cat-config and verify right now."); + + /* Having both an image and a root is not supported by the code */ + if (arg_root && arg_image) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Please specify either --root= or --image=, the combination of both is not supported."); return 1; /* work to do */ } static int run(int argc, char *argv[]) { + _cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL; + _cleanup_(decrypted_image_unrefp) DecryptedImage *decrypted_image = NULL; + _cleanup_(umount_and_rmdir_and_freep) char *unlink_dir = NULL; static const Verb verbs[] = { { "help", VERB_ANY, VERB_ANY, 0, help }, @@ -2432,6 +2450,26 @@ static int run(int argc, char *argv[]) { if (r <= 0) return r; + /* Open up and mount the image */ + if (arg_image) { + assert(!arg_root); + + r = mount_image_privately_interactively( + arg_image, + DISSECT_IMAGE_GENERIC_ROOT | + DISSECT_IMAGE_RELAX_VAR_CHECK | + DISSECT_IMAGE_READ_ONLY, + &unlink_dir, + &loop_device, + &decrypted_image); + if (r < 0) + return r; + + arg_root = strdup(unlink_dir); + if (!arg_root) + return log_oom(); + } + return dispatch_verb(argc, argv, verbs, NULL); } From 36f4af05680d0f8f67b870bf72241d7d2fefd91b Mon Sep 17 00:00:00 2001 From: Maanya Goenka Date: Wed, 4 Aug 2021 11:59:45 -0700 Subject: [PATCH 4/5] path-util: teach find_executable_full how to look into the root directory When the root parameter in find_executable_full is set, chase_symlinks prefixes this root to every check of the path name to find the complete path of the execuatble in case the path provided is not absolute. This is only done for the non NULL root because otherwise the chase_symlinks function would alter the behavior of some of the callers which would in turn alter the outputs in a way that is undesirable. The find_execuatble_full function is invoked by the verify_executable function in analyze-verify. --- src/analyze/analyze-verify.c | 2 +- src/basic/path-util.c | 38 +++++++++++++++++++++++++++++++++++- src/basic/path-util.h | 4 ++-- src/core/execute.c | 2 +- src/test/test-path-util.c | 10 +++++----- 5 files changed, 46 insertions(+), 10 deletions(-) diff --git a/src/analyze/analyze-verify.c b/src/analyze/analyze-verify.c index 4fcec2fcdc..99bce68ca4 100644 --- a/src/analyze/analyze-verify.c +++ b/src/analyze/analyze-verify.c @@ -124,7 +124,7 @@ int verify_executable(Unit *u, const ExecCommand *exec) { if (exec->flags & EXEC_COMMAND_IGNORE_FAILURE) return 0; - r = find_executable_full(exec->path, false, NULL, NULL); + r = find_executable_full(exec->path, /* root= */ NULL, false, NULL, NULL); if (r < 0) return log_unit_error_errno(u, r, "Command %s is not executable: %m", exec->path); diff --git a/src/basic/path-util.c b/src/basic/path-util.c index 4aebb6541e..fe799da20b 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -639,7 +639,7 @@ static int check_x_access(const char *path, int *ret_fd) { return 0; } -int find_executable_full(const char *name, bool use_path_envvar, char **ret_filename, int *ret_fd) { +int find_executable_full(const char *name, const char *root, bool use_path_envvar, char **ret_filename, int *ret_fd) { int last_error, r; const char *p = NULL; @@ -647,6 +647,25 @@ int find_executable_full(const char *name, bool use_path_envvar, char **ret_file if (is_path(name)) { _cleanup_close_ int fd = -1; + _cleanup_free_ char *path_name = NULL; + + /* Function chase_symlinks() is invoked only when root is not NULL, + * as using it regardless of root value would alter the behavior + * of existing callers for example: /bin/sleep would become + * /usr/bin/sleep when find_executables is called. Hence, this function + * should be invoked when needed to avoid unforeseen regression or other + * complicated changes. */ + if (root) { + r = chase_symlinks(name, + root, + CHASE_PREFIX_ROOT, + &path_name, + /* ret_fd= */ NULL); /* prefix root to name in case full paths are not specified */ + if (r < 0) + return r; + + name = path_name; + } r = check_x_access(name, ret_fd ? &fd : NULL); if (r < 0) @@ -690,6 +709,23 @@ int find_executable_full(const char *name, bool use_path_envvar, char **ret_file if (!path_extend(&element, name)) return -ENOMEM; + if (root) { + char *path_name; + + r = chase_symlinks(element, + root, + CHASE_PREFIX_ROOT, + &path_name, + /* ret_fd= */ NULL); + if (r < 0) { + if (r != -EACCES) + last_error = r; + continue; + } + + free_and_replace(element, path_name); + } + r = check_x_access(element, ret_fd ? &fd : NULL); if (r < 0) { /* PATH entries which we don't have access to are ignored, as per tradition. */ diff --git a/src/basic/path-util.h b/src/basic/path-util.h index 26e7362d1f..0d69145497 100644 --- a/src/basic/path-util.h +++ b/src/basic/path-util.h @@ -99,9 +99,9 @@ int path_strv_make_absolute_cwd(char **l); char** path_strv_resolve(char **l, const char *root); char** path_strv_resolve_uniq(char **l, const char *root); -int find_executable_full(const char *name, bool use_path_envvar, char **ret_filename, int *ret_fd); +int find_executable_full(const char *name, const char *root, bool use_path_envvar, char **ret_filename, int *ret_fd); static inline int find_executable(const char *name, char **ret_filename) { - return find_executable_full(name, true, ret_filename, NULL); + return find_executable_full(name, /* root= */ NULL, true, ret_filename, NULL); } bool paths_check_timestamp(const char* const* paths, usec_t *paths_ts_usec, bool update); diff --git a/src/core/execute.c b/src/core/execute.c index 4608956259..6635a35abf 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -4351,7 +4351,7 @@ static int exec_child( _cleanup_free_ char *executable = NULL; _cleanup_close_ int executable_fd = -1; - r = find_executable_full(command->path, false, &executable, &executable_fd); + r = find_executable_full(command->path, /* root= */ NULL, false, &executable, &executable_fd); if (r < 0) { if (r != -ENOMEM && (command->flags & EXEC_COMMAND_IGNORE_FAILURE)) { log_unit_struct_errno(unit, LOG_INFO, r, diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c index ceea7a62f5..1a0fda5d1a 100644 --- a/src/test/test-path-util.c +++ b/src/test/test-path-util.c @@ -204,12 +204,12 @@ static void test_find_executable_full(void) { log_info("/* %s */", __func__); - assert_se(find_executable_full("sh", true, &p, NULL) == 0); + assert_se(find_executable_full("sh", NULL, true, &p, NULL) == 0); puts(p); assert_se(streq(basename(p), "sh")); free(p); - assert_se(find_executable_full("sh", false, &p, NULL) == 0); + assert_se(find_executable_full("sh", NULL, false, &p, NULL) == 0); puts(p); assert_se(streq(basename(p), "sh")); free(p); @@ -221,12 +221,12 @@ static void test_find_executable_full(void) { assert_se(unsetenv("PATH") == 0); - assert_se(find_executable_full("sh", true, &p, NULL) == 0); + assert_se(find_executable_full("sh", NULL, true, &p, NULL) == 0); puts(p); assert_se(streq(basename(p), "sh")); free(p); - assert_se(find_executable_full("sh", false, &p, NULL) == 0); + assert_se(find_executable_full("sh", NULL, false, &p, NULL) == 0); puts(p); assert_se(streq(basename(p), "sh")); free(p); @@ -277,7 +277,7 @@ static void test_find_executable_exec_one(const char *path) { pid_t pid; int r; - r = find_executable_full(path, false, &t, &fd); + r = find_executable_full(path, NULL, false, &t, &fd); log_info_errno(r, "%s: %s → %s: %d/%m", __func__, path, t ?: "-", fd); From ed80366139531e196e6582ab325f1c54efa2b384 Mon Sep 17 00:00:00 2001 From: Maanya Goenka Date: Wed, 4 Aug 2021 12:00:31 -0700 Subject: [PATCH 5/5] systemd-analyze: add root to find and verify executable --- src/analyze/analyze-verify.c | 18 +++++++++--------- src/analyze/analyze-verify.h | 2 +- src/analyze/test-verify.c | 8 ++++---- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/analyze/analyze-verify.c b/src/analyze/analyze-verify.c index 99bce68ca4..9ef6868367 100644 --- a/src/analyze/analyze-verify.c +++ b/src/analyze/analyze-verify.c @@ -115,7 +115,7 @@ static int verify_socket(Unit *u) { return 0; } -int verify_executable(Unit *u, const ExecCommand *exec) { +int verify_executable(Unit *u, const ExecCommand *exec, const char *root) { int r; if (!exec) @@ -124,14 +124,14 @@ int verify_executable(Unit *u, const ExecCommand *exec) { if (exec->flags & EXEC_COMMAND_IGNORE_FAILURE) return 0; - r = find_executable_full(exec->path, /* root= */ NULL, false, NULL, NULL); + r = find_executable_full(exec->path, root, false, NULL, NULL); if (r < 0) return log_unit_error_errno(u, r, "Command %s is not executable: %m", exec->path); return 0; } -static int verify_executables(Unit *u) { +static int verify_executables(Unit *u, const char *root) { ExecCommand *exec; int r = 0, k; unsigned i; @@ -141,20 +141,20 @@ static int verify_executables(Unit *u) { exec = u->type == UNIT_SOCKET ? SOCKET(u)->control_command : u->type == UNIT_MOUNT ? MOUNT(u)->control_command : u->type == UNIT_SWAP ? SWAP(u)->control_command : NULL; - k = verify_executable(u, exec); + k = verify_executable(u, exec, root); if (k < 0 && r == 0) r = k; if (u->type == UNIT_SERVICE) for (i = 0; i < ELEMENTSOF(SERVICE(u)->exec_command); i++) { - k = verify_executable(u, SERVICE(u)->exec_command[i]); + k = verify_executable(u, SERVICE(u)->exec_command[i], root); if (k < 0 && r == 0) r = k; } if (u->type == UNIT_SOCKET) for (i = 0; i < ELEMENTSOF(SOCKET(u)->exec_command); i++) { - k = verify_executable(u, SOCKET(u)->exec_command[i]); + k = verify_executable(u, SOCKET(u)->exec_command[i], root); if (k < 0 && r == 0) r = k; } @@ -189,7 +189,7 @@ static int verify_documentation(Unit *u, bool check_man) { return r; } -static int verify_unit(Unit *u, bool check_man) { +static int verify_unit(Unit *u, bool check_man, const char *root) { _cleanup_(sd_bus_error_free) sd_bus_error err = SD_BUS_ERROR_NULL; int r, k; @@ -207,7 +207,7 @@ static int verify_unit(Unit *u, bool check_man) { if (k < 0 && r == 0) r = k; - k = verify_executables(u); + k = verify_executables(u, root); if (k < 0 && r == 0) r = k; @@ -278,7 +278,7 @@ int verify_units(char **filenames, UnitFileScope scope, bool check_man, bool run } for (i = 0; i < count; i++) { - k = verify_unit(units[i], check_man); + k = verify_unit(units[i], check_man, root); if (k < 0 && r == 0) r = k; } diff --git a/src/analyze/analyze-verify.h b/src/analyze/analyze-verify.h index b547ca6b8d..6b3d6a5ab5 100644 --- a/src/analyze/analyze-verify.h +++ b/src/analyze/analyze-verify.h @@ -6,5 +6,5 @@ #include "execute.h" #include "path-lookup.h" -int verify_executable(Unit *u, const ExecCommand *exec); +int verify_executable(Unit *u, const ExecCommand *exec, const char *root); int verify_units(char **filenames, UnitFileScope scope, bool check_man, bool run_generators, const char *root); diff --git a/src/analyze/test-verify.c b/src/analyze/test-verify.c index 12c32159e5..eaf5e0b39b 100644 --- a/src/analyze/test-verify.c +++ b/src/analyze/test-verify.c @@ -4,12 +4,12 @@ static void test_verify_nonexistent(void) { /* Negative cases */ - assert_se(verify_executable(NULL, &(ExecCommand) {.flags = EXEC_COMMAND_IGNORE_FAILURE, .path = (char*) "/non/existent"}) == 0); - assert_se(verify_executable(NULL, &(ExecCommand) {.path = (char*) "/non/existent"}) < 0); + assert_se(verify_executable(NULL, &(ExecCommand) {.flags = EXEC_COMMAND_IGNORE_FAILURE, .path = (char*) "/non/existent"}, NULL) == 0); + assert_se(verify_executable(NULL, &(ExecCommand) {.path = (char*) "/non/existent"}, NULL) < 0); /* Ordinary cases */ - assert_se(verify_executable(NULL, &(ExecCommand) {.path = (char*) "/bin/echo"}) == 0); - assert_se(verify_executable(NULL, &(ExecCommand) {.flags = EXEC_COMMAND_IGNORE_FAILURE, .path = (char*) "/bin/echo"}) == 0); + assert_se(verify_executable(NULL, &(ExecCommand) {.path = (char*) "/bin/echo"}, NULL) == 0); + assert_se(verify_executable(NULL, &(ExecCommand) {.flags = EXEC_COMMAND_IGNORE_FAILURE, .path = (char*) "/bin/echo"}, NULL) == 0); } int main(int argc, char *argv[]) {