linux-kernel - [RFC PATCH] fs/pipe.c: implement minimum pipe size for arg==0

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Mon, 11 Sep 2017 16:15:49 -0700
From:   Randy Dunlap <rdunlap@...radead.org>
To:     LKML <linux-kernel@...r.kernel.org>,
        Linux FS Devel <linux-fsdevel@...r.kernel.org>,
        Al Viro <viro@...iv.linux.org.uk>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Shankara Pailoor <sp3485@...umbia.edu>,
        Michael Kerrisk <mtk.manpages@...il.com>
Subject: [RFC PATCH] fs/pipe.c: implement minimum pipe size for arg==0

From: Randy Dunlap <rdunlap@...radead.org>

Shankara reports that running Syskaller with UBSAN causes this message:
  UBSAN: Undefined behaviour in ./include/linux/log2.h:57:13

Syzkaller is trying to set the pipe size to 0UL. The call chain is:
	pipe_set_size(pipe, 0UL)
	...
	size = round_pipe_size(arg); // arg == 0UL
which does
	nr_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT; // = 0UL
	return roundup_pow_of_two(nr_pages) << PAGE_SHIFT;
which is undefined when the argument is 0... and which calls
	fls_long(-1) // == 64
and then returns 1UL << 64. This is where UBSAN kicks in.

The fcntl() man page [http://man7.org/linux/man-pages/man2/fcntl.2.html]
says that:
	Attempts to set the pipe capacity below the page size are
	silently rounded up to the page size.

We could try to fix the basic low-level functions to handle 0 (where
<linux/log2.h> says the result is undefined when n == 0), but the safest
path for now is probably just to patch fs/pipe.c to make the documented
default happen when arg is 0.

Reported-by: Shankara Pailoor <sp3485@...umbia.edu>
Signed-off-by: Randy Dunlap <rdunlap@...radead.org>
---
 fs/pipe.c |    2 ++
 1 file changed, 2 insertions(+)

We could just return -EINVAL when arg == 0, but we don't know how that might
adversely affect some programs.


--- lnx-413.orig/fs/pipe.c
+++ lnx-413/fs/pipe.c
@@ -1038,6 +1038,8 @@ static long pipe_set_size(struct pipe_in
 	unsigned long user_bufs;
 	long ret = 0;
 
+	if (!arg)
+		arg = PAGE_SIZE;
 	size = round_pipe_size(arg);
 	nr_pages = size >> PAGE_SHIFT;