Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] ARM: imx: Fix suspend/resume crash with Clang CFI
@ 2026-07-18 11:13 Yo'av Moshe
  2026-07-20 16:53 ` Nick Desaulniers
  2026-07-20 19:29 ` Sami Tolvanen
  0 siblings, 2 replies; 6+ messages in thread
From: Yo'av Moshe @ 2026-07-18 11:13 UTC (permalink / raw)
  To: Frank Li, Sascha Hauer, Russell King
  Cc: Pengutronix Kernel Team, Fabio Estevam, Nathan Chancellor,
	Nick Desaulniers, Bill Wendling, Justin Stitt, imx,
	linux-arm-kernel, llvm, stable, linux-kernel, Yo'av Moshe

Relocated suspend code in OCRAM lacks compiler-generated CFI type
signatures. When CONFIG_CFI=y is active, the indirect call to
imx6_suspend_in_ocram_fn triggers a strict CFI violation panic.

To resolve this safely without reducing CFI protection scope:
1. Create a minimal wrapper function imx6_suspend_in_ocram annotated
   with __nocfi to handle the unverified indirect call.
2. Remove the __nocfi annotation from the main imx6q_suspend_finish
   function to preserve full CFI coverage for other indirect calls
   in that scope (such as cpu_do_idle() and flush_cache_all()).
3. Mark global variables ccm_base, suspend_ocram_base, and the
   imx6_suspend_in_ocram_fn pointer as __ro_after_init to prevent
   them from being used as target vectors for CFI bypass exploits.

Cc: stable@vger.kernel.org
Signed-off-by: Yo'av Moshe <linux@yoavmoshe.com>
---
Tested on a Kobo Clara HD (i.MX6SLL SoC) running postmarketOS edge. 
Before this patch, suspending the device caused an immediate silent 
hang requiring a hard-reboot. With this patch applied, suspend and 
resume work successfully.

Differences from v2:
- Restrained __nocfi scope by adding a dedicated, minimal 1-line 
  wrapper function (imx6_suspend_in_ocram) for the OCRAM call, 
  avoiding disabling CFI checks for cpu_do_idle() and flush_cache_all().
- Marked global pointers ccm_base and suspend_ocram_base as
  __ro_after_init to fully neutralize Write-What-Where exploit bypasses.

 arch/arm/mach-imx/pm-imx6.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-imx/pm-imx6.c b/arch/arm/mach-imx/pm-imx6.c
index a671ca498..3d5b960c5 100644
--- a/arch/arm/mach-imx/pm-imx6.c
+++ b/arch/arm/mach-imx/pm-imx6.c
@@ -61,9 +61,9 @@
 #define MX6Q_SUSPEND_OCRAM_SIZE		0x1000
 #define MX6_MAX_MMDC_IO_NUM		33
 
