SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

basic/fs-util: change CHASE_OPEN flag into a separate output parameter

Browse Source 
chase_symlinks() would return negative on error, and either a non-negative status
or a non-negative fd when CHASE_OPEN was given. This made the interface quite
complicated, because dependning on the flags used, we would get two different
"types" of return object. Coverity was always confused by this, and flagged
every use of chase_symlinks() without CHASE_OPEN as a resource leak (because it
would this that an fd is returned). This patch uses a saparate output parameter,
so there is no confusion.

(I think it is OK to have functions which return either an error or an fd. It's
only returning *either* an fd or a non-fd that is confusing.)
This commit is contained in:
Zbigniew Jędrzejewski-Szmek
2019-10-24 22:44:24 +09:00
committed by Yu Watanabe
parent 58ce85f6a1
commit a5648b8094
33 changed files with 197 additions and 180 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/basic/fs-util.c
+40 -37
View File
@@ -713,7 +713,7 @@ static int log_autofs_mount_point(int fd, const char *path, unsigned flags) {
n1, path);
}
int chase_symlinks(const char *path, const char *original_root, unsigned flags, char **ret) {
int chase_symlinks(const char *path, const char *original_root, unsigned 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 */
@@ -725,10 +725,10 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
assert(path);
/* Either the file may be missing, or we return an fd to the final object, but both make no sense */
if (FLAGS_SET(flags, CHASE_NONEXISTENT | CHASE_OPEN))
if ((flags & CHASE_NONEXISTENT) && ret_fd)
return -EINVAL;
if (FLAGS_SET(flags, CHASE_STEP | CHASE_OPEN))
if ((flags & CHASE_STEP) && ret_fd)
return -EINVAL;
if (isempty(path))
@@ -754,17 +754,17 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
* function what to do when encountering a symlink with an absolute path as directory: prefix it by the
* specified path.
*
* There are three ways to invoke this function:
* There are five ways to invoke this function:
*
* 1. Without CHASE_STEP or CHASE_OPEN: in this case the path is resolved and the normalized path is returned
* in `ret`. The return value is < 0 on error. If CHASE_NONEXISTENT is also set, 0 is returned if the file
* doesn't exist, > 0 otherwise. If CHASE_NONEXISTENT is not set, >= 0 is returned if the destination was
* found, -ENOENT if it wasn't.
* 1. Without CHASE_STEP or ret_fd: in this case the path is resolved and the normalized path is
* returned in `ret_path`. The return value is < 0 on error. If CHASE_NONEXISTENT is also set, 0
* is returned if the file doesn't exist, > 0 otherwise. If CHASE_NONEXISTENT is not set, >= 0 is
* returned if the destination was found, -ENOENT if it wasn't.
*
* 2. With CHASE_OPEN: in this case the destination is opened after chasing it as O_PATH and this file
* 2. With ret_fd: in this case the destination is opened after chasing it as O_PATH and this file
* descriptor is returned as return value. This is useful to open files relative to some root
* directory. Note that the returned O_PATH file descriptors must be converted into a regular one (using
* fd_reopen() or such) before it can be used for reading/writing. CHASE_OPEN may not be combined with
* fd_reopen() or such) before it can be used for reading/writing. ret_fd may not be combined with
* CHASE_NONEXISTENT.
*
* 3. With CHASE_STEP: in this case only a single step of the normalization is executed, i.e. only the first
@@ -780,21 +780,21 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
* 5. With CHASE_NO_AUTOFS: in this case if an autofs mount point is encountered, path normalization
* is aborted and -EREMOTE is returned. If CHASE_WARN is also set, a warning showing the path of
* the mount point is emitted.
*
*/
/* A root directory of "/" or "" is identical to none */
if (empty_or_root(original_root))
original_root = NULL;
if (!original_root && !ret && (flags & (CHASE_NONEXISTENT|CHASE_NO_AUTOFS|CHASE_SAFE|CHASE_OPEN|CHASE_STEP)) == CHASE_OPEN) {
/* Shortcut the CHASE_OPEN case if the caller isn't interested in the actual path and has no root set
if (!original_root && !ret_path && !(flags & (CHASE_NONEXISTENT|CHASE_NO_AUTOFS|CHASE_SAFE|CHASE_STEP)) && ret_fd) {
/* 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 = open(path, O_PATH|O_CLOEXEC|((flags & CHASE_NOFOLLOW) ? O_NOFOLLOW : 0));
if (r < 0)
return -errno;
return r;
*ret_fd = r;
return 0;
}
if (original_root) {
@@ -803,7 +803,6 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
return r;
if (flags & CHASE_PREFIX_ROOT) {
/* We don't support relative paths in combination with a root directory */
if (!path_is_absolute(path))
return -EINVAL;
@@ -948,7 +947,6 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
if (S_ISLNK(st.st_mode) && !((flags & CHASE_NOFOLLOW) && isempty(todo))) {
char *joined;
_cleanup_free_ char *destination = NULL;
/* This is a symlink, in this case read the destination. But let's make sure we don't follow
@@ -1034,15 +1032,15 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
return -ENOMEM;
}
if (ret)
*ret = TAKE_PTR(done);
if (ret_path)
*ret_path = TAKE_PTR(done);
if (flags & CHASE_OPEN) {
/* Return the O_PATH fd we currently are looking to the caller. It can translate it to a proper fd by
* opening /proc/self/fd/xyz. */
if (ret_fd) {
/* Return the O_PATH fd we currently are looking to the caller. It can translate it to a
* proper fd by opening /proc/self/fd/xyz. */
assert(fd >= 0);
return TAKE_FD(fd);
*ret_fd = TAKE_FD(fd);
}
if (flags & CHASE_STEP)
@@ -1051,14 +1049,14 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
return exists;
chased_one:
if (ret) {
if (ret_path) {
char *c;
c = strjoin(strempty(done), todo);
if (!c)
return -ENOMEM;
*ret = c;
*ret_path = c;
}
return 0;
@@ -1087,9 +1085,10 @@ int chase_symlinks_and_open(
return r;
}
path_fd = chase_symlinks(path, root, chase_flags|CHASE_OPEN, ret_path ? &p : NULL);
if (path_fd < 0)
return path_fd;
r = chase_symlinks(path, root, chase_flags, ret_path ? &p : NULL, &path_fd);
if (r < 0)
return r;
assert(path_fd >= 0);
r = fd_reopen(path_fd, open_flags);
if (r < 0)
@@ -1112,6 +1111,7 @@ int chase_symlinks_and_opendir(
_cleanup_close_ int path_fd = -1;
_cleanup_free_ char *p = NULL;
DIR *d;
int r;
if (!ret_dir)
return -EINVAL;
@@ -1128,9 +1128,10 @@ int chase_symlinks_and_opendir(
return 0;
}
path_fd = chase_symlinks(path, root, chase_flags|CHASE_OPEN, ret_path ? &p : NULL);
if (path_fd < 0)
return path_fd;
r = chase_symlinks(path, root, chase_flags, ret_path ? &p : NULL, &path_fd);
if (r < 0)
return r;
assert(path_fd >= 0);
xsprintf(procfs_path, "/proc/self/fd/%i", path_fd);
d = opendir(procfs_path);
@@ -1149,10 +1150,12 @@ int chase_symlinks_and_stat(
const char *root,
unsigned chase_flags,
char **ret_path,
struct stat *ret_stat) {
struct stat *ret_stat,
int *ret_fd) {
_cleanup_close_ int path_fd = -1;
_cleanup_free_ char *p = NULL;
int r;
assert(path);
assert(ret_stat);
@@ -1168,18 +1171,18 @@ int chase_symlinks_and_stat(
return 1;
}
path_fd = chase_symlinks(path, root, chase_flags|CHASE_OPEN, ret_path ? &p : NULL);
if (path_fd < 0)
return path_fd;
r = chase_symlinks(path, root, chase_flags, ret_path ? &p : NULL, &path_fd);
if (r < 0)
return r;
assert(path_fd >= 0);
if (fstat(path_fd, ret_stat) < 0)
return -errno;
if (ret_path)
*ret_path = TAKE_PTR(p);
if (chase_flags & CHASE_OPEN)
return TAKE_FD(path_fd);
if (ret_fd)
*ret_fd = TAKE_FD(path_fd);
return 1;
}
src/basic/fs-util.h
+7 -9
View File
@@ -79,23 +79,21 @@ enum {
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 */
CHASE_SAFE = 1 << 3, /* Return EPERM if we ever traverse from unprivileged to privileged files or directories */
CHASE_OPEN = 1 << 4, /* Return an O_PATH object to the final component */
CHASE_TRAIL_SLASH = 1 << 5, /* Any trailing slash will be preserved */
CHASE_STEP = 1 << 6, /* Just execute a single step of the normalization */
CHASE_NOFOLLOW = 1 << 7, /* Do not follow the path's right-most compontent. With CHASE_OPEN, when
* the path's right-most component refers to symlink, return O_PATH fd of
* the symlink. */
CHASE_WARN = 1 << 8, /* Emit an appropriate warning when an error is encountered */
CHASE_TRAIL_SLASH = 1 << 4, /* Any trailing slash will be preserved */
CHASE_STEP = 1 << 5, /* Just execute a single step of the normalization */
CHASE_NOFOLLOW = 1 << 6, /* Do not follow the path's right-most compontent. 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 */
};
/* 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);
int chase_symlinks(const char *path_with_prefix, const char *root, unsigned 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 chase_symlinks_and_stat(const char *path, const char *root, unsigned chase_flags, char **ret_path, struct stat *ret_stat, int *ret_fd);
/* Useful for usage with _cleanup_(), removes a directory and frees the pointer */
static inline void rmdir_and_free(char *p) {
src/basic/mkdir.c
+1 -1
View File
@@ -33,7 +33,7 @@ int mkdir_safe_internal(const char *path, mode_t mode, uid_t uid, gid_t gid, Mkd
if ((flags & MKDIR_FOLLOW_SYMLINK) && S_ISLNK(st.st_mode)) {
_cleanup_free_ char *p = NULL;
r = chase_symlinks(path, NULL, CHASE_NONEXISTENT, &p);
r = chase_symlinks(path, NULL, CHASE_NONEXISTENT, &p, NULL);
if (r < 0)
return r;
if (r == 0)
src/basic/mountpoint-util.c
+1 -1
View File
@@ -264,7 +264,7 @@ int path_is_mount_point(const char *t, const char *root, int flags) {
* /bin -> /usr/bin/ and /usr is a mount point, then the parent that we
* look at needs to be /usr, not /. */
if (flags & AT_SYMLINK_FOLLOW) {
r = chase_symlinks(t, root, CHASE_TRAIL_SLASH, &canonical);
r = chase_symlinks(t, root, CHASE_TRAIL_SLASH, &canonical, NULL);
if (r < 0)
return r;
src/basic/path-util.c
+1 -1
View File
@@ -273,7 +273,7 @@ char **path_strv_resolve(char **l, const char *root) {
} else
t = *s;
r = chase_symlinks(t, root, 0, &u);
r = chase_symlinks(t, root, 0, &u, NULL);
if (r == -ENOENT) {
if (root) {
u = TAKE_PTR(orig);
src/basic/stat-util.c
+1 -1
View File
@@ -335,7 +335,7 @@ int device_path_make_canonical(mode_t mode, dev_t devno, char **ret) {
if (r < 0)
return r;
return chase_symlinks(p, NULL, 0, ret);
return chase_symlinks(p, NULL, 0, ret, NULL);
}
int device_path_parse_major_minor(const char *path, mode_t *ret_mode, dev_t *ret_devno) {