From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from thejh.net ([37.221.195.125]:46742 "EHLO thejh.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750750AbcISOcM (ORCPT ); Mon, 19 Sep 2016 10:32:12 -0400 Date: Mon, 19 Sep 2016 16:32:07 +0200 From: Jann Horn To: Stephen Smalley Cc: Alexander Viro , Roland McGrath , Oleg Nesterov , John Johansen , James Morris , "Serge E. Hallyn" , Paul Moore , Eric Paris , Casey Schaufler , Kees Cook , Andrew Morton , Janis Danisevskis , Seth Forshee , "Eric . Biederman" , Thomas Gleixner , Benjamin LaHaise , linux-fsdevel@vger.kernel.org, linux-security-module@vger.kernel.org, security@kernel.org Subject: Re: [PATCH 3/9] proc: use open()-time creds for ptrace checks Message-ID: <20160919143206.GC2903@pc.thejh.net> References: <1474211117-16674-1-git-send-email-jann@thejh.net> <1474211117-16674-4-git-send-email-jann@thejh.net> <4f10ddba-fb7c-04a3-0426-550b435e6f3a@tycho.nsa.gov> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="B4IIlcmfBL/1gGOG" Content-Disposition: inline In-Reply-To: <4f10ddba-fb7c-04a3-0426-550b435e6f3a@tycho.nsa.gov> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: --B4IIlcmfBL/1gGOG Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Sep 19, 2016 at 09:01:19AM -0400, Stephen Smalley wrote: > On 09/18/2016 11:05 AM, Jann Horn wrote: > > This adds a new ptrace_may_access_noncurrent() method that > > uses the supplied credentials instead of those of the > > current task. (However, the current task may still be > > inspected for auditing purposes, e.g. by the Smack LSM.) > >=20 > > procfs used the caller's creds for a few ptrace_may_access() > > checks at read() time, which made a confused deputy attack > > by passing an FD to a procfs file to a setuid program > > possible. Therefore, the following was a local userspace > > ASLR bypass: [...] > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > > index 13185a6..f9a0be7 100644 > > --- a/security/selinux/hooks.c > > +++ b/security/selinux/hooks.c > > @@ -2133,10 +2133,10 @@ static int selinux_binder_transfer_file(struct = task_struct *from, > > } > > =20 > > static int selinux_ptrace_access_check(struct task_struct *child, > > - unsigned int mode) > > + unsigned int mode, const struct cred *cred) > > { > > if (mode & PTRACE_MODE_READ) { > > - u32 sid =3D current_sid(); > > + u32 sid =3D cred_sid(cred); > > u32 csid =3D task_sid(child); > > return avc_has_perm(sid, csid, SECCLASS_FILE, FILE__READ, NULL); > > } >=20 > For consistency, don't you also need to change the next line of code to > use cred_has_perm() rather than current_has_perm()? Oh, right. How about something like this? u32 sid =3D cred_sid(cred); u32 csid =3D task_sid(child); if (mode & PTRACE_MODE_READ) return avc_has_perm(sid, csid, SECCLASS_FILE, FILE__READ, NULL); else return avc_has_perm(sid, csid, SECCLASS_PROCESS, PROCESS__PTRACE, NULL); The current code looks as if the PTRACE_MODE_READ and PTRACE_MODE_ATTACH ca= ses behave very differently, which they actually don't. And to be able to use cred_has_perm() here, it would be necessary to explicitly grab and release an RCU read lock or so (to prevent the child creds from going away). --B4IIlcmfBL/1gGOG Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJX3/bmAAoJED4KNFJOeCOoN2IP/3gE/LgMxlENR7hbN2Fox1vG gabXM97CPQRY3dw6/TkzsP5Uz/LdEoszrX2I323I7iicTz+T16IWlk0hIdhK9Xpw NKg2wedFkMPZahCIKmrT0387Iz9hRm/Cff5aVlgw1e1aMEhFzUzGgPaNxNe8aQOl GaJJBsmZqwBNvLFGCy3pZoyQjw8OeZIw9TdHSKJUVODvYm5oXJotBba/h0iHfjF/ to42iTxQuHP2NbQTBY9EC6sHsyi1uBPmjmT4gtWBglqkm3y5oIvtuF1VWHswdmTj CbGynly2e3XCZzO+Tb45NC89J1XGCBwBazjtixT5pEUlh/++5vaIT7bvwOuR++dS vef7iWuO8elIkj5GQs6bpPraYF0+IyzEEg8MYlwfSFl7N6wFpZ4qOSyi9WmviK3x r09ZI5xoiPYBNZFez925wTnIpAxBYk+eZLT9G9PG4qXC7siD2910iQ6EnLiAHFPe amqn7kVXPdyTVx3rBEZbkHJndhG3ygqKTzmJCz+6pTXdatUC7Pvz0lxxHJgtta6x WgT2rqXhVNhCmb2YFai+sdOxyuW90P4pXf3kNY8VkXaH9IzWw/QUI9AMUWwBKwRV b/nVwOScEdNDMiwQ4ZxQljrb50PRdWRtpwrTeVo/ndKyiDdYQIQbrzjPfwcGG/dL H+No6dN3qirR0ZHQ8BwM =f7vj -----END PGP SIGNATURE----- --B4IIlcmfBL/1gGOG--