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

[Tetsuo Handa]

On 2024/02/07 9:00, Paul Moore wrote:
>> @@ -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 .