From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751863AbcFNPkq (ORCPT ); Tue, 14 Jun 2016 11:40:46 -0400 Received: from mail-wm0-f67.google.com ([74.125.82.67]:36433 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751320AbcFNPkp (ORCPT ); Tue, 14 Jun 2016 11:40:45 -0400 From: Topi Miettinen Subject: Re: [RFC 11/18] limits: track and present RLIMIT_NPROC actual max To: Jann Horn References: <1465847065-3577-1-git-send-email-toiwoton@gmail.com> <1465847065-3577-12-git-send-email-toiwoton@gmail.com> <20160613222719.GA3397@pc.thejh.net> Cc: linux-kernel@vger.kernel.org, Ingo Molnar , Peter Zijlstra , Andrew Morton , Kees Cook , Al Viro , Alexey Dobriyan , John Stultz , Janis Danisevskis , Calvin Owens , Tejun Heo , Michal Hocko , Oleg Nesterov , Vladimir Davydov , Andrea Arcangeli , Josh Triplett , "Eric W. Biederman" , Aleksa Sarai , Cyrill Gorcunov , Ben Segall , Mateusz Guzik Openpgp: id=A0F2EB0D8452DA908BEC8E911CF9ADDBD610E936 Message-ID: <3eb3040e-8f73-60d3-f9bf-0c9eaf9ffce1@gmail.com> Date: Tue, 14 Jun 2016 15:40:35 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.1.0 MIME-Version: 1.0 In-Reply-To: <20160613222719.GA3397@pc.thejh.net> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06/13/16 22:27, Jann Horn wrote: > On Mon, Jun 13, 2016 at 10:44:18PM +0300, Topi Miettinen wrote: >> Track maximum number of processes per user and present it >> in /proc/self/limits. >> >> Signed-off-by: Topi Miettinen >> --- >> fs/proc/base.c | 4 ++++ >> include/linux/sched.h | 1 + >> kernel/fork.c | 5 +++++ >> kernel/sys.c | 5 +++++ >> 4 files changed, 15 insertions(+) >> >> diff --git a/fs/proc/base.c b/fs/proc/base.c >> index 1df4fc8..02576c6 100644 >> --- a/fs/proc/base.c >> +++ b/fs/proc/base.c >> @@ -670,6 +670,10 @@ static int proc_pid_limits(struct seq_file *m, struct pid_namespace *ns, >> seq_printf(m, "%-20lu\n", psecs); >> } >> break; >> + case RLIMIT_NPROC: >> + seq_printf(m, "%-20d\n", >> + atomic_read(&task->real_cred->user->max_processes)); > > Don't you have to take an RCU read lock before dereferencing task->real_cred? In other comments in the series, cmpxchg loop was suggested, would that work here? > And shouldn't this be done with __task_cred(task) instead of task->real_cred? How about atomic_read(task_cred_xxx(task, user)->max_processes)? > > >> + break; >> default: >> seq_printf(m, "%-20lu\n", >> task->signal->rlim_curmax[i]); >> diff --git a/include/linux/sched.h b/include/linux/sched.h >> index 0150380..feb9bb7 100644 >> --- a/include/linux/sched.h >> +++ b/include/linux/sched.h >> @@ -838,6 +838,7 @@ static inline int signal_group_exit(const struct signal_struct *sig) >> struct user_struct { >> atomic_t __count; /* reference count */ >> atomic_t processes; /* How many processes does this user have? */ >> + atomic_t max_processes; /* How many processes has this user had at the same time? */ >> atomic_t sigpending; /* How many pending signals does this user have? */ >> #ifdef CONFIG_INOTIFY_USER >> atomic_t inotify_watches; /* How many inotify watches does this user have? */ >> diff --git a/kernel/fork.c b/kernel/fork.c >> index 5c2c355..667290f 100644 >> --- a/kernel/fork.c >> +++ b/kernel/fork.c >> @@ -1653,6 +1653,11 @@ static struct task_struct *copy_process(unsigned long clone_flags, >> trace_task_newtask(p, clone_flags); >> uprobe_copy_process(p, clone_flags); >> >> + if (atomic_read(&p->real_cred->user->max_processes) < >> + atomic_read(&p->real_cred->user->processes)) >> + atomic_set(&p->real_cred->user->max_processes, >> + atomic_read(&p->real_cred->user->processes)); >> + >> return p; >> >> bad_fork_cancel_cgroup: >> diff --git a/kernel/sys.c b/kernel/sys.c >> index 6629f6f..955cf21 100644 >> --- a/kernel/sys.c >> +++ b/kernel/sys.c >> @@ -439,6 +439,11 @@ static int set_user(struct cred *new) >> else >> current->flags &= ~PF_NPROC_EXCEEDED; >> >> + if (atomic_read(&new_user->max_processes) < >> + atomic_read(&new_user->processes)) >> + atomic_set(&new_user->max_processes, >> + atomic_read(&new_user->processes)); >> + > > Is this intentionally slightly racy? If so, it might be nice to have a comment > here that documents that. > I'd suppose cmpxchg loop could be used to avoid races. -Topi