[PATCH] ARM: cfi: Fix compilation corner case

[PATCH] ARM: cfi: Fix compilation corner case

[Linus Walleij]

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.

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>
---
 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>

Re: [PATCH] ARM: cfi: Fix compilation corner case

[Nathan Chancellor]

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>
> 

Re: [PATCH] ARM: cfi: Fix compilation corner case

[Linus Walleij]

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