From 0191f8697bbdfefcd36e7b8dc3eeddfe82893e4b Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Mon, 24 May 2010 19:15:57 +0200 Subject: pipe: F_SETPIPE_SZ should return -EPERM for non-root If the passed in size is larger than what has been set as the system wide limit and the user is not root, we want to return permission denied (not invalid value). Signed-off-by: Jens Axboe --- fs/pipe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/pipe.c') diff --git a/fs/pipe.c b/fs/pipe.c index d79872eba09a..01ca9fab95fa 100644 --- a/fs/pipe.c +++ b/fs/pipe.c @@ -1170,7 +1170,7 @@ long pipe_fcntl(struct file *file, unsigned int cmd, unsigned long arg) switch (cmd) { case F_SETPIPE_SZ: if (!capable(CAP_SYS_ADMIN) && arg > pipe_max_pages) - return -EINVAL; + return -EPERM; /* * The pipe needs to be at least 2 pages large to * guarantee POSIX behaviour. -- cgit v1.2.2 From b9598db3401282bb27b4aef77e3eee12015f7f29 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Mon, 24 May 2010 19:34:43 +0200 Subject: pipe: make F_{GET,SET}PIPE_SZ deal with byte sizes Instead of requiring an exact number of pages as the argument and return value, change the API to deal with number of bytes instead. This also relaxes the requirement that the passed in size must result in a power-of-2 page array size. Round up to the nearest power-of-2 automatically and return the resulting size of the pipe on success. Signed-off-by: Jens Axboe --- fs/pipe.c | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) (limited to 'fs/pipe.c') diff --git a/fs/pipe.c b/fs/pipe.c index 01ca9fab95fa..bdd3f96054b9 100644 --- a/fs/pipe.c +++ b/fs/pipe.c @@ -1112,26 +1112,20 @@ SYSCALL_DEFINE1(pipe, int __user *, fildes) * Allocate a new array of pipe buffers and copy the info over. Returns the * pipe size if successful, or return -ERROR on error. */ -static long pipe_set_size(struct pipe_inode_info *pipe, unsigned long arg) +static long pipe_set_size(struct pipe_inode_info *pipe, unsigned long nr_pages) { struct pipe_buffer *bufs; - /* - * Must be a power-of-2 currently - */ - if (!is_power_of_2(arg)) - return -EINVAL; - /* * We can shrink the pipe, if arg >= pipe->nrbufs. Since we don't * expect a lot of shrink+grow operations, just free and allocate * again like we would do for growing. If the pipe currently * contains more buffers than arg, then return busy. */ - if (arg < pipe->nrbufs) + if (nr_pages < pipe->nrbufs) return -EBUSY; - bufs = kcalloc(arg, sizeof(struct pipe_buffer), GFP_KERNEL); + bufs = kcalloc(nr_pages, sizeof(struct pipe_buffer), GFP_KERNEL); if (unlikely(!bufs)) return -ENOMEM; @@ -1152,8 +1146,8 @@ static long pipe_set_size(struct pipe_inode_info *pipe, unsigned long arg) pipe->curbuf = 0; kfree(pipe->bufs); pipe->bufs = bufs; - pipe->buffers = arg; - return arg; + pipe->buffers = nr_pages; + return nr_pages * PAGE_SIZE; } long pipe_fcntl(struct file *file, unsigned int cmd, unsigned long arg) @@ -1168,19 +1162,30 @@ long pipe_fcntl(struct file *file, unsigned int cmd, unsigned long arg) mutex_lock(&pipe->inode->i_mutex); switch (cmd) { - case F_SETPIPE_SZ: - if (!capable(CAP_SYS_ADMIN) && arg > pipe_max_pages) + case F_SETPIPE_SZ: { + unsigned long nr_pages; + + /* + * Currently the array must be a power-of-2 size, so adjust + * upwards if needed. + */ + nr_pages = (arg + PAGE_SIZE - 1) >> PAGE_SHIFT; + nr_pages = roundup_pow_of_two(nr_pages); + + if (!capable(CAP_SYS_ADMIN) && nr_pages > pipe_max_pages) return -EPERM; + /* * The pipe needs to be at least 2 pages large to * guarantee POSIX behaviour. */ - if (arg < 2) + if (nr_pages < 2) return -EINVAL; - ret = pipe_set_size(pipe, arg); + ret = pipe_set_size(pipe, nr_pages); break; + } case F_GETPIPE_SZ: - ret = pipe->buffers; + ret = pipe->buffers * PAGE_SIZE; break; default: ret = -EINVAL; -- cgit v1.2.2 From 6a6ca57de92fcae34603551ac944aa74758c30d4 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Thu, 3 Jun 2010 12:44:30 +0200 Subject: pipe: adjust minimum pipe size to 1 page We don't need to pages to guarantee the POSIX requirement that upto a page size write must be atomic to an empty pipe. Signed-off-by: Jens Axboe --- fs/pipe.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) (limited to 'fs/pipe.c') diff --git a/fs/pipe.c b/fs/pipe.c index 541d6626f9d9..369a0245aab6 100644 --- a/fs/pipe.c +++ b/fs/pipe.c @@ -1181,13 +1181,7 @@ long pipe_fcntl(struct file *file, unsigned int cmd, unsigned long arg) if (!capable(CAP_SYS_ADMIN) && nr_pages > pipe_max_pages) { ret = -EPERM; goto out; - } - - /* - * The pipe needs to be at least 2 pages large to - * guarantee POSIX behaviour. - */ - if (arg < 2) { + } else if (nr_pages < 1) { ret = -EINVAL; goto out; } -- cgit v1.2.2 From 419f8367ea37e5adc5d95479e8fd5554b92b49fe Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Thu, 3 Jun 2010 12:45:28 +0200 Subject: pipe: change the privilege required for growing a pipe beyond system max Change it to CAP_SYS_RESOURCE, as that more accurately models what we want to control. Suggested-by: Michael Kerrisk Signed-off-by: Jens Axboe --- fs/pipe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/pipe.c') diff --git a/fs/pipe.c b/fs/pipe.c index 369a0245aab6..f98fae3e36b0 100644 --- a/fs/pipe.c +++ b/fs/pipe.c @@ -1178,7 +1178,7 @@ long pipe_fcntl(struct file *file, unsigned int cmd, unsigned long arg) nr_pages = (arg + PAGE_SIZE - 1) >> PAGE_SHIFT; nr_pages = roundup_pow_of_two(nr_pages); - if (!capable(CAP_SYS_ADMIN) && nr_pages > pipe_max_pages) { + if (!capable(CAP_SYS_RESOURCE) && nr_pages > pipe_max_pages) { ret = -EPERM; goto out; } else if (nr_pages < 1) { -- cgit v1.2.2 From ff9da691c0498ff81fdd014e7a0731dab2337dac Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Thu, 3 Jun 2010 14:54:39 +0200 Subject: pipe: change /proc/sys/fs/pipe-max-pages to byte sized interface This changes the interface to be based on bytes instead. The API matches that of F_SETPIPE_SZ in that it rounds up the passed in size so that the resulting page array is a power-of-2 in size. The proc file is renamed to /proc/sys/fs/pipe-max-size to reflect this change. Signed-off-by: Jens Axboe --- fs/pipe.c | 54 ++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 42 insertions(+), 12 deletions(-) (limited to 'fs/pipe.c') diff --git a/fs/pipe.c b/fs/pipe.c index f98fae3e36b0..69c4c7c13ea9 100644 --- a/fs/pipe.c +++ b/fs/pipe.c @@ -26,9 +26,14 @@ /* * The max size that a non-root user is allowed to grow the pipe. Can - * be set by root in /proc/sys/fs/pipe-max-pages + * be set by root in /proc/sys/fs/pipe-max-size */ -unsigned int pipe_max_pages = PIPE_DEF_BUFFERS * 16; +unsigned int pipe_max_size = 1048576; + +/* + * Minimum pipe size, as required by POSIX + */ +unsigned int pipe_min_size = PAGE_SIZE; /* * We use a start+len construction, which provides full use of the @@ -1156,6 +1161,35 @@ static long pipe_set_size(struct pipe_inode_info *pipe, unsigned long nr_pages) return nr_pages * PAGE_SIZE; } +/* + * Currently we rely on the pipe array holding a power-of-2 number + * of pages. + */ +static inline unsigned int round_pipe_size(unsigned int size) +{ + unsigned long nr_pages; + + nr_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT; + return roundup_pow_of_two(nr_pages) << PAGE_SHIFT; +} + +/* + * This should work even if CONFIG_PROC_FS isn't set, as proc_dointvec_minmax + * will return an error. + */ +int pipe_proc_fn(struct ctl_table *table, int write, void __user *buf, + size_t *lenp, loff_t *ppos) +{ + int ret; + + ret = proc_dointvec_minmax(table, write, buf, lenp, ppos); + if (ret < 0 || !write) + return ret; + + pipe_max_size = round_pipe_size(pipe_max_size); + return ret; +} + long pipe_fcntl(struct file *file, unsigned int cmd, unsigned long arg) { struct pipe_inode_info *pipe; @@ -1169,23 +1203,19 @@ long pipe_fcntl(struct file *file, unsigned int cmd, unsigned long arg) switch (cmd) { case F_SETPIPE_SZ: { - unsigned long nr_pages; + unsigned int size, nr_pages; - /* - * Currently the array must be a power-of-2 size, so adjust - * upwards if needed. - */ - nr_pages = (arg + PAGE_SIZE - 1) >> PAGE_SHIFT; - nr_pages = roundup_pow_of_two(nr_pages); + size = round_pipe_size(arg); + nr_pages = size >> PAGE_SHIFT; - if (!capable(CAP_SYS_RESOURCE) && nr_pages > pipe_max_pages) { + if (!capable(CAP_SYS_RESOURCE) && size > pipe_max_size) { ret = -EPERM; goto out; - } else if (nr_pages < 1) { + } else if (nr_pages < PAGE_SIZE) { ret = -EINVAL; goto out; } - ret = pipe_set_size(pipe, arg); + ret = pipe_set_size(pipe, nr_pages); break; } case F_GETPIPE_SZ: -- cgit v1.2.2 From 1d862f41222b7f385bada9f85a67ca5592ffd33e Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Tue, 8 Jun 2010 16:28:45 +0200 Subject: pipe: fix pipe buffer resizing pipe_set_size() needs to copy pipe bufs from the old circular buffer to the new. The current code gets this wrong in multiple ways, resulting in oops. Test program is available here: http://www.kernel.org/pub/linux/kernel/people/mszeredi/piperesize/ Signed-off-by: Miklos Szeredi Signed-off-by: Jens Axboe --- fs/pipe.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) (limited to 'fs/pipe.c') diff --git a/fs/pipe.c b/fs/pipe.c index 69c4c7c13ea9..f31e2d472984 100644 --- a/fs/pipe.c +++ b/fs/pipe.c @@ -1145,13 +1145,20 @@ static long pipe_set_size(struct pipe_inode_info *pipe, unsigned long nr_pages) * and adjust the indexes. */ if (pipe->nrbufs) { - const unsigned int tail = pipe->nrbufs & (pipe->buffers - 1); - const unsigned int head = pipe->nrbufs - tail; + unsigned int tail; + unsigned int head; + tail = pipe->curbuf + pipe->nrbufs; + if (tail < pipe->buffers) + tail = 0; + else + tail &= (pipe->buffers - 1); + + head = pipe->nrbufs - tail; if (head) memcpy(bufs, pipe->bufs + pipe->curbuf, head * sizeof(struct pipe_buffer)); if (tail) - memcpy(bufs + head, pipe->bufs + pipe->curbuf, tail * sizeof(struct pipe_buffer)); + memcpy(bufs + head, pipe->bufs, tail * sizeof(struct pipe_buffer)); } pipe->curbuf = 0; -- cgit v1.2.2 From 6db40cf047a8723095caf79f5569d21b388d7b31 Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Wed, 9 Jun 2010 09:27:57 +0200 Subject: pipe: fix check in "set size" fcntl As it stands this check compares the number of pages to the page size. This makes no sense and makes the fcntl fail in almost any sane case. Fix it by checking if nr_pages is not zero (it can become zero only if arg is too big and round_pipe_size() overflows). Signed-off-by: Miklos Szeredi Signed-off-by: Jens Axboe --- fs/pipe.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'fs/pipe.c') diff --git a/fs/pipe.c b/fs/pipe.c index f31e2d472984..279eef96c51c 100644 --- a/fs/pipe.c +++ b/fs/pipe.c @@ -1215,12 +1215,13 @@ long pipe_fcntl(struct file *file, unsigned int cmd, unsigned long arg) size = round_pipe_size(arg); nr_pages = size >> PAGE_SHIFT; + ret = -EINVAL; + if (!nr_pages) + goto out; + if (!capable(CAP_SYS_RESOURCE) && size > pipe_max_size) { ret = -EPERM; goto out; - } else if (nr_pages < PAGE_SIZE) { - ret = -EINVAL; - goto out; } ret = pipe_set_size(pipe, nr_pages); break; -- cgit v1.2.2