On 2024/02/07 9:00, Paul Moore wrote:

quoted

@@ -1223,6 +1223,17 @@ void security_bprm_committed_creds(const struct linux_binprm *bprm)
 	call_void_hook(bprm_committed_creds, bprm);
 }
 
+/**
+ * security_execve_abort() - Notify that exec() has failed
+ *
+ * This hook is for undoing changes which cannot be discarded by
+ * abort_creds().
+ */
+void security_execve_abort(void)
+{
+	call_void_hook(execve_abort);
+}

I don't have a problem with reinstating something like
security_bprm_free(), but I don't like the name security_execve_abort(),
especially given that it is being called from alloc_bprm() as well as
all of the execve code.  At the risk of bikeshedding this, I'd be much
happier if this hook were renamed to security_bprm_free() and the
hook's description explained that 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.

Are you okay with that?

Hmm, that will bring us back to v1 of this series.

v3 was based on Eric W. Biederman's suggestion

  If you aren't going to change your design your new hook should be:
          security_execve_revert(current);
  Or maybe:
          security_execve_abort(current);

  At least then it is based upon the reality that you plan to revert
  changes to current->security.  Saying anything about creds or bprm when
  you don't touch them, makes no sense at all.  Causing people to
  completely misunderstand what is going on, and making it more likely
  they will change the code in ways that will break TOMOYO.

at https://lkml.kernel.org/r/8734ug9fbt.fsf@email.froward.int.ebiederm.org .