[PATCH cgroup/for-6.16] cgroup: avoid per-cpu allocation of size zero rstat cpu locks

[PATCH cgroup/for-6.16] cgroup: avoid per-cpu allocation of size zero rstat cpu locks

[JP Kobryn]

Subsystem rstat locks are dynamically allocated per-cpu. It was discovered
that a panic can occur during this allocation when the lock size is zero.
This is the case on non-smp systems, since arch_spinlock_t is defined as an
empty struct. Prevent this allocation when !CONFIG_SMP by adding a
pre-processor conditional around the affected block.

Signed-off-by: JP Kobryn <inwardvessel@gmail.com>
Reported-by: Klara Modin <klarasmodin@gmail.com>
Fixes: 748922dcfabd ("cgroup: use subsystem-specific rstat locks to avoid contention")
---
 kernel/cgroup/rstat.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/kernel/cgroup/rstat.c b/kernel/cgroup/rstat.c
index 7dd396ae3c68..ce4752ab9e09 100644
--- a/kernel/cgroup/rstat.c
+++ b/kernel/cgroup/rstat.c
@@ -510,11 +510,20 @@ int __init ss_rstat_init(struct cgroup_subsys *ss)
 {
 	int cpu;
 
+#ifdef CONFIG_SMP
+	/*
+	 * On uniprocessor machines, arch_spinlock_t is defined as an empty
+	 * struct. Avoid allocating a size of zero by having this block
+	 * excluded in this case. It's acceptable to leave the subsystem locks
+	 * unitialized since the associated lock functions are no-ops in the
+	 * non-smp case.
+	 */
 	if (ss) {
 		ss->rstat_ss_cpu_lock = alloc_percpu(raw_spinlock_t);
 		if (!ss->rstat_ss_cpu_lock)
 			return -ENOMEM;
 	}
+#endif
 
 	spin_lock_init(ss_rstat_lock(ss));
 	for_each_possible_cpu(cpu)
-- 
2.47.1

Re: [PATCH cgroup/for-6.16] cgroup: avoid per-cpu allocation of size zero rstat cpu locks

[Tejun Heo]

On Wed, May 21, 2025 at 06:32:02PM -0700, JP Kobryn wrote:
> Subsystem rstat locks are dynamically allocated per-cpu. It was discovered
> that a panic can occur during this allocation when the lock size is zero.
> This is the case on non-smp systems, since arch_spinlock_t is defined as an
> empty struct. Prevent this allocation when !CONFIG_SMP by adding a
> pre-processor conditional around the affected block.
> 
> Signed-off-by: JP Kobryn <inwardvessel@gmail.com>
> Reported-by: Klara Modin <klarasmodin@gmail.com>
> Fixes: 748922dcfabd ("cgroup: use subsystem-specific rstat locks to avoid contention")

Applied to cgroup/for-6.16.

Thanks.

-- 
tejun

Re: [PATCH cgroup/for-6.16] cgroup: avoid per-cpu allocation of size zero rstat cpu locks

[Klara Modin]

On 2025-05-21 18:32:02 -0700, JP Kobryn wrote:
> Subsystem rstat locks are dynamically allocated per-cpu. It was discovered
> that a panic can occur during this allocation when the lock size is zero.
> This is the case on non-smp systems, since arch_spinlock_t is defined as an
> empty struct. Prevent this allocation when !CONFIG_SMP by adding a
> pre-processor conditional around the affected block.
> 
> Signed-off-by: JP Kobryn <inwardvessel@gmail.com>
> Reported-by: Klara Modin <klarasmodin@gmail.com>
> Fixes: 748922dcfabd ("cgroup: use subsystem-specific rstat locks to avoid contention")

I can confirm this resolves the issue for me.

Thanks,
Tested-by: Klara Modin <klarasmodin@gmail.com>

Re: [PATCH cgroup/for-6.16] cgroup: avoid per-cpu allocation of size zero rstat cpu locks

[Guenter Roeck]

On Wed, May 21, 2025 at 06:32:02PM -0700, JP Kobryn wrote:
> Subsystem rstat locks are dynamically allocated per-cpu. It was discovered
> that a panic can occur during this allocation when the lock size is zero.
> This is the case on non-smp systems, since arch_spinlock_t is defined as an
> empty struct. Prevent this allocation when !CONFIG_SMP by adding a
> pre-processor conditional around the affected block.
> 

It may be defined as empty struct, but it is still dereferenced. This patch
is causing crashes on non-SMP systems such as xtensa, m68k, or with x86
non-SMP builds.

Examples:

m68k:

Unable to handle kernel NULL pointer dereference at virtual address 00000000
Oops: 00000000
PC: [<000593d6>] __raw_spin_lock_init+0x6/0x1c
SR: 2000  SP: 0086bef8  a2: 0086f440
d0: 00000001    d1: 000001ff    d2: 00000001    d3: 00000002
d4: 008e3a20    d5: 00000001    a0: 00000000    a1: 00000114
Process swapper (pid: 0, task=0086f440)
Frame format=7 eff addr=00000000 ssw=0405 faddr=00000000
wb 1 stat/addr/data: 0000 00000000 00000000
wb 2 stat/addr/data: 0000 00000000 00000000
wb 3 stat/addr/data: 0000 00000000 00000000
push data: 00000000 00000000 00000000 00000000
Stack from 0086bf60:
        0099d64c 00000000 007ff7a1 0092e6c0 00000002 008eed3c 00938c60 0099cf92
        008eed3c 00000008 008eed3c 008e3a84 008e3a20 0099d300 008eed3c 00000000
        00000000 00000000 00000000 00000000 00000000 00000000 00703cc2 00003b18
        0003f9a2 008e44c0 0086bff8 0099639e 00000000 00000000 00000000 00000000
        00000000 00000000 00000000 00000000 009cc000 009b8a7c 00000000 009952b8
Call Trace: [<0099d64c>] ss_rstat_init+0x5a/0x66
 [<0099cf92>] cgroup_init_subsys+0x102/0x1b4
 [<0099d300>] cgroup_init+0x18e/0x47c
 [<00703cc2>] strlen+0x0/0x1a
 [<00003b18>] _printk+0x0/0x18
 [<0003f9a2>] parse_args+0x0/0x380
 [<0099639e>] start_kernel+0x5c0/0x5cc
 [<009952b8>] _sinittext+0x2b8/0x8f0

x86 noSMP build:

[    1.151991] BUG: kernel NULL pointer dereference, address: 0000000000000020
[    1.151991] #PF: supervisor write access in kernel mode
[    1.151991] #PF: error_code(0x0002) - not-present page
[    1.151991] PGD 0 P4D 0
[    1.151991] Oops: Oops: 0002 [#1] NOPTI
[    1.151991] CPU: 0 UID: 0 PID: 0 Comm: swapper Not tainted 6.15.0-10402-g4cb6c8af8591 #1 PREEMPT(voluntary)
[    1.151991] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
[    1.151991] RIP: 0010:lockdep_init_map_type+0x1b/0x260
[    1.151991] Code: 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 f3 0f 1e fa 41 54 41 89 cc 55 48 89 d5 8b 15 9d fc b0 03 53 48 89 fb 8b 44 24 20 <48> c7 47 08 00 00 00 00 48 c7 47 10 00 00 00 00 85 d2 0f 85 8e 00
[    1.151991] RSP: 0000:ffffffff8b203e38 EFLAGS: 00010246
[    1.151991] RAX: 0000000000000000 RBX: 0000000000000018 RCX: 0000000000000000
[    1.151991] RDX: 0000000000000000 RSI: ffffffff8b0387bf RDI: 0000000000000018
[    1.151991] RBP: ffffffff8cc48c20 R08: 0000000000000002 R09: 0000000000000000
[    1.151991] R10: 0000000000000001 R11: ffffffff8a786934 R12: 0000000000000000
[    1.151991] R13: 0000000000000002 R14: ffffffff8b3c8fc0 R15: ffffffff8b3c9028
[    1.151991] FS:  0000000000000000(0000) GS:0000000000000000(0000) knlGS:0000000000000000
[    1.151991] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    1.151991] CR2: 0000000000000020 CR3: 00000000220a0000 CR4: 00000000000006f0
[    1.151991] Call Trace:
[    1.151991]  <TASK>
[    1.151991]  __raw_spin_lock_init+0x3a/0x70
[    1.151991]  ss_rstat_init+0x4b/0x80
[    1.151991]  cgroup_init_subsys+0x170/0x1c0
[    1.151991]  cgroup_init+0x3d8/0x4c0
[    1.151991]  start_kernel+0x68e/0x770
[    1.151991]  x86_64_start_reservations+0x18/0x30
[    1.151991]  x86_64_start_kernel+0x101/0x110
[    1.151991]  common_startup_64+0xc0/0xc8

Guenter

Re: [PATCH cgroup/for-6.16] cgroup: avoid per-cpu allocation of size zero rstat cpu locks

[Shakeel Butt]

On Mon, Jun 2, 2025 at 8:38 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Wed, May 21, 2025 at 06:32:02PM -0700, JP Kobryn wrote:
> > Subsystem rstat locks are dynamically allocated per-cpu. It was discovered
> > that a panic can occur during this allocation when the lock size is zero.
> > This is the case on non-smp systems, since arch_spinlock_t is defined as an
> > empty struct. Prevent this allocation when !CONFIG_SMP by adding a
> > pre-processor conditional around the affected block.
> >
>
> It may be defined as empty struct, but it is still dereferenced. This patch
> is causing crashes on non-SMP systems such as xtensa, m68k, or with x86
> non-SMP builds.
>

Does this still happen with the following patch?

https://lore.kernel.org/20250528235130.200966-1-inwardvessel@gmail.com/

Re: [PATCH cgroup/for-6.16] cgroup: avoid per-cpu allocation of size zero rstat cpu locks

[Guenter Roeck]

On 6/2/25 21:10, Shakeel Butt wrote:
> On Mon, Jun 2, 2025 at 8:38 PM Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On Wed, May 21, 2025 at 06:32:02PM -0700, JP Kobryn wrote:
>>> Subsystem rstat locks are dynamically allocated per-cpu. It was discovered
>>> that a panic can occur during this allocation when the lock size is zero.
>>> This is the case on non-smp systems, since arch_spinlock_t is defined as an
>>> empty struct. Prevent this allocation when !CONFIG_SMP by adding a
>>> pre-processor conditional around the affected block.
>>>
>>
>> It may be defined as empty struct, but it is still dereferenced. This patch
>> is causing crashes on non-SMP systems such as xtensa, m68k, or with x86
>> non-SMP builds.
>>
> 
> Does this still happen with the following patch?
> 
> https://lore.kernel.org/20250528235130.200966-1-inwardvessel@gmail.com/

I guess that should fix it. Sorry, I had not seen/found that patch.

Guenter