[PATCH v2 02/20] arm64: percpu: Fix this_cpu_and() mask generation
Mark Rutland <[email protected]> Tue, 4 Aug 2026 18:04:45 +0100
| Newsgroups | gmane.linux.kernel.stable,gmane.linux.ports.arm.kernel |
|---|---|
| Message-ID | <[email protected]> |
The arm64 implementation of this_cpu_and(pcp, val) is built in terms of
ANDNOT operations, which requires the 'val' argument to be bitwise
negated. The bitwise negation is not implemented correctly, with two
bugs described below.
(1) The bitwise negation is performed as '~val' rather than '~(val)'.
This won't always generate the expected value when 'val' is an
expression.
For example, for this_cpu_and(pcp, 1 - 1):
* 'val' is '1 - 1' ===> (int) 0x00000000
* '~val' is '~1 - 1' ===> (int) 0xfffffffd
* '~(val)' is '~(1 - 1)' ===> (int) 0xffffffff
... and thus bit[1] of 'pcp' would be preserved unexpectedly by the
ANDNOT operation.
(2) The bitwise negation is performed on 'val' before it has been cast
to (at least) the width of 'pcp'. This won't always generate the
expected value for the upper bits.
For example, for this_cpu_and(pcp, zero), where 'pcp' is a u64 and
'zero' is a u32:
* 'zero' ===> (u32) 0x00000000
* '~(zero)' ===> (u32) 0xffffffff
* '(u64)~(zero)' ===> (u64) 0x00000000ffffffff
* '~((u64)(zero))' ===> (u64) 0xffffffffffffffff
... and thus bits[63:32] of 'pcp' would be preserved unexpectedly by
the ANDNOT operation.
Fix these issues by adding brackets around 'val', and by casting 'val'
to an appropriately-sized type before bitwise negation.
The bugs described above can be seen from the disassembly of the
following test code:
| void this_cpu_and_u64_zero(u64 __percpu *pcp)
| {
| u64 zero = 0;
| this_cpu_and(*pcp, zero);
| }
|
| void this_cpu_and_u32_zero(u64 __percpu *pcp)
| {
| u32 zero = 0;
| this_cpu_and(*pcp, zero);
| }
|
| void this_cpu_and_expr_zero(u64 __percpu *pcp)
| {
| this_cpu_and(*pcp, 1 - 1);
| }
|
| void this_cpu_and_expr_zero_brackets(u64 __percpu *pcp)
| {
| this_cpu_and(*pcp, (1 - 1));
| }
Before this patch:
| <this_cpu_and_u64_zero>:
| paciasp
| stp x29, x30, [sp, #-16]!
| mrs x1, sp_el0
| mov x29, sp
| ldr w2, [x1, #8]
| add w2, w2, #0x1
| str w2, [x1, #8]
| mov x3, #0xffffffffffffffff // #-1
| mrs x2, tpidr_el1
| add x0, x0, x2
| 1: ldxr x5, [x0]
| bic x5, x5, x3
| stxr w4, x5, [x0]
| cbnz w4, 1b
| ldr x0, [x1, #8]
| add x0, x0, x3
| str w0, [x1, #8]
| cbz x0, 2f
| ldr x0, [x1, #8]
| cbnz x0, 3f
| 2: bl preempt_schedule_notrace
| 3: ldp x29, x30, [sp], #16
| autiasp
| ret
|
| <this_cpu_and_u32_zero>:
| paciasp
| stp x29, x30, [sp, #-16]!
| mrs x1, sp_el0
| mov x29, sp
| ldr w2, [x1, #8]
| add w2, w2, #0x1
| str w2, [x1, #8]
| mov x3, #0xffffffff // #4294967295
| mrs x2, tpidr_el1
| add x0, x0, x2
| 1: ldxr x5, [x0]
| bic x5, x5, x3
| stxr w4, x5, [x0]
| cbnz w4, 1b
| ldr x0, [x1, #8]
| sub x0, x0, #0x1
| str w0, [x1, #8]
| cbz x0, 2f
| ldr x0, [x1, #8]
| cbnz x0, 3f
| 2: bl preempt_schedule_notrace
| 3: ldp x29, x30, [sp], #16
| autiasp
| ret
|
| <this_cpu_and_expr_zero>:
| paciasp
| stp x29, x30, [sp, #-16]!
| mrs x1, sp_el0
| mov x29, sp
| ldr w2, [x1, #8]
| add w2, w2, #0x1
| str w2, [x1, #8]
| mov x3, #0xfffffffffffffffd // #-3
| mrs x2, tpidr_el1
| add x0, x0, x2
| 1: ldxr x5, [x0]
| bic x5, x5, x3
| stxr w4, x5, [x0]
| cbnz w4, 1b
| ldr x0, [x1, #8]
| sub x0, x0, #0x1
| str w0, [x1, #8]
| cbz x0, 2f
| ldr x0, [x1, #8]
| cbnz x0, 3f
| 2: bl preempt_schedule_notrace
| 3: ldp x29, x30, [sp], #16
| autiasp
| ret
After this patch:
| <this_cpu_and_u64_zero>:
| paciasp
| stp x29, x30, [sp, #-16]!
| mrs x1, sp_el0
| mov x29, sp
| ldr w2, [x1, #8]
| add w2, w2, #0x1
| str w2, [x1, #8]
| mov x3, #0xffffffffffffffff // #-1
| mrs x2, tpidr_el1
| add x0, x0, x2
| 1: ldxr x5, [x0]
| bic x5, x5, x3
| stxr w4, x5, [x0]
| cbnz w4, 1b
| ldr x0, [x1, #8]
| add x0, x0, x3
| str w0, [x1, #8]
| cbz x0, 2f
| ldr x0, [x1, #8]
| cbnz x0, 3f
| 2: bl 0 <preempt_schedule_notrace>
| 3: ldp x29, x30, [sp], #16
| autiasp
| ret
|
| <this_cpu_and_u32_zero>:
| b this_cpu_and_u64_zero
|
| <this_cpu_and_expr_zero>:
| b this_cpu_and_u64_zero
|
| <this_cpu_and_expr_zero_brackets>:
| b this_cpu_and_u64_zero
Fixes: 959bf2fd03b5 ("arm64: percpu: Rewrite per-cpu ops to allow use of LSE atomics")
Signed-off-by: Mark Rutland <[email protected]>
Cc: Ada Couprie Diaz <[email protected]>
Cc: Ard Biesheuvel <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: James Morse <[email protected]>
Cc: Jinjie Ruan <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Vladimir Murzin <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Yang Shi <[email protected]>
Cc: [email protected]
---
arch/arm64/include/asm/percpu.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/include/asm/percpu.h b/arch/arm64/include/asm/percpu.h
index 63bbfd4944a37..31193bcf89a2b 100644
--- a/arch/arm64/include/asm/percpu.h
+++ b/arch/arm64/include/asm/percpu.h
@@ -206,13 +206,13 @@ PERCPU_RET_OP(add, add, ldadd)
_pcp_protect_return(__percpu_add_return_case_64, pcp, val)
#define this_cpu_and_1(pcp, val) \
- _pcp_protect(__percpu_andnot_case_8, pcp, ~val)
+ _pcp_protect(__percpu_andnot_case_8, pcp, ~(u8)(val))
#define this_cpu_and_2(pcp, val) \
- _pcp_protect(__percpu_andnot_case_16, pcp, ~val)
+ _pcp_protect(__percpu_andnot_case_16, pcp, ~(u16)(val))
#define this_cpu_and_4(pcp, val) \
- _pcp_protect(__percpu_andnot_case_32, pcp, ~val)
+ _pcp_protect(__percpu_andnot_case_32, pcp, ~(u32)(val))
#define this_cpu_and_8(pcp, val) \
- _pcp_protect(__percpu_andnot_case_64, pcp, ~val)
+ _pcp_protect(__percpu_andnot_case_64, pcp, ~(u64)(val))
#define this_cpu_or_1(pcp, val) \
_pcp_protect(__percpu_or_case_8, pcp, val)
--
2.30.2