| Message ID | 20180108053542.6472-4-ebiggers3@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Jan 7, 2018 at 9:35 PM, Eric Biggers <ebiggers3@gmail.com> wrote: > From: Eric Biggers <ebiggers@google.com> > > pipe-user-pages-hard and pipe-user-pages-soft are only supposed to apply > to unprivileged users, as documented in both Documentation/sysctl/fs.txt > and the pipe(7) man page. > > However, the capabilities are actually only checked when increasing a > pipe's size using F_SETPIPE_SZ, not when creating a new pipe. > Therefore, if pipe-user-pages-hard has been set, the root user can run > into it and be unable to create pipes. Similarly, if > pipe-user-pages-soft has been set, the root user can run into it and > have their pipes limited to 1 page each. > > Fix this by allowing the privileged override in both cases. Should this be controlled per-namespace instead of via init-ns caps? -Kees > > Fixes: 759c01142a5d ("pipe: limit the per-user amount of pages allocated in pipes") > Cc: stable@vger.kernel.org > Signed-off-by: Eric Biggers <ebiggers@google.com> > --- > fs/pipe.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/fs/pipe.c b/fs/pipe.c > index d0dec5e7ef33..847ecc388820 100644 > --- a/fs/pipe.c > +++ b/fs/pipe.c > @@ -613,6 +613,11 @@ static bool too_many_pipe_buffers_hard(unsigned long user_bufs) > return pipe_user_pages_hard && user_bufs >= pipe_user_pages_hard; > } > > +static bool is_unprivileged_user(void) > +{ > + return !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN); > +} > + > struct pipe_inode_info *alloc_pipe_info(void) > { > struct pipe_inode_info *pipe; > @@ -629,12 +634,12 @@ struct pipe_inode_info *alloc_pipe_info(void) > > user_bufs = account_pipe_buffers(user, 0, pipe_bufs); > > - if (too_many_pipe_buffers_soft(user_bufs)) { > + if (too_many_pipe_buffers_soft(user_bufs) && is_unprivileged_user()) { > user_bufs = account_pipe_buffers(user, pipe_bufs, 1); > pipe_bufs = 1; > } > > - if (too_many_pipe_buffers_hard(user_bufs)) > + if (too_many_pipe_buffers_hard(user_bufs) && is_unprivileged_user()) > goto out_revert_acct; > > pipe->bufs = kcalloc(pipe_bufs, sizeof(struct pipe_buffer), > @@ -1065,7 +1070,7 @@ static long pipe_set_size(struct pipe_inode_info *pipe, unsigned long arg) > if (nr_pages > pipe->buffers && > (too_many_pipe_buffers_hard(user_bufs) || > too_many_pipe_buffers_soft(user_bufs)) && > - !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN)) { > + is_unprivileged_user()) { > ret = -EPERM; > goto out_revert_acct; > } > -- > 2.15.1 >
On Tue, Jan 09, 2018 at 02:23:32PM -0800, Kees Cook wrote: > On Sun, Jan 7, 2018 at 9:35 PM, Eric Biggers <ebiggers3@gmail.com> wrote: > > From: Eric Biggers <ebiggers@google.com> > > > > pipe-user-pages-hard and pipe-user-pages-soft are only supposed to apply > > to unprivileged users, as documented in both Documentation/sysctl/fs.txt > > and the pipe(7) man page. > > > > However, the capabilities are actually only checked when increasing a > > pipe's size using F_SETPIPE_SZ, not when creating a new pipe. > > Therefore, if pipe-user-pages-hard has been set, the root user can run > > into it and be unable to create pipes. Similarly, if > > pipe-user-pages-soft has been set, the root user can run into it and > > have their pipes limited to 1 page each. > > > > Fix this by allowing the privileged override in both cases. > > Should this be controlled per-namespace instead of via init-ns caps? > I don't think so. Users shouldn't be able to bypass the limits by creating a user namespace. Eric
diff --git a/fs/pipe.c b/fs/pipe.c index d0dec5e7ef33..847ecc388820 100644 --- a/fs/pipe.c +++ b/fs/pipe.c @@ -613,6 +613,11 @@ static bool too_many_pipe_buffers_hard(unsigned long user_bufs) return pipe_user_pages_hard && user_bufs >= pipe_user_pages_hard; } +static bool is_unprivileged_user(void) +{ + return !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN); +} + struct pipe_inode_info *alloc_pipe_info(void) { struct pipe_inode_info *pipe; @@ -629,12 +634,12 @@ struct pipe_inode_info *alloc_pipe_info(void) user_bufs = account_pipe_buffers(user, 0, pipe_bufs); - if (too_many_pipe_buffers_soft(user_bufs)) { + if (too_many_pipe_buffers_soft(user_bufs) && is_unprivileged_user()) { user_bufs = account_pipe_buffers(user, pipe_bufs, 1); pipe_bufs = 1; } - if (too_many_pipe_buffers_hard(user_bufs)) + if (too_many_pipe_buffers_hard(user_bufs) && is_unprivileged_user()) goto out_revert_acct; pipe->bufs = kcalloc(pipe_bufs, sizeof(struct pipe_buffer), @@ -1065,7 +1070,7 @@ static long pipe_set_size(struct pipe_inode_info *pipe, unsigned long arg) if (nr_pages > pipe->buffers && (too_many_pipe_buffers_hard(user_bufs) || too_many_pipe_buffers_soft(user_bufs)) && - !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN)) { + is_unprivileged_user()) { ret = -EPERM; goto out_revert_acct; }