* Re: [linux-next:master] [ns] 3a18f80918: WARNING:at_include/linux/ns_common.h:#put_cred_rcu
From: Christian Brauner @ 2025-11-11 9:36 UTC (permalink / raw)
To: kernel test robot
Cc: oe-lkp, lkp, linux-fsdevel, linux-kernel, linux-security-module
In-Reply-To: <202511111547.de480df9-lkp@intel.com>
On Tue, Nov 11, 2025 at 03:08:10PM +0800, kernel test robot wrote:
>
>
> Hello,
>
> kernel test robot noticed "WARNING:at_include/linux/ns_common.h:#put_cred_rcu" on:
>
> commit: 3a18f809184bc5a1cfad7cde5b8b026e2ff61587 ("ns: add active reference count")
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master
This is fixed in namespace-6.19 which should've made it into -next about
five hours ago.
Note that "linux-next.git master" is pretty useless. By the time I read
this mail linux-next's master branch will have already been updated. So
you should give a commit hash as well or a tag.
>
> in testcase: trinity
> version:
> with following parameters:
>
> runtime: 300s
> group: group-01
> nr_groups: 5
>
>
>
> config: x86_64-randconfig-r053-20251109
> compiler: gcc-14
> test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 32G
>
> (please refer to attached dmesg/kmsg for entire log/backtrace)
>
>
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <oliver.sang@intel.com>
> | Closes: https://lore.kernel.org/oe-lkp/202511111547.de480df9-lkp@intel.com
>
>
> [ 41.172047][ C1] ------------[ cut here ]------------
> [ 41.172821][ C1] WARNING: CPU: 1 PID: 0 at include/linux/ns_common.h:227 put_cred_rcu (include/linux/ns_common.h:227 include/linux/user_namespace.h:189 kernel/cred.c:88)
> [ 41.173907][ C1] Modules linked in: serio_raw(F) floppy(F) tiny_power_button(F) button(F)
> [ 41.174959][ C1] CPU: 1 UID: 0 PID: 0 Comm: swapper/1 Tainted: GF T 6.18.0-rc2-00014-g3a18f809184b #1 PREEMPTLAZY 9f2dc8152166a7dcc87d7d6a6b2b12a17475cded
> [ 41.176815][ C1] Tainted: [F]=FORCED_MODULE, [T]=RANDSTRUCT
> [ 41.177517][ C1] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> [ 41.178764][ C1] RIP: 0010:put_cred_rcu (include/linux/ns_common.h:227 include/linux/user_namespace.h:189 kernel/cred.c:88)
> [ 41.179419][ C1] Code: 02 48 89 e8 83 e0 07 83 c0 03 38 d0 7c 0c 84 d2 74 08 48 89 ef e8 0d 9f 55 00 8b 83 30 03 00 00 85 c0 74 09 e8 ae 24 1c 00 90 <0f> 0b 90 e8 a5 24 1c 00 48 89 df e8 fd a9 1b 00 e8 98 24 1c 00 4c
> All code
> ========
> 0: 02 48 89 add -0x77(%rax),%cl
> 3: e8 83 e0 07 83 call 0xffffffff8307e08b
> 8: c0 03 38 rolb $0x38,(%rbx)
> b: d0 7c 0c 84 sarb $1,-0x7c(%rsp,%rcx,1)
> f: d2 74 08 48 shlb %cl,0x48(%rax,%rcx,1)
> 13: 89 ef mov %ebp,%edi
> 15: e8 0d 9f 55 00 call 0x559f27
> 1a: 8b 83 30 03 00 00 mov 0x330(%rbx),%eax
> 20: 85 c0 test %eax,%eax
> 22: 74 09 je 0x2d
> 24: e8 ae 24 1c 00 call 0x1c24d7
> 29: 90 nop
> 2a:* 0f 0b ud2 <-- trapping instruction
> 2c: 90 nop
> 2d: e8 a5 24 1c 00 call 0x1c24d7
> 32: 48 89 df mov %rbx,%rdi
> 35: e8 fd a9 1b 00 call 0x1baa37
> 3a: e8 98 24 1c 00 call 0x1c24d7
> 3f: 4c rex.WR
>
> Code starting with the faulting instruction
> ===========================================
> 0: 0f 0b ud2
> 2: 90 nop
> 3: e8 a5 24 1c 00 call 0x1c24ad
> 8: 48 89 df mov %rbx,%rdi
> b: e8 fd a9 1b 00 call 0x1baa0d
> 10: e8 98 24 1c 00 call 0x1c24ad
> 15: 4c rex.WR
> [ 41.181507][ C1] RSP: 0018:ffffc900001c8e58 EFLAGS: 00010246
> [ 41.182352][ C1] RAX: 0000000000000000 RBX: ffff8881649b8780 RCX: 0000000000000000
> [ 41.183326][ C1] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
> [ 41.184311][ C1] RBP: ffff8881649b8ab0 R08: 0000000000000000 R09: 0000000000000000
> [ 41.185324][ C1] R10: 0000000000000000 R11: 0000000000000000 R12: ffff88815d490c80
> [ 41.186287][ C1] R13: ffffffff83d2b7c0 R14: 0000000000000004 R15: ffffffff813e6e70
> [ 41.187244][ C1] FS: 0000000000000000(0000) GS:ffff888799e76000(0000) knlGS:0000000000000000
> [ 41.188285][ C1] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 41.191457][ C1] CR2: 00000000004003c0 CR3: 0000000003c88000 CR4: 00000000000406b0
> [ 41.192491][ C1] Call Trace:
> [ 41.192939][ C1] <IRQ>
> [ 41.193359][ C1] rcu_do_batch (include/linux/rcupdate.h:341 kernel/rcu/tree.c:2607)
> [ 41.193952][ C1] ? rcu_pending (kernel/rcu/tree.c:2529)
> [ 41.194567][ C1] ? rcu_disable_urgency_upon_qs (kernel/rcu/tree.c:725 (discriminator 1))
> [ 41.195408][ C1] ? trace_irq_enable+0xac/0xe0
> [ 41.196177][ C1] rcu_core (kernel/rcu/tree.c:2863)
> [ 41.196782][ C1] handle_softirqs (arch/x86/include/asm/jump_label.h:36 include/trace/events/irq.h:142 kernel/softirq.c:623)
> [ 41.197442][ C1] __irq_exit_rcu (kernel/softirq.c:496 kernel/softirq.c:723)
> [ 41.198071][ C1] irq_exit_rcu (kernel/softirq.c:741 (discriminator 38))
> [ 41.198779][ C1] sysvec_apic_timer_interrupt (arch/x86/kernel/apic/apic.c:1052 (discriminator 35) arch/x86/kernel/apic/apic.c:1052 (discriminator 35))
> [ 41.199466][ C1] </IRQ>
> [ 41.199885][ C1] <TASK>
> [ 41.200311][ C1] asm_sysvec_apic_timer_interrupt (arch/x86/include/asm/idtentry.h:569)
> [ 41.201027][ C1] RIP: 0010:pv_native_safe_halt (arch/x86/kernel/paravirt.c:82)
> [ 41.201736][ C1] Code: 48 8b 3d 28 51 54 02 e8 23 00 00 00 48 2b 05 fc cf af 00 31 ff c3 cc cc cc cc cc cc cc cc cc eb 07 0f 00 2d 97 fc 0e 00 fb f4 <c3> cc cc cc cc 41 57 41 56 41 55 41 54 55 48 89 fd 53 44 8b 6d 00
> All code
> ========
> 0: 48 8b 3d 28 51 54 02 mov 0x2545128(%rip),%rdi # 0x254512f
> 7: e8 23 00 00 00 call 0x2f
> c: 48 2b 05 fc cf af 00 sub 0xafcffc(%rip),%rax # 0xafd00f
> 13: 31 ff xor %edi,%edi
> 15: c3 ret
> 16: cc int3
> 17: cc int3
> 18: cc int3
> 19: cc int3
> 1a: cc int3
> 1b: cc int3
> 1c: cc int3
> 1d: cc int3
> 1e: cc int3
> 1f: eb 07 jmp 0x28
> 21: 0f 00 2d 97 fc 0e 00 verw 0xefc97(%rip) # 0xefcbf
> 28: fb sti
> 29: f4 hlt
> 2a:* c3 ret <-- trapping instruction
> 2b: cc int3
> 2c: cc int3
> 2d: cc int3
> 2e: cc int3
> 2f: 41 57 push %r15
> 31: 41 56 push %r14
> 33: 41 55 push %r13
> 35: 41 54 push %r12
> 37: 55 push %rbp
> 38: 48 89 fd mov %rdi,%rbp
> 3b: 53 push %rbx
> 3c: 44 8b 6d 00 mov 0x0(%rbp),%r13d
>
> Code starting with the faulting instruction
> ===========================================
> 0: c3 ret
> 1: cc int3
> 2: cc int3
> 3: cc int3
> 4: cc int3
> 5: 41 57 push %r15
> 7: 41 56 push %r14
> 9: 41 55 push %r13
> b: 41 54 push %r12
> d: 55 push %rbp
> e: 48 89 fd mov %rdi,%rbp
> 11: 53 push %rbx
> 12: 44 8b 6d 00 mov 0x0(%rbp),%r13d
> [ 41.203822][ C1] RSP: 0018:ffffc9000014fe38 EFLAGS: 00000246
> [ 41.204551][ C1] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
> [ 41.205618][ C1] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
> [ 41.206601][ C1] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
> [ 41.207613][ C1] R10: 0000000000000000 R11: 0000000000000000 R12: ffff8881008322c0
> [ 41.208650][ C1] R13: 1ffff92000029fca R14: dffffc0000000000 R15: 0000000000000000
> [ 41.209689][ C1] default_idle (arch/x86/include/asm/paravirt.h:107 arch/x86/kernel/process.c:767)
> [ 41.210257][ C1] default_idle_call (include/linux/cpuidle.h:143 (discriminator 1) kernel/sched/idle.c:123 (discriminator 1))
> [ 41.210879][ C1] cpuidle_idle_call (kernel/sched/idle.c:191)
> [ 41.211506][ C1] ? arch_cpu_idle_exit+0x30/0x30
> [ 41.215390][ C1] ? tick_nohz_start_idle (kernel/time/tick-sched.c:753)
> [ 41.216069][ C1] ? tsc_verify_tsc_adjust (arch/x86/kernel/tsc_sync.c:81)
> [ 41.216763][ C1] do_idle (kernel/sched/idle.c:332)
> [ 41.217295][ C1] cpu_startup_entry (kernel/sched/idle.c:427)
> [ 41.217929][ C1] start_secondary (arch/x86/kernel/smpboot.c:315)
> [ 41.218650][ C1] ? set_cpu_sibling_map (arch/x86/kernel/smpboot.c:233)
> [ 41.219100][ C1] common_startup_64 (arch/x86/kernel/head_64.S:419)
> [ 41.219506][ C1] </TASK>
> [ 41.219783][ C1] irq event stamp: 42022
> [ 41.220131][ C1] hardirqs last enabled at (42021): tick_nohz_idle_exit (arch/x86/include/asm/irqflags.h:42 arch/x86/include/asm/irqflags.h:119 kernel/time/tick-sched.c:1472)
> [ 41.220844][ C1] hardirqs last disabled at (42022): __schedule (kernel/sched/core.c:6814)
> [ 41.221520][ C1] softirqs last enabled at (42010): handle_softirqs (kernel/softirq.c:469 (discriminator 2) kernel/softirq.c:650 (discriminator 2))
> [ 41.222380][ C1] softirqs last disabled at (42001): __irq_exit_rcu (kernel/softirq.c:496 kernel/softirq.c:723)
> [ 41.223139][ C1] ---[ end trace 0000000000000000 ]---
>
>
> The kernel config and materials to reproduce are available at:
> https://download.01.org/0day-ci/archive/20251111/202511111547.de480df9-lkp@intel.com
>
>
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
>
^ permalink raw reply
* Re: [syzbot] [fs?] WARNING in nsproxy_ns_active_put
From: Christian Brauner @ 2025-11-11 9:24 UTC (permalink / raw)
To: syzbot
Cc: Liam.Howlett, akpm, bpf, bsegall, david, dietmar.eggemann, jack,
jsavitz, juri.lelli, kartikey406, kees, linux-fsdevel,
linux-kernel, linux-mm, linux-security-module, lorenzo.stoakes,
mgorman, mhocko, mingo, mjguzik, oleg, paul, peterz, rostedt,
rppt, sergeh, surenb, syzkaller-bugs, vbabka, vincent.guittot,
viro, vschneid
In-Reply-To: <69104fa2.a70a0220.22f260.00a5.GAE@google.com>
On Sun, Nov 09, 2025 at 12:24:02AM -0800, syzbot wrote:
> syzbot has bisected this issue to:
>
> commit 3a18f809184bc5a1cfad7cde5b8b026e2ff61587
> Author: Christian Brauner <brauner@kernel.org>
> Date: Wed Oct 29 12:20:24 2025 +0000
>
> ns: add active reference count
>
> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=11a350b4580000
> start commit: 9c0826a5d9aa Add linux-next specific files for 20251107
> git tree: linux-next
> final oops: https://syzkaller.appspot.com/x/report.txt?x=13a350b4580000
> console output: https://syzkaller.appspot.com/x/log.txt?x=15a350b4580000
> kernel config: https://syzkaller.appspot.com/x/.config?x=f2ebeee52bf052b8
> dashboard link: https://syzkaller.appspot.com/bug?extid=0b2e79f91ff6579bfa5b
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1639d084580000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1625aa92580000
>
> Reported-by: syzbot+0b2e79f91ff6579bfa5b@syzkaller.appspotmail.com
> Fixes: 3a18f809184b ("ns: add active reference count")
>
> For information about bisection process see: https://goo.gl/tpsmEJ#bisection
#syz test: https://github.com/brauner/linux.git namespace-6.19
^ permalink raw reply
* Re: [PATCH v17] exec: Fix dead-lock in de_thread with ptrace_attach
From: Christian Brauner @ 2025-11-11 9:21 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Bernd Edlinger, Alexander Viro, Alexey Dobriyan, Kees Cook,
Andy Lutomirski, Will Drewry, Andrew Morton, Michal Hocko,
Serge Hallyn, James Morris, Randy Dunlap, Suren Baghdasaryan,
Yafang Shao, Helge Deller, Eric W. Biederman, Adrian Reber,
Thomas Gleixner, Jens Axboe, Alexei Starovoitov,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest, linux-mm, linux-security-module, tiozhang,
Luis Chamberlain, Paulo Alcantara (SUSE), Sergey Senozhatsky,
Frederic Weisbecker, YueHaibing, Paul Moore, Aleksa Sarai,
Stefan Roesch, Chao Yu, xu xin, Jeff Layton, Jan Kara,
David Hildenbrand, Dave Chinner, Shuah Khan, Elena Reshetova,
David Windsor, Mateusz Guzik, Ard Biesheuvel,
Joel Fernandes (Google), Matthew Wilcox (Oracle),
Hans Liljestrand, Penglei Jiang, Lorenzo Stoakes, Adrian Ratiu,
Ingo Molnar, Peter Zijlstra (Intel), Cyrill Gorcunov,
Eric Dumazet
In-Reply-To: <20251105143210.GA25535@redhat.com>
On Wed, Nov 05, 2025 at 03:32:10PM +0100, Oleg Nesterov wrote:
> I am still thinking about another approach, will write another email.
> But let me take a closer look at your patch.
>
> First of all, can you split it? See below.
>
> On 08/21, Bernd Edlinger wrote:
> >
> > -static int de_thread(struct task_struct *tsk)
> > +static int de_thread(struct task_struct *tsk, struct linux_binprm *bprm)
> > {
> > struct signal_struct *sig = tsk->signal;
> > struct sighand_struct *oldsighand = tsk->sighand;
> > spinlock_t *lock = &oldsighand->siglock;
> > + struct task_struct *t;
> > + bool unsafe_execve_in_progress = false;
> >
> > if (thread_group_empty(tsk))
> > goto no_thread_group;
> > @@ -932,6 +934,19 @@ static int de_thread(struct task_struct *tsk)
> > if (!thread_group_leader(tsk))
> > sig->notify_count--;
> >
> > + for_other_threads(tsk, t) {
> > + if (unlikely(t->ptrace)
> > + && (t != tsk->group_leader || !t->exit_state))
> > + unsafe_execve_in_progress = true;
>
> you can add "break" into the "if ()" block...
>
> But this is minor. Why do we need "bool unsafe_execve_in_progress" ?
> If this patch is correct, de_thread() can drop/reacquire cred_guard_mutex
> unconditionally.
>
> If you really think it makes sense, please make another patch with the
> changelog.
>
> I'd certainly prefer to avoid this boolean at least for the start. If nothing
> else to catch the potential problems earlier.
>
> > + if (unlikely(unsafe_execve_in_progress)) {
> > + spin_unlock_irq(lock);
> > + sig->exec_bprm = bprm;
> > + mutex_unlock(&sig->cred_guard_mutex);
> > + spin_lock_irq(lock);
>
> I don't think spin_unlock_irq() + spin_lock_irq() makes any sense...
>
> > @@ -1114,13 +1139,31 @@ int begin_new_exec(struct linux_binprm * bprm)
> > */
> > trace_sched_prepare_exec(current, bprm);
> >
> > + /* If the binary is not readable then enforce mm->dumpable=0 */
> > + would_dump(bprm, bprm->file);
> > + if (bprm->have_execfd)
> > + would_dump(bprm, bprm->executable);
> > +
> > + /*
> > + * Figure out dumpability. Note that this checking only of current
> > + * is wrong, but userspace depends on it. This should be testing
> > + * bprm->secureexec instead.
> > + */
> > + if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP ||
> > + is_dumpability_changed(current_cred(), bprm->cred) ||
> > + !(uid_eq(current_euid(), current_uid()) &&
> > + gid_eq(current_egid(), current_gid())))
> > + set_dumpable(bprm->mm, suid_dumpable);
> > + else
> > + set_dumpable(bprm->mm, SUID_DUMP_USER);
> > +
>
> OK, we need to do this before de_thread() drops cred_guard_mutex.
> But imo this too should be done in a separate patch, the changelog should
> explain this change.
>
> > @@ -1361,6 +1387,11 @@ static int prepare_bprm_creds(struct linux_binprm *bprm)
> > if (mutex_lock_interruptible(¤t->signal->cred_guard_mutex))
> > return -ERESTARTNOINTR;
> >
> > + if (unlikely(current->signal->exec_bprm)) {
> > + mutex_unlock(¤t->signal->cred_guard_mutex);
> > + return -ERESTARTNOINTR;
> > + }
>
> OK, if signal->exec_bprm != NULL, then current is already killed. But
> proc_pid_attr_write() and ptrace_traceme() do the same. So how about
> something like
>
> int lock_current_cgm(void)
> {
> if (mutex_lock_interruptible(¤t->signal->cred_guard_mutex))
> return -ERESTARTNOINTR;
>
> if (!current->signal->group_exec_task)
> return 0;
>
> WARN_ON(!fatal_signal_pending(current));
> mutex_unlock(¤t->signal->cred_guard_mutex);
> return -ERESTARTNOINTR;
> }
>
> ?
>
> Note that it checks ->group_exec_task, not ->exec_bprm. So this change can
> come in a separate patch too, but I won't insist.
>
> > @@ -453,6 +454,28 @@ static int ptrace_attach(struct task_struct *task, long request,
> > return retval;
> > }
> >
> > + if (unlikely(task == task->signal->group_exec_task)) {
> > + retval = down_write_killable(&task->signal->exec_update_lock);
> > + if (retval)
> > + return retval;
> > +
> > + scoped_guard (task_lock, task) {
> > + struct linux_binprm *bprm = task->signal->exec_bprm;
> > + const struct cred __rcu *old_cred = task->real_cred;
> > + struct mm_struct *old_mm = task->mm;
> > +
> > + rcu_assign_pointer(task->real_cred, bprm->cred);
> > + task->mm = bprm->mm;
> > + retval = __ptrace_may_access(task, PTRACE_MODE_ATTACH_REALCREDS);
> > + rcu_assign_pointer(task->real_cred, old_cred);
> > + task->mm = old_mm;
> > + }
>
> This is the most problematic change which I can't review...
>
> Firstly, it changes task->mm/real_cred for __ptrace_may_access() and this
> looks dangerous to me.
Yeah, that is not ok. This is effectively override_creds for real_cred
and that is not a pattern I want to see us establish at all! Temporary
credential overrides for the subjective credentials is already terrible
but at least we have the explicit split between real_cred and cred
expressely for that. So no, that's not an acceptable solution.
>
> Say, current_is_single_threaded() called by another CLONE_VM process can
> miss group_exec_task and falsely return true. Probably not that bad, in
> this case old_mm should go away soon, but still...
>
> And I don't know if this can fool the users of task_cred_xxx/__task_cred
> somehow.
>
> Or. check_unsafe_exec() sets LSM_UNSAFE_PTRACE if ptrace. Is it safe to
> ptrace the execing task after that? I have no idea what the security hooks
> can do...
>
> Again, can't review this part.
>
> Oleg.
>
^ permalink raw reply
* [linux-next:master] [ns] 3a18f80918: WARNING:at_include/linux/ns_common.h:#put_cred_rcu
From: kernel test robot @ 2025-11-11 7:08 UTC (permalink / raw)
To: Christian Brauner
Cc: oe-lkp, lkp, linux-fsdevel, linux-kernel, linux-security-module,
oliver.sang
Hello,
kernel test robot noticed "WARNING:at_include/linux/ns_common.h:#put_cred_rcu" on:
commit: 3a18f809184bc5a1cfad7cde5b8b026e2ff61587 ("ns: add active reference count")
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master
in testcase: trinity
version:
with following parameters:
runtime: 300s
group: group-01
nr_groups: 5
config: x86_64-randconfig-r053-20251109
compiler: gcc-14
test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 32G
(please refer to attached dmesg/kmsg for entire log/backtrace)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202511111547.de480df9-lkp@intel.com
[ 41.172047][ C1] ------------[ cut here ]------------
[ 41.172821][ C1] WARNING: CPU: 1 PID: 0 at include/linux/ns_common.h:227 put_cred_rcu (include/linux/ns_common.h:227 include/linux/user_namespace.h:189 kernel/cred.c:88)
[ 41.173907][ C1] Modules linked in: serio_raw(F) floppy(F) tiny_power_button(F) button(F)
[ 41.174959][ C1] CPU: 1 UID: 0 PID: 0 Comm: swapper/1 Tainted: GF T 6.18.0-rc2-00014-g3a18f809184b #1 PREEMPTLAZY 9f2dc8152166a7dcc87d7d6a6b2b12a17475cded
[ 41.176815][ C1] Tainted: [F]=FORCED_MODULE, [T]=RANDSTRUCT
[ 41.177517][ C1] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[ 41.178764][ C1] RIP: 0010:put_cred_rcu (include/linux/ns_common.h:227 include/linux/user_namespace.h:189 kernel/cred.c:88)
[ 41.179419][ C1] Code: 02 48 89 e8 83 e0 07 83 c0 03 38 d0 7c 0c 84 d2 74 08 48 89 ef e8 0d 9f 55 00 8b 83 30 03 00 00 85 c0 74 09 e8 ae 24 1c 00 90 <0f> 0b 90 e8 a5 24 1c 00 48 89 df e8 fd a9 1b 00 e8 98 24 1c 00 4c
All code
========
0: 02 48 89 add -0x77(%rax),%cl
3: e8 83 e0 07 83 call 0xffffffff8307e08b
8: c0 03 38 rolb $0x38,(%rbx)
b: d0 7c 0c 84 sarb $1,-0x7c(%rsp,%rcx,1)
f: d2 74 08 48 shlb %cl,0x48(%rax,%rcx,1)
13: 89 ef mov %ebp,%edi
15: e8 0d 9f 55 00 call 0x559f27
1a: 8b 83 30 03 00 00 mov 0x330(%rbx),%eax
20: 85 c0 test %eax,%eax
22: 74 09 je 0x2d
24: e8 ae 24 1c 00 call 0x1c24d7
29: 90 nop
2a:* 0f 0b ud2 <-- trapping instruction
2c: 90 nop
2d: e8 a5 24 1c 00 call 0x1c24d7
32: 48 89 df mov %rbx,%rdi
35: e8 fd a9 1b 00 call 0x1baa37
3a: e8 98 24 1c 00 call 0x1c24d7
3f: 4c rex.WR
Code starting with the faulting instruction
===========================================
0: 0f 0b ud2
2: 90 nop
3: e8 a5 24 1c 00 call 0x1c24ad
8: 48 89 df mov %rbx,%rdi
b: e8 fd a9 1b 00 call 0x1baa0d
10: e8 98 24 1c 00 call 0x1c24ad
15: 4c rex.WR
[ 41.181507][ C1] RSP: 0018:ffffc900001c8e58 EFLAGS: 00010246
[ 41.182352][ C1] RAX: 0000000000000000 RBX: ffff8881649b8780 RCX: 0000000000000000
[ 41.183326][ C1] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
[ 41.184311][ C1] RBP: ffff8881649b8ab0 R08: 0000000000000000 R09: 0000000000000000
[ 41.185324][ C1] R10: 0000000000000000 R11: 0000000000000000 R12: ffff88815d490c80
[ 41.186287][ C1] R13: ffffffff83d2b7c0 R14: 0000000000000004 R15: ffffffff813e6e70
[ 41.187244][ C1] FS: 0000000000000000(0000) GS:ffff888799e76000(0000) knlGS:0000000000000000
[ 41.188285][ C1] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 41.191457][ C1] CR2: 00000000004003c0 CR3: 0000000003c88000 CR4: 00000000000406b0
[ 41.192491][ C1] Call Trace:
[ 41.192939][ C1] <IRQ>
[ 41.193359][ C1] rcu_do_batch (include/linux/rcupdate.h:341 kernel/rcu/tree.c:2607)
[ 41.193952][ C1] ? rcu_pending (kernel/rcu/tree.c:2529)
[ 41.194567][ C1] ? rcu_disable_urgency_upon_qs (kernel/rcu/tree.c:725 (discriminator 1))
[ 41.195408][ C1] ? trace_irq_enable+0xac/0xe0
[ 41.196177][ C1] rcu_core (kernel/rcu/tree.c:2863)
[ 41.196782][ C1] handle_softirqs (arch/x86/include/asm/jump_label.h:36 include/trace/events/irq.h:142 kernel/softirq.c:623)
[ 41.197442][ C1] __irq_exit_rcu (kernel/softirq.c:496 kernel/softirq.c:723)
[ 41.198071][ C1] irq_exit_rcu (kernel/softirq.c:741 (discriminator 38))
[ 41.198779][ C1] sysvec_apic_timer_interrupt (arch/x86/kernel/apic/apic.c:1052 (discriminator 35) arch/x86/kernel/apic/apic.c:1052 (discriminator 35))
[ 41.199466][ C1] </IRQ>
[ 41.199885][ C1] <TASK>
[ 41.200311][ C1] asm_sysvec_apic_timer_interrupt (arch/x86/include/asm/idtentry.h:569)
[ 41.201027][ C1] RIP: 0010:pv_native_safe_halt (arch/x86/kernel/paravirt.c:82)
[ 41.201736][ C1] Code: 48 8b 3d 28 51 54 02 e8 23 00 00 00 48 2b 05 fc cf af 00 31 ff c3 cc cc cc cc cc cc cc cc cc eb 07 0f 00 2d 97 fc 0e 00 fb f4 <c3> cc cc cc cc 41 57 41 56 41 55 41 54 55 48 89 fd 53 44 8b 6d 00
All code
========
0: 48 8b 3d 28 51 54 02 mov 0x2545128(%rip),%rdi # 0x254512f
7: e8 23 00 00 00 call 0x2f
c: 48 2b 05 fc cf af 00 sub 0xafcffc(%rip),%rax # 0xafd00f
13: 31 ff xor %edi,%edi
15: c3 ret
16: cc int3
17: cc int3
18: cc int3
19: cc int3
1a: cc int3
1b: cc int3
1c: cc int3
1d: cc int3
1e: cc int3
1f: eb 07 jmp 0x28
21: 0f 00 2d 97 fc 0e 00 verw 0xefc97(%rip) # 0xefcbf
28: fb sti
29: f4 hlt
2a:* c3 ret <-- trapping instruction
2b: cc int3
2c: cc int3
2d: cc int3
2e: cc int3
2f: 41 57 push %r15
31: 41 56 push %r14
33: 41 55 push %r13
35: 41 54 push %r12
37: 55 push %rbp
38: 48 89 fd mov %rdi,%rbp
3b: 53 push %rbx
3c: 44 8b 6d 00 mov 0x0(%rbp),%r13d
Code starting with the faulting instruction
===========================================
0: c3 ret
1: cc int3
2: cc int3
3: cc int3
4: cc int3
5: 41 57 push %r15
7: 41 56 push %r14
9: 41 55 push %r13
b: 41 54 push %r12
d: 55 push %rbp
e: 48 89 fd mov %rdi,%rbp
11: 53 push %rbx
12: 44 8b 6d 00 mov 0x0(%rbp),%r13d
[ 41.203822][ C1] RSP: 0018:ffffc9000014fe38 EFLAGS: 00000246
[ 41.204551][ C1] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
[ 41.205618][ C1] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
[ 41.206601][ C1] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
[ 41.207613][ C1] R10: 0000000000000000 R11: 0000000000000000 R12: ffff8881008322c0
[ 41.208650][ C1] R13: 1ffff92000029fca R14: dffffc0000000000 R15: 0000000000000000
[ 41.209689][ C1] default_idle (arch/x86/include/asm/paravirt.h:107 arch/x86/kernel/process.c:767)
[ 41.210257][ C1] default_idle_call (include/linux/cpuidle.h:143 (discriminator 1) kernel/sched/idle.c:123 (discriminator 1))
[ 41.210879][ C1] cpuidle_idle_call (kernel/sched/idle.c:191)
[ 41.211506][ C1] ? arch_cpu_idle_exit+0x30/0x30
[ 41.215390][ C1] ? tick_nohz_start_idle (kernel/time/tick-sched.c:753)
[ 41.216069][ C1] ? tsc_verify_tsc_adjust (arch/x86/kernel/tsc_sync.c:81)
[ 41.216763][ C1] do_idle (kernel/sched/idle.c:332)
[ 41.217295][ C1] cpu_startup_entry (kernel/sched/idle.c:427)
[ 41.217929][ C1] start_secondary (arch/x86/kernel/smpboot.c:315)
[ 41.218650][ C1] ? set_cpu_sibling_map (arch/x86/kernel/smpboot.c:233)
[ 41.219100][ C1] common_startup_64 (arch/x86/kernel/head_64.S:419)
[ 41.219506][ C1] </TASK>
[ 41.219783][ C1] irq event stamp: 42022
[ 41.220131][ C1] hardirqs last enabled at (42021): tick_nohz_idle_exit (arch/x86/include/asm/irqflags.h:42 arch/x86/include/asm/irqflags.h:119 kernel/time/tick-sched.c:1472)
[ 41.220844][ C1] hardirqs last disabled at (42022): __schedule (kernel/sched/core.c:6814)
[ 41.221520][ C1] softirqs last enabled at (42010): handle_softirqs (kernel/softirq.c:469 (discriminator 2) kernel/softirq.c:650 (discriminator 2))
[ 41.222380][ C1] softirqs last disabled at (42001): __irq_exit_rcu (kernel/softirq.c:496 kernel/softirq.c:723)
[ 41.223139][ C1] ---[ end trace 0000000000000000 ]---
The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20251111/202511111547.de480df9-lkp@intel.com
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH] security: Add KUnit tests for rootid_owns_currentns()
From: kernel test robot @ 2025-11-11 7:03 UTC (permalink / raw)
To: Ryan Foster, selinux
Cc: oe-kbuild-all, linux-security-module, linux-kernel, serge, paul,
Ryan Foster
In-Reply-To: <20251110143748.4144288-1-foster.ryan.r@gmail.com>
Hi Ryan,
kernel test robot noticed the following build warnings:
[auto build test WARNING on pcmoore-selinux/next]
[also build test WARNING on linus/master v6.18-rc5 next-20251110]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Ryan-Foster/security-Add-KUnit-tests-for-rootid_owns_currentns/20251110-223824
base: https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git next
patch link: https://lore.kernel.org/r/20251110143748.4144288-1-foster.ryan.r%40gmail.com
patch subject: [PATCH] security: Add KUnit tests for rootid_owns_currentns()
config: x86_64-rhel-9.4-kunit (https://download.01.org/0day-ci/archive/20251111/202511111453.e1clsyyc-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251111/202511111453.e1clsyyc-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511111453.e1clsyyc-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> Warning: security/commoncap_test.c:45 function parameter 'test' not described in 'test_rootid_owns_currentns_init_ns'
>> Warning: security/commoncap_test.c:63 function parameter 'test' not described in 'test_rootid_owns_currentns_invalid'
>> Warning: security/commoncap_test.c:79 function parameter 'test' not described in 'test_rootid_owns_currentns_nonroot'
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH] security: Add KUnit tests for rootid_owns_currentns()
From: kernel test robot @ 2025-11-11 6:52 UTC (permalink / raw)
To: Ryan Foster, selinux
Cc: oe-kbuild-all, linux-security-module, linux-kernel, serge, paul,
Ryan Foster
In-Reply-To: <20251110143748.4144288-1-foster.ryan.r@gmail.com>
Hi Ryan,
kernel test robot noticed the following build errors:
[auto build test ERROR on pcmoore-selinux/next]
[also build test ERROR on linus/master v6.18-rc5 next-20251110]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Ryan-Foster/security-Add-KUnit-tests-for-rootid_owns_currentns/20251110-223824
base: https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git next
patch link: https://lore.kernel.org/r/20251110143748.4144288-1-foster.ryan.r%40gmail.com
patch subject: [PATCH] security: Add KUnit tests for rootid_owns_currentns()
config: mips-randconfig-r053-20251111 (https://download.01.org/0day-ci/archive/20251111/202511111459.1ToyUP5p-lkp@intel.com/config)
compiler: mips-linux-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251111/202511111459.1ToyUP5p-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511111459.1ToyUP5p-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
>> security/commoncap_test.c:23:3: error: conflicting types for 'vfsuid_t'
} vfsuid_t;
^~~~~~~~
In file included from include/linux/fs.h:45,
from arch/mips/include/asm/elf.h:12,
from include/linux/elf.h:6,
from include/linux/module.h:20,
from include/kunit/test.h:24,
from security/commoncap_test.c:9:
include/linux/mnt_idmapping.h:17:3: note: previous declaration of 'vfsuid_t' was here
} vfsuid_t;
^~~~~~~~
>> security/commoncap_test.c:26:21: error: conflicting types for '__vfsuid_val'
static inline uid_t __vfsuid_val(vfsuid_t uid)
^~~~~~~~~~~~
In file included from include/linux/fs.h:45,
from arch/mips/include/asm/elf.h:12,
from include/linux/elf.h:6,
from include/linux/module.h:20,
from include/kunit/test.h:24,
from security/commoncap_test.c:9:
include/linux/mnt_idmapping.h:34:21: note: previous definition of '__vfsuid_val' was here
static inline uid_t __vfsuid_val(vfsuid_t uid)
^~~~~~~~~~~~
>> security/commoncap_test.c:31: warning: "VFSUIDT_INIT" redefined
#define VFSUIDT_INIT(val) ((vfsuid_t){ __kuid_val(val) })
In file included from include/linux/fs.h:45,
from arch/mips/include/asm/elf.h:12,
from include/linux/elf.h:6,
from include/linux/module.h:20,
from include/kunit/test.h:24,
from security/commoncap_test.c:9:
include/linux/mnt_idmapping.h:109: note: this is the location of the previous definition
#define VFSUIDT_INIT(val) (vfsuid_t){ __kuid_val(val) }
vim +/vfsuid_t +23 security/commoncap_test.c
18
19 /* Forward declare the minimal types we need - match the actual kernel definitions */
20 struct mnt_idmap;
21 typedef struct {
22 uid_t val;
> 23 } vfsuid_t;
24
25 /* Minimal macros we need - match kernel definitions from mnt_idmapping.h */
> 26 static inline uid_t __vfsuid_val(vfsuid_t uid)
27 {
28 return uid.val;
29 }
30
> 31 #define VFSUIDT_INIT(val) ((vfsuid_t){ __kuid_val(val) })
32 #define INVALID_VFSUID VFSUIDT_INIT(INVALID_UID)
33
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH] security: sctp: Format type and permission checks tables
From: Bagas Sanjaya @ 2025-11-11 2:34 UTC (permalink / raw)
To: Linux Kernel Mailing List, Linux Documentation,
Linux Security Module
Cc: Jonathan Corbet, Jarkko Sakkinen, Christian Brauner, Jeff Layton,
Kees Cook, Mickaël Salaün, Stuart Yoder, Randy Dunlap
In-Reply-To: <20251103113922.61232-2-bagasdotme@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 143 bytes --]
On Mon, Nov 03, 2025 at 06:39:23PM +0700, Bagas Sanjaya wrote:
> Use reST grid tables for both type and permission checks tables.
review ping
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH] security: Add KUnit tests for rootid_owns_currentns()
From: Serge E. Hallyn @ 2025-11-11 0:33 UTC (permalink / raw)
To: Ryan Foster; +Cc: selinux, linux-security-module, linux-kernel, serge, paul
In-Reply-To: <20251110143748.4144288-1-foster.ryan.r@gmail.com>
On Mon, Nov 10, 2025 at 06:37:48AM -0800, Ryan Foster wrote:
> The rootid_owns_currentns() function in security/commoncap.c is a
> security-critical function used to determine if a root ID from a
> parent namespace owns the current namespace. This function is used
> in multiple security-critical paths:
>
> 1. cap_inode_getsecurity() - file capability retrieval
> 2. get_vfs_caps_from_disk() - file capability loading during exec
>
> Despite its security-critical nature, this function had no test
> coverage. This patch adds KUnit tests to verify the function's
> behavior in various scenarios:
>
> - Root ID in init namespace (positive case)
> - Invalid vfsuid (negative case)
> - Non-root UID (negative case)
I'd be more excited about this if it tested the actual
functionality.
The rootid_owns_currentns() could be split so that it calls
a (static if not testing) inline rootid_owns_userns(), and
then you test the latter. Then create a few user namespaces
with different values for the namespace's uid 0, and make
sure rootid_owns_userns(testns, testuid) behaves correctly.
Actually, this function should probably be renamed.
"rootid_owns_currentns()" is not correct. It should just be
"uid_owns_currentns()".
> The function is made testable by exporting it when
> CONFIG_SECURITY_COMMONCAP_KUNIT_TEST is enabled, while maintaining
> static visibility in production builds.
>
> This follows the pattern established by other security subsystems
> (IPE, Landlock) for KUnit testing.
>
> Signed-off-by: Ryan Foster <foster.ryan.r@gmail.com>
> ---
> security/Kconfig | 17 ++++++
> security/Makefile | 1 +
> security/commoncap.c | 5 ++
> security/commoncap_test.c | 109 ++++++++++++++++++++++++++++++++++++++
> 4 files changed, 132 insertions(+)
> create mode 100644 security/commoncap_test.c
>
> diff --git a/security/Kconfig b/security/Kconfig
> index 285f284dfcac..c7b3f42ef875 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -284,6 +284,23 @@ config LSM
>
> If unsure, leave this as the default.
>
> +config SECURITY_COMMONCAP_KUNIT_TEST
> + bool "Build KUnit tests for commoncap" if !KUNIT_ALL_TESTS
> + depends on KUNIT=y
> + default KUNIT_ALL_TESTS
> + help
> + This builds the commoncap KUnit tests.
> +
> + KUnit tests run during boot and output the results to the debug log
> + in TAP format (https://testanything.org/). Only useful for kernel devs
> + running KUnit test harness and are not for inclusion into a
> + production build.
> +
> + For more information on KUnit and unit tests in general please refer
> + to the KUnit documentation in Documentation/dev-tools/kunit/.
> +
> + If unsure, say N.
> +
> source "security/Kconfig.hardening"
>
> endmenu
> diff --git a/security/Makefile b/security/Makefile
> index 22ff4c8bd8ce..5b1285fde552 100644
> --- a/security/Makefile
> +++ b/security/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_KEYS) += keys/
>
> # always enable default capabilities
> obj-y += commoncap.o
> +obj-$(CONFIG_SECURITY_COMMONCAP_KUNIT_TEST) += commoncap_test.o
> obj-$(CONFIG_SECURITY) += lsm_syscalls.o
> obj-$(CONFIG_MMU) += min_addr.o
>
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 6bd4adeb4795..4d0e014ce7cd 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -358,7 +358,12 @@ int cap_inode_killpriv(struct mnt_idmap *idmap, struct dentry *dentry)
> return error;
> }
>
> +#ifdef CONFIG_SECURITY_COMMONCAP_KUNIT_TEST
> +bool rootid_owns_currentns(vfsuid_t rootvfsuid);
> +bool rootid_owns_currentns(vfsuid_t rootvfsuid)
> +#else
> static bool rootid_owns_currentns(vfsuid_t rootvfsuid)
> +#endif
> {
> struct user_namespace *ns;
> kuid_t kroot;
> diff --git a/security/commoncap_test.c b/security/commoncap_test.c
> new file mode 100644
> index 000000000000..9757d433d314
> --- /dev/null
> +++ b/security/commoncap_test.c
> @@ -0,0 +1,109 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * KUnit tests for commoncap.c security functions
> + *
> + * Tests for security-critical functions in the capability subsystem,
> + * particularly namespace-related capability checks.
> + */
> +
> +#include <kunit/test.h>
> +#include <linux/user_namespace.h>
> +#include <linux/uidgid.h>
> +#include <linux/module.h>
> +
> +/* We need to include the actual vfsuid_t definition but avoid the problematic
> + * inline functions in mnt_idmapping.h. Include just the type definitions.
> + */
> +#include <linux/types.h>
> +
> +/* Forward declare the minimal types we need - match the actual kernel definitions */
> +struct mnt_idmap;
> +typedef struct {
> + uid_t val;
> +} vfsuid_t;
> +
> +/* Minimal macros we need - match kernel definitions from mnt_idmapping.h */
> +static inline uid_t __vfsuid_val(vfsuid_t uid)
> +{
> + return uid.val;
> +}
> +
> +#define VFSUIDT_INIT(val) ((vfsuid_t){ __kuid_val(val) })
> +#define INVALID_VFSUID VFSUIDT_INIT(INVALID_UID)
> +
> +#ifdef CONFIG_SECURITY_COMMONCAP_KUNIT_TEST
> +
> +/* Forward declaration - function is exported when KUNIT_TEST is enabled */
> +extern bool rootid_owns_currentns(vfsuid_t rootvfsuid);
> +
> +/**
> + * test_rootid_owns_currentns_init_ns - Test rootid_owns_currentns with init ns
> + *
> + * Verifies that a root ID in the init namespace correctly owns the current
> + * namespace when running in init_user_ns.
> + */
> +static void test_rootid_owns_currentns_init_ns(struct kunit *test)
> +{
> + vfsuid_t root_vfsuid;
> + kuid_t root_kuid;
> +
> + /* Create a root UID in init namespace */
> + root_kuid = KUIDT_INIT(0);
> + root_vfsuid = VFSUIDT_INIT(root_kuid);
> +
> + /* In init namespace, root should own current namespace */
> + KUNIT_EXPECT_TRUE(test, rootid_owns_currentns(root_vfsuid));
> +}
> +
> +/**
> + * test_rootid_owns_currentns_invalid - Test rootid_owns_currentns with invalid vfsuid
> + *
> + * Verifies that an invalid vfsuid correctly returns false.
> + */
> +static void test_rootid_owns_currentns_invalid(struct kunit *test)
> +{
> + vfsuid_t invalid_vfsuid;
> +
> + /* Use the predefined invalid vfsuid */
> + invalid_vfsuid = INVALID_VFSUID;
> +
> + /* Invalid vfsuid should return false */
> + KUNIT_EXPECT_FALSE(test, rootid_owns_currentns(invalid_vfsuid));
> +}
> +
> +/**
> + * test_rootid_owns_currentns_nonroot - Test rootid_owns_currentns with non-root UID
> + *
> + * Verifies that a non-root UID correctly returns false.
> + */
> +static void test_rootid_owns_currentns_nonroot(struct kunit *test)
> +{
> + vfsuid_t nonroot_vfsuid;
> + kuid_t nonroot_kuid;
> +
> + /* Create a non-root UID */
> + nonroot_kuid = KUIDT_INIT(1000);
> + nonroot_vfsuid = VFSUIDT_INIT(nonroot_kuid);
> +
> + /* Non-root UID should return false */
> + KUNIT_EXPECT_FALSE(test, rootid_owns_currentns(nonroot_vfsuid));
> +}
> +
> +static struct kunit_case commoncap_test_cases[] = {
> + KUNIT_CASE(test_rootid_owns_currentns_init_ns),
> + KUNIT_CASE(test_rootid_owns_currentns_invalid),
> + KUNIT_CASE(test_rootid_owns_currentns_nonroot),
> + {}
> +};
> +
> +static struct kunit_suite commoncap_test_suite = {
> + .name = "commoncap",
> + .test_cases = commoncap_test_cases,
> +};
> +
> +kunit_test_suite(commoncap_test_suite);
> +
> +MODULE_LICENSE("GPL");
> +
> +#endif /* CONFIG_SECURITY_COMMONCAP_KUNIT_TEST */
> +
> --
> 2.43.0
^ permalink raw reply
* Re: [PATCH] security: Add KUnit tests for rootid_owns_currentns()
From: Paul Moore @ 2025-11-11 0:12 UTC (permalink / raw)
To: ryan foster; +Cc: kunit-dev, serge, linux-kernel, linux-security-module
In-Reply-To: <CAHC9VhQw2yXuwhVSZERXiOREXzuO7aU=LTUiL1go6v5m5sx3Mg@mail.gmail.com>
This time with the proper LSM list address.
On Mon, Nov 10, 2025 at 6:17 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Sun, Nov 9, 2025 at 8:13 PM ryan foster <foster.ryan.r@gmail.com> wrote:
> >
> > Please review this patch that adds KUnit tests for
> > rootid_owns_currentns() function in security/commoncap.c
>
> Hi Ryan,
>
> In case you haven't already seen this, it would be a good idea to read
> the doc below on how to submit patches to the upstream Linux kernel
> lists. For example, sending patches as attachment is frowned upon as
> it is difficult to properly review patches that way.
>
> * https://docs.kernel.org/process/submitting-patches.html
>
> If you are having difficulty configuring git to send email, you may
> want to look at the "b4" tool, doc link below. One of its primary
> goals is to make it easier for people to post kernel patches for
> review.
>
> https://b4.docs.kernel.org/en/latest
>
> Good luck!
--
paul-moore.com
^ permalink raw reply
* Re: [RFC PATCH 2/3] exec: don't wait for zombie threads with cred_guard_mutex held
From: Cyrill Gorcunov @ 2025-11-10 21:49 UTC (permalink / raw)
To: Oleg Nesterov
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest, linux-mm, linux-security-module
In-Reply-To: <aRIAEYH2iLLN-Fjg@redhat.com>
On Mon, Nov 10, 2025 at 04:09:05PM +0100, Oleg Nesterov wrote:
...
> > > if (!((sig->flags & SIGNAL_GROUP_EXIT) || sig->group_exec_task)) {
> > > sig->group_exec_task = tsk;
> > > sig->notify_count = -zap_other_threads(tsk);
> >
> > Hi Oleg! I somehow manage to miss a moment -- why negative result here?
>
> You know, initially I wrote
>
> sig->notify_count = 0 - zap_other_threads(tsk);
>
> to make it clear that this is not a typo ;)
Aha! Thanks a huge for explanation :)
>
> This is for exit_notify() which does
>
> /* mt-exec, de_thread() -> wait_for_notify_count() */
> if (tsk->signal->notify_count < 0 && !++tsk->signal->notify_count)
> wake_up_process(tsk->signal->group_exec_task);
>
> Then setup_new_exec() sets notify_count > 0 for __exit_signal() which does
>
> /* mt-exec, setup_new_exec() -> wait_for_notify_count() */
> if (sig->notify_count > 0 && !--sig->notify_count)
> wake_up_process(sig->group_exec_task);
>
> Yes this needs more comments and (with or without this patch) cleanups.
> Note that exit_notify() and __exit_signal() already (before this patch)
> use ->notify_count almost the same way, just exit_notify() assumes that
> notify_count < 0 means the !thread_group_leader() case in de_thread().
Yeah, just realized. It's been a long time since I looked into this signals
and tasks related code so to be honest don't think I would be helpful here)
Anyway while looking into patch I got wonder why
+static int wait_for_notify_count(struct task_struct *tsk)
+{
+ for (;;) {
+ return -EINTR;
+ set_current_state(TASK_KILLABLE);
+ if (!tsk->signal->notify_count)
+ break;
We have no any barrier here in fetching @notify_count? I mean updating
this value is done under locks (spin or read/write) in turn condition
test is a raw one. Not a big deal since set_current_state() and schedule()
are buffer flushers by themselves and after all not immediate update of
notify_count simply force us to yield one more schedule() call but I've
been a bit confused that we don't use some read_once here or something.
Another (more likely) that I've just said something stupid)
+ schedule();
}
+ __set_current_state(TASK_RUNNING);
+ return 0;
+}
Cyrill
^ permalink raw reply
* Re: [Patch V1] ima: avoid duplicate policy rules insertions
From: Tahera Fahimi @ 2025-11-10 19:06 UTC (permalink / raw)
To: Anirudh Venkataramanan, zohar, roberto.sassu, dmitry.kasatkin,
eric.snowberg, paul, jmorris, serge, linux-integrity,
linux-security-module, linux-kernel, code
In-Reply-To: <b36a6508-1b2a-4c87-b3b5-9af0b402dc0b@linux.microsoft.com>
On 11/6/2025 12:32 PM, Anirudh Venkataramanan wrote:
> On 11/6/2025 10:14 AM, Tahera Fahimi wrote:
>> Prevent redundant IMA policy rules by checking for duplicates before insertion. This ensures that
>> rules are not re-added when userspace is restarted (using systemd-soft-reboot) without a full system
>> reboot. ima_rule_exists() detects duplicates in both temporary and active rule lists.
>
> I have run into this too. Thanks for proposing a patch!
>
> FWIW - I am fairly new to the IMA subsystem, so feedback below is mostly structural, with some IMA specific comments.
Hi Ahirudh, Thanks for your feedback.
>>
>> Signed-off-by: Tahera Fahimi <taherafahimi@linux.microsoft.com>
>> ---
>> security/integrity/ima/ima_policy.c | 157 +++++++++++++++++++++++++++-
>> 1 file changed, 156 insertions(+), 1 deletion(-)
>>
>> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
>> index 164d62832f8ec..3dd902101dbda 100644
>> --- a/security/integrity/ima/ima_policy.c
>> +++ b/security/integrity/ima/ima_policy.c
>> @@ -1953,6 +1953,153 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>> return result;
>> }
>> +static bool template_has_field(const char *field_id, const struct ima_template_desc *template2)
>> +{
>> + int j;
>
> j is declared in the loop header below too, which is more correct because it keeps the scope of j to be within the loop. So I'd say get rid of the above declaration.
The declaration of j is at the beginning to adhere proper kernel style and ancient compile support.
>> +
>> + for (int j = 0; j < template2->num_fields; j++)
>> + if (strcmp(field_id, template2->fields[j]->field_id) == 0)
>> + return true;
> I believe the preferred kernel style is to use if (!strcmp(...)).
>
>> +
>> + return false;
>> +}
>> +
>> +static bool keyring_has_item(const char *item, const struct ima_rule_opt_list *keyrings)
>> +{
>> + int j;
>> +
>> + for (j = 0; j < keyrings->count; j++) {
>> + if (strcmp(item, keyrings->items[j]) == 0)
>> + return true;
>> + }
>> + return false;
>> +}
>> +
>> +static bool labels_has_item(const char *item, const struct ima_rule_opt_list *labels)
>> +{
>> + int j;
>> +
>> + for (j = 0; j < labels->count; j++) {
>> + if (strcmp(item, labels->items[j]) == 0)
>> + return true;
>> + }
>> + return false;
>> +}
>> +
>> +static bool ima_rules_equal(const struct ima_rule_entry *rule1, const struct ima_rule_entry *rule2)
>> +{
>> + int i;
>
> i is used further down in this function, and even in all those cases, the scope of i can be limited to the loop body where it's used.
>
> If you didn't know this already - you can use cppcheck to identify and reduce the scope of variables.
>
>> +
>> + if (rule1->flags != rule2->flags)
>> + return false;
>> +
>> + if (rule1->action != rule2->action)
>> + return false;
>> +
>> + if (((rule1->flags & IMA_FUNC) && rule1->func != rule2->func) ||
>> + ((rule1->flags & (IMA_MASK | IMA_INMASK)) && rule1->mask != rule2->mask) ||
>> + ((rule1->flags & IMA_FSMAGIC) && rule1->fsmagic != rule2->fsmagic) ||
>> + ((rule1->flags & IMA_FSUUID) && !uuid_equal(&rule1->fsuuid, &rule2->fsuuid)) ||
>> + ((rule1->flags & IMA_UID) && !uid_eq(rule1->uid, rule2->uid)) ||
>> + ((rule1->flags & IMA_GID) && !gid_eq(rule1->gid, rule2->gid)) ||
>> + ((rule1->flags & IMA_FOWNER) && !uid_eq(rule1->fowner, rule2->fowner)) ||
>> + ((rule1->flags & IMA_FGROUP) && !gid_eq(rule1->fgroup, rule2->fgroup)) ||
>> + ((rule1->flags & IMA_FSNAME) && (strcmp(rule1->fsname, rule2->fsname) != 0)) ||
>> + ((rule1->flags & IMA_PCR) && rule1->pcr != rule2->pcr) ||
>> + ((rule1->flags & IMA_VALIDATE_ALGOS) &&
>> + rule1->allowed_algos != rule2->allowed_algos) ||
>> + ((rule1->flags & IMA_EUID) && !uid_eq(rule1->uid, rule2->uid)) ||
>> + ((rule1->flags & IMA_EGID) && !gid_eq(rule1->gid, rule2->gid)))
>> + return false;
>
> So the goal is to prevent the exact same policy rule from being added, not to update an existing rule, correct? IOW, you could end up with two very similar rules, because the new rule has one thing that's different compared to the existing rule?
The purpose of this patch is to prohibit two exact same rule.
We can have other approaches like merging the new rule to the previously existing rule, ignore
new rule if a similar rule exists. However, this approaches would add more complexity to the code
and are not the purpose of this patch.
> I feel that a little bit of commentary around what makes two rules the same would be useful.
>
>> +
>> + if (!rule1->template && !rule2->template) {
>> + ;
> You're trying to do nothing and continue on. A goto statement would communicate intent better. There are other places below with the same noop structure.
>
> To be fair, I also don't completely understand what you're trying to achieve here, Regardless, this "do nothing inside a conditional" looks weird and I feel like there should be a way to structure your logic without resorting to this.
>
>> + } else if (!rule1->template || !rule2->template) {
>> + return false;
>> + } else if (rule1->template->num_fields != rule2->template->num_fields) {
>> + return false;
>> + } else if (rule1->template->num_fields != 0) {
>> + for (i = 0; i < rule1->template->num_fields; i++) {
>> + if (!template_has_field(rule1->template->fields[i]->field_id,
>> + rule2->template))
>> + return false;
>> + }
>> + }
>
> if + return will achieve the same end goals as else if + return, with lesser clutter. I have seen some static analyzers flag this pattern, but I can't remember which one at the moment.
>
> So something like this:
>
> if (!rule1->template && !rule2->template)
> goto some_target;
>
> if (!rule1->template || !rule2->template)
> return false;
>
> if (rule1->template->num_fields != rule2->template->num_fields)
> return false;
>
> if (rule1->template->num_fields != 0) {
> for (i = 0; i < rule1->template->num_fields; i++) {
> if (!template_has_field(rule1->template->fields[i]->field_id,
> rule2->template))
> return false;
> }
> }> some_target:
> ...
> ...
I don't think having two goto in the code will improve its readability.
>> +
>> + if (rule1->flags & IMA_KEYRINGS) {
>> + if (!rule1->keyrings && !rule2->keyrings) {
>> + ;
>
> Another if block no-op
>
>> + } else if (!rule1->keyrings || !rule2->keyrings) {
>> + return false;
>> + } else if (rule1->keyrings->count != rule2->keyrings->count) {
>> + return false;
>> + } else if (rule1->keyrings->count != 0) {
>
> if (rule1->keyrings->count)
>
>> + for (i = 0; i < rule1->keyrings->count; i++) {
>
> for (int i,
>
>> + if (!keyring_has_item(rule1->keyrings->items[i], rule2->keyrings))
>> + return false;
>> + }
>> + }
>> + }
>> +
>> + if (rule1->flags & IMA_LABEL) {
>> + if (!rule1->label && !rule2->label) {
>> + ;
>
> Another if block no-op
>
>> + } else if (!rule1->label || !rule2->label) {
>> + return false;
>> + } else if (rule1->label->count != rule2->label->count) {
>> + return false;
>> + } else if (rule1->label->count != 0) {
>> + for (i = 0; i < rule1->label->count; i++) {
>> + if (!labels_has_item(rule1->label->items[i], rule2->label))
>> + return false;
>> + }
>> + }
>> + }
>> +
>> + for (i = 0; i < MAX_LSM_RULES; i++) {
>
> for (int i,
>
>> + if (!rule1->lsm[i].rule && !rule2->lsm[i].rule)
>> + continue;
>> +
>> + if (!rule1->lsm[i].rule || !rule2->lsm[i].rule)
>> + return false;
>> +
>> + if (strcmp(rule1->lsm[i].args_p, rule2->lsm[i].args_p) != 0)
>> + return false;
>> + }
>> +
>> + return true;
>> +}
>> +
>> +/**
>> + * ima_rule_exists - check if a rule already exists in the policy
>> + *
>> + * Checking both the active policy and the temporary rules list.
>> + */
>> +static bool ima_rule_exists(struct ima_rule_entry *new_rule)
>> +{
>> + struct ima_rule_entry *entry;
>> + struct list_head *ima_rules_tmp;
>> +
>> + if (!list_empty(&ima_temp_rules)) {
>> + list_for_each_entry(entry, &ima_temp_rules, list) {
>> + if (ima_rules_equal(entry, new_rule))
>> + return true;
>> + }
>> + }
>> +
>> + rcu_read_lock();
>> + ima_rules_tmp = rcu_dereference(ima_rules);
>> + list_for_each_entry_rcu(entry, ima_rules_tmp, list) {
>> + if (ima_rules_equal(entry, new_rule)) {
>> + rcu_read_unlock();
>> + return true;
>> + }
>> + }
>> + rcu_read_unlock();
>> +
>> + return false;
>> +}
>> +
>> /**
>> * ima_parse_add_rule - add a rule to ima_policy_rules
>> * @rule: ima measurement policy rule
>> @@ -1993,7 +2140,15 @@ ssize_t ima_parse_add_rule(char *rule)
>> return result;
>> }
>> - list_add_tail(&entry->list, &ima_temp_rules);
>> + if (!ima_rule_exists(entry)) {
>> + list_add_tail(&entry->list, &ima_temp_rules);
>> + } else {
>> + result = -EEXIST;
> Is it necessary to set result? Or can you just pass -EEXIST to the audit call below?
>
>> + ima_free_rule(entry);
>> + integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL,
>> + NULL, op, "duplicate-policy", result,
>> + audit_info);
>> + }
>> return len;
>> }
I
^ permalink raw reply
* Re: [PATCH v5 11/14] Add start_renaming_two_dentries()
From: Stephen Smalley @ 2025-11-10 17:30 UTC (permalink / raw)
To: NeilBrown
Cc: Alexander Viro, Christian Brauner, Amir Goldstein, Jan Kara,
linux-fsdevel, Jeff Layton, Chris Mason, David Sterba,
David Howells, Greg Kroah-Hartman, Rafael J. Wysocki,
Danilo Krummrich, Tyler Hicks, Miklos Szeredi, Chuck Lever,
Olga Kornievskaia, Dai Ngo, Namjae Jeon, Steve French,
Sergey Senozhatsky, Carlos Maiolino, John Johansen, Paul Moore,
James Morris, Serge E. Hallyn, Ondrej Mosnacek, Mateusz Guzik,
Lorenzo Stoakes, Stefan Berger, Darrick J. Wong, linux-kernel,
netfs, ecryptfs, linux-nfs, linux-unionfs, linux-cifs, linux-xfs,
linux-security-module, selinux
In-Reply-To: <CAEjxPJ528Ou4dvRwHo+kXjWreGicda8BOXkQRvq3vMED6JQKOQ@mail.gmail.com>
On Mon, Nov 10, 2025 at 11:08 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Wed, Nov 5, 2025 at 7:56 PM NeilBrown <neilb@ownmail.net> wrote:
> >
> > From: NeilBrown <neil@brown.name>
> >
> > A few callers want to lock for a rename and already have both dentries.
> > Also debugfs does want to perform a lookup but doesn't want permission
> > checking, so start_renaming_dentry() cannot be used.
> >
> > This patch introduces start_renaming_two_dentries() which is given both
> > dentries. debugfs performs one lookup itself. As it will only continue
> > with a negative dentry and as those cannot be renamed or unlinked, it is
> > safe to do the lookup before getting the rename locks.
> >
> > overlayfs uses start_renaming_two_dentries() in three places and selinux
> > uses it twice in sel_make_policy_nodes().
> >
> > In sel_make_policy_nodes() we now lock for rename twice instead of just
> > once so the combined operation is no longer atomic w.r.t the parent
> > directory locks. As selinux_state.policy_mutex is held across the whole
> > operation this does open up any interesting races.
Also, I assume you mean "does NOT open up any interesting races" above.
> >
> > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> > Signed-off-by: NeilBrown <neil@brown.name>
> >
> > ---
> > changes since v3:
> > added missing assignment to rd.mnt_idmap in ovl_cleanup_and_whiteout
> > ---
>
> > diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
> > index 232e087bce3e..a224ef9bb831 100644
> > --- a/security/selinux/selinuxfs.c
> > +++ b/security/selinux/selinuxfs.c
> > @@ -539,22 +540,30 @@ static int sel_make_policy_nodes(struct selinux_fs_info *fsi,
> > if (ret)
> > goto out;
> >
> > - lock_rename(tmp_parent, fsi->sb->s_root);
> > + rd.old_parent = tmp_parent;
> > + rd.new_parent = fsi->sb->s_root;
> >
> > /* booleans */
> > - d_exchange(tmp_bool_dir, fsi->bool_dir);
> > + ret = start_renaming_two_dentries(&rd, tmp_bool_dir, fsi->bool_dir);
> > + if (!ret) {
> > + d_exchange(tmp_bool_dir, fsi->bool_dir);
>
> I would recommend an immediate goto out if ret != 0; we don't want to
> silently fall through and possibly reset ret on the next
> start_renaming_two_dentries() call, thereby ultimately returning 0 to
> the caller and acting as if nothing bad happened.
>
> >
> > - swap(fsi->bool_num, bool_num);
> > - swap(fsi->bool_pending_names, bool_names);
> > - swap(fsi->bool_pending_values, bool_values);
> > + swap(fsi->bool_num, bool_num);
> > + swap(fsi->bool_pending_names, bool_names);
> > + swap(fsi->bool_pending_values, bool_values);
> >
> > - fsi->bool_dir = tmp_bool_dir;
> > + fsi->bool_dir = tmp_bool_dir;
> > + end_renaming(&rd);
> > + }
> >
> > /* classes */
> > - d_exchange(tmp_class_dir, fsi->class_dir);
> > - fsi->class_dir = tmp_class_dir;
> > + ret = start_renaming_two_dentries(&rd, tmp_class_dir, fsi->class_dir);
> > + if (ret == 0) {
> > + d_exchange(tmp_class_dir, fsi->class_dir);
> > + fsi->class_dir = tmp_class_dir;
> >
> > - unlock_rename(tmp_parent, fsi->sb->s_root);
> > + end_renaming(&rd);
> > + }
> >
> > out:
> > sel_remove_old_bool_data(bool_num, bool_names, bool_values);
> > --
> > 2.50.0.107.gf914562f5916.dirty
> >
^ permalink raw reply
* Re: [PATCH v5 11/14] Add start_renaming_two_dentries()
From: Stephen Smalley @ 2025-11-10 16:08 UTC (permalink / raw)
To: NeilBrown
Cc: Alexander Viro, Christian Brauner, Amir Goldstein, Jan Kara,
linux-fsdevel, Jeff Layton, Chris Mason, David Sterba,
David Howells, Greg Kroah-Hartman, Rafael J. Wysocki,
Danilo Krummrich, Tyler Hicks, Miklos Szeredi, Chuck Lever,
Olga Kornievskaia, Dai Ngo, Namjae Jeon, Steve French,
Sergey Senozhatsky, Carlos Maiolino, John Johansen, Paul Moore,
James Morris, Serge E. Hallyn, Ondrej Mosnacek, Mateusz Guzik,
Lorenzo Stoakes, Stefan Berger, Darrick J. Wong, linux-kernel,
netfs, ecryptfs, linux-nfs, linux-unionfs, linux-cifs, linux-xfs,
linux-security-module, selinux
In-Reply-To: <20251106005333.956321-12-neilb@ownmail.net>
On Wed, Nov 5, 2025 at 7:56 PM NeilBrown <neilb@ownmail.net> wrote:
>
> From: NeilBrown <neil@brown.name>
>
> A few callers want to lock for a rename and already have both dentries.
> Also debugfs does want to perform a lookup but doesn't want permission
> checking, so start_renaming_dentry() cannot be used.
>
> This patch introduces start_renaming_two_dentries() which is given both
> dentries. debugfs performs one lookup itself. As it will only continue
> with a negative dentry and as those cannot be renamed or unlinked, it is
> safe to do the lookup before getting the rename locks.
>
> overlayfs uses start_renaming_two_dentries() in three places and selinux
> uses it twice in sel_make_policy_nodes().
>
> In sel_make_policy_nodes() we now lock for rename twice instead of just
> once so the combined operation is no longer atomic w.r.t the parent
> directory locks. As selinux_state.policy_mutex is held across the whole
> operation this does open up any interesting races.
>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: NeilBrown <neil@brown.name>
>
> ---
> changes since v3:
> added missing assignment to rd.mnt_idmap in ovl_cleanup_and_whiteout
> ---
> diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
> index 232e087bce3e..a224ef9bb831 100644
> --- a/security/selinux/selinuxfs.c
> +++ b/security/selinux/selinuxfs.c
> @@ -539,22 +540,30 @@ static int sel_make_policy_nodes(struct selinux_fs_info *fsi,
> if (ret)
> goto out;
>
> - lock_rename(tmp_parent, fsi->sb->s_root);
> + rd.old_parent = tmp_parent;
> + rd.new_parent = fsi->sb->s_root;
>
> /* booleans */
> - d_exchange(tmp_bool_dir, fsi->bool_dir);
> + ret = start_renaming_two_dentries(&rd, tmp_bool_dir, fsi->bool_dir);
> + if (!ret) {
> + d_exchange(tmp_bool_dir, fsi->bool_dir);
I would recommend an immediate goto out if ret != 0; we don't want to
silently fall through and possibly reset ret on the next
start_renaming_two_dentries() call, thereby ultimately returning 0 to
the caller and acting as if nothing bad happened.
>
> - swap(fsi->bool_num, bool_num);
> - swap(fsi->bool_pending_names, bool_names);
> - swap(fsi->bool_pending_values, bool_values);
> + swap(fsi->bool_num, bool_num);
> + swap(fsi->bool_pending_names, bool_names);
> + swap(fsi->bool_pending_values, bool_values);
>
> - fsi->bool_dir = tmp_bool_dir;
> + fsi->bool_dir = tmp_bool_dir;
> + end_renaming(&rd);
> + }
>
> /* classes */
> - d_exchange(tmp_class_dir, fsi->class_dir);
> - fsi->class_dir = tmp_class_dir;
> + ret = start_renaming_two_dentries(&rd, tmp_class_dir, fsi->class_dir);
> + if (ret == 0) {
> + d_exchange(tmp_class_dir, fsi->class_dir);
> + fsi->class_dir = tmp_class_dir;
>
> - unlock_rename(tmp_parent, fsi->sb->s_root);
> + end_renaming(&rd);
> + }
>
> out:
> sel_remove_old_bool_data(bool_num, bool_names, bool_values);
> --
> 2.50.0.107.gf914562f5916.dirty
>
^ permalink raw reply
* Re: [RFC PATCH 2/3] exec: don't wait for zombie threads with cred_guard_mutex held
From: Oleg Nesterov @ 2025-11-10 15:09 UTC (permalink / raw)
To: Cyrill Gorcunov
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest, linux-mm, linux-security-module
In-Reply-To: <aRHFSrTxYSOkFic7@grain>
Hi Cyrill,
On 11/10, Cyrill Gorcunov wrote:
>
> On Sun, Nov 09, 2025 at 06:15:33PM +0100, Oleg Nesterov wrote:
> ..
> > static int kill_sub_threads(struct task_struct *tsk)
> > {
> > struct signal_struct *sig = tsk->signal;
> > int err = -EINTR;
> >
> > read_lock(&tasklist_lock);
> > spin_lock_irq(&tsk->sighand->siglock);
> > if (!((sig->flags & SIGNAL_GROUP_EXIT) || sig->group_exec_task)) {
> > sig->group_exec_task = tsk;
> > sig->notify_count = -zap_other_threads(tsk);
>
> Hi Oleg! I somehow manage to miss a moment -- why negative result here?
You know, initially I wrote
sig->notify_count = 0 - zap_other_threads(tsk);
to make it clear that this is not a typo ;)
This is for exit_notify() which does
/* mt-exec, de_thread() -> wait_for_notify_count() */
if (tsk->signal->notify_count < 0 && !++tsk->signal->notify_count)
wake_up_process(tsk->signal->group_exec_task);
Then setup_new_exec() sets notify_count > 0 for __exit_signal() which does
/* mt-exec, setup_new_exec() -> wait_for_notify_count() */
if (sig->notify_count > 0 && !--sig->notify_count)
wake_up_process(sig->group_exec_task);
Yes this needs more comments and (with or without this patch) cleanups.
Note that exit_notify() and __exit_signal() already (before this patch)
use ->notify_count almost the same way, just exit_notify() assumes that
notify_count < 0 means the !thread_group_leader() case in de_thread().
Oleg.
^ permalink raw reply
* Re: [RFC PATCH 0/3] mt-exec: fix deadlock with ptrace_attach()
From: Oleg Nesterov @ 2025-11-10 14:47 UTC (permalink / raw)
To: Bernd Edlinger
Cc: Linus Torvalds, Dmitry Levin, Alexander Viro, Alexey Dobriyan,
Kees Cook, Andy Lutomirski, Will Drewry, Christian Brauner,
Andrew Morton, Michal Hocko, Serge Hallyn, James Morris,
Randy Dunlap, Suren Baghdasaryan, Yafang Shao, Helge Deller,
Eric W. Biederman, Adrian Reber, Thomas Gleixner, Jens Axboe,
Alexei Starovoitov, linux-fsdevel@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-kselftest, linux-mm,
linux-security-module, tiozhang, Luis Chamberlain,
Paulo Alcantara (SUSE), Sergey Senozhatsky, Frederic Weisbecker,
YueHaibing, Paul Moore, Aleksa Sarai, Stefan Roesch, Chao Yu,
xu xin, Jeff Layton, Jan Kara, David Hildenbrand, Dave Chinner,
Shuah Khan, Elena Reshetova, David Windsor, Mateusz Guzik,
Ard Biesheuvel, Joel Fernandes (Google), Matthew Wilcox (Oracle),
Hans Liljestrand, Penglei Jiang, Lorenzo Stoakes, Adrian Ratiu,
Ingo Molnar, Peter Zijlstra (Intel), Cyrill Gorcunov,
Eric Dumazet
In-Reply-To: <GV2PPF74270EBEE83C2CA09B945BC954FA3E4CEA@GV2PPF74270EBEE.EURP195.PROD.OUTLOOK.COM>
Hi Bernd,
On 11/10, Bernd Edlinger wrote:
>
> When the debugger wants to attach the de_thread the debug-user access rights are
> checked against the current user and additionally against the new user credentials.
> This I did by quickly switching the user credenitals to the next user and back again,
> under the cred_guard_mutex, which should make that safe.
Let me repeat, I can't really comment this part, I don't know if it is
actually safe. But the very fact your patch changes ->mm and ->cred of
the execing task in ptrace_attach() makes me worry... At least I think
you should update or remove this comment in begin_new_exec:
/*
* cred_guard_mutex must be held at least to this point to prevent
* ptrace_attach() from altering our determination of the task's
* credentials; any time after this it may be unlocked.
*/
security_bprm_committed_creds(bprm);
> So at this time I have only one request for you.
> Could you please try out how the test case in my patch behaves with your fix?
The new TEST(attach2) added by your patch fails as expected, see 3/3.
128 static long thread2_tid;
129 static void *thread2(void *arg)
130 {
131 thread2_tid = syscall(__NR_gettid);
132 sleep(2);
133 execlp("false", "false", NULL);
134 return NULL;
135 }
136
137 TEST(attach2)
138 {
139 int s, k, pid = fork();
140
141 if (!pid) {
142 pthread_t pt;
143
144 pthread_create(&pt, NULL, thread2, NULL);
145 pthread_join(pt, NULL);
146 return;
147 }
148
149 sleep(1);
150 k = ptrace(PTRACE_ATTACH, pid, 0L, 0L);
151 ASSERT_EQ(k, 0);
152 k = waitpid(-1, &s, 0);
153 ASSERT_EQ(k, pid);
154 ASSERT_EQ(WIFSTOPPED(s), 1);
155 ASSERT_EQ(WSTOPSIG(s), SIGSTOP);
156 k = ptrace(PTRACE_SETOPTIONS, pid, 0L, PTRACE_O_TRACEEXIT);
157 ASSERT_EQ(k, 0);
158 thread2_tid = ptrace(PTRACE_PEEKDATA, pid, &thread2_tid, 0L);
159 ASSERT_NE(thread2_tid, -1);
160 ASSERT_NE(thread2_tid, 0);
161 ASSERT_NE(thread2_tid, pid);
162 k = waitpid(-1, &s, WNOHANG);
163 ASSERT_EQ(k, 0);
164 sleep(2);
165 /* deadlock may happen here */
166 k = ptrace(PTRACE_ATTACH, thread2_tid, 0L, 0L);
PTRACE_ATTACH fails.
thread2() kills the old leader, takes it pid, execlp() succeeds.
Oleg.
^ permalink raw reply
* [PATCH] security: Add KUnit tests for rootid_owns_currentns()
From: Ryan Foster @ 2025-11-10 14:37 UTC (permalink / raw)
To: selinux; +Cc: linux-security-module, linux-kernel, serge, paul, Ryan Foster
The rootid_owns_currentns() function in security/commoncap.c is a
security-critical function used to determine if a root ID from a
parent namespace owns the current namespace. This function is used
in multiple security-critical paths:
1. cap_inode_getsecurity() - file capability retrieval
2. get_vfs_caps_from_disk() - file capability loading during exec
Despite its security-critical nature, this function had no test
coverage. This patch adds KUnit tests to verify the function's
behavior in various scenarios:
- Root ID in init namespace (positive case)
- Invalid vfsuid (negative case)
- Non-root UID (negative case)
The function is made testable by exporting it when
CONFIG_SECURITY_COMMONCAP_KUNIT_TEST is enabled, while maintaining
static visibility in production builds.
This follows the pattern established by other security subsystems
(IPE, Landlock) for KUnit testing.
Signed-off-by: Ryan Foster <foster.ryan.r@gmail.com>
---
security/Kconfig | 17 ++++++
security/Makefile | 1 +
security/commoncap.c | 5 ++
security/commoncap_test.c | 109 ++++++++++++++++++++++++++++++++++++++
4 files changed, 132 insertions(+)
create mode 100644 security/commoncap_test.c
diff --git a/security/Kconfig b/security/Kconfig
index 285f284dfcac..c7b3f42ef875 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -284,6 +284,23 @@ config LSM
If unsure, leave this as the default.
+config SECURITY_COMMONCAP_KUNIT_TEST
+ bool "Build KUnit tests for commoncap" if !KUNIT_ALL_TESTS
+ depends on KUNIT=y
+ default KUNIT_ALL_TESTS
+ help
+ This builds the commoncap KUnit tests.
+
+ KUnit tests run during boot and output the results to the debug log
+ in TAP format (https://testanything.org/). Only useful for kernel devs
+ running KUnit test harness and are not for inclusion into a
+ production build.
+
+ For more information on KUnit and unit tests in general please refer
+ to the KUnit documentation in Documentation/dev-tools/kunit/.
+
+ If unsure, say N.
+
source "security/Kconfig.hardening"
endmenu
diff --git a/security/Makefile b/security/Makefile
index 22ff4c8bd8ce..5b1285fde552 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_KEYS) += keys/
# always enable default capabilities
obj-y += commoncap.o
+obj-$(CONFIG_SECURITY_COMMONCAP_KUNIT_TEST) += commoncap_test.o
obj-$(CONFIG_SECURITY) += lsm_syscalls.o
obj-$(CONFIG_MMU) += min_addr.o
diff --git a/security/commoncap.c b/security/commoncap.c
index 6bd4adeb4795..4d0e014ce7cd 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -358,7 +358,12 @@ int cap_inode_killpriv(struct mnt_idmap *idmap, struct dentry *dentry)
return error;
}
+#ifdef CONFIG_SECURITY_COMMONCAP_KUNIT_TEST
+bool rootid_owns_currentns(vfsuid_t rootvfsuid);
+bool rootid_owns_currentns(vfsuid_t rootvfsuid)
+#else
static bool rootid_owns_currentns(vfsuid_t rootvfsuid)
+#endif
{
struct user_namespace *ns;
kuid_t kroot;
diff --git a/security/commoncap_test.c b/security/commoncap_test.c
new file mode 100644
index 000000000000..9757d433d314
--- /dev/null
+++ b/security/commoncap_test.c
@@ -0,0 +1,109 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * KUnit tests for commoncap.c security functions
+ *
+ * Tests for security-critical functions in the capability subsystem,
+ * particularly namespace-related capability checks.
+ */
+
+#include <kunit/test.h>
+#include <linux/user_namespace.h>
+#include <linux/uidgid.h>
+#include <linux/module.h>
+
+/* We need to include the actual vfsuid_t definition but avoid the problematic
+ * inline functions in mnt_idmapping.h. Include just the type definitions.
+ */
+#include <linux/types.h>
+
+/* Forward declare the minimal types we need - match the actual kernel definitions */
+struct mnt_idmap;
+typedef struct {
+ uid_t val;
+} vfsuid_t;
+
+/* Minimal macros we need - match kernel definitions from mnt_idmapping.h */
+static inline uid_t __vfsuid_val(vfsuid_t uid)
+{
+ return uid.val;
+}
+
+#define VFSUIDT_INIT(val) ((vfsuid_t){ __kuid_val(val) })
+#define INVALID_VFSUID VFSUIDT_INIT(INVALID_UID)
+
+#ifdef CONFIG_SECURITY_COMMONCAP_KUNIT_TEST
+
+/* Forward declaration - function is exported when KUNIT_TEST is enabled */
+extern bool rootid_owns_currentns(vfsuid_t rootvfsuid);
+
+/**
+ * test_rootid_owns_currentns_init_ns - Test rootid_owns_currentns with init ns
+ *
+ * Verifies that a root ID in the init namespace correctly owns the current
+ * namespace when running in init_user_ns.
+ */
+static void test_rootid_owns_currentns_init_ns(struct kunit *test)
+{
+ vfsuid_t root_vfsuid;
+ kuid_t root_kuid;
+
+ /* Create a root UID in init namespace */
+ root_kuid = KUIDT_INIT(0);
+ root_vfsuid = VFSUIDT_INIT(root_kuid);
+
+ /* In init namespace, root should own current namespace */
+ KUNIT_EXPECT_TRUE(test, rootid_owns_currentns(root_vfsuid));
+}
+
+/**
+ * test_rootid_owns_currentns_invalid - Test rootid_owns_currentns with invalid vfsuid
+ *
+ * Verifies that an invalid vfsuid correctly returns false.
+ */
+static void test_rootid_owns_currentns_invalid(struct kunit *test)
+{
+ vfsuid_t invalid_vfsuid;
+
+ /* Use the predefined invalid vfsuid */
+ invalid_vfsuid = INVALID_VFSUID;
+
+ /* Invalid vfsuid should return false */
+ KUNIT_EXPECT_FALSE(test, rootid_owns_currentns(invalid_vfsuid));
+}
+
+/**
+ * test_rootid_owns_currentns_nonroot - Test rootid_owns_currentns with non-root UID
+ *
+ * Verifies that a non-root UID correctly returns false.
+ */
+static void test_rootid_owns_currentns_nonroot(struct kunit *test)
+{
+ vfsuid_t nonroot_vfsuid;
+ kuid_t nonroot_kuid;
+
+ /* Create a non-root UID */
+ nonroot_kuid = KUIDT_INIT(1000);
+ nonroot_vfsuid = VFSUIDT_INIT(nonroot_kuid);
+
+ /* Non-root UID should return false */
+ KUNIT_EXPECT_FALSE(test, rootid_owns_currentns(nonroot_vfsuid));
+}
+
+static struct kunit_case commoncap_test_cases[] = {
+ KUNIT_CASE(test_rootid_owns_currentns_init_ns),
+ KUNIT_CASE(test_rootid_owns_currentns_invalid),
+ KUNIT_CASE(test_rootid_owns_currentns_nonroot),
+ {}
+};
+
+static struct kunit_suite commoncap_test_suite = {
+ .name = "commoncap",
+ .test_cases = commoncap_test_cases,
+};
+
+kunit_test_suite(commoncap_test_suite);
+
+MODULE_LICENSE("GPL");
+
+#endif /* CONFIG_SECURITY_COMMONCAP_KUNIT_TEST */
+
--
2.43.0
^ permalink raw reply related
* Re: [RFC PATCH 2/3] exec: don't wait for zombie threads with cred_guard_mutex held
From: Cyrill Gorcunov @ 2025-11-10 10:58 UTC (permalink / raw)
To: Oleg Nesterov
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest, linux-mm, linux-security-module
In-Reply-To: <aRDMNWx-69fL_gf-@redhat.com>
On Sun, Nov 09, 2025 at 06:15:33PM +0100, Oleg Nesterov wrote:
..
> static int kill_sub_threads(struct task_struct *tsk)
> {
> struct signal_struct *sig = tsk->signal;
> int err = -EINTR;
>
> read_lock(&tasklist_lock);
> spin_lock_irq(&tsk->sighand->siglock);
> if (!((sig->flags & SIGNAL_GROUP_EXIT) || sig->group_exec_task)) {
> sig->group_exec_task = tsk;
> sig->notify_count = -zap_other_threads(tsk);
Hi Oleg! I somehow manage to miss a moment -- why negative result here?
> err = 0;
> }
> spin_unlock_irq(&tsk->sighand->siglock);
> read_unlock(&tasklist_lock);
>
> return err;
> }
p.s. i've dropped long CC but left ML intact)
Cyrill
^ permalink raw reply
* Re: [syzbot] [lsm?] WARNING in put_cred_rcu
From: syzbot @ 2025-11-10 10:09 UTC (permalink / raw)
To: brauner, linux-kernel, linux-security-module, luto, peterz,
syzkaller-bugs, tglx
In-Reply-To: <20251110-hufen-klang-9cf8ec9dda59@brauner>
Hello,
syzbot has tested the proposed patch and the reproducer did not trigger any issue:
Reported-by: syzbot+553c4078ab14e3cf3358@syzkaller.appspotmail.com
Tested-by: syzbot+553c4078ab14e3cf3358@syzkaller.appspotmail.com
Tested on:
commit: 80156be0 ns: fixes for namespace iteration and active ..
git tree: https://github.com/brauner/linux.git work.namespace-6.19.fixes
console output: https://syzkaller.appspot.com/x/log.txt?x=17fc5412580000
kernel config: https://syzkaller.appspot.com/x/.config?x=e31f5f45f87b6763
dashboard link: https://syzkaller.appspot.com/bug?extid=553c4078ab14e3cf3358
compiler: Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8
Note: no patches were applied.
Note: testing is done by a robot and is best-effort only.
^ permalink raw reply
* Re: [syzbot] [lsm?] WARNING in put_cred_rcu
From: Christian Brauner @ 2025-11-10 8:45 UTC (permalink / raw)
To: syzbot
Cc: linux-kernel, linux-security-module, luto, peterz, syzkaller-bugs,
tglx
In-Reply-To: <690dfca7.a70a0220.22f260.004d.GAE@google.com>
On Fri, Nov 07, 2025 at 06:05:27AM -0800, syzbot wrote:
> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit: 17490bd0527f Add linux-next specific files for 20251104
> git tree: linux-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=16c09bcd980000
> kernel config: https://syzkaller.appspot.com/x/.config?x=9995c0d2611ab121
> dashboard link: https://syzkaller.appspot.com/bug?extid=553c4078ab14e3cf3358
> compiler: Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1500a532580000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=12e0a114580000
>
> Downloadable assets:
> disk image: https://storage.googleapis.com/syzbot-assets/a4d318147846/disk-17490bd0.raw.xz
> vmlinux: https://storage.googleapis.com/syzbot-assets/86641a470170/vmlinux-17490bd0.xz
> kernel image: https://storage.googleapis.com/syzbot-assets/35c008a540c8/bzImage-17490bd0.xz
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+553c4078ab14e3cf3358@syzkaller.appspotmail.com
#syz test: https://github.com/brauner/linux.git work.namespace-6.19.fixes
^ permalink raw reply
* Re: [RFC PATCH 0/3] mt-exec: fix deadlock with ptrace_attach()
From: Bernd Edlinger @ 2025-11-10 5:28 UTC (permalink / raw)
To: Oleg Nesterov, Linus Torvalds, Dmitry Levin
Cc: Alexander Viro, Alexey Dobriyan, Kees Cook, Andy Lutomirski,
Will Drewry, Christian Brauner, Andrew Morton, Michal Hocko,
Serge Hallyn, James Morris, Randy Dunlap, Suren Baghdasaryan,
Yafang Shao, Helge Deller, Eric W. Biederman, Adrian Reber,
Thomas Gleixner, Jens Axboe, Alexei Starovoitov,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest, linux-mm, linux-security-module, tiozhang,
Luis Chamberlain, Paulo Alcantara (SUSE), Sergey Senozhatsky,
Frederic Weisbecker, YueHaibing, Paul Moore, Aleksa Sarai,
Stefan Roesch, Chao Yu, xu xin, Jeff Layton, Jan Kara,
David Hildenbrand, Dave Chinner, Shuah Khan, Elena Reshetova,
David Windsor, Mateusz Guzik, Ard Biesheuvel,
Joel Fernandes (Google), Matthew Wilcox (Oracle),
Hans Liljestrand, Penglei Jiang, Lorenzo Stoakes, Adrian Ratiu,
Ingo Molnar, Peter Zijlstra (Intel), Cyrill Gorcunov,
Eric Dumazet
In-Reply-To: <aRDL3HOB21pMVMWC@redhat.com>
Hi Oleg,
I have not been able to update my patch with your and Peter Peter Zijlstra's
kind suggestions, because I am currently too busy with my role as openssl maintainer.
Just for clarification, my patch is 10% about deadlocks, and 90% about security.
The idea is that if the de_thread is blocked, and the debugger may be trying to
ptrace the exec thread. That must succeed or fail. So the debugger can release
the zombie threads.
The security issue is when the debugged process tries to exec a SUID process
like /usr/bin/passwd
In that case the new credentials are determined differently when the PTRACE is
already attached (i.e. non-root), than when it is not yet attached (root user).
My attempt at fixing this, determines the new credentials and the new dumpability
as root user when the debugger did not yet attach before the de_thread.
And keeps this decision.
When the debugger wants to attach the de_thread the debug-user access rights are
checked against the current user and additionally against the new user credentials.
This I did by quickly switching the user credenitals to the next user and back again,
under the cred_guard_mutex, which should make that safe.
So at this time I have only one request for you.
Could you please try out how the test case in my patch behaves with your fix?
Thanks
Bernd.
On 11/9/25 18:14, Oleg Nesterov wrote:
> Not for inclusion yet. 2/2 is untested, incomplete, possibly buggy.
>
> But could you review at least the intent? Do you see any problem with
> this approach?
>
> This problem is very, very old. It seems that nobody can suggest a
> simple/clean fix...
>
> Oleg.
> ---
>
> fs/binfmt_elf.c | 4 +-
> fs/binfmt_elf_fdpic.c | 4 +-
> fs/binfmt_flat.c | 4 +-
> fs/exec.c | 142 +++++++++++++++++++++++-------------------------
> include/linux/binfmts.h | 2 +-
> kernel/exit.c | 9 +--
> kernel/signal.c | 6 +-
> 7 files changed, 87 insertions(+), 84 deletions(-)
>
^ permalink raw reply
* [PATCH] security: Add KUnit tests for rootid_owns_currentns()
From: ryan foster @ 2025-11-10 1:16 UTC (permalink / raw)
To: linux-security-module; +Cc: linux-kernel, kunit-dev, serge, paul
[-- Attachment #1.1: Type: text/plain, Size: 111 bytes --]
Please review this patch that adds KUnit tests for
rootid_owns_currentns() function in security/commoncap.c
[-- Attachment #1.2: Type: text/html, Size: 138 bytes --]
[-- Attachment #2: 0001-security-Add-KUnit-tests-for-rootid_owns_currentns.patch --]
[-- Type: text/x-patch, Size: 6553 bytes --]
From 12ab4ef96bdc856cb7287b442ab0b545c6c8a4cb Mon Sep 17 00:00:00 2001
From: Ryan Foster <foster.ryan.r@gmail.com>
Date: Sun, 9 Nov 2025 16:51:26 -0800
Subject: [PATCH] security: Add KUnit tests for rootid_owns_currentns()
The rootid_owns_currentns() function in security/commoncap.c is a
security-critical function used to determine if a root ID from a
parent namespace owns the current namespace. This function is used
in multiple security-critical paths:
1. cap_inode_getsecurity() - file capability retrieval
2. get_vfs_caps_from_disk() - file capability loading during exec
Despite its security-critical nature, this function had no test
coverage. This patch adds KUnit tests to verify the function's
behavior in various scenarios:
- Root ID in init namespace (positive case)
- Invalid vfsuid (negative case)
- Non-root UID (negative case)
The function is made testable by exporting it when
CONFIG_SECURITY_COMMONCAP_KUNIT_TEST is enabled, while maintaining
static visibility in production builds.
This follows the pattern established by other security subsystems
(IPE, Landlock) for KUnit testing.
Signed-off-by: Ryan Foster <foster.ryan.r@gmail.com>
---
security/Kconfig | 17 ++++++
security/Makefile | 1 +
security/commoncap.c | 5 ++
security/commoncap_test.c | 109 ++++++++++++++++++++++++++++++++++++++
4 files changed, 132 insertions(+)
create mode 100644 security/commoncap_test.c
diff --git a/security/Kconfig b/security/Kconfig
index 285f284dfcac..c7b3f42ef875 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -284,6 +284,23 @@ config LSM
If unsure, leave this as the default.
+config SECURITY_COMMONCAP_KUNIT_TEST
+ bool "Build KUnit tests for commoncap" if !KUNIT_ALL_TESTS
+ depends on KUNIT=y
+ default KUNIT_ALL_TESTS
+ help
+ This builds the commoncap KUnit tests.
+
+ KUnit tests run during boot and output the results to the debug log
+ in TAP format (https://testanything.org/). Only useful for kernel devs
+ running KUnit test harness and are not for inclusion into a
+ production build.
+
+ For more information on KUnit and unit tests in general please refer
+ to the KUnit documentation in Documentation/dev-tools/kunit/.
+
+ If unsure, say N.
+
source "security/Kconfig.hardening"
endmenu
diff --git a/security/Makefile b/security/Makefile
index 22ff4c8bd8ce..5b1285fde552 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_KEYS) += keys/
# always enable default capabilities
obj-y += commoncap.o
+obj-$(CONFIG_SECURITY_COMMONCAP_KUNIT_TEST) += commoncap_test.o
obj-$(CONFIG_SECURITY) += lsm_syscalls.o
obj-$(CONFIG_MMU) += min_addr.o
diff --git a/security/commoncap.c b/security/commoncap.c
index 6bd4adeb4795..4d0e014ce7cd 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -358,7 +358,12 @@ int cap_inode_killpriv(struct mnt_idmap *idmap, struct dentry *dentry)
return error;
}
+#ifdef CONFIG_SECURITY_COMMONCAP_KUNIT_TEST
+bool rootid_owns_currentns(vfsuid_t rootvfsuid);
+bool rootid_owns_currentns(vfsuid_t rootvfsuid)
+#else
static bool rootid_owns_currentns(vfsuid_t rootvfsuid)
+#endif
{
struct user_namespace *ns;
kuid_t kroot;
diff --git a/security/commoncap_test.c b/security/commoncap_test.c
new file mode 100644
index 000000000000..9757d433d314
--- /dev/null
+++ b/security/commoncap_test.c
@@ -0,0 +1,109 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * KUnit tests for commoncap.c security functions
+ *
+ * Tests for security-critical functions in the capability subsystem,
+ * particularly namespace-related capability checks.
+ */
+
+#include <kunit/test.h>
+#include <linux/user_namespace.h>
+#include <linux/uidgid.h>
+#include <linux/module.h>
+
+/* We need to include the actual vfsuid_t definition but avoid the problematic
+ * inline functions in mnt_idmapping.h. Include just the type definitions.
+ */
+#include <linux/types.h>
+
+/* Forward declare the minimal types we need - match the actual kernel definitions */
+struct mnt_idmap;
+typedef struct {
+ uid_t val;
+} vfsuid_t;
+
+/* Minimal macros we need - match kernel definitions from mnt_idmapping.h */
+static inline uid_t __vfsuid_val(vfsuid_t uid)
+{
+ return uid.val;
+}
+
+#define VFSUIDT_INIT(val) ((vfsuid_t){ __kuid_val(val) })
+#define INVALID_VFSUID VFSUIDT_INIT(INVALID_UID)
+
+#ifdef CONFIG_SECURITY_COMMONCAP_KUNIT_TEST
+
+/* Forward declaration - function is exported when KUNIT_TEST is enabled */
+extern bool rootid_owns_currentns(vfsuid_t rootvfsuid);
+
+/**
+ * test_rootid_owns_currentns_init_ns - Test rootid_owns_currentns with init ns
+ *
+ * Verifies that a root ID in the init namespace correctly owns the current
+ * namespace when running in init_user_ns.
+ */
+static void test_rootid_owns_currentns_init_ns(struct kunit *test)
+{
+ vfsuid_t root_vfsuid;
+ kuid_t root_kuid;
+
+ /* Create a root UID in init namespace */
+ root_kuid = KUIDT_INIT(0);
+ root_vfsuid = VFSUIDT_INIT(root_kuid);
+
+ /* In init namespace, root should own current namespace */
+ KUNIT_EXPECT_TRUE(test, rootid_owns_currentns(root_vfsuid));
+}
+
+/**
+ * test_rootid_owns_currentns_invalid - Test rootid_owns_currentns with invalid vfsuid
+ *
+ * Verifies that an invalid vfsuid correctly returns false.
+ */
+static void test_rootid_owns_currentns_invalid(struct kunit *test)
+{
+ vfsuid_t invalid_vfsuid;
+
+ /* Use the predefined invalid vfsuid */
+ invalid_vfsuid = INVALID_VFSUID;
+
+ /* Invalid vfsuid should return false */
+ KUNIT_EXPECT_FALSE(test, rootid_owns_currentns(invalid_vfsuid));
+}
+
+/**
+ * test_rootid_owns_currentns_nonroot - Test rootid_owns_currentns with non-root UID
+ *
+ * Verifies that a non-root UID correctly returns false.
+ */
+static void test_rootid_owns_currentns_nonroot(struct kunit *test)
+{
+ vfsuid_t nonroot_vfsuid;
+ kuid_t nonroot_kuid;
+
+ /* Create a non-root UID */
+ nonroot_kuid = KUIDT_INIT(1000);
+ nonroot_vfsuid = VFSUIDT_INIT(nonroot_kuid);
+
+ /* Non-root UID should return false */
+ KUNIT_EXPECT_FALSE(test, rootid_owns_currentns(nonroot_vfsuid));
+}
+
+static struct kunit_case commoncap_test_cases[] = {
+ KUNIT_CASE(test_rootid_owns_currentns_init_ns),
+ KUNIT_CASE(test_rootid_owns_currentns_invalid),
+ KUNIT_CASE(test_rootid_owns_currentns_nonroot),
+ {}
+};
+
+static struct kunit_suite commoncap_test_suite = {
+ .name = "commoncap",
+ .test_cases = commoncap_test_cases,
+};
+
+kunit_test_suite(commoncap_test_suite);
+
+MODULE_LICENSE("GPL");
+
+#endif /* CONFIG_SECURITY_COMMONCAP_KUNIT_TEST */
+
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] security: provide an inlined static branch for security_inode_permission()
From: Mateusz Guzik @ 2025-11-09 21:52 UTC (permalink / raw)
To: Paul Moore
Cc: casey, keescook, song, andrii, kpsingh, linux-security-module,
linux-fsdevel
In-Reply-To: <CAHC9VhRCvoXrUESCjbxz5Bcxjq8GXLj4GvSoXq+sukdP1cuXNg@mail.gmail.com>
On Sun, Nov 9, 2025 at 10:29 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Sun, Nov 9, 2025 at 2:29 PM Mateusz Guzik <mjguzik@gmail.com> wrote:
> >
> > The routine is executing for every path component during name resolution in
> > vfs and shows up on the profile to the tune of 2% of CPU time in my
> > tests.
> >
> > The only LSMs which install hoooks there are selinux and smack, meaning
> > most installs don't have it and this ends up being a call to do nothing.
>
> Unless you have a reputable survey or analysis to back up claims like
> this, please refrain from making comments like these in commit
> messages. I can't speak to Smack's adoption numbers, but last I
> looked in 2023, and considering only Android since numbers were easy
> to source, SELinux was deployed in enforcing mode on over 3 billion
> systems. Of course I don't have numbers handy for *all* Linux
> systems, and there are some numbers that simply are never going to be
> public, but given the +3 billion Android systems alone, I think there
> is a very real likelihood that there are more systems running SELinux
> than those that are not.
>
Fair, I was mostly thinking stuff like Ubuntu. Phone stuff is not on my radar.
> > While perhaps a more generic mechanism covering all hoooks would be
> > preferred, I implemented a bare minimum version which gets out of the
> > way for my needs.
>
> I'd much rather see a generalized solution than hacks for a small
> number of hooks.
>
I'll ponder about this.
> --
> paul-moore.com
^ permalink raw reply
* Re: [PATCH] security: provide an inlined static branch for security_inode_permission()
From: Paul Moore @ 2025-11-09 21:29 UTC (permalink / raw)
To: Mateusz Guzik
Cc: casey, keescook, song, andrii, kpsingh, linux-security-module,
linux-fsdevel
In-Reply-To: <20251109192940.1334775-1-mjguzik@gmail.com>
On Sun, Nov 9, 2025 at 2:29 PM Mateusz Guzik <mjguzik@gmail.com> wrote:
>
> The routine is executing for every path component during name resolution in
> vfs and shows up on the profile to the tune of 2% of CPU time in my
> tests.
>
> The only LSMs which install hoooks there are selinux and smack, meaning
> most installs don't have it and this ends up being a call to do nothing.
Unless you have a reputable survey or analysis to back up claims like
this, please refrain from making comments like these in commit
messages. I can't speak to Smack's adoption numbers, but last I
looked in 2023, and considering only Android since numbers were easy
to source, SELinux was deployed in enforcing mode on over 3 billion
systems. Of course I don't have numbers handy for *all* Linux
systems, and there are some numbers that simply are never going to be
public, but given the +3 billion Android systems alone, I think there
is a very real likelihood that there are more systems running SELinux
than those that are not.
> While perhaps a more generic mechanism covering all hoooks would be
> preferred, I implemented a bare minimum version which gets out of the
> way for my needs.
I'd much rather see a generalized solution than hacks for a small
number of hooks.
--
paul-moore.com
^ permalink raw reply
* RE: Module signing and post-quantum crypto public key algorithms
From: Elliott, Robert (Servers) @ 2025-11-09 19:30 UTC (permalink / raw)
To: David Howells
Cc: Simo Sorce, James Bottomley, Ignat Korchagin, Herbert Xu,
Stephan Mueller, torvalds@linux-foundation.org, Paul Moore,
Lukas Wunner, Clemens Lang, David Bohannon, Roberto Sassu,
keyrings@vger.kernel.org, linux-crypto@vger.kernel.org,
linux-security-module@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <534145.1762588015@warthog.procyon.org.uk>
> -----Original Message-----
> From: David Howells <dhowells@redhat.com>
> Sent: Saturday, November 8, 2025 1:47 AM
> To: Elliott, Robert (Servers) <elliott@hpe.com>
> Cc: dhowells@redhat.com; Simo Sorce <simo@redhat.com>; James Bottomley
> <James.Bottomley@HansenPartnership.com>; Ignat Korchagin
> <ignat@cloudflare.com>; Herbert Xu <herbert@gondor.apana.org.au>;
> Stephan Mueller <smueller@chronox.de>; torvalds@linux-foundation.org;
> Paul Moore <paul@paul-moore.com>; Lukas Wunner <lukas@wunner.de>;
> Clemens Lang <cllang@redhat.com>; David Bohannon <dbohanno@redhat.com>;
> Roberto Sassu <roberto.sassu@huawei.com>; keyrings@vger.kernel.org;
> linux-crypto@vger.kernel.org; linux-security-module@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: Re: Module signing and post-quantum crypto public key
> algorithms
>
> Elliott, Robert (Servers) <elliott@hpe.com> wrote:
>
> > The traditional signature would use whatever algorithm is used today.
> > Legacy verifiers would only check that.
>
> Would there be any legacy verifiers? Kernel modules are generally tied
> to the kernel version for which they were compiled. Granted there are
> things like the wifi regulatory stuff that are also signed.
If the system building and signing a module doesn't have OpenSSL 3.5 yet,
it can only provide a legacy signature. If the kernel is only looking
for a composite by default, a kernel command line allowing a legacy
signature to be accepted would help the transition.
Userspace tools can check signatures, but I don't know if that is done
in any non-developer situations. I did it manually while testing x86
optimized crypto module changes a few years ago.
If and when a distro is confident there are no userspace tools, it could
stop signing with legacy signatures.
> But note also, PKCS#7 supports multiple independent signatures in a
> single object. The kernel will handle this already. At least one
> signature must be verifiable and none must be blacklisted.
Yes, I think that makes multiple signatures viable.
> I assume that the main aim of using a composite algorithm is to share
> the result of the content hash - but in this case only the
> authenticatedAttributes are going to be hashed for the signature, and
> that's relatively small.
The composite motivation is to provide some protection if someone
discovers a basic flaw in the PQC algorithm. If quantum computers
haven't arrived yet to break the traditional algorithm, the
composite still proves validity.
The CMS and OpenPGP constructions ensure that the signatures are
non-separable; an attacker cannot strip off the strong one and leave
the broken one. The verifier always runs both algorithms (unlike
the current OR policy with multiple signatures).
^ permalink raw reply
* [PATCH] security: provide an inlined static branch for security_inode_permission()
From: Mateusz Guzik @ 2025-11-09 19:29 UTC (permalink / raw)
To: paul
Cc: casey, keescook, song, andrii, kpsingh, linux-security-module,
linux-fsdevel, Mateusz Guzik
The routine is executing for every path component during name resolution in
vfs and shows up on the profile to the tune of 2% of CPU time in my
tests.
The only LSMs which install hoooks there are selinux and smack, meaning
most installs don't have it and this ends up being a call to do nothing.
While perhaps a more generic mechanism covering all hoooks would be
preferred, I implemented a bare minimum version which gets out of the
way for my needs.
Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---
include/linux/security.h | 11 ++++++++++-
security/security.c | 12 ++++++++++--
2 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/include/linux/security.h b/include/linux/security.h
index 92ac3f27b973..0ce1d73167ed 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -68,6 +68,8 @@ struct watch;
struct watch_notification;
struct lsm_ctx;
+DECLARE_STATIC_KEY_FALSE(security_inode_permission_has_hooks);
+
/* Default (no) options for the capable function */
#define CAP_OPT_NONE 0x0
/* If capable should audit the security request */
@@ -421,7 +423,14 @@ int security_inode_rename(struct inode *old_dir, struct dentry *old_dentry,
int security_inode_readlink(struct dentry *dentry);
int security_inode_follow_link(struct dentry *dentry, struct inode *inode,
bool rcu);
-int security_inode_permission(struct inode *inode, int mask);
+int __security_inode_permission(struct inode *inode, int mask);
+static inline int security_inode_permission(struct inode *inode, int mask)
+{
+ if (static_branch_unlikely(&security_inode_permission_has_hooks))
+ return __security_inode_permission(inode, mask);
+ return 0;
+}
+
int security_inode_setattr(struct mnt_idmap *idmap,
struct dentry *dentry, struct iattr *attr);
void security_inode_post_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
diff --git a/security/security.c b/security/security.c
index 4d3c03a4524c..e879f034a77c 100644
--- a/security/security.c
+++ b/security/security.c
@@ -51,6 +51,8 @@ do { \
#define LSM_DEFINE_UNROLL(M, ...) UNROLL(MAX_LSM_COUNT, M, __VA_ARGS__)
+DEFINE_STATIC_KEY_FALSE(security_inode_permission_has_hooks);
+
/*
* These are descriptions of the reasons that can be passed to the
* security_locked_down() LSM hook. Placing this array here allows
@@ -639,6 +641,12 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
lsm_static_call_init(&hooks[i]);
}
+ if (static_key_enabled(&static_calls_table.inode_permission->active->key)) {
+ if (!static_key_enabled(&security_inode_permission_has_hooks.key)) {
+ static_branch_enable(&security_inode_permission_has_hooks);
+ }
+ }
+
/*
* Don't try to append during early_security_init(), we'll come back
* and fix this up afterwards.
@@ -2343,7 +2351,7 @@ int security_inode_follow_link(struct dentry *dentry, struct inode *inode,
}
/**
- * security_inode_permission() - Check if accessing an inode is allowed
+ * __security_inode_permission() - Check if accessing an inode is allowed
* @inode: inode
* @mask: access mask
*
@@ -2356,7 +2364,7 @@ int security_inode_follow_link(struct dentry *dentry, struct inode *inode,
*
* Return: Returns 0 if permission is granted.
*/
-int security_inode_permission(struct inode *inode, int mask)
+int __security_inode_permission(struct inode *inode, int mask)
{
if (unlikely(IS_PRIVATE(inode)))
return 0;
--
2.48.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox