From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from thejh.net ([37.221.195.125]:46566 "EHLO thejh.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759445AbcIRUiW (ORCPT ); Sun, 18 Sep 2016 16:38:22 -0400 Date: Sun, 18 Sep 2016 22:38:17 +0200 From: Jann Horn To: Andy Lutomirski Cc: Thomas Gleixner , Stephen Smalley , Andrew Morton , "security@kernel.org" , James Morris , Janis Danisevskis , Casey Schaufler , Kees Cook , Roland McGrath , 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 7/9] ptrace: forbid ptrace checks against current_cred() from VFS context Message-ID: <20160918203817.GA2903@pc.thejh.net> References: <1474211117-16674-1-git-send-email-jann@thejh.net> <1474211117-16674-8-git-send-email-jann@thejh.net> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="zYM0uCDKw75PZbzx" Content-Disposition: inline In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: --zYM0uCDKw75PZbzx Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Sep 18, 2016 at 12:57:57PM -0700, Andy Lutomirski wrote: > On Sep 18, 2016 5:05 AM, "Jann Horn" wrote: > > > > This ensures that VFS implementations don't call ptrace_may_access() fr= om > > VFS read or write handlers. In order for file descriptor passing to have > > its intended security properties, VFS read/write handlers must not do a= ny > > kind of privilege checking. > > >=20 > Ooh, nifty! Can you warn about capable() too? >=20 > Warning about all access to current->cred could be fun. I expect we > have zillions of these bugs. Think keys, netlink, proc, etc. True, that would probably spam dmesg quite a bit. %pK under printk() is pretty nonsensical (heh, maybe we even have code doing capable() checks in IRQ context via printk()), then there are all those broken capable() checks for root-only sysctls and so on. For capable(), I've been thinking about adopting Linus' old suggestion of simply overriding the creds on VFS read/write entry. If we make current->cred non-refcounted and task-private (no RCU-safe pointer assignment), it's going to basically be two extra pointer reads and writes per read/write call - even for such a hot code path, that should be fine IMO. (And if we don't want to do this for all read/write calls, we could at least do it for sysctls, that would already help quite a bit.) --zYM0uCDKw75PZbzx Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJX3vs4AAoJED4KNFJOeCOo/tEP/0ajr+sSo4jzF76F0tuiM1Ta 5iH+jQbfmuBhPY0C8/auZ0pFQ5VNt9Ion+8PC41JqecFB4B3vnBxlqksUdoO3/H9 03AmikN1LlrBRAMCE0JmPcFZuTLSO+wbxpDZNi3TxaoL4rkk9QY9CtjKnrCDvCh0 bgNI32ugKuQvAUgXj1Q7hr8f8safBcyilCpICual0H1A1/Sug7Es/y4Bvw3GG+Hy lACXe67MqpsUc+ueKEZZQCZlNsm/Y+htmzPQ7xkoQRjNxTxzRMcy60I+q/241lBR 8c52DY+Tzs+bsaNsASc6eq/oj9uUH7xbG4IDjgPrcZYDLhLz2S99iRxB/itSTUKG IvpqgXopKMbTxoRADs3KOUEb5bwhcPiuCXiMIwi0vQQ3zNK8VF8m4/pvBqcWkmKu zR/T1XGXpdqB6k62BjRmbTagmkxm7E7OxbcQrAC/FKGREivgFWU1f9wGbdkYyZ7r E2W5qLud3NiMGBO3R9cV4LV8IbqFnUMJn0sZhiqXoZXGqJqoNOzhKmbVLPPlBWuq f8ie41HO5X0Fa/RE5z77gxszcxXN2zwl7tT01rc7dChB7WUz1IUxuW5MvJSlEN+O qWsa3GvviolybjBHllWBNFjNJ7vF1kpcGQTHGyytWTCTiQMLIkR579ANBajuaqCD cn7bBrTRsGK8GqbaHp5e =sEiE -----END PGP SIGNATURE----- --zYM0uCDKw75PZbzx--