On 11/21/25 08:18, Eric W. Biederman wrote: > Bernd Edlinger <[email protected]> writes: > >> Hi Eric, >> >> thanks for you valuable input on the topic. >> >> On 11/21/25 00:50, Eric W. Biederman wrote: >>> "Eric W. Biederman" <[email protected]> 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. >
Well, I dont know for sure, but the security engine could deny the execution for any reason, not only because of being ptraced. Maybe there can be a policy which denies user X to execute e.g. any suid programs. Bernd.
