| Message ID | 1453115211-24270-1-git-send-email-w@1wt.eu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Willy,
[auto build test WARNING on linus/master]
[also build test WARNING on next-20160118]
[cannot apply to tip/sched/core v4.4]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Willy-Tarreau/pipe-limit-the-per-user-amount-of-pages-allocated-in-pipes/20160118-190854
reproduce: make htmldocs
All warnings (new ones prefixed by >>):
include/linux/jbd2.h:439: warning: No description found for parameter 'i_transaction'
include/linux/jbd2.h:439: warning: No description found for parameter 'i_next_transaction'
include/linux/jbd2.h:439: warning: No description found for parameter 'i_list'
include/linux/jbd2.h:439: warning: No description found for parameter 'i_vfs_inode'
include/linux/jbd2.h:439: warning: No description found for parameter 'i_flags'
include/linux/jbd2.h:495: warning: No description found for parameter 'h_rsv_handle'
include/linux/jbd2.h:495: warning: No description found for parameter 'h_reserved'
include/linux/jbd2.h:495: warning: No description found for parameter 'h_type'
include/linux/jbd2.h:495: warning: No description found for parameter 'h_line_no'
include/linux/jbd2.h:495: warning: No description found for parameter 'h_start_jiffies'
include/linux/jbd2.h:495: warning: No description found for parameter 'h_requested_credits'
include/linux/jbd2.h:495: warning: No description found for parameter 'h_lockdep_map'
include/linux/jbd2.h:1038: warning: No description found for parameter 'j_chkpt_bhs[JBD2_NR_BATCH]'
include/linux/jbd2.h:1038: warning: No description found for parameter 'j_devname[BDEVNAME_SIZE+24]'
include/linux/jbd2.h:1038: warning: No description found for parameter 'j_average_commit_time'
include/linux/jbd2.h:1038: warning: No description found for parameter 'j_min_batch_time'
include/linux/jbd2.h:1038: warning: No description found for parameter 'j_max_batch_time'
include/linux/jbd2.h:1038: warning: No description found for parameter 'j_commit_callback'
include/linux/jbd2.h:1038: warning: No description found for parameter 'j_failed_commit'
include/linux/jbd2.h:1038: warning: No description found for parameter 'j_chksum_driver'
include/linux/jbd2.h:1038: warning: No description found for parameter 'j_csum_seed'
include/linux/jbd2.h:1038: warning: Excess struct/union/enum/typedef member 'j_history' description in 'journal_s'
include/linux/jbd2.h:1038: warning: Excess struct/union/enum/typedef member 'j_history_max' description in 'journal_s'
include/linux/jbd2.h:1038: warning: Excess struct/union/enum/typedef member 'j_history_cur' description in 'journal_s'
fs/jbd2/transaction.c:429: warning: No description found for parameter 'rsv_blocks'
fs/jbd2/transaction.c:429: warning: No description found for parameter 'gfp_mask'
fs/jbd2/transaction.c:429: warning: No description found for parameter 'type'
fs/jbd2/transaction.c:429: warning: No description found for parameter 'line_no'
fs/jbd2/transaction.c:505: warning: No description found for parameter 'type'
fs/jbd2/transaction.c:505: warning: No description found for parameter 'line_no'
fs/jbd2/transaction.c:635: warning: No description found for parameter 'gfp_mask'
>> include/linux/pipe_fs_i.h:61: warning: No description found for parameter 'user'
vim +/user +61 include/linux/pipe_fs_i.h
0845718da Jens Axboe 2007-06-12 45 **/
17374ff1a Jens Axboe 2007-06-04 46 struct pipe_inode_info {
72b0d9aac Al Viro 2013-03-21 47 struct mutex mutex;
17374ff1a Jens Axboe 2007-06-04 48 wait_queue_head_t wait;
35f3d14db Jens Axboe 2010-05-20 49 unsigned int nrbufs, curbuf, buffers;
17374ff1a Jens Axboe 2007-06-04 50 unsigned int readers;
17374ff1a Jens Axboe 2007-06-04 51 unsigned int writers;
ba5bb1473 Al Viro 2013-03-21 52 unsigned int files;
17374ff1a Jens Axboe 2007-06-04 53 unsigned int waiting_writers;
17374ff1a Jens Axboe 2007-06-04 54 unsigned int r_counter;
17374ff1a Jens Axboe 2007-06-04 55 unsigned int w_counter;
35f3d14db Jens Axboe 2010-05-20 56 struct page *tmp_page;
17374ff1a Jens Axboe 2007-06-04 57 struct fasync_struct *fasync_readers;
17374ff1a Jens Axboe 2007-06-04 58 struct fasync_struct *fasync_writers;
35f3d14db Jens Axboe 2010-05-20 59 struct pipe_buffer *bufs;
89cb9d10e Willy Tarreau 2016-01-18 60 struct user_struct *user;
17374ff1a Jens Axboe 2007-06-04 @61 };
17374ff1a Jens Axboe 2007-06-04 62
f84d75199 Jens Axboe 2006-05-01 63 /*
f84d75199 Jens Axboe 2006-05-01 64 * Note on the nesting of these functions:
f84d75199 Jens Axboe 2006-05-01 65 *
cac36bb06 Jens Axboe 2007-06-14 66 * ->confirm()
f84d75199 Jens Axboe 2006-05-01 67 * ->steal()
f84d75199 Jens Axboe 2006-05-01 68 * ...
f84d75199 Jens Axboe 2006-05-01 69 * ->map()
:::::: The code at line 61 was first introduced by commit
:::::: 17374ff1aa9ce2a0597416a16729474b538af443 pipe: move pipe_inode_info structure decleration up before it's used
:::::: TO: Jens Axboe <jens.axboe@oracle.com>
:::::: CC: Jens Axboe <jens.axboe@oracle.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Mon, Jan 18, 2016 at 08:22:10PM +0800, kbuild test robot wrote: > Hi Willy, > > [auto build test WARNING on linus/master] > [also build test WARNING on next-20160118] > [cannot apply to tip/sched/core v4.4] > [if your patch is applied to the wrong git tree, please drop us a note to help improving the system] Well, I don't understand how to read the output here, I'm sorry. Since this patch modifies the user_struct, it has a dependency on the following patch just merged into Linus tree : 712f4aa ("unix: properly account for FDs passed over unix sockets") That's probably why you have the "cannot apply" error above. Willy > url: https://github.com/0day-ci/linux/commits/Willy-Tarreau/pipe-limit-the-per-user-amount-of-pages-allocated-in-pipes/20160118-190854 > reproduce: make htmldocs > > All warnings (new ones prefixed by >>): > > include/linux/jbd2.h:439: warning: No description found for parameter 'i_transaction' > include/linux/jbd2.h:439: warning: No description found for parameter 'i_next_transaction' > include/linux/jbd2.h:439: warning: No description found for parameter 'i_list' > include/linux/jbd2.h:439: warning: No description found for parameter 'i_vfs_inode' > include/linux/jbd2.h:439: warning: No description found for parameter 'i_flags' > include/linux/jbd2.h:495: warning: No description found for parameter 'h_rsv_handle' > include/linux/jbd2.h:495: warning: No description found for parameter 'h_reserved' > include/linux/jbd2.h:495: warning: No description found for parameter 'h_type' > include/linux/jbd2.h:495: warning: No description found for parameter 'h_line_no' > include/linux/jbd2.h:495: warning: No description found for parameter 'h_start_jiffies' > include/linux/jbd2.h:495: warning: No description found for parameter 'h_requested_credits' > include/linux/jbd2.h:495: warning: No description found for parameter 'h_lockdep_map' > include/linux/jbd2.h:1038: warning: No description found for parameter 'j_chkpt_bhs[JBD2_NR_BATCH]' > include/linux/jbd2.h:1038: warning: No description found for parameter 'j_devname[BDEVNAME_SIZE+24]' > include/linux/jbd2.h:1038: warning: No description found for parameter 'j_average_commit_time' > include/linux/jbd2.h:1038: warning: No description found for parameter 'j_min_batch_time' > include/linux/jbd2.h:1038: warning: No description found for parameter 'j_max_batch_time' > include/linux/jbd2.h:1038: warning: No description found for parameter 'j_commit_callback' > include/linux/jbd2.h:1038: warning: No description found for parameter 'j_failed_commit' > include/linux/jbd2.h:1038: warning: No description found for parameter 'j_chksum_driver' > include/linux/jbd2.h:1038: warning: No description found for parameter 'j_csum_seed' > include/linux/jbd2.h:1038: warning: Excess struct/union/enum/typedef member 'j_history' description in 'journal_s' > include/linux/jbd2.h:1038: warning: Excess struct/union/enum/typedef member 'j_history_max' description in 'journal_s' > include/linux/jbd2.h:1038: warning: Excess struct/union/enum/typedef member 'j_history_cur' description in 'journal_s' > fs/jbd2/transaction.c:429: warning: No description found for parameter 'rsv_blocks' > fs/jbd2/transaction.c:429: warning: No description found for parameter 'gfp_mask' > fs/jbd2/transaction.c:429: warning: No description found for parameter 'type' > fs/jbd2/transaction.c:429: warning: No description found for parameter 'line_no' > fs/jbd2/transaction.c:505: warning: No description found for parameter 'type' > fs/jbd2/transaction.c:505: warning: No description found for parameter 'line_no' > fs/jbd2/transaction.c:635: warning: No description found for parameter 'gfp_mask' > >> include/linux/pipe_fs_i.h:61: warning: No description found for parameter 'user' > > vim +/user +61 include/linux/pipe_fs_i.h > > 0845718da Jens Axboe 2007-06-12 45 **/ > 17374ff1a Jens Axboe 2007-06-04 46 struct pipe_inode_info { > 72b0d9aac Al Viro 2013-03-21 47 struct mutex mutex; > 17374ff1a Jens Axboe 2007-06-04 48 wait_queue_head_t wait; > 35f3d14db Jens Axboe 2010-05-20 49 unsigned int nrbufs, curbuf, buffers; > 17374ff1a Jens Axboe 2007-06-04 50 unsigned int readers; > 17374ff1a Jens Axboe 2007-06-04 51 unsigned int writers; > ba5bb1473 Al Viro 2013-03-21 52 unsigned int files; > 17374ff1a Jens Axboe 2007-06-04 53 unsigned int waiting_writers; > 17374ff1a Jens Axboe 2007-06-04 54 unsigned int r_counter; > 17374ff1a Jens Axboe 2007-06-04 55 unsigned int w_counter; > 35f3d14db Jens Axboe 2010-05-20 56 struct page *tmp_page; > 17374ff1a Jens Axboe 2007-06-04 57 struct fasync_struct *fasync_readers; > 17374ff1a Jens Axboe 2007-06-04 58 struct fasync_struct *fasync_writers; > 35f3d14db Jens Axboe 2010-05-20 59 struct pipe_buffer *bufs; > 89cb9d10e Willy Tarreau 2016-01-18 60 struct user_struct *user; > 17374ff1a Jens Axboe 2007-06-04 @61 }; > 17374ff1a Jens Axboe 2007-06-04 62 > f84d75199 Jens Axboe 2006-05-01 63 /* > f84d75199 Jens Axboe 2006-05-01 64 * Note on the nesting of these functions: > f84d75199 Jens Axboe 2006-05-01 65 * > cac36bb06 Jens Axboe 2007-06-14 66 * ->confirm() > f84d75199 Jens Axboe 2006-05-01 67 * ->steal() > f84d75199 Jens Axboe 2006-05-01 68 * ... > f84d75199 Jens Axboe 2006-05-01 69 * ->map() > > :::::: The code at line 61 was first introduced by commit > :::::: 17374ff1aa9ce2a0597416a16729474b538af443 pipe: move pipe_inode_info structure decleration up before it's used > > :::::: TO: Jens Axboe <jens.axboe@oracle.com> > :::::: CC: Jens Axboe <jens.axboe@oracle.com> > > --- > 0-DAY kernel test infrastructure Open Source Technology Center > https://lists.01.org/pipermail/kbuild-all Intel Corporation -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Willy Tarreau wrote: > On Mon, Jan 18, 2016 at 08:22:10PM +0800, kbuild test robot wrote: > > Hi Willy, > > > > [auto build test WARNING on linus/master] > > [also build test WARNING on next-20160118] > > [cannot apply to tip/sched/core v4.4] > > [if your patch is applied to the wrong git tree, please drop us a note to help improving the system] > > Well, I don't understand how to read the output here, I'm sorry. Since this > patch modifies the user_struct, it has a dependency on the following patch > just merged into Linus tree : > > 712f4aa ("unix: properly account for FDs passed over unix sockets") > > That's probably why you have the "cannot apply" error above. The output says that description for newly added field is missing. I think adding some description like * @fasync_readers: reader side fasync * @fasync_writers: writer side fasync * @bufs: the circular array of pipe buffers + * @user: the user who created this pipe **/ struct pipe_inode_info { struct mutex mutex; will fix. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 19, 2016 at 12:12:44AM +0900, Tetsuo Handa wrote: > Willy Tarreau wrote: > > On Mon, Jan 18, 2016 at 08:22:10PM +0800, kbuild test robot wrote: > > > Hi Willy, > > > > > > [auto build test WARNING on linus/master] > > > [also build test WARNING on next-20160118] > > > [cannot apply to tip/sched/core v4.4] > > > [if your patch is applied to the wrong git tree, please drop us a note to help improving the system] > > > > Well, I don't understand how to read the output here, I'm sorry. Since this > > patch modifies the user_struct, it has a dependency on the following patch > > just merged into Linus tree : > > > > 712f4aa ("unix: properly account for FDs passed over unix sockets") > > > > That's probably why you have the "cannot apply" error above. > > The output says that description for newly added field is missing. > I think adding some description like > > * @fasync_readers: reader side fasync > * @fasync_writers: writer side fasync > * @bufs: the circular array of pipe buffers > + * @user: the user who created this pipe > **/ > struct pipe_inode_info { > struct mutex mutex; > > will fix. Ah you're right, I've checked the other warnings that were reported around and that's it. I wasn't aware that a bot was reading comments, I thought it was a build warning. I'm going to resend a v4. Thanks, Willy -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/sysctl/fs.txt b/Documentation/sysctl/fs.txt index 88152f2..302b5ed 100644 --- a/Documentation/sysctl/fs.txt +++ b/Documentation/sysctl/fs.txt @@ -32,6 +32,8 @@ Currently, these files are in /proc/sys/fs: - nr_open - overflowuid - overflowgid +- pipe-user-pages-hard +- pipe-user-pages-soft - protected_hardlinks - protected_symlinks - suid_dumpable @@ -159,6 +161,27 @@ The default is 65534. ============================================================== +pipe-user-pages-hard: + +Maximum total number of pages a non-privileged user may allocate for pipes. +Once this limit is reached, no new pipes may be allocated until usage goes +below the limit again. When set to 0, no limit is applied, which is the default +setting. + +============================================================== + +pipe-user-pages-soft: + +Maximum total number of pages a non-privileged user may allocate for pipes +before the pipe size gets limited to a single page. Once this limit is reached, +new pipes will be limited to a single page in size for this user in order to +limit total memory usage, and trying to increase them using fcntl() will be +denied until usage goes below the limit again. The default value allows to +allocate up to 1024 pipes at their default size. When set to 0, no limit is +applied. + +============================================================== + protected_hardlinks: A long-standing class of security issues is the hardlink-based diff --git a/fs/pipe.c b/fs/pipe.c index 42cf8dd..ab8dad3 100644 --- a/fs/pipe.c +++ b/fs/pipe.c @@ -38,6 +38,12 @@ unsigned int pipe_max_size = 1048576; */ unsigned int pipe_min_size = PAGE_SIZE; +/* Maximum allocatable pages per user. Hard limit is unset by default, soft + * matches default values. + */ +unsigned long pipe_user_pages_hard; +unsigned long pipe_user_pages_soft = PIPE_DEF_BUFFERS * INR_OPEN_CUR; + /* * We use a start+len construction, which provides full use of the * allocated memory. @@ -583,20 +589,49 @@ pipe_fasync(int fd, struct file *filp, int on) return retval; } +static void account_pipe_buffers(struct pipe_inode_info *pipe, + unsigned long old, unsigned long new) +{ + atomic_long_add(new - old, &pipe->user->pipe_bufs); +} + +static bool too_many_pipe_buffers_soft(struct user_struct *user) +{ + return pipe_user_pages_soft && + atomic_long_read(&user->pipe_bufs) >= pipe_user_pages_soft; +} + +static bool too_many_pipe_buffers_hard(struct user_struct *user) +{ + return pipe_user_pages_hard && + atomic_long_read(&user->pipe_bufs) >= pipe_user_pages_hard; +} + struct pipe_inode_info *alloc_pipe_info(void) { struct pipe_inode_info *pipe; pipe = kzalloc(sizeof(struct pipe_inode_info), GFP_KERNEL); if (pipe) { - pipe->bufs = kzalloc(sizeof(struct pipe_buffer) * PIPE_DEF_BUFFERS, GFP_KERNEL); + unsigned long pipe_bufs = PIPE_DEF_BUFFERS; + struct user_struct *user = get_current_user(); + + if (!too_many_pipe_buffers_hard(user)) { + if (too_many_pipe_buffers_soft(user)) + pipe_bufs = 1; + pipe->bufs = kzalloc(sizeof(struct pipe_buffer) * pipe_bufs, GFP_KERNEL); + } + if (pipe->bufs) { init_waitqueue_head(&pipe->wait); pipe->r_counter = pipe->w_counter = 1; - pipe->buffers = PIPE_DEF_BUFFERS; + pipe->buffers = pipe_bufs; + pipe->user = user; + account_pipe_buffers(pipe, 0, pipe_bufs); mutex_init(&pipe->mutex); return pipe; } + free_uid(user); kfree(pipe); } @@ -607,6 +642,8 @@ void free_pipe_info(struct pipe_inode_info *pipe) { int i; + account_pipe_buffers(pipe, pipe->buffers, 0); + free_uid(pipe->user); for (i = 0; i < pipe->buffers; i++) { struct pipe_buffer *buf = pipe->bufs + i; if (buf->ops) @@ -998,6 +1035,7 @@ static long pipe_set_size(struct pipe_inode_info *pipe, unsigned long nr_pages) memcpy(bufs + head, pipe->bufs, tail * sizeof(struct pipe_buffer)); } + account_pipe_buffers(pipe, pipe->buffers, nr_pages); pipe->curbuf = 0; kfree(pipe->bufs); pipe->bufs = bufs; @@ -1069,6 +1107,11 @@ long pipe_fcntl(struct file *file, unsigned int cmd, unsigned long arg) if (!capable(CAP_SYS_RESOURCE) && size > pipe_max_size) { ret = -EPERM; goto out; + } else if ((too_many_pipe_buffers_hard(pipe->user) || + too_many_pipe_buffers_soft(pipe->user)) && + !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN)) { + ret = -EPERM; + goto out; } ret = pipe_set_size(pipe, nr_pages); break; diff --git a/include/linux/pipe_fs_i.h b/include/linux/pipe_fs_i.h index eb8b8ac..7878980 100644 --- a/include/linux/pipe_fs_i.h +++ b/include/linux/pipe_fs_i.h @@ -57,6 +57,7 @@ struct pipe_inode_info { struct fasync_struct *fasync_readers; struct fasync_struct *fasync_writers; struct pipe_buffer *bufs; + struct user_struct *user; }; /* @@ -123,6 +124,8 @@ void pipe_unlock(struct pipe_inode_info *); void pipe_double_lock(struct pipe_inode_info *, struct pipe_inode_info *); extern unsigned int pipe_max_size, pipe_min_size; +extern unsigned long pipe_user_pages_hard; +extern unsigned long pipe_user_pages_soft; int pipe_proc_fn(struct ctl_table *, int, void __user *, size_t *, loff_t *); diff --git a/include/linux/sched.h b/include/linux/sched.h index 21a6e96..1c0193b 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -831,6 +831,7 @@ struct user_struct { #endif unsigned long locked_shm; /* How many pages of mlocked shm ? */ unsigned long unix_inflight; /* How many files in flight in unix sockets */ + atomic_long_t pipe_bufs; /* how many pages are allocated in pipe buffers */ #ifdef CONFIG_KEYS struct key *uid_keyring; /* UID specific keyring */ diff --git a/kernel/sysctl.c b/kernel/sysctl.c index dc6858d..2781141 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -1735,6 +1735,20 @@ static struct ctl_table fs_table[] = { .proc_handler = &pipe_proc_fn, .extra1 = &pipe_min_size, }, + { + .procname = "pipe-user-pages-hard", + .data = &pipe_user_pages_hard, + .maxlen = sizeof(pipe_user_pages_hard), + .mode = 0644, + .proc_handler = proc_doulongvec_minmax, + }, + { + .procname = "pipe-user-pages-soft", + .data = &pipe_user_pages_soft, + .maxlen = sizeof(pipe_user_pages_soft), + .mode = 0644, + .proc_handler = proc_doulongvec_minmax, + }, { } };
On no-so-small systems, it is possible for a single process to cause an OOM condition by filling large pipes with data that are never read. A typical process filling 4000 pipes with 1 MB of data will use 4 GB of memory. On small systems it may be tricky to set the pipe max size to prevent this from happening. This patch makes it possible to enforce a per-user soft limit above which new pipes will be limited to a single page, effectively limiting them to 4 kB each, as well as a hard limit above which no new pipes may be created for this user. This has the effect of protecting the system against memory abuse without hurting other users, and still allowing pipes to work correctly though with less data at once. The limit are controlled by two new sysctls : pipe-user-pages-soft, and pipe-user-pages-hard. Both may be disabled by setting them to zero. The default soft limit allows the default number of FDs per process (1024) to create pipes of the default size (64kB), thus reaching a limit of 64MB before starting to create only smaller pipes. With 256 processes limited to 1024 FDs each, this results in 1024*64kB + (256*1024 - 1024) * 4kB = 1084 MB of memory allocated for a user. The hard limit is disabled by default to avoid breaking existing applications that make intensive use of pipes (eg: for splicing). Reported-by: socketpair@gmail.com Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Mitigates: CVE-2013-4312 (Linux 2.0+) Suggested-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Willy Tarreau <w@1wt.eu> --- This one cleans up the capable() test as suggested by Tetsuo Handa, and includes the hard-limit control as well in order to properly protect the system against large numbers of processes, which shows a clear improvement in tests. Linus I think this one is ready for merging, I don't know if you want to pick it yourself or let it flow via Al. Either way is fine to me. v3: - changed the test so that capable(CAP_SYS_ADMIN) is only made if necessary - renamed the sysctl pipe-user-pages-soft - added a new sysctl to enforce a hard limit v2: - added && !capable(CAP_SYS_ADMIN) - removed EXPORT_SYMBOL(user_max_pipe_pages) - added reported-by + mitigates, explain better example. --- Documentation/sysctl/fs.txt | 23 ++++++++++++++++++++++ fs/pipe.c | 47 +++++++++++++++++++++++++++++++++++++++++++-- include/linux/pipe_fs_i.h | 3 +++ include/linux/sched.h | 1 + kernel/sysctl.c | 14 ++++++++++++++ 5 files changed, 86 insertions(+), 2 deletions(-)