Linux-Fsdevel Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Eric Biggers <ebiggers3@gmail.com>
To: linux-fsdevel@vger.kernel.org
Cc: Alexander Viro <viro@zeniv.linux.org.uk>,
	Joe Lawrence <joe.lawrence@redhat.com>,
	Michael Kerrisk <mtk.manpages@gmail.com>,
	Willy Tarreau <w@1wt.eu>, Mikulas Patocka <mpatocka@redhat.com>,
	"Luis R . Rodriguez" <mcgrof@kernel.org>,
	Kees Cook <keescook@chromium.org>,
	linux-kernel@vger.kernel.org, Eric Biggers <ebiggers@google.com>
Subject: [PATCH v2 7/7] pipe: read buffer limits atomically
Date: Wed, 10 Jan 2018 21:29:02 -0800	[thread overview]
Message-ID: <20180111052902.14409-8-ebiggers3@gmail.com> (raw)
In-Reply-To: <20180111052902.14409-1-ebiggers3@gmail.com>

From: Eric Biggers <ebiggers@google.com>

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.

Acked-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/pipe.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/fs/pipe.c b/fs/pipe.c
index 458f866caf53..deeb303aa4ce 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -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)
 {
-	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)
 {
-	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)
@@ -624,13 +628,14 @@ struct pipe_inode_info *alloc_pipe_info(void)
 	unsigned long pipe_bufs = PIPE_DEF_BUFFERS;
 	struct user_struct *user = get_current_user();
 	unsigned long user_bufs;
+	unsigned int max_size = READ_ONCE(pipe_max_size);
 
 	pipe = kzalloc(sizeof(struct pipe_inode_info), GFP_KERNEL_ACCOUNT);
 	if (pipe == NULL)
 		goto out_free_uid;
 
-	if (pipe_bufs * PAGE_SIZE > pipe_max_size && !capable(CAP_SYS_RESOURCE))
-		pipe_bufs = pipe_max_size >> PAGE_SHIFT;
+	if (pipe_bufs * PAGE_SIZE > max_size && !capable(CAP_SYS_RESOURCE))
+		pipe_bufs = max_size >> PAGE_SHIFT;
 
 	user_bufs = account_pipe_buffers(user, 0, pipe_bufs);
 
-- 
2.15.1

  parent reply	other threads:[~2018-01-11  5:29 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-11  5:28 [PATCH v2 0/7] pipe: buffer limits fixes and cleanups Eric Biggers
2018-01-11  5:28 ` [PATCH v2 1/7] pipe, sysctl: drop 'min' parameter from pipe-max-size converter Eric Biggers
2018-01-11  5:28 ` [PATCH v2 2/7] pipe, sysctl: remove pipe_proc_fn() Eric Biggers
2018-01-11  5:28 ` [PATCH v2 3/7] pipe: actually allow root to exceed the pipe buffer limits Eric Biggers
2018-01-11  5:28 ` [PATCH v2 4/7] pipe: fix off-by-one error when checking " Eric Biggers
2018-01-11  5:29 ` [PATCH v2 5/7] pipe: reject F_SETPIPE_SZ with size over UINT_MAX Eric Biggers
2018-01-11  5:29 ` [PATCH v2 6/7] pipe: simplify round_pipe_size() Eric Biggers
2018-01-11  5:29 ` Eric Biggers [this message]
2018-01-11 20:36 ` [PATCH v2 0/7] pipe: buffer limits fixes and cleanups Kees Cook
2018-01-11 21:41 ` Joe Lawrence

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180111052902.14409-8-ebiggers3@gmail.com \
    --to=ebiggers3@gmail.com \
    --cc=ebiggers@google.com \
    --cc=joe.lawrence@redhat.com \
    --cc=keescook@chromium.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=mpatocka@redhat.com \
    --cc=mtk.manpages@gmail.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=w@1wt.eu \
    --subject='Re: [PATCH v2 7/7] pipe: read buffer limits atomically' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).