From: Joe Lawrence <joe.lawrence@redhat.com>
To: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Cc: Alexander Viro <viro@zeniv.linux.org.uk>,
"Luis R. Rodriguez" <mcgrof@kernel.org>,
Kees Cook <keescook@chromium.org>,
Mikulas Patocka <mpatocka@redhat.com>,
Michael Kerrisk <mtk.manpages@gmail.com>
Subject: [PATCH RFC 1/3] pipe: avoid round_pipe_size() nr_pages overflow on 32-bit
Date: Tue, 5 Sep 2017 10:44:34 -0400 [thread overview]
Message-ID: <1504622676-2992-2-git-send-email-joe.lawrence@redhat.com> (raw)
In-Reply-To: <1504622676-2992-1-git-send-email-joe.lawrence@redhat.com>
The round_pipe_size() function contains a right-bit-shift expression
which may overflow, which would cause undefined results in a subsequent
roundup_pow_of_two() call.
static inline unsigned int round_pipe_size(unsigned int size)
{
unsigned long nr_pages;
nr_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
return roundup_pow_of_two(nr_pages) << PAGE_SHIFT;
}
PAGE_SIZE is defined as (1UL << PAGE_SHIFT), so:
- 4 bytes wide on 32-bit (0 to 0xffffffff)
- 8 bytes wide on 64-bit (0 to 0xffffffffffffffff)
That means that 32-bit round_pipe_size(), nr_pages may overflow to 0:
size=0x00000000 nr_pages=0x0
size=0x00000001 nr_pages=0x1
size=0xfffff000 nr_pages=0xfffff
size=0xfffff001 nr_pages=0x0 << !
size=0xffffffff nr_pages=0x0 << !
This is bad because roundup_pow_of_two(n) is undefined when n == 0!
64-bit is not a problem* as the unsigned int size is 4 bytes wide
(similar to 32-bit) and the larger, 8 byte wide unsigned long, is
sufficient to handle the largest value of the bit shift expression:
size=0xffffffff nr_pages=100000
(*On 64-bit, round_pipe_size(0xffffffff) will later overflow when the
result of roundup_pow_of_two(0x100000) is shifted left by PAGE_SHIFT.
This behavior is at least defined by the language, so callers can safely
sanity-check a 0 return value from round_pipe_size.)
Modify round_pipe_size() to return 0 if n == 0, reset the original
pipe_max_size value, and update callers to handle accordingly.
Reported-by: Mikulas Patocka <mpatocka@redhat.com>
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
---
fs/pipe.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/fs/pipe.c b/fs/pipe.c
index 97e5be897753..fa28910b3c59 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -1017,13 +1017,16 @@ static int fifo_open(struct inode *inode, struct file *filp)
/*
* Currently we rely on the pipe array holding a power-of-2 number
- * of pages.
+ * of pages. Returns 0 on error.
*/
static inline unsigned int round_pipe_size(unsigned int size)
{
unsigned long nr_pages;
nr_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
+ if (nr_pages == 0)
+ return 0;
+
return roundup_pow_of_two(nr_pages) << PAGE_SHIFT;
}
@@ -1039,6 +1042,8 @@ static long pipe_set_size(struct pipe_inode_info *pipe, unsigned long arg)
long ret = 0;
size = round_pipe_size(arg);
+ if (size == 0)
+ return -EINVAL;
nr_pages = size >> PAGE_SHIFT;
if (!nr_pages)
@@ -1122,13 +1127,22 @@ static long pipe_set_size(struct pipe_inode_info *pipe, unsigned long arg)
int pipe_proc_fn(struct ctl_table *table, int write, void __user *buf,
size_t *lenp, loff_t *ppos)
{
+ unsigned int orig_pipe_max_size;
+ unsigned int rounded_pipe_max_size;
int ret;
+ orig_pipe_max_size = pipe_max_size;
ret = proc_dointvec_minmax(table, write, buf, lenp, ppos);
if (ret < 0 || !write)
return ret;
- pipe_max_size = round_pipe_size(pipe_max_size);
+ rounded_pipe_max_size = round_pipe_size(pipe_max_size);
+ if (rounded_pipe_max_size == 0) {
+ pipe_max_size = orig_pipe_max_size;
+ return -EINVAL;
+ }
+
+ pipe_max_size = rounded_pipe_max_size;
return ret;
}
--
1.8.3.1
next prev parent reply other threads:[~2017-09-05 14:44 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-05 14:44 [PATCH RFC 0/3] A few round_pipe_size() and pipe-max-size fixups Joe Lawrence
2017-09-05 14:44 ` Joe Lawrence [this message]
2017-09-05 14:44 ` [PATCH RFC 2/3] pipe: protect pipe_max_size access with a mutex Joe Lawrence
2017-09-14 23:09 ` Mikulas Patocka
2017-09-15 14:08 ` Joe Lawrence
2017-09-19 7:53 ` Mikulas Patocka
2017-09-19 21:32 ` Joe Lawrence
2017-09-21 10:05 ` Mikulas Patocka
2017-09-19 21:47 ` Joe Lawrence
2017-09-25 10:44 ` Mikulas Patocka
2017-09-05 14:44 ` [PATCH RFC 3/3] pipe: match pipe_max_size data type with procfs Joe Lawrence
2017-09-14 13:26 ` [PATCH RFC 0/3] A few round_pipe_size() and pipe-max-size fixups Michael Kerrisk (man-pages)
2017-09-14 16:57 ` Randy Dunlap
2017-09-14 19:19 ` Joe Lawrence
2017-09-14 20:22 ` Randy Dunlap
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=1504622676-2992-2-git-send-email-joe.lawrence@redhat.com \
--to=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 \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.