| Message ID | 20241108-fix-kcfi-bug-v1-1-f8497c4bccae@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ARM: cfi: Fix compilation corner case | expand |
On Fri, Nov 08, 2024 at 08:37:36AM +0100, Linus Walleij wrote: > When enabling expert mode CONFIG_EXPERT and using that power > user mode to disable the branch prediction hardening > !CONFIG_HARDEN_BRANCH_PREDICTOR, the picky assembly linker > in CLANG notices that some assembly in proc-v7.S does > not have corresponding C call sites, i.e. the prototypes > in proc-v7-bugs.c are enclosed in ifdef > CONFIG_HARDEN_BRANCH_PREDICTOR so this assembly: > > SYM_TYPED_FUNC_START(cpu_v7_smc_switch_mm) > SYM_TYPED_FUNC_START(cpu_v7_hvc_switch_mm) > > Results in: > > ld.lld: error: undefined symbol: __kcfi_typeid_cpu_v7_smc_switch_mm > >>> referenced by proc-v7.S:94 (.../arch/arm/mm/proc-v7.S:94) > >>> arch/arm/mm/proc-v7.o:(.text+0x108) in archive vmlinux.a > > ld.lld: error: undefined symbol: __kcfi_typeid_cpu_v7_hvc_switch_mm > >>> referenced by proc-v7.S:105 (.../arch/arm/mm/proc-v7.S:105) > >>> arch/arm/mm/proc-v7.o:(.text+0x124) in archive vmlinux.a > > Fix this by adding an additional requirement that > CONFIG_HARDEN_BRANCH_PREDICTOR has to be enabled to compile > these assembly calls. > > I suppose it wasn't a problem before because the linker is not > so picky that other assembly symbols are actually being > used. I think this has been a problem since the original CFI change, so I think it is worth dropping this block; it is more likely that CONFIG_HARDEN_BRANCH_PREDICTOR is not often disabled outside of randconfig, so it is hard to hit this issue in practice. As far as I can tell, this is totally expected with the given configuration. SYM_TYPED_FUNC_START makes use of __kcfi_typeid_<func>, which the compiler generates for address-taken function declarations, but in this configuration, cpu_v7_smc_switch_mm() and cpu_v7_hvc_switch_mm() have no actual uses aside from their definition, so the compiler does not generate these symbols, resulting in the link time error above. Perhaps some of this could be encorporated into the beginning of the commit message to make the issue a little more clear and less like the compiler is at fault here ("the picky assembly linker" stands out a bit to me there). > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202411041456.ZsoEiD7T-lkp@intel.com/ > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> Regardless of the commit message nits, this seems like the right thing to do. Reviewed-by: Nathan Chancellor <nathan@kernel.org> > --- > arch/arm/mm/proc-v7.S | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S > index 5fb9a6aecb00..2cd933342679 100644 > --- a/arch/arm/mm/proc-v7.S > +++ b/arch/arm/mm/proc-v7.S > @@ -94,7 +94,7 @@ SYM_TYPED_FUNC_START(cpu_v7_dcache_clean_area) > ret lr > SYM_FUNC_END(cpu_v7_dcache_clean_area) > > -#ifdef CONFIG_ARM_PSCI > +#if defined(CONFIG_ARM_PSCI) && defined(CONFIG_HARDEN_BRANCH_PREDICTOR) > .arch_extension sec > SYM_TYPED_FUNC_START(cpu_v7_smc_switch_mm) > stmfd sp!, {r0 - r3} > > --- > base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc > change-id: 20241107-fix-kcfi-bug-ae3b08cbc167 > > Best regards, > -- > Linus Walleij <linus.walleij@linaro.org> >
On Sat, Nov 9, 2024 at 5:31 AM Nathan Chancellor <nathan@kernel.org> wrote: > Regardless of the commit message nits, this seems like the right thing > to do. > > Reviewed-by: Nathan Chancellor <nathan@kernel.org> Thanks Nathan, I dropped all the annoying complaints about pickiness in the commit message (after all, compilers should be picky, it's a defining trait...) and put the patch into Russell's arfmlpatch tracker. Yours, Linus Walleij
diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S index 5fb9a6aecb00..2cd933342679 100644 --- a/arch/arm/mm/proc-v7.S +++ b/arch/arm/mm/proc-v7.S @@ -94,7 +94,7 @@ SYM_TYPED_FUNC_START(cpu_v7_dcache_clean_area) ret lr SYM_FUNC_END(cpu_v7_dcache_clean_area) -#ifdef CONFIG_ARM_PSCI +#if defined(CONFIG_ARM_PSCI) && defined(CONFIG_HARDEN_BRANCH_PREDICTOR) .arch_extension sec SYM_TYPED_FUNC_START(cpu_v7_smc_switch_mm) stmfd sp!, {r0 - r3}