All of 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-18 11:29 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ 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] 8+ 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-18 11:29 ` sashiko-bot
  2026-07-20 16:53 ` Nick Desaulniers
  2026-07-20 19:29 ` Sami Tolvanen
  2 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-07-18 11:29 UTC (permalink / raw)
  To: Yo'av Moshe; +Cc: Frank.Li, imx

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

Pre-existing issues:
- [Critical] W^X Violation / Exploit Mitigation Bypass via Writable OCRAM Alias
--

--- Patch [1]: [PATCH v3] ARM: imx: Fix suspend/resume crash with Clang CFI ---
commit b6dc32274a7f40075998a9c67f552a3982169e81
Author: Yo'av Moshe <linux@yoavmoshe.com>

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

This commit fixes a suspend/resume crash caused by CFI violations on indirect 
calls to relocated OCRAM code. It introduces a minimal __nocfi wrapper for 
the OCRAM call and marks relevant pointers as __ro_after_init to mitigate 
potential exploit risks.

> diff --git a/arch/arm/mach-imx/pm-imx6.c b/arch/arm/mach-imx/pm-imx6.c
> index a671ca498f887..3d5b960c583bd 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;

[ ... ]

> @@ -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);
> +}

[Severity: Critical]
This is a pre-existing issue, but does adding __ro_after_init fully protect 
this memory from write-what-where attacks?

Looking at imx6q_suspend_init(), physical OCRAM memory is mapped with both a 
persistent writable alias (ocram_base) and an executable alias 
(suspend_ocram_base):

arch/arm/mach-imx/pm-imx6.c:imx6q_suspend_init() {
    ocram_base = gen_pool_alloc(ocram_pool, MX6Q_SUSPEND_OCRAM_SIZE);
    ...
    ocram_pbase = gen_pool_virt_to_phys(ocram_pool, ocram_base);

    suspend_ocram_base = __arm_ioremap_exec(ocram_pbase,
        MX6Q_SUSPEND_OCRAM_SIZE, false);
    ...
    __arm_iomem_set_ro(suspend_ocram_base, MX6Q_SUSPEND_OCRAM_SIZE);
}

Even though suspend_ocram_base is marked read-only and its pointer is now 
__ro_after_init, the initial writable alias (ocram_base) appears to remain 
fully active without being unmapped.

Since this patch introduces a __nocfi wrapper to execute this memory, could an 
attacker still bypass CFI by writing shellcode directly into the active 
writable ocram_base alias?

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

^ permalink raw reply	[flat|nested] 8+ 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-18 11:29 ` sashiko-bot
@ 2026-07-20 16:53 ` Nick Desaulniers
  2026-07-21  5:20   ` Yo'av Moshe
  2026-07-20 19:29 ` Sami Tolvanen
  2 siblings, 1 reply; 8+ 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] 8+ 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-18 11:29 ` sashiko-bot
  2026-07-20 16:53 ` Nick Desaulniers
@ 2026-07-20 19:29 ` Sami Tolvanen
  2026-07-21  5:29   ` Yo'av Moshe
  2 siblings, 1 reply; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ 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
  2026-07-22 12:53       ` Yo'av Moshe
  0 siblings, 1 reply; 8+ 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] 8+ messages in thread

* Re: [PATCH v3] ARM: imx: Fix suspend/resume crash with Clang CFI
  2026-07-21 18:09     ` Nathan Chancellor
@ 2026-07-22 12:53       ` Yo'av Moshe
  0 siblings, 0 replies; 8+ messages in thread
From: Yo'av Moshe @ 2026-07-22 12:53 UTC (permalink / raw)
  To: Nathan Chancellor, 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 2026-07-21 8:09 PM, Nathan Chancellor wrote:
> 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.
Hi Nathan,

Thanks for the suggestion!

Unfortunately my physical board died, so I couldn't test this on hardware.
However, I gave your patch a try under QEMU emulation (with CONFIG_CFI=y
and copying the 4-byte hash into OCRAM before fncpy), and it still
triggered a CFI panic when suspending:

    __und_svc_finish from imx6q_suspend_finish+0x78/0xd0

It seems like the hash generated for assembly doesn't match what the C code
expects at the call site.

Best regards,
Yo'av

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

end of thread, other threads:[~2026-07-22 12:54 UTC | newest]

Thread overview: 8+ 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-18 11:29 ` sashiko-bot
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
2026-07-22 12:53       ` Yo'av Moshe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.