Hello Joe, On 5 September 2017 at 16:44, Joe Lawrence <joe.lawrence@xxxxxxxxxx> 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@xxxxxxxxx> 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/
