From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from thejh.net ([37.221.195.125]:46762 "EHLO thejh.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932090AbcISPbZ (ORCPT ); Mon, 19 Sep 2016 11:31:25 -0400 Date: Mon, 19 Sep 2016 17:31:21 +0200 From: Jann Horn To: Andy Lutomirski Cc: Ben Hutchings , Thomas Gleixner , Stephen Smalley , Andrew Morton , "security@kernel.org" , James Morris , Janis Danisevskis , Casey Schaufler , Roland McGrath , Kees Cook , Alexander Viro , LSM List , "Serge E. Hallyn" , "Eric . Biederman" , Paul Moore , Linux FS Devel , Oleg Nesterov , Benjamin LaHaise , Eric Paris , Seth Forshee , John Johansen Subject: Re: [PATCH 2/9] exec: turn self_exec_id into self_privunit_id Message-ID: <20160919153121.GD2903@pc.thejh.net> References: <1474211117-16674-1-git-send-email-jann@thejh.net> <1474211117-16674-3-git-send-email-jann@thejh.net> <1474222407.2428.2.camel@decadent.org.uk> <20160918183137.GA17170@pc.thejh.net> <20160918184507.GT10601@decadent.org.uk> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="AbQceqfdZEv+FvjW" Content-Disposition: inline In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: --AbQceqfdZEv+FvjW Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Sep 18, 2016 at 12:57:46PM -0700, Andy Lutomirski wrote: > On Sep 18, 2016 8:45 AM, "Ben Hutchings" wrote: > > > > 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 =3D 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 p= trace > > > 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 <=3D2^16, so in the wors= t 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 l= et's > > > disregard that), so 2^48 non-thread clone() calls should need over 10= 0 years. > > > > > > But I guess both the kernel and machines get faster - if you think th= e margin > > > might not be future-proof enough (or if you think I measured wrong an= d 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. > > >=20 > But how big can NR_CPUs get before this gets uncomfortable? >=20 > We could do: >=20 > struct luid { > u64 count: > unsigned cpu; > }; >=20 > (LUID =3D locally unique ID). >=20 > IIRC my draft PCID code does something similar to uniquely identify > mms. If I accidentally reused a PCID without a flush, everything > would explode. So I guess for generating a new LUID, I'd have to do something like this? struct luid new_luid; preempt_disable(); raw_cpu_add(luid_counters, 1); new_luid.count =3D raw_cpu_read(luid_counters, 1); new_luid.cpu =3D smp_processor_id(); preempt_enable(); Disabling preemption should be sufficient as long as nobody generates LUIDs =66rom IRQ context, right? --AbQceqfdZEv+FvjW Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJX4ATJAAoJED4KNFJOeCOos2UQAJ3YdzU+aT4eH398WOM5/mGn ak+as70Z4AsmpoNj6ReChV7C5/qjdQ0g8gY5GPriyEB0nrxF4YNYYQd1rKlwtOlX ika2pRRQljB/Lk3STWZjrONKQgtM+htqNaVAHKJUCgovA/gBoXTQxKW1U0sHrHxl A8kIoa2MzXVRSy4OYWm9znmYnfYuoYGwjvNk39aDv7QS5DlsoQz682qx0E6hwirl JBYeoQyfXPYnBxVrUFPqi9h6Fsmu7gDzTfvzM6WoMDwo+He22IL22HWB7Ze0P5OP T9OW2UpRv6dkTuY31awZ9sFguvLNCvADu+Gcu1QALHQnXXkXXUMjUWmcam/kNRAF ftNEZEfvLTXMJhK7uJllgtztCg+wRx6w1GMx04drPy/K38R4M4PpcsW8CBUEtOe/ IThkEtwzOMCMDCKOZcIPDUHGM4FgC+riuYD6g0tjQO0IbH9g+mBp0HSzhgnyOEAg zzNHlfXpF65TbEoP1luzqmpLZ+F4I41xz2N78E5wRQD17vWBoeyy+FmhASOWEOov K1DWrbQKN2dqqXNIQ0wLPQRMDYugGooJcMPe3SpbbgciFxEEX/ul9tVitVL9WMkt arR45ni/HOkParlXntkg+qR2y7c0UYaYOmTNeTA5ZdgftkeEeeTrQZpfZj18oaO7 CoXl69WHCLoE6rdIo9+n =Y/TC -----END PGP SIGNATURE----- --AbQceqfdZEv+FvjW--