Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751473AbdINN1F (ORCPT ); Thu, 14 Sep 2017 09:27:05 -0400 Received: from mail-wm0-f65.google.com ([74.125.82.65]:35484 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751131AbdINN1E (ORCPT ); Thu, 14 Sep 2017 09:27:04 -0400 X-Google-Smtp-Source: AOwi7QBgth8nQP9n8NjVqqgpxhJj7TdGXG8+HmraNd4MXj7A8pz2nFs5ZcJ1pqZss9v9jipnoTi6kB0qanOGm4Oh55s= MIME-Version: 1.0 Reply-To: mtk.manpages@gmail.com In-Reply-To: <1504622676-2992-1-git-send-email-joe.lawrence@redhat.com> References: <1504622676-2992-1-git-send-email-joe.lawrence@redhat.com> From: "Michael Kerrisk (man-pages)" Date: Thu, 14 Sep 2017 15:26:42 +0200 Message-ID: Subject: Re: [PATCH RFC 0/3] A few round_pipe_size() and pipe-max-size fixups To: Joe Lawrence Cc: lkml , "linux-fsdevel@vger.kernel.org" , Alexander Viro , "Luis R. Rodriguez" , Kees Cook , Mikulas Patocka Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2916 Lines: 102 Hello Joe, On 5 September 2017 at 16:44, Joe Lawrence 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 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/