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.
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/fs/exec.c b/fs/exec.c
index af4fbb61cd53..cbee988445c6 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1530,6 +1530,7 @@ static void free_bprm(struct linux_binprm *bprm)
kfree(bprm->interp);
kfree(bprm->fdpath);
kfree(bprm);
+ security_bprm_free();
}
static struct linux_binprm *alloc_bprm(int fd, struct filename *filename, int flags)
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 76458b6d53da..0ef298231de2 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -54,6 +54,7 @@ LSM_HOOK(int, 0, bprm_creds_from_file, struct linux_binprm *bprm, const struct f
LSM_HOOK(int, 0, bprm_check_security, struct linux_binprm *bprm)
LSM_HOOK(void, LSM_RET_VOID, bprm_committing_creds, const struct linux_binprm *bprm)
LSM_HOOK(void, LSM_RET_VOID, bprm_committed_creds, const struct linux_binprm *bprm)
+LSM_HOOK(void, LSM_RET_VOID, bprm_free, void)
LSM_HOOK(int, 0, fs_context_submount, struct fs_context *fc, struct super_block *reference)
LSM_HOOK(int, 0, fs_context_dup, struct fs_context *fc,
struct fs_context *src_sc)
diff --git a/include/linux/security.h b/include/linux/security.h
index d0eb20f90b26..b12d20071a8f 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -299,6 +299,7 @@ int security_bprm_creds_from_file(struct linux_binprm *bprm, const struct file *
int security_bprm_check(struct linux_binprm *bprm);
void security_bprm_committing_creds(const struct linux_binprm *bprm);
void security_bprm_committed_creds(const struct linux_binprm *bprm);
+void security_bprm_free(void);
int security_fs_context_submount(struct fs_context *fc, struct super_block *reference);
int security_fs_context_dup(struct fs_context *fc, struct fs_context *src_fc);
int security_fs_context_parse_param(struct fs_context *fc, struct fs_parameter *param);
@@ -648,6 +649,10 @@ static inline void security_bprm_committed_creds(const struct linux_binprm *bprm
{
}
+static inline void security_bprm_free(void)
+{
+}
+
static inline int security_fs_context_submount(struct fs_context *fc,
struct super_block *reference)
{
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()
+ *
+ * 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
