Re: [PATCH v5] ARM: imx: Fix suspend/resume crash with Clang CFI

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Global CFI Check Failure on fncpy() copied Suspend/PM routines (IMX5, AT91, OMAP1) due to missing kCFI hashes.
--

commit 24f4d3cbee4d4f64e23b67ce2fbbb6d3e3eb8db1
Author: Yo'av Moshe <linux@yoavmoshe.com>

ARM: imx: Fix suspend/resume crash with Clang CFI

The suspend code that runs from OCRAM is copied there with fncpy(), which does
not copy the kCFI type hash preceding the function. With CONFIG_CFI=y the
indirect call through imx6_suspend_in_ocram_fn therefore panics.

> diff --git a/arch/arm/mach-imx/pm-imx6.c b/arch/arm/mach-imx/pm-imx6.c
> index a671ca498f887..3c73e2cdb8ceb 100644
> --- a/arch/arm/mach-imx/pm-imx6.c
> +++ b/arch/arm/mach-imx/pm-imx6.c

[ ... ]

> @@ -568,6 +581,13 @@ static int __init imx6q_suspend_init(const struct imx6_pm_socdata *socdata)
>  			mmdc_offset_array[i]);
>  	}
>  
> +	/*
> +	 * Mask out the Thumb bit, as cfi_get_func_hash() expects the
> +	 * function's actual start address. Returns 0 if CONFIG_CFI=n.
> +	 */
> +	pm_info->cfi_type =
> +		cfi_get_func_hash((void *)((uintptr_t)&imx6_suspend & ~1UL));
> +
>  	imx6_suspend_in_ocram_fn = fncpy(
>  		suspend_ocram_base + sizeof(*pm_info),
>  		&imx6_suspend,
[Severity: High]
This isn't a bug introduced by this patch, but since fncpy() strips the 4-byte
kCFI type hash during SRAM copying, wouldn't other ARM architectures using
fncpy() for suspend routines also fail their CFI checks and panic in the exact
same way?

For instance, this happens during suspend or idle when the copied PM routine
is called indirectly.

In arch/arm/mach-imx/pm-imx5.c:mx5_suspend_enter():
    if (imx5_suspend_in_ocram_fn)
        imx5_suspend_in_ocram_fn(suspend_ocram_base);

In arch/arm/mach-at91/pm.c:at91_pm_suspend():
    at91_suspend_sram_fn(&soc_pm.data);

In arch/arm/mach-omap1/pm.c:omap1_pm_idle():
    omap_sram_suspend(omap_readl(ARM_IDLECT1),
                      omap_readl(ARM_IDLECT2));

In arch/arm/mach-omap1/pm.c:omap1_pm_suspend():
    omap_sram_suspend(arg0, arg1);

Do these architectures need a similar fix to ensure the kCFI hash is
physically located before the function copy in SRAM?

>  		MX6Q_SUSPEND_OCRAM_SIZE - sizeof(*pm_info));

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260830155100.1020620-1-linux@yoavmoshe.com?part=1