| From 4dc57d26cb547243c8a569faba53428d17b090c4 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Fri, 28 May 2021 11:24:16 +0200 |
| Subject: open: don't silently ignore unknown O-flags in openat2() |
| |
| From: Christian Brauner <christian.brauner@ubuntu.com> |
| |
| [ Upstream commit cfe80306a0dd6d363934913e47c3f30d71b721e5 ] |
| |
| 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/20210528092417.3942079-3-brauner@kernel.org |
| Cc: Christoph Hellwig <hch@lst.de> |
| Cc: Aleksa Sarai <cyphar@cyphar.com> |
| Cc: Al Viro <viro@zeniv.linux.org.uk> |
| Cc: linux-fsdevel@vger.kernel.org |
| Reported-by: Richard Guy Briggs <rgb@redhat.com> |
| Reviewed-by: Christoph Hellwig <hch@lst.de> |
| Reviewed-by: Aleksa Sarai <cyphar@cyphar.com> |
| Reviewed-by: Richard Guy Briggs <rgb@redhat.com> |
| Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| fs/open.c | 14 +++++++++++--- |
| 1 file changed, 11 insertions(+), 3 deletions(-) |
| |
| diff --git a/fs/open.c b/fs/open.c |
| index 4d7537ae59df..3aaaad47d9ca 100644 |
| --- a/fs/open.c |
| +++ b/fs/open.c |
| @@ -993,12 +993,20 @@ inline struct open_how build_open_how(int flags, umode_t mode) |
| |
| inline int build_open_flags(const struct open_how *how, struct open_flags *op) |
| { |
| - int flags = how->flags; |
| + u64 flags = how->flags; |
| + u64 strip = FMODE_NONOTIFY | O_CLOEXEC; |
| int lookup_flags = 0; |
| int acc_mode = ACC_MODE(flags); |
| |
| - /* Must never be set by userspace */ |
| - flags &= ~(FMODE_NONOTIFY | O_CLOEXEC); |
| + BUILD_BUG_ON_MSG(upper_32_bits(VALID_OPEN_FLAGS), |
| + "struct open_flags doesn't yet handle flags > 32 bits"); |
| + |
| + /* |
| + * Strip flags that either shouldn't be set by userspace like |
| + * FMODE_NONOTIFY or that aren't relevant in determining struct |
| + * open_flags like O_CLOEXEC. |
| + */ |
| + flags &= ~strip; |
| |
| /* |
| * Older syscalls implicitly clear all of the invalid flags or argument |
| -- |
| 2.30.2 |
| |