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@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. > > 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. Thanks, -- Joe
