Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757657AbcKCNSz (ORCPT ); Thu, 3 Nov 2016 09:18:55 -0400 Received: from smtp.nsa.gov ([8.44.101.8]:58743 "EHLO emsm-gh1-uea10.nsa.gov" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755927AbcKCNSx (ORCPT ); Thu, 3 Nov 2016 09:18:53 -0400 X-IronPort-AV: E=Sophos;i="5.31,438,1473120000"; d="scan'208";a="569544" IronPort-PHdr: =?us-ascii?q?9a23=3Awa2yWB33S57Ft4+ysmDT+DRfVm0co7zxezQtwd8Z?= =?us-ascii?q?seweKfad9pjvdHbS+e9qxAeQG96KsbQU1KGM4+jJYi8p2d65qncMcZhBBVcuqP?= =?us-ascii?q?49uEgeOvODElDxN/XwbiY3T4xoXV5h+GynYwAOQJ6tL2PbrnD61zMOABK3bVMz?= =?us-ascii?q?fbWvXN6NxJ7nn8mJuLTrKz1SgzS8Zb4gZD6Xli728vcsvI15N6wqwQHIqHYbM8?= =?us-ascii?q?5fxGdvOE7B102kvpT41NdZ/i9Ro/Ms8dJbGeW/JvxgDO9lFjBuD0QZrI2u70GC?= =?us-ascii?q?HkOz4S4+W2MQ2jpPGQ6NuBPzWJHZriv3tOo73iSGa4m+Vr0/RC6j87ZDSxLyji?= =?us-ascii?q?oDcTkj/yWfo8h9nKtdrB+77yJ+2YmcNJ+ULv1WbK7bfM1cQWtHQ9YXUDZORJ6/?= =?us-ascii?q?Oc9HN+McOa59qI7nqhNatRKjASG0Df7rjzpPgWX7m6Y91rJlWSzc3QdoJ9sUsW?= =?us-ascii?q?+c+NjtPb0TSsitxbPJ1i3HZvhbnzDn596MOjIopPyXFZd3a9DQ0gF7FQrAg07W?= =?us-ascii?q?rcrgOCmP1/8ltHKS5O5tE+mojjhj40tKryKgy48BzMHpj4YR21aOvXFizZw6KP?= =?us-ascii?q?W4QUp/cNjiG5xV4WXSMoB2RcUta25vvyk+x/sNvpv/NAEMxIVv4wPDbPmGaZOL?= =?us-ascii?q?41q3UPuNJh9xgXtucaq+mx+2t06t1ru4Huiy31ECiy1BlNDW/iQI1hrc7eCER+?= =?us-ascii?q?F780Pn3iyAgUSbzeVJLggSmLHHJoQm3PZkkZ4evmzZEyP2kVmwh6iTIAFs3Omj?= =?us-ascii?q?6KzEeLziq4GdPI883gf4MYwhncuwBel+OQ8LCSzT2+261aar0UT/Qa5HkOY1k7?= =?us-ascii?q?KR5JLWLMIavYa2BAha1otl4BG6WWSIytMdyEIbIUpFdRTPtI3gP1XDMbisFvuk?= =?us-ascii?q?q0i9mzdsgfbdN/vuBYubfSuLq6voYbsosx0U8wE0190KossOUrw=3D?= X-IPAS-Result: =?us-ascii?q?A2EwBAA1OBtY/wHyM5BdGgEBAQECAQEBAQgBAQEBFQEBAQE?= =?us-ascii?q?CAQEBAQgBAQEBgwUBAQEBAR+BAlKjFAEBBoEbkjiEF4YiAoIlUwEBAQEBAQEBA?= =?us-ascii?q?gECXyiCMwQBFQEEghABAQQjBFIQCw0LAgImAgJXBgEMBgIBAYhSrgyNAwEBAQE?= =?us-ascii?q?BAQEDAQEBAQEBIYEJhGuCSIJYhBeDNIJcBY5Ri0+QPYoMhX6NHYQEVVkJCYMmH?= =?us-ascii?q?IF5IjSFP26BTQEBAQ?= Subject: Re: [PATCH v3 3/3] selinux: require EXECMEM for forced ptrace poke To: Jann Horn , security@kernel.org, Alexander Viro , Paul Moore , Eric Paris , James Morris , "Serge E. Hallyn" , mchong@google.com, Andy Lutomirski , Ingo Molnar , Oleg Nesterov , Nick Kralevich , Janis Danisevskis References: <1478142286-18427-1-git-send-email-jann@thejh.net> <1478142286-18427-6-git-send-email-jann@thejh.net> Cc: linux-security-module@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org From: Stephen Smalley Organization: National Security Agency Message-ID: <7b880f9b-63ac-baa6-e4ac-f751afcaffa2@tycho.nsa.gov> Date: Thu, 3 Nov 2016 09:21:34 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <1478142286-18427-6-git-send-email-jann@thejh.net> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4644 Lines: 118 On 11/02/2016 11:04 PM, Jann Horn wrote: > This restricts forced writes to private R+X mappings using the EXECMEM > permission. To avoid a breaking change, a new policy capability needs to > be enabled before the new restrictions take effect. > > Unlike most other SELinux hooks, this one takes the subject credentials as > an argument instead of looking up current_cred(). This is done because the > security_forced_write() LSM hook can be invoked from within the write > handler of /proc/$pid/mem, where current_cred() is pretty useless. > > Changed in v3: > - minor: symmetric comment (Ingo Molnar) > - use helper struct (Ingo Molnar) > - add new policy capability for enabling forced write checks > (Stephen Smalley) > > Signed-off-by: Jann Horn > --- > security/selinux/hooks.c | 15 +++++++++++++++ > security/selinux/include/security.h | 2 ++ > security/selinux/selinuxfs.c | 3 ++- > security/selinux/ss/services.c | 3 +++ > 4 files changed, 22 insertions(+), 1 deletion(-) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 09fd6108e421..cdd9c53db2ed 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -2144,6 +2144,20 @@ static int selinux_ptrace_traceme(struct task_struct *parent) > return task_has_perm(parent, current, PROCESS__PTRACE); > } > > +static int selinux_forced_write(struct vm_area_struct *vma, > + const struct gup_creds *creds) > +{ > + /* > + * Permitting a write to readonly memory is fine - making the readonly > + * memory executable afterwards would require EXECMOD permission because > + * anon_vma would be non-NULL. > + */ > + if (!selinux_policycap_forcedwrite || (vma->vm_flags & VM_EXEC) == 0) > + return 0; > + > + return cred_has_perm(creds->subject, creds->object, PROCESS__EXECMEM); > +} > + > static int selinux_capget(struct task_struct *target, kernel_cap_t *effective, > kernel_cap_t *inheritable, kernel_cap_t *permitted) > { > @@ -6085,6 +6099,7 @@ static struct security_hook_list selinux_hooks[] = { > > LSM_HOOK_INIT(ptrace_access_check, selinux_ptrace_access_check), > LSM_HOOK_INIT(ptrace_traceme, selinux_ptrace_traceme), > + LSM_HOOK_INIT(forced_write, selinux_forced_write), > LSM_HOOK_INIT(capget, selinux_capget), > LSM_HOOK_INIT(capset, selinux_capset), > LSM_HOOK_INIT(capable, selinux_capable), > diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h > index 308a286c6cbe..87228f0ff09c 100644 > --- a/security/selinux/include/security.h > +++ b/security/selinux/include/security.h > @@ -71,6 +71,7 @@ enum { > POLICYDB_CAPABILITY_OPENPERM, > POLICYDB_CAPABILITY_REDHAT1, > POLICYDB_CAPABILITY_ALWAYSNETWORK, > + POLICYDB_CAPABILITY_FORCEDWRITE, > __POLICYDB_CAPABILITY_MAX > }; > #define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1) > @@ -78,6 +79,7 @@ enum { > extern int selinux_policycap_netpeer; > extern int selinux_policycap_openperm; > extern int selinux_policycap_alwaysnetwork; > +extern int selinux_policycap_forcedwrite; > > /* > * type_datum properties > diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c > index 72c145dd799f..a646cb801242 100644 > --- a/security/selinux/selinuxfs.c > +++ b/security/selinux/selinuxfs.c > @@ -46,7 +46,8 @@ static char *policycap_names[] = { > "network_peer_controls", > "open_perms", > "redhat1", > - "always_check_network" > + "always_check_network", > + "forced_write" This is a nit, but can you provide a more descriptive capability name that would be meaningful to policy writers and signifies that this policy capability enables checking execmem in these situations? > }; > > unsigned int selinux_checkreqprot = CONFIG_SECURITY_SELINUX_CHECKREQPROT_VALUE; > diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c > index 082b20c78363..4017810030d6 100644 > --- a/security/selinux/ss/services.c > +++ b/security/selinux/ss/services.c > @@ -73,6 +73,7 @@ > int selinux_policycap_netpeer; > int selinux_policycap_openperm; > int selinux_policycap_alwaysnetwork; > +int selinux_policycap_forcedwrite; > > static DEFINE_RWLOCK(policy_rwlock); > > @@ -1990,6 +1991,8 @@ static void security_load_policycaps(void) > POLICYDB_CAPABILITY_OPENPERM); > selinux_policycap_alwaysnetwork = ebitmap_get_bit(&policydb.policycaps, > POLICYDB_CAPABILITY_ALWAYSNETWORK); > + selinux_policycap_forcedwrite = ebitmap_get_bit(&policydb.policycaps, > + POLICYDB_CAPABILITY_FORCEDWRITE); > } > > static int security_preserve_bools(struct policydb *p); >