Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751705AbdINUWZ (ORCPT ); Thu, 14 Sep 2017 16:22:25 -0400 Received: from bombadil.infradead.org ([65.50.211.133]:49822 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751606AbdINUWX (ORCPT ); Thu, 14 Sep 2017 16:22:23 -0400 Subject: Re: [PATCH RFC 0/3] A few round_pipe_size() and pipe-max-size fixups To: Joe Lawrence , mtk.manpages@gmail.com Cc: lkml , "linux-fsdevel@vger.kernel.org" , Alexander Viro , "Luis R. Rodriguez" , Kees Cook , Mikulas Patocka References: <1504622676-2992-1-git-send-email-joe.lawrence@redhat.com> <201487e9-235c-8604-50f2-02bd1170d8e8@infradead.org> <70559d91-c90b-0060-2b16-3a392ade62a5@redhat.com> From: Randy Dunlap Message-ID: <4bf4f17b-f202-3166-9b76-59c9409d5223@infradead.org> Date: Thu, 14 Sep 2017 13:22:19 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <70559d91-c90b-0060-2b16-3a392ade62a5@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1600 Lines: 54 On 09/14/17 12:19, Joe Lawrence wrote: > On 09/14/2017 12:57 PM, Randy Dunlap wrote: >> On 09/14/17 06:26, Michael Kerrisk (man-pages) wrote: >>> 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. >> >> Hi, >> Sorry I missed the initial posting of this. >> >> The man page for F_SETPIPE_SZ (http://man7.org/linux/man-pages/man2/fcntl.2.html) >> says: >> "Attempts to set the pipe capacity below the page size are >> silently rounded up to the page size." >> >> That implies to me that setting pipe size to 0 would round up to PAGE_SIZE. >> Doesn't patch 1/3 change that to return -EINVAL? > > Good catch. How about something like this: > > /* > * Minimum pipe size, as required by POSIX > */ > unsigned int pipe_min_size = PAGE_SIZE; > > ... > > static inline unsigned int round_pipe_size(unsigned int size) > { > unsigned long nr_pages; > > + if (size < pipe_min_size) > + size = pipe_min_size; > + > nr_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT; > if (nr_pages == 0) > return 0; > >> >> Otherwise all 3 patches look good to me. > > If the above is good, I can fold this into patch 1 and respin the set. Yes, looks good to me. Thanks. -- ~Randy