Commit f7340761 authored by Eric Biggers's avatar Eric Biggers Committed by Linus Torvalds

pipe: read buffer limits atomically

The pipe buffer limits are accessed without any locking, and may be
changed at any time by the sysctl handlers.  In theory this could cause
problems for expressions like the following:

    pipe_user_pages_hard && user_bufs > pipe_user_pages_hard

...  since the assembly code might reference the 'pipe_user_pages_hard'
memory location multiple times, and if the admin removes the limit by
setting it to 0, there is a very brief window where processes could
incorrectly observe the limit to be exceeded.

Fix this by loading the limits with READ_ONCE() prior to use.

Link: http://lkml.kernel.org/r/20180111052902.14409-8-ebiggers3@gmail.comSigned-off-by: default avatarEric Biggers <ebiggers@google.com>
Acked-by: default avatarKees Cook <keescook@chromium.org>
Acked-by: default avatarJoe Lawrence <joe.lawrence@redhat.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Michael Kerrisk <mtk.manpages@gmail.com>
Cc: Willy Tarreau <w@1wt.eu>
Cc: Mikulas Patocka <mpatocka@redhat.com>
Cc: "Luis R . Rodriguez" <mcgrof@kernel.org>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent c4fed5a9
...@@ -605,12 +605,16 @@ static unsigned long account_pipe_buffers(struct user_struct *user, ...@@ -605,12 +605,16 @@ static unsigned long account_pipe_buffers(struct user_struct *user,
static bool too_many_pipe_buffers_soft(unsigned long user_bufs) static bool too_many_pipe_buffers_soft(unsigned long user_bufs)
{ {
return pipe_user_pages_soft && user_bufs > pipe_user_pages_soft; unsigned long soft_limit = READ_ONCE(pipe_user_pages_soft);
return soft_limit && user_bufs > soft_limit;
} }
static bool too_many_pipe_buffers_hard(unsigned long user_bufs) static bool too_many_pipe_buffers_hard(unsigned long user_bufs)
{ {
return pipe_user_pages_hard && user_bufs > pipe_user_pages_hard; unsigned long hard_limit = READ_ONCE(pipe_user_pages_hard);
return hard_limit && user_bufs > hard_limit;
} }
static bool is_unprivileged_user(void) static bool is_unprivileged_user(void)
...@@ -624,13 +628,14 @@ struct pipe_inode_info *alloc_pipe_info(void) ...@@ -624,13 +628,14 @@ struct pipe_inode_info *alloc_pipe_info(void)
unsigned long pipe_bufs = PIPE_DEF_BUFFERS; unsigned long pipe_bufs = PIPE_DEF_BUFFERS;
struct user_struct *user = get_current_user(); struct user_struct *user = get_current_user();
unsigned long user_bufs; unsigned long user_bufs;
unsigned int max_size = READ_ONCE(pipe_max_size);
pipe = kzalloc(sizeof(struct pipe_inode_info), GFP_KERNEL_ACCOUNT); pipe = kzalloc(sizeof(struct pipe_inode_info), GFP_KERNEL_ACCOUNT);
if (pipe == NULL) if (pipe == NULL)
goto out_free_uid; goto out_free_uid;
if (pipe_bufs * PAGE_SIZE > pipe_max_size && !capable(CAP_SYS_RESOURCE)) if (pipe_bufs * PAGE_SIZE > max_size && !capable(CAP_SYS_RESOURCE))
pipe_bufs = pipe_max_size >> PAGE_SHIFT; pipe_bufs = max_size >> PAGE_SHIFT;
user_bufs = account_pipe_buffers(user, 0, pipe_bufs); user_bufs = account_pipe_buffers(user, 0, pipe_bufs);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment