Re: [PATCH 2/9] exec: turn self_exec_id into self_privunit_id

From: Ben Hutchings <ben@decadent.org.uk>
To: Jann Horn <jann@thejh.net>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>,
	Roland McGrath <roland@hack.frob.com>,
	Oleg Nesterov <oleg@redhat.com>,
	John Johansen <john.johansen@canonical.com>,
	James Morris <james.l.morris@oracle.com>,
	"Serge E. Hallyn" <serge@hallyn.com>,
	Paul Moore <aul@paul-moore.com>,
	Stephen Smalley <sds@tycho.nsa.gov>,
	Eric Paris <eparis@parisplace.org>,
	Casey Schaufler <casey@schaufler-ca.com>,
	Kees Cook <keescook@chromium.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Janis Danisevskis <jdanis@google.com>,
	Seth Forshee <seth.forshee@canonical.com>,
	"Eric . Biederman" <ebiederm@xmission.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Benjamin LaHaise <bcrl@kvack.org>,
	linux-fsdevel@vger.kernel.org,
	linux-security-module@vger.kernel.org, security@kernel.org
Subject: Re: [PATCH 2/9] exec: turn self_exec_id into self_privunit_id
Date: Sun, 18 Sep 2016 19:45:07 +0100	[thread overview]
Message-ID: <20160918184507.GT10601@decadent.org.uk> (raw)
In-Reply-To: <20160918183137.GA17170@pc.thejh.net>

[-- Attachment #1: Type: text/plain, Size: 2120 bytes --]

On Sun, Sep 18, 2016 at 08:31:37PM +0200, Jann Horn wrote:
> On Sun, Sep 18, 2016 at 07:13:27PM +0100, Ben Hutchings wrote:
> > On Sun, 2016-09-18 at 17:05 +0200, Jann Horn wrote:
> > > This ensures that self_privunit_id ("privilege unit ID") is only shared by
> > > processes that share the mm_struct and the signal_struct; not just
> > > spatially, but also temporally. In other words, if you do execve() or
> > > clone() without CLONE_THREAD, you get a new privunit_id that has never been
> > > used before.
> > [...]
> > > +void increment_privunit_counter(void)
> > > +{
> > > +	BUILD_BUG_ON(NR_CPUS > (1 << 16));
> > > +	current->self_privunit_id = this_cpu_add_return(exec_counter, NR_CPUS);
> > > +}
> > [...]
> > 
> > This will wrap incorrectly if NR_CPUS is not a power of 2 (which is
> > unusual but allowed).
> 
> If this wraps, hell breaks loose permission-wise - processes that have
> no relationship whatsoever with each other will suddenly be able to ptrace
> each other.
> 
> The idea is that it never wraps.

That's what I suspected, but wasn't sure.  In that case you can
initialise each counter to U64_MAX/NR_CPUS*cpu and increment by
1 each time, which might be more efficient on some architectures.

> It wraps after (2^64)/NR_CPUS execs or
> forks on one CPU core. NR_CPUS is bounded to <=2^16, so in the worst case,
> it wraps after 2^48 execs or forks.
> 
> On my system with 3.7GHz per core, 2^16 minimal sequential non-thread clone()
> calls need 1 second system time (and 2 seconds wall clock time, but let's
> disregard that), so 2^48 non-thread clone() calls should need over 100 years.
> 
> But I guess both the kernel and machines get faster - if you think the margin
> might not be future-proof enough (or if you think I measured wrong and it's
> actually much faster), I guess I could bump this to a 128bit number.

Sequential execution speed isn't likely to get significantly faster so
with those current numbers this seems to be quite safe.

Ben.

-- 
Ben Hutchings
Reality is just a crutch for people who can't handle science fiction.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]