From 91d149cfb45fc2fad7ce18fb651297ee50ecc1f8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Feb 2024 11:30:27 +0100 Subject: [PATCH 1/8] basic: add "build path" logic We have a number of components these days that are split into multiple binaries, i.e. a primary one, and a worker callout usually. These binaries are closely related, they typically speak a protocol that is internal, and not safe to mix and match. Examples for this: - homed and its worker binary homework - userdbd and its worker binary userwork - import and the various pull/import/export handlers - sysupdate the same - the service manager and the executor binary Running any of these daemons directly from the meson build tree is messy, since the implementations will typically invoke the installed callout binaries, not the ones from the build tree. This is very annoying, and not obvious at first. Now, we could always invoke relevant binaries from $(dirname /proc/self/exe) first, before using the OS installed ones. But that's typically not what is desired, because this means in the installed case (i.e. the usual one) we'll look for these callout binaries at a place they typically will not be found (because these callouts generally are located in libexecdir, not bindir when installed). Hence, let's try to do things a bit smarter, and follow what build systems such as meson have already been doing to make sure dynamic library discovery works correctly when binaries are run from a build directory: let's start looking at rpath/runpath in the main binary that is executed: if there's an rpath/runpath set, then we'll look for the callout binaries next to the main binary, otherwise we won't. This should generally be the right thing to do as meson strips the rpath during installation, and thus we'll look for the callouts in the build dir if in build dir mode, and in the OS otherwise. --- src/basic/build-path.c | 207 +++++++++++++++++++++++++++++++++++++ src/basic/build-path.h | 6 ++ src/basic/meson.build | 1 + src/test/meson.build | 1 + src/test/test-build-path.c | 20 ++++ 5 files changed, 235 insertions(+) create mode 100644 src/basic/build-path.c create mode 100644 src/basic/build-path.h create mode 100644 src/test/test-build-path.c diff --git a/src/basic/build-path.c b/src/basic/build-path.c new file mode 100644 index 0000000000..d81ed561dc --- /dev/null +++ b/src/basic/build-path.c @@ -0,0 +1,207 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include +#include +#include + +#include "build-path.h" +#include "errno-list.h" +#include "macro.h" +#include "path-util.h" +#include "process-util.h" +#include "unistd.h" + +static int get_runpath_from_dynamic(const ElfW(Dyn) *d, const char **ret) { + size_t runpath_index = SIZE_MAX, rpath_index = SIZE_MAX; + const char *strtab = NULL; + + assert(d); + + /* Iterates through the PT_DYNAMIC section to find the DT_RUNPATH/DT_RPATH entries */ + + for (; d->d_tag != DT_NULL; d++) { + + switch (d->d_tag) { + + case DT_RUNPATH: + runpath_index = (size_t) d->d_un.d_val; + break; + + case DT_RPATH: + rpath_index = (size_t) d->d_un.d_val; + break; + + case DT_STRTAB: + strtab = (const char *) d->d_un.d_val; + break; + } + + /* runpath wins, hence if we have the table and runpath we can exit the loop early */ + if (strtab && runpath_index != SIZE_MAX) + break; + } + + if (!strtab) + return -ENOTRECOVERABLE; + + /* According to dl.so runpath wins of both runpath and rpath are defined. */ + if (runpath_index != SIZE_MAX) { + if (ret) + *ret = strtab + runpath_index; + return 1; + } + + if (rpath_index != SIZE_MAX) { + if (ret) + *ret = strtab + rpath_index; + return 1; + } + + if (ret) + *ret = NULL; + + return 0; +} + +static int get_runpath(const char **ret) { + unsigned long phdr, phent, phnum; + + /* Finds the rpath/runpath in the program headers of the main executable we are running in */ + + phdr = getauxval(AT_PHDR); /* Start offset of phdr */ + if (phdr == 0) + return -ENOTRECOVERABLE; + + phnum = getauxval(AT_PHNUM); /* Number of entries in phdr */ + if (phnum == 0) + return -ENOTRECOVERABLE; + + phent = getauxval(AT_PHENT); /* Size of entries in phdr */ + if (phent < sizeof(ElfW(Phdr))) /* Safety check, that our idea of the structure matches the file */ + return -ENOTRECOVERABLE; + + ElfW(Addr) bias = 0, dyn = 0; + bool found_bias = false, found_dyn = false; + + /* Iterate through the Phdr structures to find the PT_PHDR and PT_DYNAMIC sections */ + for (unsigned long i = 0; i < phnum; i++) { + const ElfW(Phdr) *p = (const ElfW(Phdr)*) (phdr + (i * phent)); + + switch (p->p_type) { + + case PT_PHDR: + if (p->p_vaddr > phdr) /* safety overflow check */ + return -ENOTRECOVERABLE; + + bias = (ElfW(Addr)) phdr - p->p_vaddr; + found_bias = true; + break; + + case PT_DYNAMIC: + dyn = p->p_vaddr; + found_dyn = true; + break; + } + + if (found_bias && found_dyn) + break; + } + + if (!found_dyn) + return -ENOTRECOVERABLE; + + return get_runpath_from_dynamic((const ElfW(Dyn)*) (bias + dyn), ret); +} + +int get_build_exec_dir(char **ret) { + int r; + + /* Returns the build execution directory if we are invoked in a build environment. Specifically, this + * checks if the main program binary has an rpath/runpath set (i.e. an explicit directory where to + * look for shared libraries) to $ORIGIN. If so we know that this is not a regular installed binary, + * but one which shall acquire its libraries from below a directory it is located in, i.e. a build + * directory or similar. In that case it typically makes sense to also search for our auxiliary + * executables we fork() off in a directory close to our main program binary, rather than in the + * system. + * + * This function is supposed to be used when looking for "callout" binaries that are closely related + * to the main program (i.e. speak a specific protocol between each other). And where it's generally + * a good idea to use the binary from the build tree (if there is one) instead of the system. + * + * Note that this does *not* actually return the rpath/runpath but the instead the directory the main + * executable was found in. This follows the logic that the result is supposed to be used for + * executable binaries (i.e. stuff in bindir), not for shared libraries (i.e. stuff in libdir), and + * hence the literal shared library path would just be wrong. + * + * TLDR: if we look for callouts in this dir first, running binaries from the meson build tree + * automatically uses the right callout. + * + * Returns: + * -ENOEXEC → We are not running in an rpath/runpath $ORIGIN environment + * -ENOENT → We don't know our own binary path + * -NOTRECOVERABLE → Dynamic binary information missing? + */ + + static int runpath_cached = -ERRNO_MAX-1; + if (runpath_cached == -ERRNO_MAX-1) { + const char *runpath = NULL; + + runpath_cached = get_runpath(&runpath); + + /* We only care if the runpath starts with $ORIGIN/ */ + if (runpath_cached > 0 && !startswith(runpath, "$ORIGIN/")) + runpath_cached = 0; + } + if (runpath_cached < 0) + return runpath_cached; + if (runpath_cached == 0) + return -ENOEXEC; + + _cleanup_free_ char *exe = NULL; + r = get_process_exe(0, &exe); + if (r < 0) + return runpath_cached = r; + + return path_extract_directory(exe, ret); +} + +static int find_build_dir_binary(const char *fn, char **ret) { + int r; + + assert(fn); + assert(ret); + + _cleanup_free_ char *build_dir = NULL; + r = get_build_exec_dir(&build_dir); + if (r < 0) + return r; + + _cleanup_free_ char *np = path_join(build_dir, fn); + if (!np) + return -ENOMEM; + + *ret = TAKE_PTR(np); + return 0; +} + +int invoke_callout_binary(const char *path, char *const argv[]) { + int r; + + assert(path); + + /* Just like execv(), but tries to execute the specified binary in the build dir instead, if known */ + + _cleanup_free_ char *fn = NULL; + r = path_extract_filename(path, &fn); + if (r < 0) + return r; + if (r == O_DIRECTORY) /* Uh? */ + return -EISDIR; + + _cleanup_free_ char *np = NULL; + if (find_build_dir_binary(fn, &np) >= 0) + execv(np, argv); + + execv(path, argv); + return -errno; +} diff --git a/src/basic/build-path.h b/src/basic/build-path.h new file mode 100644 index 0000000000..bf42783f1b --- /dev/null +++ b/src/basic/build-path.h @@ -0,0 +1,6 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +#pragma once + +int get_build_exec_dir(char **ret); + +int invoke_callout_binary(const char *path, char *const argv[]); diff --git a/src/basic/meson.build b/src/basic/meson.build index 6b30908ce1..a9722d2121 100644 --- a/src/basic/meson.build +++ b/src/basic/meson.build @@ -10,6 +10,7 @@ basic_sources = files( 'audit-util.c', 'btrfs.c', 'build.c', + 'build-path.c', 'bus-label.c', 'cap-list.c', 'capability-util.c', diff --git a/src/test/meson.build b/src/test/meson.build index ef741388d5..88e46cc03e 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -50,6 +50,7 @@ simple_tests += files( 'test-bitmap.c', 'test-blockdev-util.c', 'test-bootspec.c', + 'test-build-path.c', 'test-bus-util.c', 'test-calendarspec.c', 'test-cgroup-setup.c', diff --git a/src/test/test-build-path.c b/src/test/test-build-path.c new file mode 100644 index 0000000000..661b5fc9e6 --- /dev/null +++ b/src/test/test-build-path.c @@ -0,0 +1,20 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "build-path.h" +#include "log.h" +#include "string-util.h" + +int main(int argc, char* argv[]) { + _cleanup_free_ char *p = NULL; + int r; + + r = get_build_exec_dir(&p); + if (r == -ENOEXEC) + log_info("Not run from build dir."); + else if (r < 0) + log_error_errno(r, "Failed to find build dir: %m"); + else + log_info("%s", strna(p)); + + return 0; +} From 950ca1788e5cc024adbaf34fb24a87e20a510e81 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Feb 2024 11:57:39 +0100 Subject: [PATCH 2/8] build-path: allow overriding of all callout binary paths via an env var --- src/basic/build-path.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/basic/build-path.c b/src/basic/build-path.c index d81ed561dc..0c43afaefd 100644 --- a/src/basic/build-path.c +++ b/src/basic/build-path.c @@ -184,6 +184,30 @@ static int find_build_dir_binary(const char *fn, char **ret) { return 0; } +static int find_environment_binary(const char *fn, const char **ret) { + + /* If a path such as /usr/lib/systemd/systemd-foobar is specified, then this will check for an + * environment variable SYSTEMD_FOOBAR_PATH and return it if set. */ + + _cleanup_free_ char *s = strdup(fn); + if (!s) + return -ENOMEM; + + ascii_strupper(s); + string_replace_char(s, '-', '_'); + + if (!strextend(&s, "_PATH")) + return -ENOMEM; + + const char *e; + e = secure_getenv(s); + if (!e) + return -ENXIO; + + *ret = e; + return 0; +} + int invoke_callout_binary(const char *path, char *const argv[]) { int r; @@ -198,6 +222,13 @@ int invoke_callout_binary(const char *path, char *const argv[]) { if (r == O_DIRECTORY) /* Uh? */ return -EISDIR; + const char *e; + if (find_environment_binary(fn, &e) >= 0) { + /* If there's an explicit environment variable set for this binary, prefer it */ + execv(e, argv); + return -errno; /* The environment variable counts, let's fail otherwise */ + } + _cleanup_free_ char *np = NULL; if (find_build_dir_binary(fn, &np) >= 0) execv(np, argv); From d39a161a46d61cb926328eb274336eff962de297 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Feb 2024 12:46:41 +0100 Subject: [PATCH 3/8] homed: port to use new invoke_callout_binary() API --- src/home/homed-home.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/home/homed-home.c b/src/home/homed-home.c index a487eb1fd1..f3c72926d7 100644 --- a/src/home/homed-home.c +++ b/src/home/homed-home.c @@ -10,6 +10,7 @@ #include "blockdev-util.h" #include "btrfs-util.h" +#include "build-path.h" #include "bus-common-errors.h" #include "bus-locator.h" #include "data-fd-util.h" @@ -1276,7 +1277,7 @@ static int home_start_work( return r; if (r == 0) { _cleanup_free_ char *joined = NULL; - const char *homework, *suffix, *unix_path; + const char *suffix, *unix_path; /* Child */ @@ -1320,12 +1321,8 @@ static int home_start_work( if (r < 0) log_warning_errno(r, "Failed to update $SYSTEMD_LOG_LEVEL, ignoring: %m"); - /* Allow overriding the homework path via an environment variable, to make debugging - * easier. */ - homework = getenv("SYSTEMD_HOMEWORK_PATH") ?: SYSTEMD_HOMEWORK_PATH; - - execl(homework, homework, verb, NULL); - log_error_errno(errno, "Failed to invoke %s: %m", homework); + r = invoke_callout_binary(SYSTEMD_HOMEWORK_PATH, STRV_MAKE(SYSTEMD_HOMEWORK_PATH, verb)); + log_error_errno(r, "Failed to invoke %s: %m", SYSTEMD_HOMEWORK_PATH); _exit(EXIT_FAILURE); } From a0afd4e7337d7463e1a9d91d47a5d9b5067d7a6b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Feb 2024 12:47:31 +0100 Subject: [PATCH 4/8] importd: port importd over to new invoke_callout_binary() API --- src/import/importd.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/import/importd.c b/src/import/importd.c index 3321155e84..d25c23c6dc 100644 --- a/src/import/importd.c +++ b/src/import/importd.c @@ -6,6 +6,7 @@ #include "sd-bus.h" #include "alloc-util.h" +#include "build-path.h" #include "bus-common-errors.h" #include "bus-get-properties.h" #include "bus-log-control-api.h" @@ -475,8 +476,10 @@ static int transfer_start(Transfer *t) { cmd[k++] = t->local; cmd[k] = NULL; - execv(cmd[0], (char * const *) cmd); - log_error_errno(errno, "Failed to execute %s tool: %m", cmd[0]); + assert(k < ELEMENTSOF(cmd)); + + r = invoke_callout_binary(cmd[0], (char * const *) cmd); + log_error_errno(r, "Failed to execute %s tool: %m", cmd[0]); _exit(EXIT_FAILURE); } From c17c467144724e86a80d8aa4c8aca4e8e5eb8d53 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Feb 2024 12:47:44 +0100 Subject: [PATCH 5/8] userdbd: port userdbd over to invoke_callout_binary() --- src/userdb/userdbd-manager.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/userdb/userdbd-manager.c b/src/userdb/userdbd-manager.c index 73b6d72026..8720721f43 100644 --- a/src/userdb/userdbd-manager.c +++ b/src/userdb/userdbd-manager.c @@ -4,6 +4,7 @@ #include "sd-daemon.h" +#include "build-path.h" #include "common-signal.h" #include "env-util.h" #include "fd-util.h" @@ -191,11 +192,8 @@ static int start_one_worker(Manager *m) { _exit(EXIT_FAILURE); } - /* execl("/home/lennart/projects/systemd/build/systemd-userwork", "systemd-userwork", "xxxxxxxxxxxxxxxx", NULL); /\* With some extra space rename_process() can make use of *\/ */ - /* execl("/usr/bin/valgrind", "valgrind", "/home/lennart/projects/systemd/build/systemd-userwork", "systemd-userwork", "xxxxxxxxxxxxxxxx", NULL); /\* With some extra space rename_process() can make use of *\/ */ - - execl(SYSTEMD_USERWORK_PATH, "systemd-userwork", "xxxxxxxxxxxxxxxx", NULL); /* With some extra space rename_process() can make use of */ - log_error_errno(errno, "Failed start worker process: %m"); + r = invoke_callout_binary(SYSTEMD_USERWORK_PATH, STRV_MAKE(SYSTEMD_USERWORK_PATH, "xxxxxxxxxxxxxxxx")); /* With some extra space rename_process() can make use of */ + log_error_errno(r, "Failed start worker process: %m"); _exit(EXIT_FAILURE); } From 84ab5d1f9564982a5e6029d5e558912948ee5535 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Feb 2024 13:06:17 +0100 Subject: [PATCH 6/8] sysupdate: port over to new invoke_callout_binary() call --- src/sysupdate/sysupdate-resource.c | 7 ++- src/sysupdate/sysupdate-transfer.c | 90 +++++++++++------------------- src/sysupdate/sysupdate.h | 12 ---- 3 files changed, 38 insertions(+), 71 deletions(-) diff --git a/src/sysupdate/sysupdate-resource.c b/src/sysupdate/sysupdate-resource.c index 96422626f4..5b7aee2b54 100644 --- a/src/sysupdate/sysupdate-resource.c +++ b/src/sysupdate/sysupdate-resource.c @@ -6,6 +6,7 @@ #include "alloc-util.h" #include "blockdev-util.h" +#include "build-path.h" #include "chase.h" #include "device-util.h" #include "devnum-util.h" @@ -300,7 +301,7 @@ static int download_manifest( /* Child */ const char *cmdline[] = { - "systemd-pull", + SYSTEMD_PULL_PATH, "raw", "--direct", /* just download the specified URL, don't download anything else */ "--verify", verify_signature ? "signature" : "no", /* verify the manifest file */ @@ -309,8 +310,8 @@ static int download_manifest( NULL }; - execv(pull_binary_path(), (char *const*) cmdline); - log_error_errno(errno, "Failed to execute %s tool: %m", pull_binary_path()); + r = invoke_callout_binary(SYSTEMD_PULL_PATH, (char *const*) cmdline); + log_error_errno(r, "Failed to execute %s tool: %m", SYSTEMD_PULL_PATH); _exit(EXIT_FAILURE); }; diff --git a/src/sysupdate/sysupdate-transfer.c b/src/sysupdate/sysupdate-transfer.c index f8f4a154f9..b350f83065 100644 --- a/src/sysupdate/sysupdate-transfer.c +++ b/src/sysupdate/sysupdate-transfer.c @@ -4,6 +4,7 @@ #include "alloc-util.h" #include "blockdev-util.h" +#include "build-path.h" #include "chase.h" #include "conf-parser.h" #include "dirent-util.h" @@ -782,25 +783,23 @@ static void compile_pattern_fields( memcpy(ret->sha256sum, i->metadata.sha256sum, sizeof(ret->sha256sum)); } -static int run_helper( +static int run_callout( const char *name, - const char *path, - const char * const cmdline[]) { + char *cmdline[]) { int r; assert(name); - assert(path); assert(cmdline); + assert(cmdline[0]); r = safe_fork(name, FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGTERM|FORK_LOG|FORK_WAIT, NULL); if (r < 0) return r; if (r == 0) { /* Child */ - - execv(path, (char *const*) cmdline); - log_error_errno(errno, "Failed to execute %s tool: %m", path); + r = invoke_callout_binary(cmdline[0], (char *const*) cmdline); + log_error_errno(r, "Failed to execute %s tool: %m", cmdline[0]); _exit(EXIT_FAILURE); } @@ -907,36 +906,30 @@ int transfer_acquire_instance(Transfer *t, Instance *i) { * importer has some tricks up its sleeve, such as sparse file generation, which we * want to take benefit of, too.) */ - r = run_helper("(sd-import-raw)", - import_binary_path(), - (const char* const[]) { - "systemd-import", + r = run_callout("(sd-import-raw)", + STRV_MAKE( + SYSTEMD_IMPORT_PATH, "raw", "--direct", /* just copy/unpack the specified file, don't do anything else */ arg_sync ? "--sync=yes" : "--sync=no", i->path, - t->temporary_path, - NULL - }); + t->temporary_path)); break; case RESOURCE_PARTITION: /* regular file → partition */ - r = run_helper("(sd-import-raw)", - import_binary_path(), - (const char* const[]) { - "systemd-import", + r = run_callout("(sd-import-raw)", + STRV_MAKE( + SYSTEMD_IMPORT_PATH, "raw", "--direct", /* just copy/unpack the specified file, don't do anything else */ "--offset", offset, "--size-max", max_size, arg_sync ? "--sync=yes" : "--sync=no", i->path, - t->target.path, - NULL - }); + t->target.path)); break; default: @@ -951,18 +944,15 @@ int transfer_acquire_instance(Transfer *t, Instance *i) { /* directory/subvolume → directory/subvolume */ - r = run_helper("(sd-import-fs)", - import_fs_binary_path(), - (const char* const[]) { - "systemd-import-fs", + r = run_callout("(sd-import-fs)", + STRV_MAKE( + SYSTEMD_IMPORT_FS_PATH, "run", "--direct", /* just untar the specified file, don't do anything else */ arg_sync ? "--sync=yes" : "--sync=no", t->target.type == RESOURCE_SUBVOLUME ? "--btrfs-subvol=yes" : "--btrfs-subvol=no", i->path, - t->temporary_path, - NULL - }); + t->temporary_path)); break; case RESOURCE_TAR: @@ -970,18 +960,15 @@ int transfer_acquire_instance(Transfer *t, Instance *i) { /* tar → directory/subvolume */ - r = run_helper("(sd-import-tar)", - import_binary_path(), - (const char* const[]) { - "systemd-import", + r = run_callout("(sd-import-tar)", + STRV_MAKE( + SYSTEMD_IMPORT_PATH, "tar", "--direct", /* just untar the specified file, don't do anything else */ arg_sync ? "--sync=yes" : "--sync=no", t->target.type == RESOURCE_SUBVOLUME ? "--btrfs-subvol=yes" : "--btrfs-subvol=no", i->path, - t->temporary_path, - NULL - }); + t->temporary_path)); break; case RESOURCE_URL_FILE: @@ -992,28 +979,24 @@ int transfer_acquire_instance(Transfer *t, Instance *i) { /* url file → regular file */ - r = run_helper("(sd-pull-raw)", - pull_binary_path(), - (const char* const[]) { - "systemd-pull", + r = run_callout("(sd-pull-raw)", + STRV_MAKE( + SYSTEMD_PULL_PATH, "raw", "--direct", /* just download the specified URL, don't download anything else */ "--verify", digest, /* validate by explicit SHA256 sum */ arg_sync ? "--sync=yes" : "--sync=no", i->path, - t->temporary_path, - NULL - }); + t->temporary_path)); break; case RESOURCE_PARTITION: /* url file → partition */ - r = run_helper("(sd-pull-raw)", - pull_binary_path(), - (const char* const[]) { - "systemd-pull", + r = run_callout("(sd-pull-raw)", + STRV_MAKE( + SYSTEMD_PULL_PATH, "raw", "--direct", /* just download the specified URL, don't download anything else */ "--verify", digest, /* validate by explicit SHA256 sum */ @@ -1021,9 +1004,7 @@ int transfer_acquire_instance(Transfer *t, Instance *i) { "--size-max", max_size, arg_sync ? "--sync=yes" : "--sync=no", i->path, - t->target.path, - NULL - }); + t->target.path)); break; default: @@ -1035,19 +1016,16 @@ int transfer_acquire_instance(Transfer *t, Instance *i) { case RESOURCE_URL_TAR: assert(IN_SET(t->target.type, RESOURCE_DIRECTORY, RESOURCE_SUBVOLUME)); - r = run_helper("(sd-pull-tar)", - pull_binary_path(), - (const char*const[]) { - "systemd-pull", + r = run_callout("(sd-pull-tar)", + STRV_MAKE( + SYSTEMD_PULL_PATH, "tar", "--direct", /* just download the specified URL, don't download anything else */ "--verify", digest, /* validate by explicit SHA256 sum */ t->target.type == RESOURCE_SUBVOLUME ? "--btrfs-subvol=yes" : "--btrfs-subvol=no", arg_sync ? "--sync=yes" : "--sync=no", i->path, - t->temporary_path, - NULL - }); + t->temporary_path)); break; default: diff --git a/src/sysupdate/sysupdate.h b/src/sysupdate/sysupdate.h index 6d387b7a5d..cba9bf489f 100644 --- a/src/sysupdate/sysupdate.h +++ b/src/sysupdate/sysupdate.h @@ -7,15 +7,3 @@ extern bool arg_sync; extern uint64_t arg_instances_max; extern char *arg_root; - -static inline const char* import_binary_path(void) { - return secure_getenv("SYSTEMD_IMPORT_PATH") ?: SYSTEMD_IMPORT_PATH; -} - -static inline const char* import_fs_binary_path(void) { - return secure_getenv("SYSTEMD_IMPORT_FS_PATH") ?: SYSTEMD_IMPORT_FS_PATH; -} - -static inline const char *pull_binary_path(void) { - return secure_getenv("SYSTEMD_PULL_PATH") ?: SYSTEMD_PULL_PATH; -} From 2287565d4697053311fde60bcd8ae74c964a3b31 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Feb 2024 12:47:10 +0100 Subject: [PATCH 7/8] pid1: port executor binary pinning to new build path logic --- src/basic/build-path.c | 29 +++++++++++++++++++++++++++++ src/basic/build-path.h | 2 ++ src/core/manager.c | 36 +++++++----------------------------- 3 files changed, 38 insertions(+), 29 deletions(-) diff --git a/src/basic/build-path.c b/src/basic/build-path.c index 0c43afaefd..8ddef7b2dd 100644 --- a/src/basic/build-path.c +++ b/src/basic/build-path.c @@ -6,6 +6,7 @@ #include "build-path.h" #include "errno-list.h" +#include "errno-util.h" #include "macro.h" #include "path-util.h" #include "process-util.h" @@ -236,3 +237,31 @@ int invoke_callout_binary(const char *path, char *const argv[]) { execv(path, argv); return -errno; } + +int pin_callout_binary(const char *path) { + int r; + + assert(path); + + /* Similar to invoke_callout_binary(), but pins (i.e. O_PATH opens) the binary instead of executing it. */ + + _cleanup_free_ char *fn = NULL; + r = path_extract_filename(path, &fn); + if (r < 0) + return r; + if (r == O_DIRECTORY) /* Uh? */ + return -EISDIR; + + const char *e; + if (find_environment_binary(fn, &e) >= 0) + return RET_NERRNO(open(e, O_CLOEXEC|O_PATH)); + + _cleanup_free_ char *np = NULL; + if (find_build_dir_binary(fn, &np) >= 0) { + r = RET_NERRNO(open(np, O_CLOEXEC|O_PATH)); + if (r >= 0) + return r; + } + + return RET_NERRNO(open(path, O_CLOEXEC|O_PATH)); +} diff --git a/src/basic/build-path.h b/src/basic/build-path.h index bf42783f1b..6c38a4a3bb 100644 --- a/src/basic/build-path.h +++ b/src/basic/build-path.h @@ -4,3 +4,5 @@ int get_build_exec_dir(char **ret); int invoke_callout_binary(const char *path, char *const argv[]); + +int pin_callout_binary(const char *path); diff --git a/src/core/manager.c b/src/core/manager.c index e8c747d96d..7a7669f315 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -25,6 +25,7 @@ #include "alloc-util.h" #include "audit-fd.h" #include "boot-timestamps.h" +#include "build-path.h" #include "bus-common-errors.h" #include "bus-error.h" #include "bus-kernel.h" @@ -1024,42 +1025,19 @@ int manager_new(RuntimeScope runtime_scope, ManagerTestRunFlags test_run_flags, if (r < 0 && r != -EEXIST) return r; + } - m->executor_fd = open(SYSTEMD_EXECUTOR_BINARY_PATH, O_CLOEXEC|O_PATH); + if (!FLAGS_SET(test_run_flags, MANAGER_TEST_DONT_OPEN_EXECUTOR)) { + m->executor_fd = pin_callout_binary(SYSTEMD_EXECUTOR_BINARY_PATH); if (m->executor_fd < 0) - return log_emergency_errno(errno, - "Failed to open executor binary '%s': %m", - SYSTEMD_EXECUTOR_BINARY_PATH); - } else if (!FLAGS_SET(test_run_flags, MANAGER_TEST_DONT_OPEN_EXECUTOR)) { - _cleanup_free_ char *self_exe = NULL, *executor_path = NULL; - _cleanup_close_ int self_dir_fd = -EBADF; - int level = LOG_DEBUG; - - /* Prefer sd-executor from the same directory as the test, e.g.: when running unit tests from the - * build directory. Fallback to working directory and then the installation path. */ - r = readlink_and_make_absolute("/proc/self/exe", &self_exe); - if (r < 0) - return r; - - self_dir_fd = open_parent(self_exe, O_CLOEXEC|O_PATH|O_DIRECTORY, 0); - if (self_dir_fd < 0) - return self_dir_fd; - - m->executor_fd = RET_NERRNO(openat(self_dir_fd, "systemd-executor", O_CLOEXEC|O_PATH)); - if (m->executor_fd == -ENOENT) - m->executor_fd = RET_NERRNO(openat(AT_FDCWD, "systemd-executor", O_CLOEXEC|O_PATH)); - if (m->executor_fd == -ENOENT) { - m->executor_fd = RET_NERRNO(open(SYSTEMD_EXECUTOR_BINARY_PATH, O_CLOEXEC|O_PATH)); - level = LOG_WARNING; /* Tests should normally use local builds */ - } - if (m->executor_fd < 0) - return m->executor_fd; + return log_debug_errno(m->executor_fd, "Failed to pin executor binary: %m"); + _cleanup_free_ char *executor_path = NULL; r = fd_get_path(m->executor_fd, &executor_path); if (r < 0) return r; - log_full(level, "Using systemd-executor binary from '%s'.", executor_path); + log_debug("Using systemd-executor binary from '%s'.", executor_path); } /* Note that we do not set up the notify fd here. We do that after deserialization, From 234bdd9c99fe853cf15ce9ad493426713133bc2e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Feb 2024 14:39:38 +0100 Subject: [PATCH 8/8] process-util: use proc_mounted() check at one more place --- src/basic/process-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/basic/process-util.c b/src/basic/process-util.c index fb8f4ef06e..69635e65f8 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -510,7 +510,7 @@ static int get_process_link_contents(pid_t pid, const char *proc_file, char **re p = procfs_file_alloca(pid, proc_file); r = readlink_malloc(p, ret); - return r == -ENOENT ? -ESRCH : r; + return (r == -ENOENT && proc_mounted() > 0) ? -ESRCH : r; } int get_process_exe(pid_t pid, char **ret) {