Re: [PATCH v3 1/3] LSM: add security_execve_abort() hook

[Paul Moore <paul@paul-moore.com>]

On Thu, Feb 15, 2024 at 9:33 AM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> 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