From 5a2f674a005a8f31648dfed1dde0b34ed02ee7dd Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 14 Apr 2023 16:28:54 +0900 Subject: [PATCH 1/2] chase: use FLAGS_SET() macro --- src/basic/chase.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/basic/chase.c b/src/basic/chase.c index e8d38279fd..f167a439ad 100644 --- a/src/basic/chase.c +++ b/src/basic/chase.c @@ -88,10 +88,10 @@ int chaseat(int dir_fd, const char *path, ChaseFlags flags, char **ret_path, int assert(dir_fd >= 0 || dir_fd == AT_FDCWD); /* Either the file may be missing, or we return an fd to the final object, but both make no sense */ - if ((flags & CHASE_NONEXISTENT)) + if (FLAGS_SET(flags, CHASE_NONEXISTENT)) assert(!ret_fd); - if ((flags & CHASE_STEP)) + if (FLAGS_SET(flags, CHASE_STEP)) assert(!ret_fd); if (isempty(path)) @@ -176,7 +176,7 @@ int chaseat(int dir_fd, const char *path, ChaseFlags flags, char **ret_path, int /* Shortcut the ret_fd case if the caller isn't interested in the actual path and has no root * set and doesn't care about any of the other special features we provide either. */ - r = openat(dir_fd, path, O_PATH|O_CLOEXEC|((flags & CHASE_NOFOLLOW) ? O_NOFOLLOW : 0)); + r = openat(dir_fd, path, O_PATH|O_CLOEXEC|(FLAGS_SET(flags, CHASE_NOFOLLOW) ? O_NOFOLLOW : 0)); if (r < 0) return -errno; @@ -221,7 +221,7 @@ int chaseat(int dir_fd, const char *path, ChaseFlags flags, char **ret_path, int if (fstat(fd, &st) < 0) return -errno; - if (flags & CHASE_TRAIL_SLASH) + if (FLAGS_SET(flags, CHASE_TRAIL_SLASH)) append_trail_slash = ENDSWITH_SET(buffer, "/", "/."); for (todo = buffer;;) { @@ -283,10 +283,10 @@ int chaseat(int dir_fd, const char *path, ChaseFlags flags, char **ret_path, int } else return r; - if (flags & CHASE_STEP) + if (FLAGS_SET(flags, CHASE_STEP)) goto chased_one; - if (flags & CHASE_SAFE && + if (FLAGS_SET(flags, CHASE_SAFE) && unsafe_transition(&st, &st_parent)) return log_unsafe_transition(fd, fd_parent, path, flags); @@ -317,7 +317,7 @@ int chaseat(int dir_fd, const char *path, ChaseFlags flags, char **ret_path, int return -ENOMEM; break; - } else if (flags & CHASE_NONEXISTENT) { + } else if (FLAGS_SET(flags, CHASE_NONEXISTENT)) { if (!path_extend(&done, first, todo)) return -ENOMEM; @@ -330,18 +330,18 @@ int chaseat(int dir_fd, const char *path, ChaseFlags flags, char **ret_path, int if (fstat(child, &st_child) < 0) return -errno; - if ((flags & CHASE_SAFE) && + if (FLAGS_SET(flags, CHASE_SAFE) && unsafe_transition(&st, &st_child)) return log_unsafe_transition(fd, child, path, flags); - if ((flags & CHASE_NO_AUTOFS) && + if (FLAGS_SET(flags, CHASE_NO_AUTOFS) && fd_is_fs_type(child, AUTOFS_SUPER_MAGIC) > 0) return log_autofs_mount_point(child, path, flags); - if (S_ISLNK(st_child.st_mode) && !((flags & CHASE_NOFOLLOW) && isempty(todo))) { + if (S_ISLNK(st_child.st_mode) && !(FLAGS_SET(flags, CHASE_NOFOLLOW) && isempty(todo))) { _cleanup_free_ char *destination = NULL; - if (flags & CHASE_PROHIBIT_SYMLINKS) + if (FLAGS_SET(flags, CHASE_PROHIBIT_SYMLINKS)) return log_prohibited_symlink(child, flags); /* This is a symlink, in this case read the destination. But let's make sure we @@ -368,7 +368,7 @@ int chaseat(int dir_fd, const char *path, ChaseFlags flags, char **ret_path, int if (fstat(fd, &st) < 0) return -errno; - if (flags & CHASE_SAFE && + if (FLAGS_SET(flags, CHASE_SAFE) && unsafe_transition(&st_child, &st)) return log_unsafe_transition(child, fd, path, flags); @@ -385,7 +385,7 @@ int chaseat(int dir_fd, const char *path, ChaseFlags flags, char **ret_path, int free_and_replace(buffer, destination); todo = buffer; - if (flags & CHASE_STEP) + if (FLAGS_SET(flags, CHASE_STEP)) goto chased_one; continue; @@ -403,7 +403,7 @@ int chaseat(int dir_fd, const char *path, ChaseFlags flags, char **ret_path, int close_and_replace(fd, child); } - if (flags & CHASE_PARENT) { + if (FLAGS_SET(flags, CHASE_PARENT)) { r = stat_verify_directory(&st); if (r < 0) return r; @@ -438,7 +438,7 @@ int chaseat(int dir_fd, const char *path, ChaseFlags flags, char **ret_path, int *ret_fd = TAKE_FD(fd); } - if (flags & CHASE_STEP) + if (FLAGS_SET(flags, CHASE_STEP)) return 1; return exists; @@ -499,7 +499,7 @@ int chase(const char *path, const char *original_root, ChaseFlags flags, char ** assert(path_is_absolute(root)); assert(!empty_or_root(root)); - if (flags & CHASE_PREFIX_ROOT) { + if (FLAGS_SET(flags, CHASE_PREFIX_ROOT)) { absolute = path_join(root, path); if (!absolute) return -ENOMEM; @@ -516,7 +516,7 @@ int chase(const char *path, const char *original_root, ChaseFlags flags, char ** path = path_startswith(absolute, empty_to_root(root)); if (!path) - return log_full_errno(flags & CHASE_WARN ? LOG_WARNING : LOG_DEBUG, + return log_full_errno(FLAGS_SET(flags, CHASE_WARN) ? LOG_WARNING : LOG_DEBUG, SYNTHETIC_ERRNO(ECHRNG), "Specified path '%s' is outside of specified root directory '%s', refusing to resolve.", absolute, empty_to_root(root)); From 4ea0bcb9229fe12e0c428659d76934351b821872 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 14 Apr 2023 16:29:08 +0900 Subject: [PATCH 2/2] chase: CHASE_MKDIR_0755 requires CHASE_NONEXISTENT and/or CHASE_PARENT When CHASE_MKDIR_0755 is specified without CHASE_NONEXISTENT and CHASE_PARENT, then chase() succeeds only when the file specified by the path already exists, and in that case, chase() does not create any parent directories, and CHASE_MKDIR_0755 is meaningless. Let's mention that CHASE_MKDIR_0755 needs to be specified with CHASE_NONEXISTENT or CHASE_PARENT, and adds a assertion about that. --- src/basic/chase.c | 1 + src/basic/chase.h | 10 ++++++++-- src/test/test-chase.c | 2 +- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/basic/chase.c b/src/basic/chase.c index f167a439ad..eb4bda07a6 100644 --- a/src/basic/chase.c +++ b/src/basic/chase.c @@ -85,6 +85,7 @@ int chaseat(int dir_fd, const char *path, ChaseFlags flags, char **ret_path, int assert(!FLAGS_SET(flags, CHASE_PREFIX_ROOT)); assert(!FLAGS_SET(flags, CHASE_STEP|CHASE_EXTRACT_FILENAME)); assert(!FLAGS_SET(flags, CHASE_TRAIL_SLASH|CHASE_EXTRACT_FILENAME)); + assert(!FLAGS_SET(flags, CHASE_MKDIR_0755) || (flags & (CHASE_NONEXISTENT | CHASE_PARENT)) != 0); assert(dir_fd >= 0 || dir_fd == AT_FDCWD); /* Either the file may be missing, or we return an fd to the final object, but both make no sense */ diff --git a/src/basic/chase.h b/src/basic/chase.h index 7e9ebe0685..40121f7d70 100644 --- a/src/basic/chase.h +++ b/src/basic/chase.h @@ -24,8 +24,14 @@ typedef enum ChaseFlags { * full path is still stored in ret_path and only the returned * file descriptor will point to the parent directory. Note that * the result path is the root or '.', then the file descriptor - * also points to the result path even if this flag is set. */ - CHASE_MKDIR_0755 = 1 << 11, /* Create any missing parent directories in the given path. */ + * also points to the result path even if this flag is set. + * When this specified, chase() will succeed with 1 even if the + * file points to the last path component does not exist. */ + CHASE_MKDIR_0755 = 1 << 11, /* Create any missing parent directories in the given path. This + * needs to be set with CHASE_NONEXISTENT and/or CHASE_PARENT. + * Note, chase_and_open() or friends always add CHASE_PARENT flag + * when internally call chase(), hence CHASE_MKDIR_0755 can be + * safely set without CHASE_NONEXISTENT and CHASE_PARENT. */ CHASE_EXTRACT_FILENAME = 1 << 12, /* Only return the last component of the resolved path */ } ChaseFlags; diff --git a/src/test/test-chase.c b/src/test/test-chase.c index dee781929d..1e98f5c6ed 100644 --- a/src/test/test-chase.c +++ b/src/test/test-chase.c @@ -509,7 +509,7 @@ TEST(chaseat) { assert_se(streq(result, "q")); result = mfree(result); - assert_se(chaseat(tfd, "i/../p", CHASE_MKDIR_0755, NULL, NULL) == -ENOENT); + assert_se(chaseat(tfd, "i/../p", CHASE_MKDIR_0755|CHASE_NONEXISTENT, NULL, NULL) == -ENOENT); /* Test CHASE_FILENAME */