On Wed, Sep 16, 2026 at 11:17:36AM -0500, Andrey Ryabinin wrote: > "Mukesh Kumar Chaurasiya (IBM)" <[email protected]> writes: > > Hi, > I fed this patch to an AI for review, and the review results are included > below. > Please take a look. From my side, I agree with all of the points > raised by AI in the > review, and I think they all need to be addressed. There is also a diff with > the suggested changes at the very end of this mail. > > Hey Andrey,
Thanks for the effort. > > powerpc unconditionally selects GENERIC_ENTRY. The GENERIC_ENTRY > > infrastructure relies on the compiler emitting __asan_mem*() calls at > > instrumented mem*() sites rather than plain memset/memcpy/memmove, so > > that entry/exit paths calling those functions are not instrumented. > > > > [ ... ] > > > > When GENERIC_ENTRY is set, both guards suppress the C wrappers for > > memset/memcpy/memmove and the __underlying_mem*() redirections. This > > is only safe when the compiler supports the prefixed __asan_mem*() > > intrinsics. On older toolchains (e.g. GCC 9) that lack this support, > > plain mem*() calls from instrumented code fall through to the raw > > assembly implementations in mem_64.S / copy_32.S, completely bypassing > > the KASAN shadow check. > > > > Other arches with GENERIC_ENTRY (x86, s390, loongarch, riscv) do not > > hit this because their CI toolchains are always new enough to support > > the prefix flag. > > > > [ ... ] > > > > Reported-by: Venkat Rao Bagalkote <[email protected]> > > Closes: > > https://lore.kernel.org/all/[email protected] > > Tested-by: Venkat Rao Bagalkote <[email protected]> > > Signed-off-by: Mukesh Kumar Chaurasiya (IBM) <[email protected]> > > The thread in the Closes: link is about an early boot hang, but the > commit message only describes mem*() calls bypassing the shadow check. > Bypassed checks would lose coverage, not hang the machine. Is the > mechanism of the hang understood? > > Looking at the state before this commit, the !CC_HAS_KASAN_MEMINTRINSIC_PREFIX > macros that bee25f97ad24 ("powerpc: Enable GENERIC_ENTRY feature") added to > asm/kasan.h emit two ELFv2 global entry points back to back: > > #define _GLOBAL_TOC_KASAN(fn) > _GLOBAL_TOC(fn); > _GLOBAL_TOC(__##fn) > > With the _GLOBAL_TOC() definition from asm/ppc_asm.h that expands, for > memcpy_64.S, to: > > memcpy: > 0: addis r2,r12,(.TOC.-0b)@ha > addi r2,r2,(.TOC.-0b)@l > .localentry memcpy,.-memcpy <- local entry is memcpy+8 > __memcpy: > 0: addis r2,r12,(.TOC.-0b)@ha <- this is memcpy+8 > addi r2,r2,(.TOC.-0b)@l > .localentry __memcpy,.-__memcpy > > Assembling exactly that for powerpc64le gives memcpy st_other 0x60 > (local entry offset 8), and memcpy+8 is the __memcpy TOC prologue. > > Every same-TOC caller of memcpy() or memmove() is resolved by the > linker to the local entry, so it lands on that second prologue with r12 > holding whatever the caller left there, and returns with r2 pointing > at garbage. Same-TOC callers do not reload r2 after the call. With > GENERIC_ENTRY, mm/kasan/shadow.c no longer provides memcpy(), so on a > toolchain without the prefix parameter every instrumented file calls > the memcpy symbol directly and hits this. That matches an early hang > that only shows up with GCC 9. > > This commit makes the hang go away because the dual-entry macro is > deleted, but the commit message attributes the fix to something else. > Could the message describe the r2 corruption, and since this repairs a > regression from the GENERIC_ENTRY conversion, should it carry: > > Fixes: bee25f97ad24 ("powerpc: Enable GENERIC_ENTRY feature") Yeah the commit message needs to clearly specify the corruption of r2, and the Fixes tag should be there. > > On the claim that other GENERIC_ENTRY architectures "do not hit this > because their CI toolchains are always new enough": x86, s390, riscv > and loongarch with GCC 8 to 12 build exactly the same configuration, > CONFIG_KASAN=y without CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX, and > run with mem*() unchecked. scripts/Makefile.kasan documents that as > the intended behaviour: > > # Instrument memcpy/memset/memmove calls by using instrumented __asan_mem*() > # instead. With compilers that don't support this option, compiler-inserted > # memintrinsics won't be checked by KASAN on GENERIC_ENTRY architectures. > > and mm/kasan/kasan_test_c.c skips the affected tests with "Test > requires checked mem*()". So the situation the message describes is > the accepted upstream state for old toolchains, not something specific > to powerpc. > > > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig > > index 2580e27e4328..b27ed9739eea 100644 > > --- a/arch/powerpc/Kconfig > > +++ b/arch/powerpc/Kconfig > > @@ -7,6 +7,10 @@ config CC_HAS_ELFV2 > > config CC_HAS_PREFIXED > > def_bool PPC64 && $(cc-option, -mcpu=power10 -mprefixed) > > > > +config PPC_CC_HAS_KASAN_MEMINTRINSIC_PREFIX > > + def_bool (CC_IS_CLANG && $(cc-option,-fsanitize=kernel-address -mllvm > > -asan-kernel-mem-intrinsic-prefix=1)) || \ > > + (CC_IS_GCC && $(cc-option,-fsanitize=kernel-address --param > > asan-kernel-mem-intrinsic-prefix=1)) > > + > > config CC_HAS_PCREL > > # Clang has a bug (https://github.com/llvm/llvm-project/issues/62372) > > # where pcrel code is not generated if -msoft-float, -mno-altivec, or > > [ ... ] > > > @@ -220,9 +224,9 @@ config PPC > > select HAVE_ARCH_HUGE_VMAP if PPC_RADIX_MMU || PPC_8xx > > select HAVE_ARCH_JUMP_LABEL > > select HAVE_ARCH_JUMP_LABEL_RELATIVE > > - select HAVE_ARCH_KASAN if PPC32 && PAGE_SHIFT <= 14 > > - select HAVE_ARCH_KASAN if PPC_RADIX_MMU > > - select HAVE_ARCH_KASAN if PPC_BOOK3E_64 > > + select HAVE_ARCH_KASAN if PPC32 && PAGE_SHIFT <= 14 && > > PPC_CC_HAS_KASAN_MEMINTRINSIC_PREFIX > > + select HAVE_ARCH_KASAN if PPC_RADIX_MMU && > > PPC_CC_HAS_KASAN_MEMINTRINSIC_PREFIX > > + select HAVE_ARCH_KASAN if PPC_BOOK3E_64 && > > PPC_CC_HAS_KASAN_MEMINTRINSIC_PREFIX > > select HAVE_ARCH_KASAN_VMALLOC if HAVE_ARCH_KASAN > > select HAVE_ARCH_KCSAN > > select HAVE_ARCH_KFENCE if ARCH_SUPPORTS_DEBUG_PAGEALLOC > > This drops KASAN from powerpc entirely for GCC 8 through 12, which are > inside the supported range in Documentation/process/changes.rst (GNU C > 8.1 minimum). An existing .config with CONFIG_KASAN=y silently loses > it on olddefconfig once HAVE_ARCH_KASAN is no longer selected. > > Given that the actual breakage is the broken memcpy/memmove entry > points, is it necessary to go this far? Keeping one _GLOBAL_TOC() > prologue for __memcpy and making memcpy a plain alias of it (a second > label plus a matching .localentry, or a global entry that branches to > __memcpy) would restore the pre-GENERIC_ENTRY behaviour, with mem*() > unchecked on old compilers exactly like the other GENERIC_ENTRY > architectures. That also keeps a Fixes-tagged backport candidate from > removing a feature on stable kernels. > Ok, yeah this is too aggressive. I will rectify this in next revision. > > > diff --git a/arch/powerpc/include/asm/kasan.h > > b/arch/powerpc/include/asm/kasan.h > > index a690e7da53c2..d62756b87ba4 100644 > > --- a/arch/powerpc/include/asm/kasan.h > > +++ b/arch/powerpc/include/asm/kasan.h > > @@ -2,20 +2,9 @@ > > #ifndef __ASM_KASAN_H > > #define __ASM_KASAN_H > > > > -#if defined(CONFIG_KASAN) && > > !defined(CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX) > > -#define _GLOBAL_KASAN(fn) \ > > - _GLOBAL(fn); \ > > - _GLOBAL(__##fn) > > -#define _GLOBAL_TOC_KASAN(fn) \ > > - _GLOBAL_TOC(fn); \ > > - _GLOBAL_TOC(__##fn) > > -#define EXPORT_SYMBOL_KASAN(fn) \ > > - EXPORT_SYMBOL(__##fn) > > -#else /* CONFIG_KASAN && !CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX */ > > #define _GLOBAL_KASAN(fn) _GLOBAL(fn) > > #define _GLOBAL_TOC_KASAN(fn) _GLOBAL_TOC(fn) > > #define EXPORT_SYMBOL_KASAN(fn) > > -#endif /* CONFIG_KASAN && !CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX */ > > > > #ifndef __ASSEMBLER__ > > > > this isn't a bug, but after this change _GLOBAL_KASAN(), _GLOBAL_TOC_KASAN() > and EXPORT_SYMBOL_KASAN() are unconditional identity macros with a single > empty one. Should the five users in mem_64.S, memcpy_64.S and copy_32.S > switch to _GLOBAL()/_GLOBAL_TOC() and the macros go away? > > Related leftover: arch/powerpc/kernel/prom_init_check.sh still has > > has_renamed_memintrinsics() > { > grep -q "^CONFIG_KASAN=y$" "${KCONFIG_CONFIG}" && \ > ! grep -q > "^CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX=y" "${KCONFIG_CONFIG}" > } > > if has_renamed_memintrinsics > then > MEM_FUNCS="__memcpy __memset" > > which can no longer be true on powerpc after this commit. Should that > branch be removed in the same cleanup? > Yes it should be. It's a dead code now. > > diff --git a/arch/powerpc/include/asm/string.h > > b/arch/powerpc/include/asm/string.h > > index 1981bd4036b5..72b5c93a2b84 100644 > > --- a/arch/powerpc/include/asm/string.h > > +++ b/arch/powerpc/include/asm/string.h > > @@ -29,29 +29,10 @@ extern void * memchr(const void *,int,__kernel_size_t); > > void memcpy_flushcache(void *dest, const void *src, size_t size); > > > > #ifdef CONFIG_KASAN > > -/* __mem variants are used by KASAN to implement instrumented > > meminstrinsics. */ > > -#ifdef CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX > > +/* Used by mm/kasan/shadow.c as raw backends to bypass KASAN checking. */ > > #define __memset memset > > this isn't a bug, but the new comment names only mm/kasan/shadow.c. > The same __memset()/__memcpy() names are used by mm/kasan/generic.c as > well (DEFINE_ASAN_SET_SHADOW() and release_alloc_meta()), so would > "used by mm/kasan as raw backends" be more accurate? > > > --- > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig > index b27ed9739eea..2580e27e4328 100644 > --- a/arch/powerpc/Kconfig > +++ b/arch/powerpc/Kconfig > @@ -7,10 +7,6 @@ config CC_HAS_ELFV2 > config CC_HAS_PREFIXED > def_bool PPC64 && $(cc-option, -mcpu=power10 -mprefixed) > > -config PPC_CC_HAS_KASAN_MEMINTRINSIC_PREFIX > - def_bool (CC_IS_CLANG && $(cc-option,-fsanitize=kernel-address > -mllvm -asan-kernel-mem-intrinsic-prefix=1)) || \ > - (CC_IS_GCC && $(cc-option,-fsanitize=kernel-address --param > asan-kernel-mem-intrinsic-prefix=1)) > - > config CC_HAS_PCREL > # Clang has a bug (https://github.com/llvm/llvm-project/issues/62372) > # where pcrel code is not generated if -msoft-float, -mno-altivec, or > @@ -224,9 +220,9 @@ config PPC > select HAVE_ARCH_HUGE_VMAP if PPC_RADIX_MMU || PPC_8xx > select HAVE_ARCH_JUMP_LABEL > select HAVE_ARCH_JUMP_LABEL_RELATIVE > - select HAVE_ARCH_KASAN if PPC32 && PAGE_SHIFT <= 14 && > PPC_CC_HAS_KASAN_MEMINTRINSIC_PREFIX > - select HAVE_ARCH_KASAN if PPC_RADIX_MMU && > PPC_CC_HAS_KASAN_MEMINTRINSIC_PREFIX > - select HAVE_ARCH_KASAN if PPC_BOOK3E_64 && > PPC_CC_HAS_KASAN_MEMINTRINSIC_PREFIX > + select HAVE_ARCH_KASAN if PPC32 && PAGE_SHIFT <= 14 > + select HAVE_ARCH_KASAN if PPC_RADIX_MMU > + select HAVE_ARCH_KASAN if PPC_BOOK3E_64 > select HAVE_ARCH_KASAN_VMALLOC if HAVE_ARCH_KASAN > select HAVE_ARCH_KCSAN > select HAVE_ARCH_KFENCE if ARCH_SUPPORTS_DEBUG_PAGEALLOC > diff --git a/arch/powerpc/include/asm/kasan.h > b/arch/powerpc/include/asm/kasan.h > index d62756b87ba4..599a9e02af02 100644 > --- a/arch/powerpc/include/asm/kasan.h > +++ b/arch/powerpc/include/asm/kasan.h > @@ -2,10 +2,6 @@ > #ifndef __ASM_KASAN_H > #define __ASM_KASAN_H > > -#define _GLOBAL_KASAN(fn) _GLOBAL(fn) > -#define _GLOBAL_TOC_KASAN(fn) _GLOBAL_TOC(fn) > -#define EXPORT_SYMBOL_KASAN(fn) > - > #ifndef __ASSEMBLER__ > > #include <asm/page.h> > diff --git a/arch/powerpc/kernel/prom_init_check.sh > b/arch/powerpc/kernel/prom_init_check.sh > index 3090b97258ae..3155cc722e48 100644 > --- a/arch/powerpc/kernel/prom_init_check.sh > +++ b/arch/powerpc/kernel/prom_init_check.sh > @@ -13,21 +13,8 @@ > # If you really need to reference something from prom_init.o add > # it to the list below: > > -has_renamed_memintrinsics() > -{ > - grep -q "^CONFIG_KASAN=y$" "${KCONFIG_CONFIG}" && \ > - ! grep -q "^CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX=y" > "${KCONFIG_CONFIG}" > -} > - > -if has_renamed_memintrinsics > -then > - MEM_FUNCS="__memcpy __memset" > -else > - MEM_FUNCS="memcpy memset" > -fi > - > WHITELIST="add_reloc_offset __bss_start __bss_stop copy_and_flush > -_end enter_prom $MEM_FUNCS reloc_offset __secondary_hold > +_end enter_prom memcpy memset reloc_offset __secondary_hold > __secondary_hold_acknowledge __secondary_hold_spinloop __start > logo_linux_clut224 btext_prepare_BAT > reloc_got2 kernstart_addr memstart_addr linux_banner _stext > diff --git a/arch/powerpc/lib/copy_32.S b/arch/powerpc/lib/copy_32.S > index 933b685e7ab6..97eb9ca0cc23 100644 > --- a/arch/powerpc/lib/copy_32.S > +++ b/arch/powerpc/lib/copy_32.S > @@ -10,7 +10,6 @@ > #include <asm/errno.h> > #include <asm/ppc_asm.h> > #include <asm/code-patching-asm.h> > -#include <asm/kasan.h> > > #define COPY_16_BYTES \ > lwz r7,4(r4); \ > @@ -87,7 +86,7 @@ EXPORT_SYMBOL(memset16) > * We therefore skip the optimised bloc that uses dcbz. This jump is > * replaced by a nop once cache is active. This is done in machine_init() > */ > -_GLOBAL_KASAN(memset) > +_GLOBAL(memset) > cmplwi 0,r5,4 > blt 7f > > @@ -147,7 +146,6 @@ _GLOBAL_KASAN(memset) > bdnz 9b > blr > EXPORT_SYMBOL(memset) > -EXPORT_SYMBOL_KASAN(memset) > > /* > * This version uses dcbz on the complete cache lines in the > @@ -160,12 +158,12 @@ EXPORT_SYMBOL_KASAN(memset) > * We therefore jump to generic_memcpy which doesn't use dcbz. This jump is > * replaced by a nop once cache is active. This is done in machine_init() > */ > -_GLOBAL_KASAN(memmove) > +_GLOBAL(memmove) > cmplw 0,r3,r4 > bgt backwards_memcpy > /* fall through */ > > -_GLOBAL_KASAN(memcpy) > +_GLOBAL(memcpy) > 1: b generic_memcpy > patch_site 1b, patch__memcpy_nocache > > @@ -241,8 +239,6 @@ _GLOBAL_KASAN(memcpy) > 65: blr > EXPORT_SYMBOL(memcpy) > EXPORT_SYMBOL(memmove) > -EXPORT_SYMBOL_KASAN(memcpy) > -EXPORT_SYMBOL_KASAN(memmove) > > generic_memcpy: > srwi. r7,r5,3 > diff --git a/arch/powerpc/lib/mem_64.S b/arch/powerpc/lib/mem_64.S > index 6fd06cd20faa..40eaedd31486 100644 > --- a/arch/powerpc/lib/mem_64.S > +++ b/arch/powerpc/lib/mem_64.S > @@ -8,7 +8,6 @@ > #include <asm/processor.h> > #include <asm/errno.h> > #include <asm/ppc_asm.h> > -#include <asm/kasan.h> > > #ifndef CONFIG_KASAN > _GLOBAL(__memset16) > @@ -29,7 +28,7 @@ EXPORT_SYMBOL(__memset32) > EXPORT_SYMBOL(__memset64) > #endif > > -_GLOBAL_KASAN(memset) > +_GLOBAL(memset) > neg r0,r3 > rlwimi r4,r4,8,16,23 > andi. r0,r0,7 /* # bytes to be 8-byte aligned */ > @@ -95,9 +94,8 @@ _GLOBAL_KASAN(memset) > stb r4,0(r6) > blr > EXPORT_SYMBOL(memset) > -EXPORT_SYMBOL_KASAN(memset) > > -_GLOBAL_TOC_KASAN(memmove) > +_GLOBAL_TOC(memmove) > cmplw 0,r3,r4 > bgt backwards_memcpy > b memcpy > @@ -139,4 +137,3 @@ _GLOBAL(backwards_memcpy) > mtctr r7 > b 1b > EXPORT_SYMBOL(memmove) > -EXPORT_SYMBOL_KASAN(memmove) > diff --git a/arch/powerpc/lib/memcpy_64.S b/arch/powerpc/lib/memcpy_64.S > index b5a67e20143f..0cedd455231a 100644 > --- a/arch/powerpc/lib/memcpy_64.S > +++ b/arch/powerpc/lib/memcpy_64.S > @@ -7,7 +7,6 @@ > #include <asm/ppc_asm.h> > #include <asm/asm-compat.h> > #include <asm/feature-fixups.h> > -#include <asm/kasan.h> > > #ifndef SELFTEST_CASE > /* For big-endian, 0 == most CPUs, 1 == POWER6, 2 == Cell */ > @@ -15,7 +14,7 @@ > #endif > > .align 7 > -_GLOBAL_TOC_KASAN(memcpy) > +_GLOBAL_TOC(memcpy) > BEGIN_FTR_SECTION > #ifdef __LITTLE_ENDIAN__ > cmpdi cr7,r5,0 > @@ -227,4 +226,3 @@ END_FTR_SECTION_IFCLR(CPU_FTR_UNALIGNED_LD_STD) > blr > #endif > EXPORT_SYMBOL(memcpy) > -EXPORT_SYMBOL_KASAN(memcpy) Yeah this seems like it could work. Letme try this. Thanks, Mukesh
