From 326d60a9d0605f02936d0aa8ec5cf78cae72f5dd Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 5 Apr 2025 22:29:33 +0200 Subject: [PATCH 1/3] path-util: generalize open_and_check_executable() --- src/basic/path-util.c | 71 ++++++++++++++++++------------------------- src/basic/path-util.h | 1 + 2 files changed, 31 insertions(+), 41 deletions(-) diff --git a/src/basic/path-util.c b/src/basic/path-util.c index 78ba10ed80..dbde364eff 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -613,37 +613,9 @@ char* path_extend_internal(char **x, ...) { return nx; } -static int check_x_access(const char *path, int *ret_fd) { +int open_and_check_executable(const char *name, const char *root, char **ret_path, int *ret_fd) { _cleanup_close_ int fd = -EBADF; - int r; - - /* We need to use O_PATH because there may be executables for which we have only exec - * permissions, but not read (usually suid executables). */ - fd = open(path, O_PATH|O_CLOEXEC); - if (fd < 0) - return -errno; - - r = fd_verify_regular(fd); - if (r < 0) - return r; - - r = access_fd(fd, X_OK); - if (r == -ENOSYS) { - /* /proc is not mounted. Fallback to access(). */ - if (access(path, X_OK) < 0) - return -errno; - } else if (r < 0) - return r; - - if (ret_fd) - *ret_fd = TAKE_FD(fd); - - return 0; -} - -static int find_executable_impl(const char *name, const char *root, char **ret_filename, int *ret_fd) { - _cleanup_close_ int fd = -EBADF; - _cleanup_free_ char *path_name = NULL; + _cleanup_free_ char *resolved = NULL; int r; assert(name); @@ -654,23 +626,40 @@ static int find_executable_impl(const char *name, const char *root, char **ret_f * needed to avoid unforeseen regression or other complicated changes. */ if (root) { /* prefix root to name in case full paths are not specified */ - r = chase(name, root, CHASE_PREFIX_ROOT, &path_name, /* ret_fd= */ NULL); + r = chase(name, root, CHASE_PREFIX_ROOT, &resolved, &fd); if (r < 0) return r; - name = path_name; + name = resolved; + } else { + /* We need to use O_PATH because there may be executables for which we have only exec permissions, + * but not read (usually suid executables). */ + fd = open(name, O_PATH|O_CLOEXEC); + if (fd < 0) + return -errno; } - r = check_x_access(name, ret_fd ? &fd : NULL); + r = fd_verify_regular(fd); if (r < 0) return r; - if (ret_filename) { - r = path_make_absolute_cwd(name, ret_filename); - if (r < 0) - return r; + r = access_fd(fd, X_OK); + if (r == -ENOSYS) + /* /proc/ is not mounted. Fall back to access(). */ + r = RET_NERRNO(access(name, X_OK)); + if (r < 0) + return r; - path_simplify(*ret_filename); + if (ret_path) { + if (resolved) + *ret_path = TAKE_PTR(resolved); + else { + r = path_make_absolute_cwd(name, ret_path); + if (r < 0) + return r; + + path_simplify(*ret_path); + } } if (ret_fd) @@ -692,7 +681,7 @@ int find_executable_full( assert(name); if (is_path(name)) - return find_executable_impl(name, root, ret_filename, ret_fd); + return open_and_check_executable(name, root, ret_filename, ret_fd); if (exec_search_path) { STRV_FOREACH(element, exec_search_path) { @@ -707,7 +696,7 @@ int find_executable_full( if (!full_path) return -ENOMEM; - r = find_executable_impl(full_path, root, ret_filename, ret_fd); + r = open_and_check_executable(full_path, root, ret_filename, ret_fd); if (r >= 0) return 0; if (r != -EACCES) @@ -743,7 +732,7 @@ int find_executable_full( if (!path_extend(&element, name)) return -ENOMEM; - r = find_executable_impl(element, root, ret_filename, ret_fd); + r = open_and_check_executable(element, root, ret_filename, ret_fd); if (r >= 0) /* Found it! */ return 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 dff5a3a549..436bd5d892 100644 --- a/src/basic/path-util.h +++ b/src/basic/path-util.h @@ -115,6 +115,7 @@ 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 open_and_check_executable(const char *name, const char *root, char **ret_path, int *ret_fd); int find_executable_full( const char *name, const char *root, From 28b6bfd3704916479fc59d0a150c0c22d2297fec Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 5 Apr 2025 22:35:01 +0200 Subject: [PATCH 2/3] build-path: port to open_and_check_executable(), do not return resolved path Follow-up for b58c240312a5cc0f9f9eab3018d6459e44d085e0 We need to be extremely careful with using the path associated with fd, since it contains the resolved path if a symlink was opened. In particular, it's really not desirable to return the resolved executable path in pin_callout_binary(), which would end up as argv[0] in udev_event_spawn(), potentially changing the behavior of spawned process. --- src/basic/build-path.c | 48 ++++++++++++++---------------------------- 1 file changed, 16 insertions(+), 32 deletions(-) diff --git a/src/basic/build-path.c b/src/basic/build-path.c index ba49273cd3..95de7d3ca3 100644 --- a/src/basic/build-path.c +++ b/src/basic/build-path.c @@ -246,34 +246,8 @@ int invoke_callout_binary(const char *path, char *const argv[]) { return -errno; } -static int open_executable(const char *path, char **ret_path) { - int r; - - assert(path); - - _cleanup_close_ int fd = RET_NERRNO(open(path, O_CLOEXEC|O_PATH)); - if (fd < 0) - return fd; - - r = fd_verify_regular(fd); - if (r < 0) - return r; - - r = access_fd(fd, X_OK); - if (r < 0) - return r; - - if (ret_path) { - r = fd_get_path(fd, ret_path); - if (r < 0) - return r; - } - - return TAKE_FD(fd); -} - int pin_callout_binary(const char *path, char **ret_path) { - int r; + int r, fd; assert(path); @@ -288,15 +262,25 @@ int pin_callout_binary(const char *path, char **ret_path) { return -EISDIR; const char *e; - if (find_environment_binary(fn, &e) >= 0) - return open_executable(e, ret_path); + if (find_environment_binary(fn, &e) >= 0) { + /* The environment variable counts. We'd fail if the executable is not available/invalid. */ + r = open_and_check_executable(e, /* root = */ NULL, ret_path, &fd); + if (r < 0) + return r; + + return fd; + } _cleanup_free_ char *np = NULL; if (find_build_dir_binary(fn, &np) >= 0) { - r = open_executable(np, ret_path); + r = open_and_check_executable(np, /* root = */ NULL, ret_path, &fd); if (r >= 0) - return r; + return fd; } - return open_executable(path, ret_path); + r = open_and_check_executable(path, /* root = */ NULL, ret_path, &fd); + if (r < 0) + return r; + + return fd; } From c0ebcb818efe8d828c6ba7dd1155ec14ac7161d4 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 5 Apr 2025 22:56:19 +0200 Subject: [PATCH 3/3] build-path: teach pin_callout_binary() to search $PATH too Prompted by 2caa462504af4f29a2df6805639b026d901099cd --- src/basic/build-path.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/basic/build-path.c b/src/basic/build-path.c index 95de7d3ca3..5d93bded4d 100644 --- a/src/basic/build-path.c +++ b/src/basic/build-path.c @@ -278,7 +278,9 @@ int pin_callout_binary(const char *path, char **ret_path) { return fd; } - r = open_and_check_executable(path, /* root = */ NULL, ret_path, &fd); + r = find_executable_full(path, /* root = */ NULL, + /* exec_search_path = */ NULL, /* use_path_envvar = */ true, + ret_path, &fd); if (r < 0) return r;