Skip to content

Commit cfe8030

Browse files
author
Christian Brauner
committed
open: don't silently ignore unknown O-flags in openat2()
The new openat2() syscall verifies that no unknown O-flag values are set and returns an error to userspace if they are while the older open syscalls like open() and openat() simply ignore unknown flag values: #define O_FLAG_CURRENTLY_INVALID (1 << 31) struct open_how how = { .flags = O_RDONLY | O_FLAG_CURRENTLY_INVALID, .resolve = 0, }; /* fails */ fd = openat2(-EBADF, "/dev/null", &how, sizeof(how)); /* succeeds */ fd = openat(-EBADF, "/dev/null", O_RDONLY | O_FLAG_CURRENTLY_INVALID); However, openat2() silently truncates the upper 32 bits meaning: #define O_FLAG_CURRENTLY_INVALID_LOWER32 (1 << 31) #define O_FLAG_CURRENTLY_INVALID_UPPER32 (1 << 40) struct open_how how_lowe32 = { .flags = O_RDONLY | O_FLAG_CURRENTLY_INVALID_LOWER32, }; struct open_how how_upper32 = { .flags = O_RDONLY | O_FLAG_CURRENTLY_INVALID_UPPER32, }; /* fails */ fd = openat2(-EBADF, "/dev/null", &how_lower32, sizeof(how_lower32)); /* succeeds */ fd = openat2(-EBADF, "/dev/null", &how_upper32, sizeof(how_upper32)); Fix this by preventing the immediate truncation in build_open_flags(). There's a snafu here though stripping FMODE_* directly from flags would cause the upper 32 bits to be truncated as well due to integer promotion rules since FMODE_* is unsigned int, O_* are signed ints (yuck). In addition, struct open_flags currently defines flags to be 32 bit which is reasonable. If we simply were to bump it to 64 bit we would need to change a lot of code preemptively which doesn't seem worth it. So simply add a compile-time check verifying that all currently known O_* flags are within the 32 bit range and fail to build if they aren't anymore. This change shouldn't regress old open syscalls since they silently truncate any unknown values anyway. It is a tiny semantic change for openat2() but it is very unlikely people pass ing > 32 bit unknown flags and the syscall is relatively new too. Link: https://lore.kernel.org/r/[email protected] Cc: Christoph Hellwig <[email protected]> Cc: Aleksa Sarai <[email protected]> Cc: Al Viro <[email protected]> Cc: [email protected] Reported-by: Richard Guy Briggs <[email protected]> Reviewed-by: Christoph Hellwig <[email protected]> Reviewed-by: Aleksa Sarai <[email protected]> Reviewed-by: Richard Guy Briggs <[email protected]> Signed-off-by: Christian Brauner <[email protected]>
1 parent 5c350aa commit cfe8030

File tree

1 file changed

+11
-3
lines changed

1 file changed

+11
-3
lines changed

fs/open.c

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1002,12 +1002,20 @@ inline struct open_how build_open_how(int flags, umode_t mode)
10021002

10031003
inline int build_open_flags(const struct open_how *how, struct open_flags *op)
10041004
{
1005-
int flags = how->flags;
1005+
u64 flags = how->flags;
1006+
u64 strip = FMODE_NONOTIFY | O_CLOEXEC;
10061007
int lookup_flags = 0;
10071008
int acc_mode = ACC_MODE(flags);
10081009

1009-
/* Must never be set by userspace */
1010-
flags &= ~(FMODE_NONOTIFY | O_CLOEXEC);
1010+
BUILD_BUG_ON_MSG(upper_32_bits(VALID_OPEN_FLAGS),
1011+
"struct open_flags doesn't yet handle flags > 32 bits");
1012+
1013+
/*
1014+
* Strip flags that either shouldn't be set by userspace like
1015+
* FMODE_NONOTIFY or that aren't relevant in determining struct
1016+
* open_flags like O_CLOEXEC.
1017+
*/
1018+
flags &= ~strip;
10111019

10121020
/*
10131021
* Older syscalls implicitly clear all of the invalid flags or argument

0 commit comments

Comments
 (0)