linux-kernel - Re: [RFC 11/18] limits: track and present RLIMIT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160614231554.GA18261@pc.thejh.net>
Date:	Wed, 15 Jun 2016 01:15:54 +0200
From:	Jann Horn <jann@...jh.net>
To:	Topi Miettinen <toiwoton@...il.com>
Cc:	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Kees Cook <keescook@...omium.org>,
	Al Viro <viro@...iv.linux.org.uk>,
	Alexey Dobriyan <adobriyan@...il.com>,
	John Stultz <john.stultz@...aro.org>,
	Janis Danisevskis <jdanis@...gle.com>,
	Calvin Owens <calvinowens@...com>, Tejun Heo <tj@...nel.org>,
	Michal Hocko <mhocko@...e.com>,
	Oleg Nesterov <oleg@...hat.com>,
	Vladimir Davydov <vdavydov@...tuozzo.com>,
	Andrea Arcangeli <aarcange@...hat.com>,
	Josh Triplett <josh@...htriplett.org>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Aleksa Sarai <cyphar@...har.com>,
	Cyrill Gorcunov <gorcunov@...nvz.org>,
	Ben Segall <bsegall@...gle.com>,
	Mateusz Guzik <mguzik@...hat.com>
Subject: Re: [RFC 11/18] limits: track and present RLIMIT_NPROC actual max

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 <toiwoton@...il.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;

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)