On Thu, Feb 15, 2024 at 9:33 AM Tetsuo Handa
<penguin-kernel@xxxxxxxxxxxxxxxxxxx> wrote:
> On 2024/02/15 6:46, Paul Moore wrote:
> >> To quickly summarize, there are two paths forward that I believe are
> >> acceptable from a LSM perspective, pick either one and send me an
> >> updated patchset.
> >>
> >> 1. Rename the hook to security_bprm_free() and update the LSM hook
> >> description as I mentioned earlier in this thread.
> >>
> >> 2. Rename the hook to security_execve_revert(), move it into the
> >> execve related functions, and update the LSM hook description to
> >> reflect that this hook is for reverting execve related changes to the
> >> current task's internal LSM state beyond what is possible via the
> >> credential hooks.
> >
> > Hi Tetsuo, I just wanted to check on this and see if you've been able
> > to make any progress?
> >
>
> I'm fine with either approach. Just worrying that someone doesn't like
> overhead of unconditionally calling security_bprm_free() hook.
> If everyone is fine with below one, I'll post v4 patchset.
My guess is that based on the previous comments people are going to
prefer option #2 above, but we'll see what everyone says. I did have
one comment, below ...
> fs/exec.c | 1 +
> include/linux/lsm_hook_defs.h | 1 +
> include/linux/security.h | 5 +++++
> security/security.c | 12 ++++++++++++
> 4 files changed, 19 insertions(+)
...
> diff --git a/security/security.c b/security/security.c
> index 3aaad75c9ce8..ba2f480b2bdb 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1223,6 +1223,18 @@ void security_bprm_committed_creds(const struct linux_binprm *bprm)
> call_void_hook(bprm_committed_creds, bprm);
> }
>
> +/**
> + * security_bprm_free() - Notify of completion of an exec()
The short summary above doesn't match the summary below. If we stick
with the security_bprm_free() approach please change "Notify of
completion of an exec()" to "Notify LSMs of a bprm free event" or
similar.
> + * This hook is called when a linux_bprm instance is being destroyed, after
> + * the bprm creds have been released, and is intended to cleanup any internal
> + * LSM state associated with the linux_bprm instance.
> + */
> +void security_bprm_free(void)
> +{
> + call_void_hook(bprm_free);
> +}
> +
> /**
> * security_fs_context_submount() - Initialise fc->security
> * @fc: new filesystem context
>
--
paul-moore.com
