lkml.org 
[lkml]   [2020]   [May]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 2/8] exec: Factor security_bprm_creds_for_exec out of security_bprm_set_creds
On Mon, May 18, 2020 at 07:30:10PM -0500, Eric W. Biederman wrote:
>
> Today security_bprm_set_creds has several implementations:
> apparmor_bprm_set_creds, cap_bprm_set_creds, selinux_bprm_set_creds,
> smack_bprm_set_creds, and tomoyo_bprm_set_creds.
>
> Except for cap_bprm_set_creds they all test bprm->called_set_creds and
> return immediately if it is true. The function cap_bprm_set_creds
> ignores bprm->calld_sed_creds entirely.
>
> Create a new LSM hook security_bprm_creds_for_exec that is called just
> before prepare_binprm in __do_execve_file, resulting in a LSM hook
> that is called exactly once for the entire of exec. Modify the bits
> of security_bprm_set_creds that only want to be called once per exec
> into security_bprm_creds_for_exec, leaving only cap_bprm_set_creds
> behind.
>
> Remove bprm->called_set_creds all of it's former users have been moved
> to security_bprm_creds_for_exec.
>
> Add or upate comments a appropriate to bring them up to date and
> to reflect this change.

Yup, awesome. One nit below.

Reviewed-by: Kees Cook <keescook@chromium.org>

> [...]
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 0b4e32161b77..718345dd76bb 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> [...]
> @@ -2297,8 +2297,6 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm)
>
> /* SELinux context only depends on initial program or script and not
> * the script interpreter */
> - if (bprm->called_set_creds)
> - return 0;
>
> old_tsec = selinux_cred(current_cred());
> new_tsec = selinux_cred(bprm->cred);

As you've done in the other LSMs, I think this comment can be removed
(or moved to the top of the function) too.

--
Kees Cook

\
 
 \ /
  Last update: 2020-05-19 20:10    [W:0.229 / U:0.180 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site