lkml.org 
[lkml]   [2017]   [Sep]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [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@redhat.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@gmail.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/

\
 
 \ /
  Last update: 2017-09-14 19:28    [W:0.120 / U:0.096 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site