From 8f47f880a44be97635ac085d67ac33e217c1607d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 12 Apr 2022 15:41:09 +0200 Subject: [PATCH 1/6] chase-symlinks: use proper typedef for flags argument --- src/basic/chase-symlinks.c | 20 +++++++++++++------- src/basic/chase-symlinks.h | 14 +++++++------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/basic/chase-symlinks.c b/src/basic/chase-symlinks.c index 61f8b3351b..8558944a69 100644 --- a/src/basic/chase-symlinks.c +++ b/src/basic/chase-symlinks.c @@ -24,7 +24,7 @@ bool unsafe_transition(const struct stat *a, const struct stat *b) { return a->st_uid != b->st_uid; /* Otherwise we need to stay within the same UID */ } -static int log_unsafe_transition(int a, int b, const char *path, unsigned flags) { +static int log_unsafe_transition(int a, int b, const char *path, ChaseSymlinksFlags flags) { _cleanup_free_ char *n1 = NULL, *n2 = NULL, *user_a = NULL, *user_b = NULL; struct stat st; @@ -44,7 +44,7 @@ static int log_unsafe_transition(int a, int b, const char *path, unsigned flags) strna(n1), strna(user_a), special_glyph(SPECIAL_GLYPH_ARROW_RIGHT), strna(n2), strna(user_b), path); } -static int log_autofs_mount_point(int fd, const char *path, unsigned flags) { +static int log_autofs_mount_point(int fd, const char *path, ChaseSymlinksFlags flags) { _cleanup_free_ char *n1 = NULL; if (!FLAGS_SET(flags, CHASE_WARN)) @@ -57,7 +57,13 @@ static int log_autofs_mount_point(int fd, const char *path, unsigned flags) { strna(n1), path); } -int chase_symlinks(const char *path, const char *original_root, unsigned flags, char **ret_path, int *ret_fd) { +int chase_symlinks( + const char *path, + const char *original_root, + ChaseSymlinksFlags flags, + char **ret_path, + int *ret_fd) { + _cleanup_free_ char *buffer = NULL, *done = NULL, *root = NULL; _cleanup_close_ int fd = -1; unsigned max_follow = CHASE_SYMLINKS_MAX; /* how many symlinks to follow before giving up and returning ELOOP */ @@ -397,7 +403,7 @@ chased_one: int chase_symlinks_and_open( const char *path, const char *root, - unsigned chase_flags, + ChaseSymlinksFlags chase_flags, int open_flags, char **ret_path) { @@ -435,7 +441,7 @@ int chase_symlinks_and_open( int chase_symlinks_and_opendir( const char *path, const char *root, - unsigned chase_flags, + ChaseSymlinksFlags chase_flags, char **ret_path, DIR **ret_dir) { @@ -478,7 +484,7 @@ int chase_symlinks_and_opendir( int chase_symlinks_and_stat( const char *path, const char *root, - unsigned chase_flags, + ChaseSymlinksFlags chase_flags, char **ret_path, struct stat *ret_stat, int *ret_fd) { @@ -520,7 +526,7 @@ int chase_symlinks_and_stat( int chase_symlinks_and_fopen_unlocked( const char *path, const char *root, - unsigned chase_flags, + ChaseSymlinksFlags chase_flags, const char *open_flags, char **ret_path, FILE **ret_file) { diff --git a/src/basic/chase-symlinks.h b/src/basic/chase-symlinks.h index e2b22fe041..491138a698 100644 --- a/src/basic/chase-symlinks.h +++ b/src/basic/chase-symlinks.h @@ -6,7 +6,7 @@ #include "stat-util.h" -enum { +typedef enum ChaseSymlinksFlags { CHASE_PREFIX_ROOT = 1 << 0, /* The specified path will be prefixed by the specified root before beginning the iteration */ CHASE_NONEXISTENT = 1 << 1, /* It's OK if the path doesn't actually exist. */ CHASE_NO_AUTOFS = 1 << 2, /* Return -EREMOTE if autofs mount point found */ @@ -16,17 +16,17 @@ enum { CHASE_NOFOLLOW = 1 << 6, /* Do not follow the path's right-most component. With ret_fd, when the path's * right-most component refers to symlink, return O_PATH fd of the symlink. */ CHASE_WARN = 1 << 7, /* Emit an appropriate warning when an error is encountered */ -}; +} ChaseSymlinksFlags; bool unsafe_transition(const struct stat *a, const struct stat *b); /* How many iterations to execute before returning -ELOOP */ #define CHASE_SYMLINKS_MAX 32 -int chase_symlinks(const char *path_with_prefix, const char *root, unsigned flags, char **ret_path, int *ret_fd); +int chase_symlinks(const char *path_with_prefix, const char *root, ChaseSymlinksFlags chase_flags, char **ret_path, int *ret_fd); -int chase_symlinks_and_open(const char *path, const char *root, unsigned chase_flags, int open_flags, char **ret_path); -int chase_symlinks_and_opendir(const char *path, const char *root, unsigned chase_flags, char **ret_path, DIR **ret_dir); -int chase_symlinks_and_stat(const char *path, const char *root, unsigned chase_flags, char **ret_path, struct stat *ret_stat, int *ret_fd); +int chase_symlinks_and_open(const char *path, const char *root, ChaseSymlinksFlags chase_flags, int open_flags, char **ret_path); +int chase_symlinks_and_opendir(const char *path, const char *root, ChaseSymlinksFlags chase_flags, char **ret_path, DIR **ret_dir); +int chase_symlinks_and_stat(const char *path, const char *root, ChaseSymlinksFlags chase_flags, char **ret_path, struct stat *ret_stat, int *ret_fd); -int chase_symlinks_and_fopen_unlocked(const char *path, const char *root, unsigned chase_flags, const char *open_flags, char **ret_path, FILE **ret_file); +int chase_symlinks_and_fopen_unlocked(const char *path, const char *root, ChaseSymlinksFlags chase_flags, const char *open_flags, char **ret_path, FILE **ret_file); From 7b9be862c708249a534b56dba4703f4c808d82fa Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 12 Apr 2022 15:41:48 +0200 Subject: [PATCH 2/6] chase-symlinks: avoid using stack allocation for potentially huge paths let's better be safe and use heap allocation for paths which might be unbounded. In particular as previously we copied the stack memory to heap anyway, via a noop path_make_absolute_cwd() call. --- src/basic/chase-symlinks.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/basic/chase-symlinks.c b/src/basic/chase-symlinks.c index 8558944a69..3e2f2a6622 100644 --- a/src/basic/chase-symlinks.c +++ b/src/basic/chase-symlinks.c @@ -84,6 +84,10 @@ int chase_symlinks( if (isempty(path)) return -EINVAL; + /* We don't support relative paths in combination with a root directory */ + if (FLAGS_SET(flags, CHASE_PREFIX_ROOT) && !path_is_absolute(path)) + return -EINVAL; + /* This is a lot like canonicalize_file_name(), but takes an additional "root" parameter, that allows following * symlinks relative to a root directory, instead of the root of the host. * @@ -161,17 +165,17 @@ int chase_symlinks( path_simplify(root); if (flags & CHASE_PREFIX_ROOT) { - /* We don't support relative paths in combination with a root directory */ - if (!path_is_absolute(path)) - return -EINVAL; - - path = prefix_roota(root, path); + buffer = path_join(root, path); + if (!buffer) + return -ENOMEM; } } - r = path_make_absolute_cwd(path, &buffer); - if (r < 0) - return r; + if (!buffer) { + r = path_make_absolute_cwd(path, &buffer); + if (r < 0) + return r; + } fd = open(root ?: "/", O_CLOEXEC|O_DIRECTORY|O_PATH); if (fd < 0) From 69cf392f8e53a3e5fb3fb0a18308182ff3323143 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 12 Apr 2022 15:43:13 +0200 Subject: [PATCH 3/6] chase-symlinks: use empty_to_root() where appropriate --- src/basic/chase-symlinks.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/basic/chase-symlinks.c b/src/basic/chase-symlinks.c index 3e2f2a6622..63d3e44913 100644 --- a/src/basic/chase-symlinks.c +++ b/src/basic/chase-symlinks.c @@ -177,7 +177,7 @@ int chase_symlinks( return r; } - fd = open(root ?: "/", O_CLOEXEC|O_DIRECTORY|O_PATH); + fd = open(empty_to_root(root), O_CLOEXEC|O_DIRECTORY|O_PATH); if (fd < 0) return -errno; @@ -321,7 +321,7 @@ int chase_symlinks( * directory as base. */ safe_close(fd); - fd = open(root ?: "/", O_CLOEXEC|O_DIRECTORY|O_PATH); + fd = open(empty_to_root(root), O_CLOEXEC|O_DIRECTORY|O_PATH); if (fd < 0) return -errno; From 0ac6cdd6aeae23fff751e5292e3011be44f64c48 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 12 Apr 2022 15:43:23 +0200 Subject: [PATCH 4/6] chase-symlinks: add missing OOM check --- src/basic/chase-symlinks.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/basic/chase-symlinks.c b/src/basic/chase-symlinks.c index 63d3e44913..eff5785a29 100644 --- a/src/basic/chase-symlinks.c +++ b/src/basic/chase-symlinks.c @@ -203,6 +203,8 @@ int chase_symlinks( todo = buffer; done = strdup("/"); } + if (!done) + return -ENOMEM; for (;;) { _cleanup_free_ char *first = NULL; From 860f4c6aa64020d9705b017a90d75eb4cac5f800 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 12 Apr 2022 15:43:39 +0200 Subject: [PATCH 5/6] chase-symlinks: add comment for opaque argument --- src/basic/chase-symlinks.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/basic/chase-symlinks.c b/src/basic/chase-symlinks.c index eff5785a29..1fca871495 100644 --- a/src/basic/chase-symlinks.c +++ b/src/basic/chase-symlinks.c @@ -212,7 +212,7 @@ int chase_symlinks( struct stat st; const char *e; - r = path_find_first_component(&todo, true, &e); + r = path_find_first_component(&todo, /* accept_dot_dot= */ true, &e); if (r < 0) return r; if (r == 0) { /* We reached the end. */ @@ -387,7 +387,7 @@ chased_one: const char *e; /* todo may contain slashes at the beginning. */ - r = path_find_first_component(&todo, true, &e); + r = path_find_first_component(&todo, /* accept_dot_dot= */ true, &e); if (r < 0) return r; if (r == 0) From 57f9ca3aa02d224243066e35702256fa0f24e873 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 12 Apr 2022 15:44:02 +0200 Subject: [PATCH 6/6] chase-symlinks: prefer path_extract_directory() over dirname() --- src/basic/chase-symlinks.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/basic/chase-symlinks.c b/src/basic/chase-symlinks.c index 1fca871495..e93419d635 100644 --- a/src/basic/chase-symlinks.c +++ b/src/basic/chase-symlinks.c @@ -236,9 +236,9 @@ int chase_symlinks( if (empty_or_root(done)) continue; - parent = dirname_malloc(done); - if (!parent) - return -ENOMEM; + r = path_extract_directory(done, &parent); + if (r < 0) + return r; /* Don't allow this to leave the root dir. */ if (root &&