linux-kernel - Re: [PATCH RFC 0/3] A few round_pipe

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKgNAkjrv_y9XMu947YxjfugAFS=539gJQhAZE9yASuooXFqxw@mail.gmail.com>
Date:   Thu, 14 Sep 2017 15:26:42 +0200
From:   "Michael Kerrisk (man-pages)" <mtk.manpages@...il.com>
To:     Joe Lawrence <joe.lawrence@...hat.com>
Cc:     lkml <linux-kernel@...r.kernel.org>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        "Luis R. Rodriguez" <mcgrof@...nel.org>,
        Kees Cook <keescook@...omium.org>,
        Mikulas Patocka <mpatocka@...hat.com>
Subject: Re: [PATCH RFC 0/3] A few round_pipe_size() and pipe-max-size fixups

Hello Joe,

On 5 September 2017 at 16:44, Joe Lawrence <joe.lawrence@...hat.com> wrote:
> While backporting Michael's "pipe: fix limit handling" [1] patchset to a
> distro-kernel, Mikulas noticed that current upstream pipe limit handling
> contains a few problems:
>
>   1 - round_pipe_size() nr_pages overflow on 32bit:  this would
>       subsequently try roundup_pow_of_two(0), which is undefined.
>
>   2 - visible non-rounded pipe-max-size value: there is no mutual
>       exclusion or protection between the time pipe_max_size is assigned
>       a raw value from proc_dointvec_minmax() and when it is rounded.
>
>   3 - procfs signed wrap: echo'ing a large number into
>       /proc/sys/fs/pipe-max-size and then cat'ing it back out shows a
>       negative value.
>
>
> This RFC serves as a bug report and a contains a few possible fixes.
> There may be better / more consistent ways to fix the overflows and
> procfs bugs, but I figured I'd throw an RFC w/code out there for initial
> conversation.  Suggestions welcome!

Thank for working on this. I have no improvements to suggest. The
patches all look sane to me. For the whole series:

Reviewed-by: MIchael Kerrisk <mtk.manpages@...il.com>

Cheers,

Michael


> Testing
> =======
>
> Patch 1 - 32bit overflow
> ------------------------
> From userspace:
>
>   fcntl(fd, F_SETPIPE_SZ, 0xffffffff);
>
> - Before the fix, return value was 4096 as pipe size overflowed and
>   was set to 4096
>
> - After the fix, returns -1 and sets errno EINVAL, pipe size remains
>   untouched
>
>
> Patch 2 - non-rounded pipe-max-size value
> -----------------------------------------
> Keep plugging in values that need to be rounded:
>
>   while (true); do echo 1048570 > /proc/sys/fs/pipe-max-size; done
>
> and in another terminal, loop around reading the value:
>
>   time (while (true); do SIZE=$(cat /proc/sys/fs/pipe-max-size); [[ $(( $SIZE % 4096 )) -ne 0 ]] && break; done; echo "$SIZE")
>   1048570
>
>   real    0m46.213s
>   user    0m29.688s
>   sys     0m20.042s
>
> after the fix, the test loop never encountered a non-page-rounded value.
>
>
> Patch 3 - procfs signed wrap
> ----------------------------
> Before:
>
>   % echo 2147483647 >/proc/sys/fs/pipe-max-size
>   % cat /proc/sys/fs/pipe-max-size
>   -2147483648
>
> After:
>
>   % echo 2147483647 >/proc/sys/fs/pipe-max-size
>   % cat /proc/sys/fs/pipe-max-size
>   2147483648
>
>
> Joe Lawrence (3):
>   pipe: avoid round_pipe_size() nr_pages overflow on 32-bit
>   pipe: protect pipe_max_size access with a mutex
>   pipe: match pipe_max_size data type with procfs
>
>  fs/pipe.c       | 48 +++++++++++++++++++++++++++++++++++++++++-------
>  kernel/sysctl.c |  2 +-
>  2 files changed, 42 insertions(+), 8 deletions(-)
>
> --
> 1.8.3.1
>



-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/