linux-kernel - [PATCH 3.16 216/410] pipe: read buffer limits atomically

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <lsq.1528380321.615608833@decadent.org.uk>
Date:   Thu, 07 Jun 2018 15:05:21 +0100
From:   Ben Hutchings <ben@...adent.org.uk>
To:     linux-kernel@...r.kernel.org, stable@...r.kernel.org
CC:     akpm@...ux-foundation.org,
        "Michael Kerrisk" <mtk.manpages@...il.com>,
        "Kees Cook" <keescook@...omium.org>,
        "Mikulas Patocka" <mpatocka@...hat.com>,
        "Eric Biggers" <ebiggers@...gle.com>,
        "Alexander Viro" <viro@...iv.linux.org.uk>,
        "Willy Tarreau" <w@....eu>,
        "Joe Lawrence" <joe.lawrence@...hat.com>,
        "Linus Torvalds" <torvalds@...ux-foundation.org>,
        "Luis R . Rodriguez" <mcgrof@...nel.org>
Subject: [PATCH 3.16 216/410] pipe: read buffer limits atomically

3.16.57-rc1 review patch.  If anyone has any objections, please let me know.

------------------

From: Eric Biggers <ebiggers@...gle.com>

commit f7340761812fc10313e6fcc115e0bc4f7a799112 upstream.

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.com
Signed-off-by: Eric Biggers <ebiggers@...gle.com>
Acked-by: Kees Cook <keescook@...omium.org>
Acked-by: Joe Lawrence <joe.lawrence@...hat.com>
Cc: Alexander Viro <viro@...iv.linux.org.uk>
Cc: Michael Kerrisk <mtk.manpages@...il.com>
Cc: Willy Tarreau <w@....eu>
Cc: Mikulas Patocka <mpatocka@...hat.com>
Cc: "Luis R . Rodriguez" <mcgrof@...nel.org>
Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@...ux-foundation.org>
[bwh: Backported to 3.16:
 - Use ACCESS_ONCE() instead of READ_ONCE()
 - Adjust context]
Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
---
 fs/pipe.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -593,12 +593,16 @@ static unsigned long account_pipe_buffer
 
 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 = ACCESS_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 = ACCESS_ONCE(pipe_user_pages_hard);
+
+	return hard_limit && user_bufs > hard_limit;
 }
 
 static bool is_unprivileged_user(void)
@@ -612,13 +616,14 @@ struct pipe_inode_info *alloc_pipe_info(
 	unsigned long pipe_bufs = PIPE_DEF_BUFFERS;
 	struct user_struct *user = get_current_user();
 	unsigned long user_bufs;
+	unsigned int max_size = ACCESS_ONCE(pipe_max_size);
 
 	pipe = kzalloc(sizeof(struct pipe_inode_info), GFP_KERNEL);
 	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);