Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1732555

Re: [PATCH RFC 0/3] A few round_pipe_size() and pipe-max-size fixups

From Randy Dunlap <rdunlap@infradead.org>
Newsgroups linux.kernel
Subject Re: [PATCH RFC 0/3] A few round_pipe_size() and pipe-max-size fixups
Date 2017-09-14 22:30 +0200
Message-ID <upIVP-1fS-5@gated-at.bofh.it> (permalink)
References <umnkR-3Wq-7@gated-at.bofh.it> <upCnn-5ys-13@gated-at.bofh.it> <upFEC-7zL-15@gated-at.bofh.it> <upHQ6-Dd-11@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


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 <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.
>>
>> 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

Back to linux.kernel | Previous | NextPrevious in thread | Find similar | Unroll thread


Thread

Re: [PATCH RFC 0/3] A few round_pipe_size() and pipe-max-size fixups "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com> - 2017-09-14 15:30 +0200
  Re: [PATCH RFC 0/3] A few round_pipe_size() and pipe-max-size fixups Randy Dunlap <rdunlap@infradead.org> - 2017-09-14 19:00 +0200
    Re: [PATCH RFC 0/3] A few round_pipe_size() and pipe-max-size fixups Joe Lawrence <joe.lawrence@redhat.com> - 2017-09-14 21:20 +0200
      Re: [PATCH RFC 0/3] A few round_pipe_size() and pipe-max-size fixups Randy Dunlap <rdunlap@infradead.org> - 2017-09-14 22:30 +0200

csiph-web