From: Uros Bizjak <ubizjak@gmail.com>
To: Gal Pressman <gal@nvidia.com>
Cc: Dennis Zhou <dennis@kernel.org>, Tejun Heo <tj@kernel.org>,
Christoph Lameter <cl@linux.com>,
Nathan Chancellor <nathan@kernel.org>,
Nick Desaulniers <ndesaulniers@google.com>,
Bill Wendling <morbo@google.com>,
Justin Stitt <justinstitt@google.com>,
Andrew Morton <akpm@linux-foundation.org>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
llvm@lists.linux.dev, netdev@vger.kernel.org
Subject: Re: [PATCH] percpu: Remove intermediate variable in PERCPU_PTR()
Date: Thu, 19 Dec 2024 18:03:47 +0100 [thread overview]
Message-ID: <CAFULd4ZcSY+1WPn2T9dHVJZyyg1p+YaexQMJzAXHnCDy90j2fA@mail.gmail.com> (raw)
In-Reply-To: <CAFULd4Z0PSzwvsFx_5deMKb7tV34uJWcHEadYGdk+D72QuHonA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1886 bytes --]
On Thu, Dec 19, 2024 at 5:02 PM Uros Bizjak <ubizjak@gmail.com> 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.
[-- Attachment #2: 0004-percpu-Use-TYPEOF_UNQUAL-in-_cpu_ptr-accessors.patch --]
[-- Type: text/x-patch, Size: 2284 bytes --]
From fc933c978446d6aae977b540f00cfcfbfc65b755 Mon Sep 17 00:00:00 2001
From: Uros Bizjak <ubizjak@gmail.com>
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@gmail.com>
Acked-by: Nadav Amit <nadav.amit@gmail.com>
Acked-by: Christoph Lameter <cl@linux.com>
Cc: Dennis Zhou <dennis@kernel.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
---
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
next prev parent reply other threads:[~2024-12-19 17:04 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-19 12:18 Gal Pressman
2024-12-19 12:30 ` Uros Bizjak
2024-12-19 16:02 ` Uros Bizjak
2024-12-19 17:03 ` Uros Bizjak [this message]
2024-12-19 22:51 ` Andrew Morton
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAFULd4ZcSY+1WPn2T9dHVJZyyg1p+YaexQMJzAXHnCDy90j2fA@mail.gmail.com \
--to=ubizjak@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=cl@linux.com \
--cc=dennis@kernel.org \
--cc=gal@nvidia.com \
--cc=justinstitt@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=llvm@lists.linux.dev \
--cc=morbo@google.com \
--cc=nathan@kernel.org \
--cc=ndesaulniers@google.com \
--cc=netdev@vger.kernel.org \
--cc=tj@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox