[PATCH] pipe: handle zero sized F_SETPIPE_SZ fcntl

From: Nicolai Stange
Date: Sun Feb 19 2017 - 14:20:39 EST


Syzkaller reported this

UBSAN: Undefined behaviour in include/linux/log2.h:63:13
shift exponent 64 is too large for 64-bit type 'long unsigned int'
Call Trace:
__dump_stack lib/dump_stack.c:15 [inline]
dump_stack+0x108/0x19b lib/dump_stack.c:51
ubsan_epilogue+0x12/0x8f lib/ubsan.c:164
__ubsan_handle_shift_out_of_bounds+0x29c/0x300 lib/ubsan.c:421
__roundup_pow_of_two include/linux/log2.h:63 [inline]
round_pipe_size fs/pipe.c:1049 [inline]
pipe_set_size fs/pipe.c:1042 [inline]
pipe_fcntl+0x12a/0x6e0 fs/pipe.c:1159
do_fcntl fs/fcntl.c:332 [inline]
SYSC_fcntl fs/fcntl.c:372 [inline]
SyS_fcntl+0xa90/0xb80 fs/fcntl.c:380
entry_SYSCALL_64_fastpath+0x1f/0xc2

after it had tried to do this:

fcntl(fd, F_SETPIPE_SZ, 0);

The reason is that round_pipe_size() rounds the requested size towards the
next multiple of PAGE_SIZE and passes the thus resulting number of pages
to roundup_pow_of_two(). Now, roundup_pow_of_two(0) is defined to be
undefined. More specifically,

roundup_pow_of_two(0) == 1UL << fls_long(-1) == 1UL << BITS_PER_LONG

which evaluates to 1 on x86 (good), but to zero on ARMv7 (not good), for
example.

Whatever the resulting value is, pipe_set_size() will hand it to kcalloc(),
so it should better be nonzero.

Make round_pipe_size() handle the size == 0 case explicitly and let it
return PAGE_SIZE then.

Signed-off-by: Nicolai Stange <nicstange@xxxxxxxxx>
---
fs/pipe.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/fs/pipe.c b/fs/pipe.c
index 73b84baf58f8..2c88fa8cbe3b 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -1024,6 +1024,9 @@ static inline unsigned int round_pipe_size(unsigned int size)
{
unsigned long nr_pages;

+ if (!size)
+ return PAGE_SIZE;
+
nr_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
return roundup_pow_of_two(nr_pages) << PAGE_SHIFT;
}
--
2.11.1