Re: [RFC][PATCH] exec: Move cred computation under exec_update_lock

["Eric W. Biederman" <ebiederm@xmission.com>]

Bernd Edlinger <bernd.edlinger@hotmail.de> writes:

> Hi Eric,
>
> thanks for you valuable input on the topic.
>
> On 11/21/25 00:50, Eric W. Biederman wrote:
>> "Eric W. Biederman" <ebiederm@xmission.com> writes:
>> 
>>> Instead of computing the new cred before we pass the point of no
>>> return compute the new cred just before we use it.
>>>
>>> This allows the removal of fs_struct->in_exec and cred_guard_mutex.
>>>
>>> I am not certain why we wanted to compute the cred for the new
>>> executable so early.  Perhaps I missed something but I did not see any
>>> common errors being signaled.   So I don't think we loose anything by
>>> computing the new cred later.
>> 
>> I should add that the permission checks happen in open_exec,
>> everything that follows credential wise is just about representing in
>> struct cred the credentials the new executable will have.
>> 
>> So I am really at a loss why we have had this complicated way of
>> computing of computed the credentials all of these years full of
>> time of check to time of use problems.
>> 
>
> Well, I think I see a problem with your patch:
>
> When the security engine gets the LSM_UNSAFE_PTRACE flag, it might
> e.g. return -EPERM in bprm_creds_for_exec in the apparmor, selinux
> or the smack security engines at least.  Previously that callback
> was called before the point of no return, and the return code should
> be returned as a return code the the caller of execve.  But if we move
> that check after the point of no return, the caller will get killed
> due to the failed security check.
>
> Or did I miss something?

I think we definitely need to document this change in behavior.  I would
call ending the exec with SIGSEGV vs -EPERM a quality of implementation
issue.  The exec is failing one way or the other so I don't see it as a
correctness issue.

In the case of ptrace in general I think it is a bug if the mere act of
debugging a program changes it's behavior.  So which buggy behavior
should we prefer?  SIGSEGV where it is totally clear that the behavior
has changed or -EPERM and ask the debugged program to handle it.
I lean towards SIGSEGV because then it is clear the code should not
handle it.

In the case of LSM_UNSAFE_NO_NEW_PRIVS I believe the preferred way to
handle unexpected things happening is to terminate the application.

In the case of LSM_UNSAFE_SHARE -EPERM might be better.  I don't know
of any good uses of any good uses of sys_clone(CLONE_FS ...) outside
of CLONE_THREAD.


Plus all of these things are only considerations if we are exec'ing a
program that transitions to a different set of credentials.  Something
that happens but is quite rare itself.

In practice I don't expect there is anything that depends on the exact
behavior of what happens when exec'ing a suid executable to gain
privileges when ptraced.   The closes I can imagine is upstart and
I think upstart ran as root when ptracing other programs so there is no
gaining of privilege and thus no reason for a security module to
complain.

Who knows I could be wrong, and someone could actually care.  Which is
hy I think we should document it.

Eric