-static void __iomem *ccm_base;
-static void __iomem *suspend_ocram_base;
-static void (*imx6_suspend_in_ocram_fn)(void __iomem *ocram_vbase);
+static void __iomem *ccm_base __ro_after_init;
+static void __iomem *suspend_ocram_base __ro_after_init;
+static void (*imx6_suspend_in_ocram_fn)(void __iomem *ocram_vbase) __ro_after_init;
 
 /*
  * suspend ocram space layout:
@@ -360,6 +360,11 @@ int imx6_set_lpm(enum mxc_cpu_pwr_mode mode)
 	return 0;
 }
 
+static void __nocfi imx6_suspend_in_ocram(void __iomem *ocram_vbase)
+{
+	imx6_suspend_in_ocram_fn(ocram_vbase);
+}
+
 static int imx6q_suspend_finish(unsigned long val)
 {
 	if (!imx6_suspend_in_ocram_fn) {
@@ -374,7 +379,7 @@ static int imx6q_suspend_finish(unsigned long val)
 		if (!((struct imx6_cpu_pm_info *)
 			suspend_ocram_base)->l2_base.vbase)
 			flush_cache_all();
-		imx6_suspend_in_ocram_fn(suspend_ocram_base);
+		imx6_suspend_in_ocram(suspend_ocram_base);
 	}
 
 	return 0;
-- 
2.55.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] ARM: imx: Fix suspend/resume crash with Clang CFI
  2026-07-18 11:13 [PATCH v3] ARM: imx: Fix suspend/resume crash with Clang CFI Yo'av Moshe
@ 2026-07-20 16:53 ` Nick Desaulniers
  2026-07-21  5:20   ` Yo'av Moshe
  2026-07-20 19:29 ` Sami Tolvanen
  1 sibling, 1 reply; 6+ messages in thread
From: Nick Desaulniers @ 2026-07-20 16:53 UTC (permalink / raw)
  To: Yo'av Moshe
  Cc: Frank Li, Sascha Hauer, Russell King, Pengutronix Kernel Team,
	Fabio Estevam, Nathan Chancellor, Bill Wendling, Justin Stitt,
	imx, linux-arm-kernel, llvm, stable, linux-kernel

On Sat, Jul 18, 2026 at 4:14 AM Yo'av Moshe <linux@yoavmoshe.com> wrote:
>
> Relocated suspend code in OCRAM lacks compiler-generated CFI type
> signatures. When CONFIG_CFI=y is active, the indirect call to
> imx6_suspend_in_ocram_fn triggers a strict CFI violation panic.
>
> To resolve this safely without reducing CFI protection scope:
> 1. Create a minimal wrapper function imx6_suspend_in_ocram annotated
>    with __nocfi to handle the unverified indirect call.
> 2. Remove the __nocfi annotation from the main imx6q_suspend_finish
>    function to preserve full CFI coverage for other indirect calls
>    in that scope (such as cpu_do_idle() and flush_cache_all()).
> 3. Mark global variables ccm_base, suspend_ocram_base, and the
>    imx6_suspend_in_ocram_fn pointer as __ro_after_init to prevent
>    them from being used as target vectors for CFI bypass exploits.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Yo'av Moshe <linux@yoavmoshe.com>
> ---
> Tested on a Kobo Clara HD (i.MX6SLL SoC) running postmarketOS edge.
> Before this patch, suspending the device caused an immediate silent
> hang requiring a hard-reboot. With this patch applied, suspend and
> resume work successfully.
>
> Differences from v2:
> - Restrained __nocfi scope by adding a dedicated, minimal 1-line
>   wrapper function (imx6_suspend_in_ocram) for the OCRAM call,
>   avoiding disabling CFI checks for cpu_do_idle() and flush_cache_all().
> - Marked global pointers ccm_base and suspend_ocram_base as
>   __ro_after_init to fully neutralize Write-What-Where exploit bypasses.
>
>  arch/arm/mach-imx/pm-imx6.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/mach-imx/pm-imx6.c b/arch/arm/mach-imx/pm-imx6.c
> index a671ca498..3d5b960c5 100644
> --- a/arch/arm/mach-imx/pm-imx6.c
> +++ b/arch/arm/mach-imx/pm-imx6.c
> @@ -61,9 +61,9 @@
>  #define MX6Q_SUSPEND_OCRAM_SIZE                0x1000
>  #define MX6_MAX_MMDC_IO_NUM            33
>
> -static void __iomem *ccm_base;
> -static void __iomem *suspend_ocram_base;
> -static void (*imx6_suspend_in_ocram_fn)(void __iomem *ocram_vbase);
> +static void __iomem *ccm_base __ro_after_init;
> +static void __iomem *suspend_ocram_base __ro_after_init;
> +static void (*imx6_suspend_in_ocram_fn)(void __iomem *ocram_vbase) __ro_after_init;

Are we able to just put __nocfi on the declaration of
`imx6_suspend_in_ocram_fn`, rather than bother with a wrapper
(imx6_suspend_in_ocram)? I don't know if that works, but surely you
can test that quickly?

>
>  /*
>   * suspend ocram space layout:
> @@ -360,6 +360,11 @@ int imx6_set_lpm(enum mxc_cpu_pwr_mode mode)
>         return 0;
>  }
>
> +static void __nocfi imx6_suspend_in_ocram(void __iomem *ocram_vbase)
> +{
> +       imx6_suspend_in_ocram_fn(ocram_vbase);
> +}
> +
>  static int imx6q_suspend_finish(unsigned long val)
>  {
>         if (!imx6_suspend_in_ocram_fn) {
> @@ -374,7 +379,7 @@ static int imx6q_suspend_finish(unsigned long val)
>                 if (!((struct imx6_cpu_pm_info *)
>                         suspend_ocram_base)->l2_base.vbase)
>                         flush_cache_all();
> -               imx6_suspend_in_ocram_fn(suspend_ocram_base);
> +               imx6_suspend_in_ocram(suspend_ocram_base);
>         }
>
>         return 0;
> --
> 2.55.0
>


-- 
Thanks,
~Nick Desaulniers


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] ARM: imx: Fix suspend/resume crash with Clang CFI
  2026-07-18 11:13 [PATCH v3] ARM: imx: Fix suspend/resume crash with Clang CFI Yo'av Moshe
  2026-07-20 16:53 ` Nick Desaulniers
@ 2026-07-20 19:29 ` Sami Tolvanen
  2026-07-21  5:29   ` Yo'av Moshe
  1 sibling, 1 reply; 6+ messages in thread
From: Sami Tolvanen @ 2026-07-20 19:29 UTC (permalink / raw)
  To: Yo'av Moshe
  Cc: Frank Li, Sascha Hauer, Russell King, Pengutronix Kernel Team,
	Fabio Estevam, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
	Justin Stitt, imx, linux-arm-kernel, llvm, stable, linux-kernel

On Sat, Jul 18, 2026 at 4:14 AM Yo'av Moshe <linux@yoavmoshe.com> wrote:
>
> Relocated suspend code in OCRAM lacks compiler-generated CFI type
> signatures. When CONFIG_CFI=y is active, the indirect call to
> imx6_suspend_in_ocram_fn triggers a strict CFI violation panic.

Would it be possible to just copy the 4-byte CFI hash prefix to OCRAM
when relocating the function? If not, the __nocfi approach seems
reasonable to me.

Sami


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] ARM: imx: Fix suspend/resume crash with Clang CFI
  2026-07-20 16:53 ` Nick Desaulniers
@ 2026-07-21  5:20   ` Yo'av Moshe
  0 siblings, 0 replies; 6+ messages in thread
From: Yo'av Moshe @ 2026-07-21  5:20 UTC (permalink / raw)
  To: Nick Desaulniers, Yo'av Moshe
  Cc: Frank Li, Sascha Hauer, Russell King, Pengutronix Kernel Team,
	Fabio Estevam, Nathan Chancellor, Bill Wendling, Justin Stitt,
	imx, linux-arm-kernel, llvm, stable, linux-kernel

On 2026-07-20 6:53 PM, Nick Desaulniers wrote:
> 
> Are we able to just put __nocfi on the declaration of
> `imx6_suspend_in_ocram_fn`, rather than bother with a wrapper
> (imx6_suspend_in_ocram)? I don't know if that works, but surely you
> can test that quickly?
Thanks for the suggestion! I tested placing __nocfi directly on the
imx6_suspend_in_ocram_fn variable declaration:

    static void (* __nocfi imx6_suspend_in_ocram_fn)(void __iomem *ocram_vbase);

Unfortunately, Clang ignores no_sanitize("cfi") on variable declarations and
emits a compiler warning:

    warning: 'no_sanitize' attribute argument 'cfi' not supported on a
    global variable [-Wignored-attributes]

Because Clang ignores it, it still injects the CFI check at the call site.
I tested this on physical hardware (Kobo Clara HD), and it crashes on suspend.

Best regards,
Yo'av


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] ARM: imx: Fix suspend/resume crash with Clang CFI
  2026-07-20 19:29 ` Sami Tolvanen
@ 2026-07-21  5:29   ` Yo'av Moshe
  2026-07-21 18:09     ` Nathan Chancellor
  0 siblings, 1 reply; 6+ messages in thread
From: Yo'av Moshe @ 2026-07-21  5:29 UTC (permalink / raw)
  To: Sami Tolvanen, Yo'av Moshe
  Cc: Frank Li, Sascha Hauer, Russell King, Pengutronix Kernel Team,
	Fabio Estevam, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
	Justin Stitt, imx, linux-arm-kernel, llvm, stable, linux-kernel

On 2026-07-20 9:29 PM, Sami Tolvanen wrote:
> Would it be possible to just copy the 4-byte CFI hash prefix to OCRAM
> when relocating the function? If not, the __nocfi approach seems
> reasonable to me.
> 
> Sami

I gave this a try - I tried copying 4 bytes from before imx6_suspend
into OCRAM, but when I tested it on physical hardware (Kobo Clara HD),
it still crashed on suspend.

I suspect it's because imx6_suspend is written in assembly
(suspend-imx6.S) rather than C, so Clang doesn't emit a CFI hash prefix
before it in the first place.

Best regards,
Yo'av


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] ARM: imx: Fix suspend/resume crash with Clang CFI
  2026-07-21  5:29   ` Yo'av Moshe
@ 2026-07-21 18:09     ` Nathan Chancellor
  0 siblings, 0 replies; 6+ messages in thread
From: Nathan Chancellor @ 2026-07-21 18:09 UTC (permalink / raw)
  To: Yo'av Moshe
  Cc: Sami Tolvanen, Frank Li, Sascha Hauer, Russell King,
	Pengutronix Kernel Team, Fabio Estevam, Nick Desaulniers,
	Bill Wendling, Justin Stitt, imx, linux-arm-kernel, llvm, stable,
	linux-kernel

On Tue, Jul 21, 2026 at 07:29:50AM +0200, Yo'av Moshe wrote:
> On 2026-07-20 9:29 PM, Sami Tolvanen wrote:
> > Would it be possible to just copy the 4-byte CFI hash prefix to OCRAM
> > when relocating the function? If not, the __nocfi approach seems
> > reasonable to me.
> > 
> > Sami
> 
> I gave this a try - I tried copying 4 bytes from before imx6_suspend
> into OCRAM, but when I tested it on physical hardware (Kobo Clara HD),
> it still crashed on suspend.
> 
> I suspect it's because imx6_suspend is written in assembly
> (suspend-imx6.S) rather than C, so Clang doesn't emit a CFI hash prefix
> before it in the first place.

Does using SYM_TYPED_FUNC_START for imx6_suspend() make that work?
Something like this builds fine for me and I see
__kcfi_typeid_imx6_suspend generated by Clang.

diff --git a/arch/arm/mach-imx/suspend-imx6.S b/arch/arm/mach-imx/suspend-imx6.S
index 63ccc2d0e920..6ded29a38c99 100644
--- a/arch/arm/mach-imx/suspend-imx6.S
+++ b/arch/arm/mach-imx/suspend-imx6.S
@@ -3,6 +3,7 @@
  * Copyright 2014 Freescale Semiconductor, Inc.
  */
 
+#include <linux/cfi_types.h>
 #include <linux/linkage.h>
 #include <asm/assembler.h>
 #include <asm/asm-offsets.h>
@@ -148,7 +149,7 @@
 
 	.endm
 
-ENTRY(imx6_suspend)
+SYM_TYPED_FUNC_START(imx6_suspend)
 	ldr	r1, [r0, #PM_INFO_PBASE_OFFSET]
 	ldr	r2, [r0, #PM_INFO_RESUME_ADDR_OFFSET]
 	ldr	r3, [r0, #PM_INFO_DDR_TYPE_OFFSET]
@@ -329,4 +330,4 @@ resume:
 	resume_mmdc
 
 	ret	lr
-ENDPROC(imx6_suspend)
+SYM_FUNC_END(imx6_suspend)

-- 
Cheers,
Nathan


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-07-21 18:10 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-18 11:13 [PATCH v3] ARM: imx: Fix suspend/resume crash with Clang CFI Yo'av Moshe
2026-07-20 16:53 ` Nick Desaulniers
2026-07-21  5:20   ` Yo'av Moshe
2026-07-20 19:29 ` Sami Tolvanen
2026-07-21  5:29   ` Yo'av Moshe
2026-07-21 18:09     ` Nathan Chancellor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox