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@xxxxxxxxxxxx> > [...] > 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
