From 05314c9c87cf13fc71976d6770a7bf00f75503f1 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Thu, 9 Nov 2023 01:10:06 +0800 Subject: [PATCH 1/2] fd-util: refuse O_CREAT in fd_reopen O_CREAT doesn't make sense for fd_reopen, since we're working on an already opened fd. Also, in fd_reopen we don't handle the mode parameter of open(2), which means we may get runtime error like #29938. --- src/basic/fd-util.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index 32289cfd09..0690bcd830 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -761,9 +761,10 @@ finish: } int fd_reopen(int fd, int flags) { - int new_fd, r; + int r; assert(fd >= 0 || fd == AT_FDCWD); + assert(!FLAGS_SET(flags, O_CREAT)); /* Reopens the specified fd with new flags. This is useful for convert an O_PATH fd into a regular one, or to * turn O_RDWR fds into O_RDONLY fds. @@ -787,19 +788,12 @@ int fd_reopen(int fd, int flags) { * the same way as the non-O_DIRECTORY case. */ return -ELOOP; - if (FLAGS_SET(flags, O_DIRECTORY) || fd == AT_FDCWD) { + if (FLAGS_SET(flags, O_DIRECTORY) || fd == AT_FDCWD) /* If we shall reopen the fd as directory we can just go via "." and thus bypass the whole * magic /proc/ directory, and make ourselves independent of that being mounted. */ - new_fd = openat(fd, ".", flags | O_DIRECTORY); - if (new_fd < 0) - return -errno; + return RET_NERRNO(openat(fd, ".", flags | O_DIRECTORY)); - return new_fd; - } - - assert(fd >= 0); - - new_fd = open(FORMAT_PROC_FD_PATH(fd), flags); + int new_fd = open(FORMAT_PROC_FD_PATH(fd), flags); if (new_fd < 0) { if (errno != ENOENT) return -errno; @@ -825,6 +819,7 @@ int fd_reopen_condition( int r, new_fd; assert(fd >= 0); + assert(!FLAGS_SET(flags, O_CREAT)); /* Invokes fd_reopen(fd, flags), but only if the existing F_GETFL flags don't match the specified * flags (masked by the specified mask). This is useful for converting O_PATH fds into real fds if From 6f9677bc7f6139c7ea805ae7c7064a7f67568bee Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Thu, 9 Nov 2023 01:43:08 +0800 Subject: [PATCH 2/2] basic/fileio: drop O_CREAT before passing flags to fd_reopen Follow-up for 78c21009bfcf0758f9c85dc70ac896c8aab6b535 Fixes #29938 --- src/basic/fileio.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/basic/fileio.c b/src/basic/fileio.c index 6129461e66..a050b61db4 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -1064,7 +1064,9 @@ int fdopen_independent(int fd, const char *mode, FILE **ret) { if (mode_flags < 0) return mode_flags; - copy_fd = fd_reopen(fd, mode_flags); + /* Flags returned by fopen_mode_to_flags might contain O_CREAT, but it doesn't make sense for fd_reopen + * since we're working on an existing fd anyway. Let's drop it here to avoid triggering assertion. */ + copy_fd = fd_reopen(fd, mode_flags & ~O_CREAT); if (copy_fd < 0) return copy_fd;