Re: [PATCH] percpu: Remove intermediate variable in PERCPU_PTR()

[Uros Bizjak]

On Thu, Dec 19, 2024 at 5:02 PM Uros Bizjak <ubizjak@xxxxxxxxx> wrote:

> > > The intermediate variable in the PERCPU_PTR() macro results in a kernel
> > > panic on boot [1] due to a compiler bug seen when compiling the kernel
> > > (+ KASAN) with gcc 11.3.1, but not when compiling with latest gcc
> > > (v14.2)/clang(v18.1).
> > >
> > > To solve it, remove the intermediate variable (which is not needed) and
> > > keep the casting that resolves the address space checks.

[...]

> > >  include/linux/percpu-defs.h | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/include/linux/percpu-defs.h b/include/linux/percpu-defs.h
> > > index 35842d1e3879..573adb643d90 100644
> > > --- a/include/linux/percpu-defs.h
> > > +++ b/include/linux/percpu-defs.h
> > > @@ -222,8 +222,7 @@ do {                                                                        \
> > >
> > >  #define PERCPU_PTR(__p)                                                        \
> > >  ({                                                                     \
> > > -       unsigned long __pcpu_ptr = (__force unsigned long)(__p);        \
> > > -       (typeof(*(__p)) __force __kernel *)(__pcpu_ptr);                \
> > > +       (typeof(*(__p)) __force __kernel *)((__force unsigned long)(__p)); \
> > >  })
>
> Actually, you can simplify the above a bit by writing it as:
>
> #define PERCPU_PTR(__p)                            \
>     ((typeof(*(__p)) __force __kernel *)(__force unsigned long)(__p)) \

Andrew, please find attached a substitute patch "[PATCH 4/6] percpu:
Use TYPEOF_UNQUAL() in *_cpu_ptr() accessors" for your MM tree
relative to the above hotfix. The whole patch series (+ hotfix) has
been re-tested against the current mainline defconfig (+ KASAN),
compiled once with gcc-11.4.1 and once with gcc-14.2.1.

Uros.
From fc933c978446d6aae977b540f00cfcfbfc65b755 Mon Sep 17 00:00:00 2001
From: Uros Bizjak <ubizjak@xxxxxxxxx>
Date: Tue, 26 Nov 2024 16:46:42 +0100
Subject: [PATCH 4/6] percpu: Use TYPEOF_UNQUAL() in *_cpu_ptr() accessors

Use TYPEOF_UNQUAL() macro to declare the return type of *_cpu_ptr()
accessors in the generic named address space to avoid access to
data from pointer to non-enclosed address space type of errors.

Signed-off-by: Uros Bizjak <ubizjak@xxxxxxxxx>
Acked-by: Nadav Amit <nadav.amit@xxxxxxxxx>
Acked-by: Christoph Lameter <cl@xxxxxxxxx>
Cc: Dennis Zhou <dennis@xxxxxxxxxx>
Cc: Tejun Heo <tj@xxxxxxxxxx>
Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
Cc: Ingo Molnar <mingo@xxxxxxxxxx>
Cc: Borislav Petkov <bp@xxxxxxxxx>
Cc: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
Cc: "H. Peter Anvin" <hpa@xxxxxxxxx>
Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Cc: Andy Lutomirski <luto@xxxxxxxxxx>
Cc: Brian Gerst <brgerst@xxxxxxxxx>
Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
---
 arch/x86/include/asm/percpu.h | 8 ++++++--
 include/linux/percpu-defs.h   | 2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
index 666e4137b09f..27f668660abe 100644
--- a/arch/x86/include/asm/percpu.h
+++ b/arch/x86/include/asm/percpu.h
@@ -73,10 +73,14 @@
 	unsigned long tcp_ptr__ = raw_cpu_read_long(this_cpu_off);	\
 									\
 	tcp_ptr__ += (__force unsigned long)(_ptr);			\
-	(typeof(*(_ptr)) __kernel __force *)tcp_ptr__;			\
+	(TYPEOF_UNQUAL(*(_ptr)) __force __kernel *)tcp_ptr__;		\
 })
 #else
-#define arch_raw_cpu_ptr(_ptr) ({ BUILD_BUG(); (typeof(_ptr))0; })
+#define arch_raw_cpu_ptr(_ptr)						\
+({									\
+	BUILD_BUG();							\
+	(TYPEOF_UNQUAL(*(_ptr)) __force __kernel *)0;			\
+})
 #endif
 
 #define PER_CPU_VAR(var)	%__percpu_seg:(var)__percpu_rel
diff --git a/include/linux/percpu-defs.h b/include/linux/percpu-defs.h
index 4fde93334ac3..8a7c8d2d570d 100644
--- a/include/linux/percpu-defs.h
+++ b/include/linux/percpu-defs.h
@@ -221,7 +221,7 @@ do {									\
 } while (0)
 
 #define PERCPU_PTR(__p)							\
-	((typeof(*(__p)) __force __kernel *)(__force unsigned long)(__p)) \
+	((TYPEOF_UNQUAL(*(__p)) __force __kernel *)(__force unsigned long)(__p)) \
 
 #ifdef CONFIG_SMP
 
-- 
2.42.0