Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933845AbdGTATk (ORCPT ); Wed, 19 Jul 2017 20:19:40 -0400 Received: from mail-lf0-f66.google.com ([209.85.215.66]:35420 "EHLO mail-lf0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932336AbdGTATh (ORCPT ); Wed, 19 Jul 2017 20:19:37 -0400 MIME-Version: 1.0 X-Originating-IP: [108.49.102.27] In-Reply-To: References: <1500416736-49829-1-git-send-email-keescook@chromium.org> <1500416736-49829-5-git-send-email-keescook@chromium.org> From: Paul Moore Date: Wed, 19 Jul 2017 20:19:35 -0400 Message-ID: Subject: Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook To: Kees Cook , Stephen Smalley Cc: Andrew Morton , David Howells , "Eric W. Biederman" , John Johansen , "Serge E. Hallyn" , Casey Schaufler , Tetsuo Handa , James Morris , Andy Lutomirski , Linus Torvalds , linux-fsdevel@vger.kernel.org, linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3409 Lines: 90 On Wed, Jul 19, 2017 at 8:03 PM, Paul Moore wrote: > On Tue, Jul 18, 2017 at 6:25 PM, Kees Cook wrote: >> The SELinux bprm_secureexec hook can be merged with the bprm_set_creds >> hook since it's dealing with the same information, and all of the details >> are finalized during the first call to the bprm_set_creds hook via >> prepare_binprm() (subsequent calls due to binfmt_script, etc, are ignored >> via bprm->called_set_creds). >> >> Here, the test can just happen at the end of the bprm_set_creds hook, >> and the bprm_secureexec hook can be dropped. >> >> Cc: Paul Moore >> Cc: Stephen Smalley >> Signed-off-by: Kees Cook >> --- >> security/selinux/hooks.c | 24 +++++------------------- >> 1 file changed, 5 insertions(+), 19 deletions(-) > > This seems reasonable in the context of the other changes. > > Stephen just posted an AT_SECURE test for the selinux-testsuite on the > SELinux mailing list, it would be nice to ensure that this patchset > doesn't run afoul of that. Quick follow-up: I just merged Stephen's test into the test suite: * https://github.com/SELinuxProject/selinux-testsuite > Acked-by: Paul Moore > >> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c >> index 0f1450a06b02..18038f73a2f7 100644 >> --- a/security/selinux/hooks.c >> +++ b/security/selinux/hooks.c >> @@ -2413,30 +2413,17 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm) >> >> /* Clear any possibly unsafe personality bits on exec: */ >> bprm->per_clear |= PER_CLEAR_ON_SETID; >> - } >> - >> - return 0; >> -} >> - >> -static int selinux_bprm_secureexec(struct linux_binprm *bprm) >> -{ >> - const struct task_security_struct *tsec = current_security(); >> - u32 sid, osid; >> - int atsecure = 0; >> - >> - sid = tsec->sid; >> - osid = tsec->osid; >> >> - if (osid != sid) { >> /* Enable secure mode for SIDs transitions unless >> the noatsecure permission is granted between >> the two SIDs, i.e. ahp returns 0. */ >> - atsecure = avc_has_perm(osid, sid, >> - SECCLASS_PROCESS, >> - PROCESS__NOATSECURE, NULL); >> + rc = avc_has_perm(old_tsec->sid, new_tsec->sid, >> + SECCLASS_PROCESS, PROCESS__NOATSECURE, >> + NULL); >> + bprm->secureexec |= !!rc; >> } >> >> - return !!atsecure; >> + return 0; >> } >> >> static int match_file(const void *p, struct file *file, unsigned fd) >> @@ -6151,7 +6138,6 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { >> LSM_HOOK_INIT(bprm_set_creds, selinux_bprm_set_creds), >> LSM_HOOK_INIT(bprm_committing_creds, selinux_bprm_committing_creds), >> LSM_HOOK_INIT(bprm_committed_creds, selinux_bprm_committed_creds), >> - LSM_HOOK_INIT(bprm_secureexec, selinux_bprm_secureexec), >> >> LSM_HOOK_INIT(sb_alloc_security, selinux_sb_alloc_security), >> LSM_HOOK_INIT(sb_free_security, selinux_sb_free_security), >> -- >> 2.7.4 > > -- > paul moore > www.paul-moore.com -- paul moore www.paul-moore.com