On Tue, Jun 14, 2016 at 03:40:35PM +0000, Topi Miettinen wrote: > 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 <toiwo...@gmail.com> > >> --- > >> 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?
What would a cmpxchg loop have to do with missing RCU locking?
> > 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)?
No. You'd still end up dereferencing max_processes in the user_struct without
any guarantee that it hasn't been freed. I think the code should look this way:
case RLIMIT_NPROC:
rcu_read_lock();
seq_printf(m, "%-20d\n",
atomic_read(&__task_cred(task)->user->max_processes));
rcu_read_unlock();
break;
signature.asc
Description: Digital signature
