Message-ID: <87ikf3w5us.fsf@email.froward.int.ebiederm.org>
Date: Fri, 21 Nov 2025 13:19:55 -0600
From: "Eric W. Biederman" <ebiederm@...ssion.com>
To: Bernd Edlinger <bernd.edlinger@...mail.de>
Cc: Alexander Viro <viro@...iv.linux.org.uk>,  Alexey Dobriyan
 <adobriyan@...il.com>,  Oleg Nesterov <oleg@...hat.com>,  Kees Cook
 <kees@...nel.org>,  Andy Lutomirski <luto@...capital.net>,  Will Drewry
 <wad@...omium.org>,  Christian Brauner <brauner@...nel.org>,  Andrew
 Morton <akpm@...ux-foundation.org>,  Michal Hocko <mhocko@...e.com>,
  Serge Hallyn <serge@...lyn.com>,  James Morris
 <jamorris@...ux.microsoft.com>,  Randy Dunlap <rdunlap@...radead.org>,
  Suren Baghdasaryan <surenb@...gle.com>,  Yafang Shao
 <laoar.shao@...il.com>,  Helge Deller <deller@....de>,  Adrian Reber
 <areber@...hat.com>,  Thomas Gleixner <tglx@...utronix.de>,  Jens Axboe
 <axboe@...nel.dk>,  Alexei Starovoitov <ast@...nel.org>,
  "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
  "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
  linux-kselftest@...r.kernel.org,  linux-mm@...ck.org,
  linux-security-module@...r.kernel.org,  tiozhang
 <tiozhang@...iglobal.com>,  Luis Chamberlain <mcgrof@...nel.org>,  "Paulo
 Alcantara (SUSE)" <pc@...guebit.com>,  Sergey Senozhatsky
 <senozhatsky@...omium.org>,  Frederic Weisbecker <frederic@...nel.org>,
  YueHaibing <yuehaibing@...wei.com>,  Paul Moore <paul@...l-moore.com>,
  Aleksa Sarai <cyphar@...har.com>,  Stefan Roesch <shr@...kernel.io>,
  Chao Yu <chao@...nel.org>,  xu xin <xu.xin16@....com.cn>,  Jeff Layton
 <jlayton@...nel.org>,  Jan Kara <jack@...e.cz>,  David Hildenbrand
 <david@...hat.com>,  Dave Chinner <dchinner@...hat.com>,  Shuah Khan
 <shuah@...nel.org>,  Elena Reshetova <elena.reshetova@...el.com>,  David
 Windsor <dwindsor@...il.com>,  Mateusz Guzik <mjguzik@...il.com>,  Ard
 Biesheuvel <ardb@...nel.org>,  "Joel Fernandes (Google)"
 <joel@...lfernandes.org>,  "Matthew Wilcox (Oracle)"
 <willy@...radead.org>,  Hans Liljestrand <ishkamiel@...il.com>,  Penglei
 Jiang <superman.xpt@...il.com>,  Lorenzo Stoakes
 <lorenzo.stoakes@...cle.com>,  Adrian Ratiu <adrian.ratiu@...labora.com>,
  Ingo Molnar <mingo@...nel.org>,  "Peter Zijlstra (Intel)"
 <peterz@...radead.org>,  Cyrill Gorcunov <gorcunov@...il.com>,  Eric
 Dumazet <edumazet@...gle.com>
Subject: Re: [RFC][PATCH] exec: Move cred computation under exec_update_lock

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

> On 11/21/25 10:35, Bernd Edlinger wrote:
>> On 11/21/25 08:18, Eric W. Biederman wrote:
>>> Bernd Edlinger <bernd.edlinger@...mail.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@...ssion.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.>>
>> 
>> 
>> 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.
>> 
>
> Hmm, funny..
>
> I installed this patch on top of
>
> commit fd95357fd8c6778ac7dea6c57a19b8b182b6e91f (HEAD -> master, origin/master, origin/HEAD)
> Merge: c966813ea120 7b6216baae75
> Author: Linus Torvalds <torvalds@...ux-foundation.org>
> Date:   Thu Nov 20 11:04:37 2025 -0800
>
> but it does panic when I try to boot:
>
> [  0.870539]     TERM=1inux
> [  0.870573] Starting init: /bin/sh exists but couldn't execute it (error -14) 0.8705751 Kernel panic- not syncing: No working init found. Try passing i mit= option to kernel. See Linux Documentation/admin-guide/init.rst for guidance
> [  0.870577] CPU: UID: 0 PID: 1 Comm: sh Not tainted 6.18.0-rc6+ #1 PREEMPT(voluntary)
> [  0.870579] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBo x 12/01/2006
> [  0.870580] Call Trace:
> [  0.870590]  <TASK>
> [  0.870592]  vpanic+0x36d/0x380
> [  0.870607]  ? __pfx_kernel_init+0x10/0x10
> [  0.870615]  panic+0x5b/0x60
> [  0.870617]  kernel_init+0x17d/0x1c0
> [  0.870623]  ret_from_fork+0x124/0x150
> [  0.870625}  ? __pfx_kernel_init+0x10/0x10
> [  0.870627]  ret_from_fork_asm+0x1a/0x30
> [  0.870632]  </TASK>
> [  0.8706631 Kernel Offset: 0x3a800000 from Oxffffffff81000000 (relocation ran ge: 0xffffffff80000000-0xffffffffbfffffff)
> [  0.880034] ---[ end Kernel panic - not syncing: No working init found. Try passing init option to kernel. See Linux Documentation/admin-guide/init.rst for guidance. 1---`
>
>
> Is that a known problem?

Nope.  It looks like the code needs a little bit bug fixing testing.

I will take see about taking a look.

Eric

Powered by blists - more mailing lists