From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from thejh.net ([37.221.195.125]:35186 "EHLO thejh.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757414AbcKBVyV (ORCPT ); Wed, 2 Nov 2016 17:54:21 -0400 Date: Wed, 2 Nov 2016 22:54:16 +0100 From: Jann Horn To: Ben Hutchings Cc: Oleg Nesterov , Alexander Viro , Roland McGrath , John Johansen , James Morris , "Serge E. Hallyn" , Paul Moore , Stephen Smalley , Eric Paris , Casey Schaufler , Kees Cook , Andrew Morton , Janis Danisevskis , Seth Forshee , "Eric W. Biederman" , Thomas Gleixner , Benjamin LaHaise , Andy Lutomirski , Linus Torvalds , Krister Johansen , linux-fsdevel@vger.kernel.org, linux-security-module@vger.kernel.org, security@kernel.org Subject: Re: [PATCH v3 1/8] exec: introduce cred_guard_light Message-ID: <20161102215416.GH8196@pc.thejh.net> References: <1477863998-3298-1-git-send-email-jann@thejh.net> <1477863998-3298-2-git-send-email-jann@thejh.net> <20161102181806.GB1112@redhat.com> <20161102205011.GF8196@pc.thejh.net> <1478122733.29107.1.camel@decadent.org.uk> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="DesjdUuHQDwS2t4N" Content-Disposition: inline In-Reply-To: <1478122733.29107.1.camel@decadent.org.uk> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: --DesjdUuHQDwS2t4N Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Nov 02, 2016 at 03:38:53PM -0600, Ben Hutchings wrote: > On Wed, 2016-11-02 at 21:50 +0100, Jann Horn wrote: > > On Wed, Nov 02, 2016 at 07:18:06PM +0100, Oleg Nesterov wrote: > > > On 10/30, Jann Horn wrote: > > > >=20 > > > > This is a new per-threadgroup lock that can often be taken instead = of > > > > cred_guard_mutex and has less deadlock potential. I'm doing this be= cause > > > > Oleg Nesterov mentioned the potential for deadlocks, in particular = if a > > > > debugged task is stuck in execve, trying to get rid of a ptrace-sto= pped > > > > thread, and the debugger attempts to inspect procfs files of the de= bugged > > > > task. > > >=20 > > >=20 > > > Yes, but let me repeat that we need to fix this anyway. So I don't re= ally > > > understand why should we add yet another mutex. > >=20 > >=20 > > execve() only takes the new mutex immediately after de_thread(), so this > > problem shouldn't occur there. Basically, I think that I'm not making t= he > > problem worse with my patches this way. > >=20 > > I believe that it should be possible to convert most existing users of = the > > cred_guard_mutex to the new cred_guard_light - exceptions to that that I > > see are: > >=20 > > =A0- PTRACE_ATTACH > > =A0- SECCOMP_FILTER_FLAG_TSYNC (sets NO_NEW_PRIVS on remote task) > >=20 > > Beyond that, conceptually, the new cred_guard_light could also be turned > > into a read-write mutex to prevent deadlocks between its users (where > > execve would take it for writing and everyone else would take it for > > reading), but afaik the kernel doesn't have an implementation of > > read-write mutexes yet? > [...] >=20 > It does (rw_semaphore) Aren't those spinlocks? I don't think those would be appropriate here, considering that the cred_guard_light can be held during filesystem access during execve(). > but with PREEMPT_RT enabled they become simple > mutexes. =A0So I don't think we should rely on that to avoid deadlock. Well, I don't think it's really necessary - as far as I can tell, the only locking operations that would be executed with the cred_guard_light held would be taking the sighand lock, filesystem stuff and LSM stuff. --DesjdUuHQDwS2t4N Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJYGmCIAAoJED4KNFJOeCOoz4cP/2zVsl0PuEdKuRWTS1ZCpgRM egFRV0vUzBAqOHXNMu9OuhMzUggiG+FXJP/WfgqEoLOHdUh9OkHgxxyN80wK9LNm UYjveOo6DOQ4UQTFxh5IuIEz8wFwidYc+7UP4oKHc5mCdjsh1rUdqDGKX+ZW6ovP mck3ed6PN5NTW3KSBAAMTmp3vd/6SDyMnoQHbg3iNT0OSE9ZHhHVVVzZq1Yy9O9/ EeMI8jNCxt+lLhUqbqgmz7U9OGrm8FHXtHyGb0W0yiAppNHAvv/heMwQgfzBTfGc 4c87VRxycoJcFTSvoPiCo1s3bsJdQPnmq7g2PSoPK4FMuO5v0xns/OJMAzy7PRg6 J338PYgxP3qnuXLgkLR1NBOSnWKmG0SWaOtAlANTmZWXhr0vhaEwE+CB5U4ReeLw H3xkJx0Ahg/BThNhUGjH82VMs1WCnt9B4PoF1JH2uXBIWsjPzZ9UVUcxHv/TTgK3 qe9W+YDRW4v3jLFbjd3AmrgFd5THqg8SzYW6frP4tB+5WxA0K1iif2Zh+XuIWaGN KUMYlWhFirUJu67uxzkl/gX9tACEeSCib6HDWGl+js56fS5vViyO8Vo4ayTiSJKZ fUYGza3+mX8PU/xocD2yhz7h0URuxlt9jz1NXMz9Nt9lSXSLCwH0qwBtYETCSlU0 7fn1fs2tqJ+nzLFncl9M =2eJp -----END PGP SIGNATURE----- --DesjdUuHQDwS2t4N--