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

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

[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

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

[Nick Desaulniers]

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

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

[Sami Tolvanen]

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

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

[Yo'av Moshe]

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

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

[Yo'av Moshe]

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

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

[Nathan Chancellor]

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

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

[Yo'av Moshe]

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

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

[Sami Tolvanen]

On Wed, Jul 22, 2026 at 5:54 AM Yo'av Moshe <linux@yoavmoshe.com> wrote:
>
> 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.

Are relocations applied to this code before the function is copied?
Because SYM_TYPED_FUNC_START doesn't embed the actual CFI hash in the
binary, just a reference to a symbol that contains the hash, and it
relies on the kernel to apply relocations before indirect calls are
made to the function.

Sami

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

[Yo'av Moshe]

On 2026-07-22 5:00 PM, Sami Tolvanen wrote:
> Are relocations applied to this code before the function is copied?
> Because SYM_TYPED_FUNC_START doesn't embed the actual CFI hash in the
> binary, just a reference to a symbol that contains the hash, and it
> relies on the kernel to apply relocations before indirect calls are
> made to the function.
Hi Sami,

AFAICT, kernel relocations are already applied at boot before fncpy()
is called during init.

I think the issue might be a parameter mismatch: in C, the pointer is
declared as taking a parameter `void (*)(void __iomem *)`, but in assembly
`imx6_suspend` has no parameters, so Clang generates two different hashes
for them?

Best regards,
Yo'av

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

[Sami Tolvanen]

On Wed, Jul 22, 2026 at 10:47 AM Yo'av Moshe <linux@yoavmoshe.com> wrote:
>
> I think the issue might be a parameter mismatch: in C, the pointer is
> declared as taking a parameter `void (*)(void __iomem *)`, but in assembly
> `imx6_suspend` has no parameters, so Clang generates two different hashes
> for them?

As long as the C prototype for the assembly function matches the
function pointer type, the hashes should also match. Can you dump the
hash prefix from the OCRAM and compare it to the disassembly of the
call site?

Sami

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

[Yo'av Moshe]

On 2026-07-22 8:03 PM, Sami Tolvanen wrote:
> As long as the C prototype for the assembly function matches the
> function pointer type, the hashes should also match. Can you dump the
> hash prefix from the OCRAM and compare it to the disassembly of the
> call site?
Here is what I saw when checking the disassembly in vmlinux:

1. Call site expected hash in pm-imx6.c: 0xA488EBFC
2. SYM_TYPED_FUNC_START hash prefix in suspend-imx6.S: 0xd4d4d4d4

It looks like because suspend-imx6.S is an assembly file, Clang emits
the default assembly type ID (0xd4d4d4d4) for it, which doesn't match
the C call site expectation (0xA488EBFC).

Yo'av

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

[Sami Tolvanen]

On Thu, Jul 23, 2026 at 6:30 AM Yo'av Moshe <linux@yoavmoshe.com> wrote:
>
> On 2026-07-22 8:03 PM, Sami Tolvanen wrote:
> > As long as the C prototype for the assembly function matches the
> > function pointer type, the hashes should also match. Can you dump the
> > hash prefix from the OCRAM and compare it to the disassembly of the
> > call site?
> Here is what I saw when checking the disassembly in vmlinux:
>
> 1. Call site expected hash in pm-imx6.c: 0xA488EBFC
> 2. SYM_TYPED_FUNC_START hash prefix in suspend-imx6.S: 0xd4d4d4d4

What's the value for the __kcfi_typeid_imx6_suspend symbol in
vmlinux.o? I would expect the hash value to look a bit more random.

> It looks like because suspend-imx6.S is an assembly file, Clang emits
> the default assembly type ID (0xd4d4d4d4) for it, which doesn't match
> the C call site expectation (0xA488EBFC).

Clang doesn't actually emit type hashes for assembly functions.
SYM_TYPED_FUNC_START just adds a reference to the
__kcfi_typeid_<functionname> symbol that contains a hash calculated
for the C prototype.

Sami

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

[Yo'av Moshe]

On 2026-07-23 9:21 PM, Sami Tolvanen wrote:
> What's the value for the __kcfi_typeid_imx6_suspend symbol in
> vmlinux.o? I would expect the hash value to look a bit more random.
Checking vmlinux.o with `llvm-readelf -s vmlinux.o` indeed shows:

  a488ebfc  0 NOTYPE  WEAK  DEFAULT  ABS __kcfi_typeid_imx6_suspend

Yo'av

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

[Sami Tolvanen]

On Thu, Jul 23, 2026 at 12:56 PM Yo'av Moshe <linux@yoavmoshe.com> wrote:
>
> On 2026-07-23 9:21 PM, Sami Tolvanen wrote:
> > What's the value for the __kcfi_typeid_imx6_suspend symbol in
> > vmlinux.o? I would expect the hash value to look a bit more random.
> Checking vmlinux.o with `llvm-readelf -s vmlinux.o` indeed shows:
>
>   a488ebfc  0 NOTYPE  WEAK  DEFAULT  ABS __kcfi_typeid_imx6_suspend

OK, so the hashes should match. I can only assume that the hash prefix
doesn't end up getting copied to OCRAM for some reason then, or
perhaps the source of the copy operation doesn't have relocations
applied? Either way, if this turns out to be infeasible, I think the
__nocfi approach is reasonable in this case too.

Sami

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

[Nick Desaulniers]

On Fri, Jul 24, 2026 at 8:54 AM Sami Tolvanen <samitolvanen@google.com> wrote:
>
> On Thu, Jul 23, 2026 at 12:56 PM Yo'av Moshe <linux@yoavmoshe.com> wrote:
> >
> > On 2026-07-23 9:21 PM, Sami Tolvanen wrote:
> > > What's the value for the __kcfi_typeid_imx6_suspend symbol in
> > > vmlinux.o? I would expect the hash value to look a bit more random.
> > Checking vmlinux.o with `llvm-readelf -s vmlinux.o` indeed shows:
> >
> >   a488ebfc  0 NOTYPE  WEAK  DEFAULT  ABS __kcfi_typeid_imx6_suspend
>
> OK, so the hashes should match. I can only assume that the hash prefix
> doesn't end up getting copied to OCRAM for some reason then, or

Yeah, Yo'av did you test further modifications that explicitly copy
the hash prefix? I assumed Nathan's change _plus some additional tweak
to copy not just imx6_suspend to OCRAM which I assume the code already
does, but I would imagine you need another change to copy
__kcfi_typeid_imx6_suspend into OCRAM just before the function.  Was
that tested?

> perhaps the source of the copy operation doesn't have relocations
> applied? Either way, if this turns out to be infeasible, I think the
> __nocfi approach is reasonable in this case too.
>
> Sami



-- 
Thanks,
~Nick Desaulniers

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

[Yo'av Moshe]

On 2026-07-24 10:49 PM, Nick Desaulniers wrote:
> Yeah, Yo'av did you test further modifications that explicitly copy
> the hash prefix? I assumed Nathan's change _plus some additional tweak
> to copy not just imx6_suspend to OCRAM which I assume the code already
> does, but I would imagine you need another change to copy
> __kcfi_typeid_imx6_suspend into OCRAM just before the function.  Was
> that tested?
Yes, I tried that as well and it unfortunately didn't work.

I tested Nathan's change along with explicitly copying the 4-byte
prefix from before imx6_suspend into OCRAM right before calling fncpy().
However, when checking the compiled binary, "&imx6_suspend - 4" just
contained padding rather than the hash, so the copied 4-byte value
didn't match what the call site expected.

Yo'av

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

[Yo'av Moshe]

On 2026-07-26 5:36 PM, Yo'av Moshe wrote:
> Yes, I tried that as well and it unfortunately didn't work.
> 
> I tested Nathan's change along with explicitly copying the 4-byte
> prefix from before imx6_suspend into OCRAM right before calling fncpy().
> However, when checking the compiled binary, "&imx6_suspend - 4" just
> contained padding rather than the hash, so the copied 4-byte value
> didn't match what the call site expected.
Hi Sascha, Frank,

Gentle ping on this patch. Is there anything else needed from my end?

Best regards,
Yo'av

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

[Frank Li]

On Mon, Aug 17, 2026 at 07:45:18PM +0200, Yo'av Moshe wrote:
> On 2026-07-26 5:36 PM, Yo'av Moshe wrote:
> > Yes, I tried that as well and it unfortunately didn't work.
> >
> > I tested Nathan's change along with explicitly copying the 4-byte
> > prefix from before imx6_suspend into OCRAM right before calling fncpy().
> > However, when checking the compiled binary, "&imx6_suspend - 4" just
> > contained padding rather than the hash, so the copied 4-byte value
> > didn't match what the call site expected.
> Hi Sascha, Frank,
>
> Gentle ping on this patch. Is there anything else needed from my end?

There are long discussion on this thread.

Sami Tolvanen and Nick Desaulniers:

	Do you agree on Yo's implement?

Frank

>
> Best regards,
> Yo'av
>

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

[Nick Desaulniers]

On Sun, Jul 26, 2026 at 8:37 AM Yo'av Moshe <linux@yoavmoshe.com> wrote:
>
> On 2026-07-24 10:49 PM, Nick Desaulniers wrote:
> > Yeah, Yo'av did you test further modifications that explicitly copy
> > the hash prefix? I assumed Nathan's change _plus some additional tweak
> > to copy not just imx6_suspend to OCRAM which I assume the code already
> > does, but I would imagine you need another change to copy
> > __kcfi_typeid_imx6_suspend into OCRAM just before the function.  Was
> > that tested?
> Yes, I tried that as well and it unfortunately didn't work.
>
> I tested Nathan's change along with explicitly copying the 4-byte
> prefix from before imx6_suspend into OCRAM right before calling fncpy().

Can you share that diff? I would have expected that to work. Perhaps a
minor mistake in your implementation?

> However, when checking the compiled binary, "&imx6_suspend - 4" just
> contained padding rather than the hash, so the copied 4-byte value
> didn't match what the call site expected.

How did you verify this? Can you share the command line invocations and output?

>
> Yo'av



-- 
Thanks,
~Nick Desaulniers

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

[Yo'av Moshe]

Sorry it took so long, I finally got it to work using your method!

On 2026-08-17 11:58 PM, Nick Desaulniers wrote:
> Can you share that diff? I would have expected that to work. Perhaps a
> minor mistake in your implementation?

I started with something very simple: using SYM_TYPED_FUNC_START and
copying the 4-byte hash into OCRAM before fncpy(). But that didn't boot
on my hardware (Kobo Clara HD, i.MX6SLL).

I think the issues were:
1. My first attempt wrote the hash to suspend_ocram_base +
sizeof(*pm_info) - 4, which I suspect overwrote the last member of
struct imx6_cpu_pm_info, corrupting the memory controller setup.
2. Moving the hash after pm_info and offsetting fncpy by +4 still failed
— I think because fncpy requires 8-byte aligned source and destination
addresses (FNCPY_ALIGN, with a BUG_ON check).
3. Offsetting by +8 for alignment also failed. I suspect it is because
SYM_TYPED_FUNC_START emits the 4-byte hash after the .align directive,
which shifts the imx6_suspend label out of 8-byte alignment, again
triggering fncpy's source alignment BUG_ON.

The version that finally boots on hardware emits the hash manually with
explicit alignment padding, instead of using SYM_TYPED_FUNC_START:

diff --git a/arch/arm/mach-imx/pm-imx6.c b/arch/arm/mach-imx/pm-imx6.c
index a671ca4..b500922 100644
--- a/arch/arm/mach-imx/pm-imx6.c
+++ b/arch/arm/mach-imx/pm-imx6.c
@@ -515,7 +515,7 @@ static int __init imx6q_suspend_init(const struct imx6_pm_socdata *socdata)
 	pm_info = suspend_ocram_base;
 	pm_info->pbase = ocram_pbase;
 	pm_info->resume_addr = __pa_symbol(v7_cpu_resume);
-	pm_info->pm_info_size = sizeof(*pm_info);
+	pm_info->pm_info_size = sizeof(*pm_info) + 8;
 
 	/*
 	 * ccm physical address is not used by asm code currently,
@@ -568,10 +568,16 @@ static int __init imx6q_suspend_init(const struct imx6_pm_socdata *socdata)
 			mmdc_offset_array[i]);
 	}
 
+	/* Reserve 8 bytes between pm_info and the function copy in OCRAM:
+	 * 4 bytes padding + 4 bytes kCFI type hash, so that the hash sits
+	 * at fncpy_dest - 4 and fncpy_dest remains 8-byte aligned. */
+	*(u32 *)(suspend_ocram_base + sizeof(*pm_info) + 4) =
+		*(((u32 *)&imx6_suspend) - 1);
+
 	imx6_suspend_in_ocram_fn = fncpy(
-		suspend_ocram_base + sizeof(*pm_info),
+		suspend_ocram_base + sizeof(*pm_info) + 8,
 		&imx6_suspend,
-		MX6Q_SUSPEND_OCRAM_SIZE - sizeof(*pm_info));
+		MX6Q_SUSPEND_OCRAM_SIZE - sizeof(*pm_info) - 8);
 
 	__arm_iomem_set_ro(suspend_ocram_base, MX6Q_SUSPEND_OCRAM_SIZE);
 
diff --git a/arch/arm/mach-imx/suspend-imx6.S b/arch/arm/mach-imx/suspend-imx6.S
index 63ccc2d..c06e474 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,6 +149,15 @@
 
 	.endm
 
+#ifdef CONFIG_CFI
+	/*
+	 * Emit kCFI type hash before imx6_suspend with padding to preserve
+	 * the 8-byte alignment that fncpy requires for the source address.
+	 */
+	.align	3
+	.4byte	0
+	__CFI_TYPE(imx6_suspend)
+#endif
 ENTRY(imx6_suspend)
 	ldr	r1, [r0, #PM_INFO_PBASE_OFFSET]
 	ldr	r2, [r0, #PM_INFO_RESUME_ADDR_OFFSET]


> How did you verify this? Can you share the command line invocations and output?
I tested each iteration on postmarketOS by building the kernel with
pmbootstrap (Clang/LLVM, CONFIG_CFI=y), replacing the vmlinuz on the SD
card, and booting the Kobo Clara HD. The earlier attempts all failed to
boot (though they worked fine in QEMU, which doesn't emulate the i.MX6
MMDC hardware I guess?).

I'm not really sure which approach is better now. The version above
preserves CFI on the indirect call, but it's quite involved compared to
the v3 __nocfi wrapper. If you think this is the better way to go, I'm
happy to clean it up and resubmit as v4.

Yo'av

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

[Nick Desaulniers]

On Mon, Aug 24, 2026 at 7:10 AM Yo'av Moshe <linux@yoavmoshe.com> wrote:
>
> Sorry it took so long, I finally got it to work using your method!

Great! (It's somewhat funny to me the level of effort going into this;
I love it. Keep up the good work).  It's also good to hear that we can
keep this working _with_ CFI still enabled.

>
> On 2026-08-17 11:58 PM, Nick Desaulniers wrote:
> > Can you share that diff? I would have expected that to work. Perhaps a
> > minor mistake in your implementation?
>
> I started with something very simple: using SYM_TYPED_FUNC_START and
> copying the 4-byte hash into OCRAM before fncpy(). But that didn't boot
> on my hardware (Kobo Clara HD, i.MX6SLL).
>
> I think the issues were:
> 1. My first attempt wrote the hash to suspend_ocram_base +
> sizeof(*pm_info) - 4, which I suspect overwrote the last member of
> struct imx6_cpu_pm_info, corrupting the memory controller setup.
> 2. Moving the hash after pm_info and offsetting fncpy by +4 still failed
> — I think because fncpy requires 8-byte aligned source and destination
> addresses (FNCPY_ALIGN, with a BUG_ON check).
> 3. Offsetting by +8 for alignment also failed. I suspect it is because
> SYM_TYPED_FUNC_START emits the 4-byte hash after the .align directive,
> which shifts the imx6_suspend label out of 8-byte alignment, again
> triggering fncpy's source alignment BUG_ON.
>
> The version that finally boots on hardware emits the hash manually with
> explicit alignment padding, instead of using SYM_TYPED_FUNC_START:
>
> diff --git a/arch/arm/mach-imx/pm-imx6.c b/arch/arm/mach-imx/pm-imx6.c
> index a671ca4..b500922 100644
> --- a/arch/arm/mach-imx/pm-imx6.c
> +++ b/arch/arm/mach-imx/pm-imx6.c
> @@ -515,7 +515,7 @@ static int __init imx6q_suspend_init(const struct imx6_pm_socdata *socdata)
>         pm_info = suspend_ocram_base;
>         pm_info->pbase = ocram_pbase;
>         pm_info->resume_addr = __pa_symbol(v7_cpu_resume);
> -       pm_info->pm_info_size = sizeof(*pm_info);
> +       pm_info->pm_info_size = sizeof(*pm_info) + 8;

I wonder if it would be better to modify the `struct imx6_cpu_pm_info`
struct definition instead to add explicit padding there?  The comment
below about reserving padding seems to make more sense for this hunk
here than down there.

Like add a `u32 cfi_type;` member last, maybe?  That's not 8 bytes,
but I think it fits in the padding, then you might not need +/- 8 in
your pointer arithmetic anymore? I _think_.

>
>         /*
>          * ccm physical address is not used by asm code currently,
> @@ -568,10 +568,16 @@ static int __init imx6q_suspend_init(const struct imx6_pm_socdata *socdata)
>                         mmdc_offset_array[i]);
>         }
>
> +       /* Reserve 8 bytes between pm_info and the function copy in OCRAM:
> +        * 4 bytes padding + 4 bytes kCFI type hash, so that the hash sits
> +        * at fncpy_dest - 4 and fncpy_dest remains 8-byte aligned. */
> +       *(u32 *)(suspend_ocram_base + sizeof(*pm_info) + 4) =
> +               *(((u32 *)&imx6_suspend) - 1);

This is incredibly hard to follow.  This is copying the hash, not
reserving anything?

Is cfi_get_func_hash() something we can use here?

```
#include <linux/cfi.h>

#ifdef CONFIG_CFI
    // Make sure to clear the thumb bit!
    pm_info->cfi_type = cfi_get_func_hash((void
*)((uintptr_t)&imx6_suspend & ~1));
#endif
```

then remove the below hunk?

> +
>         imx6_suspend_in_ocram_fn = fncpy(
> -               suspend_ocram_base + sizeof(*pm_info),
> +               suspend_ocram_base + sizeof(*pm_info) + 8,
>                 &imx6_suspend,
> -               MX6Q_SUSPEND_OCRAM_SIZE - sizeof(*pm_info));
> +               MX6Q_SUSPEND_OCRAM_SIZE - sizeof(*pm_info) - 8);
>
>         __arm_iomem_set_ro(suspend_ocram_base, MX6Q_SUSPEND_OCRAM_SIZE);
>
> diff --git a/arch/arm/mach-imx/suspend-imx6.S b/arch/arm/mach-imx/suspend-imx6.S
> index 63ccc2d..c06e474 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,6 +149,15 @@
>
>         .endm
>
> +#ifdef CONFIG_CFI
> +       /*
> +        * Emit kCFI type hash before imx6_suspend with padding to preserve
> +        * the 8-byte alignment that fncpy requires for the source address.
> +        */
> +       .align  3
> +       .4byte  0
> +       __CFI_TYPE(imx6_suspend)
> +#endif
>  ENTRY(imx6_suspend)

Should you replace ENTRY (and everything you added to this file) with
SYM_TYPED_START?  Or perhaps we should add a new macro to cfi_types.h
for this?  ENTRY_TYPED or something?

>         ldr     r1, [r0, #PM_INFO_PBASE_OFFSET]
>         ldr     r2, [r0, #PM_INFO_RESUME_ADDR_OFFSET]
>
>
> > How did you verify this? Can you share the command line invocations and output?
> I tested each iteration on postmarketOS by building the kernel with
> pmbootstrap (Clang/LLVM, CONFIG_CFI=y), replacing the vmlinuz on the SD
> card, and booting the Kobo Clara HD. The earlier attempts all failed to
> boot (though they worked fine in QEMU, which doesn't emulate the i.MX6
> MMDC hardware I guess?).
>
> I'm not really sure which approach is better now. The version above
> preserves CFI on the indirect call, but it's quite involved compared to
> the v3 __nocfi wrapper. If you think this is the better way to go, I'm
> happy to clean it up and resubmit as v4.

I do think it's better, but probably still needs revision.

FWIW, I still have a bunch of i.mx8 family boards in a box under my
desk.  I have a wafer gifted to me from my best man that we think is
some i.mx chip from his time at NXP.  Out of nostalgia for that line,
let's get this wrapped up neatly with a bow.

>
> Yo'av



-- 
Thanks,
~Nick Desaulniers

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

[Yo'av Moshe]

On 2026-08-25 7:33 PM, Nick Desaulniers wrote:
> I do think it's better, but probably still needs revision.
All three suggestions worked great. cfi_type fits exactly in the
struct's tail padding so I removed the ±8 arithmetic,
cfi_get_func_hash() works as you suggested, and I've switched to
SYM_TYPED_FUNC_START/SYM_FUNC_END. I only kept a small manual alignment
pad before the macro, since it aligns the type hash but not the function
entry itself — I'll include details in the v4 notes.

Yo'av

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

[Yo'av Moshe]

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.

Keep the call covered by CFI instead of exempting it:

- Declare imx6_suspend() with SYM_TYPED_FUNC_START() so a type hash
  is emitted for it. fncpy() requires the entry point to be 8-byte
  aligned, and the macro's alignment applies to the hash rather than
  the entry that follows it, so pad manually before the macro.

- Add a cfi_type member at the end of struct imx6_cpu_pm_info, which
  directly precedes the OCRAM copy of the function. It fits in the
  struct's tail padding, so no sizes or offsets change. Fill it using
  cfi_get_func_hash(), putting the hash where the caller's CFI check
  expects it: four bytes before the function entry.

Also mark ccm_base, suspend_ocram_base and imx6_suspend_in_ocram_fn
as __ro_after_init: they are only written during __init, and the
function pointer in particular should not be writable afterwards.

Suggested-by: Nick Desaulniers <ndesaulniers@google.com>
Cc: stable@vger.kernel.org
Signed-off-by: Yo'av Moshe <linux@yoavmoshe.com>
---
Tested on a Kobo Clara HD (i.MX6SLL) running postmarketOS
(clang/LLVM, CONFIG_CFI=y): suspend and resume work.

Changes in v4, all following Nick's suggestions:
- Drop the v3 __nocfi wrapper; keep the indirect call CFI-checked.
- Store the hash in a cfi_type member in the struct's tail padding
  instead of open-coded pointer arithmetic.
- Use cfi_get_func_hash() instead of reading the hash manually.
- Use SYM_TYPED_FUNC_START()/SYM_FUNC_END() instead of ENTRY()/
  ENDPROC() with a hand-rolled __CFI_TYPE.

Note: linux/uaccess.h is included before linux/cfi.h because
cfi_get_func_hash() uses get_kernel_nofault() and cfi.h does not
include uaccess.h itself.

 arch/arm/mach-imx/pm-imx6.c      | 25 ++++++++++++++++++++++---
 arch/arm/mach-imx/suspend-imx6.S | 14 ++++++++++++--
 2 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-imx/pm-imx6.c b/arch/arm/mach-imx/pm-imx6.c
index a671ca4..2d3d3cd 100644
--- a/arch/arm/mach-imx/pm-imx6.c
+++ b/arch/arm/mach-imx/pm-imx6.c
@@ -4,6 +4,8 @@
  * Copyright 2011 Linaro Ltd.
  */
 
+#include <linux/uaccess.h>
+#include <linux/cfi.h>
 #include <linux/clk/imx.h>
 #include <linux/delay.h>
 #include <linux/init.h>
@@ -61,9 +63,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:
@@ -229,8 +231,18 @@ struct imx6_cpu_pm_info {
 	struct imx6_pm_base l2_base;
 	u32 mmdc_io_num; /* Number of MMDC IOs which need saved/restored. */
 	u32 mmdc_io_val[MX6_MAX_MMDC_IO_NUM][2]; /* To save offset and value */
+	u32 cfi_type; /* kCFI type hash of imx6_suspend() */
 } __aligned(8);
 
+/*
+ * The ocram copy of imx6_suspend() starts right after struct imx6_cpu_pm_info,
+ * and the CFI check on the indirect call reads the kCFI type hash from the
+ * four bytes preceding the function entry, so cfi_type must occupy the last
+ * four bytes of the struct, i.e. fit into its tail padding.
+ */
+static_assert(offsetofend(struct imx6_cpu_pm_info, cfi_type) ==
+	      sizeof(struct imx6_cpu_pm_info));
+
 void imx6_set_int_mem_clk_lpm(bool enable)
 {
 	u32 val = readl_relaxed(ccm_base + CGPR);
@@ -568,6 +580,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,
diff --git a/arch/arm/mach-imx/suspend-imx6.S b/arch/arm/mach-imx/suspend-imx6.S
index 63ccc2d..f60c4e5 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,16 @@
 
 	.endm
 
-ENTRY(imx6_suspend)
+#ifdef CONFIG_CFI
+	/*
+	 * Pad the location counter so that the type hash emitted by
+	 * SYM_TYPED_FUNC_START() below ends on an 8-byte boundary:
+	 * fncpy() requires the function entry to be 8-byte aligned.
+	 */
+	.align	3
+	.4byte	0
+#endif
+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 +339,4 @@ resume:
 	resume_mmdc
 
 	ret	lr
-ENDPROC(imx6_suspend)
+SYM_FUNC_END(imx6_suspend)
-- 
2.55.0

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

[Nick Desaulniers]

On Thu, Aug 27, 2026 at 12:46 PM Yo'av Moshe <linux@yoavmoshe.com> wrote:
>
> 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.
>
> Keep the call covered by CFI instead of exempting it:

Nice!  This is a pretty good progression.

>
> - Declare imx6_suspend() with SYM_TYPED_FUNC_START() so a type hash
>   is emitted for it. fncpy() requires the entry point to be 8-byte
>   aligned, and the macro's alignment applies to the hash rather than
>   the entry that follows it, so pad manually before the macro.
>
> - Add a cfi_type member at the end of struct imx6_cpu_pm_info, which
>   directly precedes the OCRAM copy of the function. It fits in the
>   struct's tail padding, so no sizes or offsets change. Fill it using
>   cfi_get_func_hash(), putting the hash where the caller's CFI check
>   expects it: four bytes before the function entry.
>
> Also mark ccm_base, suspend_ocram_base and imx6_suspend_in_ocram_fn
> as __ro_after_init: they are only written during __init, and the
> function pointer in particular should not be writable afterwards.
>
> Suggested-by: Nick Desaulniers <ndesaulniers@google.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Yo'av Moshe <linux@yoavmoshe.com>
> ---
> Tested on a Kobo Clara HD (i.MX6SLL) running postmarketOS
> (clang/LLVM, CONFIG_CFI=y): suspend and resume work.
>
> Changes in v4, all following Nick's suggestions:
> - Drop the v3 __nocfi wrapper; keep the indirect call CFI-checked.
> - Store the hash in a cfi_type member in the struct's tail padding
>   instead of open-coded pointer arithmetic.
> - Use cfi_get_func_hash() instead of reading the hash manually.
> - Use SYM_TYPED_FUNC_START()/SYM_FUNC_END() instead of ENTRY()/
>   ENDPROC() with a hand-rolled __CFI_TYPE.
>
> Note: linux/uaccess.h is included before linux/cfi.h because
> cfi_get_func_hash() uses get_kernel_nofault() and cfi.h does not
> include uaccess.h itself.

Ah, no, we (you) should fix that. include/linux/cfi.h should IWYU.

>
>  arch/arm/mach-imx/pm-imx6.c      | 25 ++++++++++++++++++++++---
>  arch/arm/mach-imx/suspend-imx6.S | 14 ++++++++++++--
>  2 files changed, 34 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/mach-imx/pm-imx6.c b/arch/arm/mach-imx/pm-imx6.c
> index a671ca4..2d3d3cd 100644
> --- a/arch/arm/mach-imx/pm-imx6.c
> +++ b/arch/arm/mach-imx/pm-imx6.c
> @@ -4,6 +4,8 @@
>   * Copyright 2011 Linaro Ltd.
>   */
>
> +#include <linux/uaccess.h>
> +#include <linux/cfi.h>

This is a bug in include/linux/cfi.h; don't work around it. Fix it.

>  #include <linux/clk/imx.h>
>  #include <linux/delay.h>
>  #include <linux/init.h>
> @@ -61,9 +63,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:
> @@ -229,8 +231,18 @@ struct imx6_cpu_pm_info {
>         struct imx6_pm_base l2_base;
>         u32 mmdc_io_num; /* Number of MMDC IOs which need saved/restored. */
>         u32 mmdc_io_val[MX6_MAX_MMDC_IO_NUM][2]; /* To save offset and value */
> +       u32 cfi_type; /* kCFI type hash of imx6_suspend() */
>  } __aligned(8);
>
> +/*
> + * The ocram copy of imx6_suspend() starts right after struct imx6_cpu_pm_info,
> + * and the CFI check on the indirect call reads the kCFI type hash from the
> + * four bytes preceding the function entry, so cfi_type must occupy the last
> + * four bytes of the struct, i.e. fit into its tail padding.
> + */
> +static_assert(offsetofend(struct imx6_cpu_pm_info, cfi_type) ==
> +             sizeof(struct imx6_cpu_pm_info));

on this IWYU thread, shall we also include linux/build_bug.h for
static_assert or linux/stddef.h for offsetofend?

> +
>  void imx6_set_int_mem_clk_lpm(bool enable)
>  {
>         u32 val = readl_relaxed(ccm_base + CGPR);
> @@ -568,6 +580,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,
> diff --git a/arch/arm/mach-imx/suspend-imx6.S b/arch/arm/mach-imx/suspend-imx6.S
> index 63ccc2d..f60c4e5 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,16 @@
>
>         .endm
>
> -ENTRY(imx6_suspend)
> +#ifdef CONFIG_CFI
> +       /*
> +        * Pad the location counter so that the type hash emitted by
> +        * SYM_TYPED_FUNC_START() below ends on an 8-byte boundary:
> +        * fncpy() requires the function entry to be 8-byte aligned.
> +        */
> +       .align  3
> +       .4byte  0

I still don't like these assembler directives inline like this; this
feels like we should have perhaps a new macro in
include/linux/cfi_types.h. Thoughts, Sami?

One that garuntees the 8B alignment of the symbol for w/e that
function patching routine requires?

> +#endif
> +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 +339,4 @@ resume:
>         resume_mmdc
>
>         ret     lr
> -ENDPROC(imx6_suspend)
> +SYM_FUNC_END(imx6_suspend)
> --
> 2.55.0
>


-- 
Thanks,
~Nick Desaulniers

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

[Sami Tolvanen]

On Fri, Aug 28, 2026 at 11:29 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Thu, Aug 27, 2026 at 12:46 PM Yo'av Moshe <linux@yoavmoshe.com> wrote:
> >
> > Note: linux/uaccess.h is included before linux/cfi.h because
> > cfi_get_func_hash() uses get_kernel_nofault() and cfi.h does not
> > include uaccess.h itself.
>
> Ah, no, we (you) should fix that. include/linux/cfi.h should IWYU.

Nathan fixed this here:

https://lore.kernel.org/lkml/20260604-tracing-fix-cfi-h-build-error-v1-1-b27015390901@kernel.org/

> > -ENTRY(imx6_suspend)
> > +#ifdef CONFIG_CFI
> > +       /*
> > +        * Pad the location counter so that the type hash emitted by
> > +        * SYM_TYPED_FUNC_START() below ends on an 8-byte boundary:
> > +        * fncpy() requires the function entry to be 8-byte aligned.
> > +        */
> > +       .align  3
> > +       .4byte  0
>
> I still don't like these assembler directives inline like this; this
> feels like we should have perhaps a new macro in
> include/linux/cfi_types.h. Thoughts, Sami?
>
> One that garuntees the 8B alignment of the symbol for w/e that
> function patching routine requires?

Sounds like arm should override SYM_TYPED_FUNC_START to add whatever
alignment is needed?

Sami

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

[Nick Desaulniers]

On Fri, Aug 28, 2026 at 11:45 AM Sami Tolvanen <samitolvanen@google.com> wrote:
>
> On Fri, Aug 28, 2026 at 11:29 AM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> >
> > On Thu, Aug 27, 2026 at 12:46 PM Yo'av Moshe <linux@yoavmoshe.com> wrote:
> > >
> > > Note: linux/uaccess.h is included before linux/cfi.h because
> > > cfi_get_func_hash() uses get_kernel_nofault() and cfi.h does not
> > > include uaccess.h itself.
> >
> > Ah, no, we (you) should fix that. include/linux/cfi.h should IWYU.
>
> Nathan fixed this here:
>
> https://lore.kernel.org/lkml/20260604-tracing-fix-cfi-h-build-error-v1-1-b27015390901@kernel.org/

Ah! Then this series should probably be rebased or include that patch.

>
> > > -ENTRY(imx6_suspend)
> > > +#ifdef CONFIG_CFI
> > > +       /*
> > > +        * Pad the location counter so that the type hash emitted by
> > > +        * SYM_TYPED_FUNC_START() below ends on an 8-byte boundary:
> > > +        * fncpy() requires the function entry to be 8-byte aligned.
> > > +        */
> > > +       .align  3
> > > +       .4byte  0
> >
> > I still don't like these assembler directives inline like this; this
> > feels like we should have perhaps a new macro in
> > include/linux/cfi_types.h. Thoughts, Sami?
> >
> > One that garuntees the 8B alignment of the symbol for w/e that
> > function patching routine requires?
>
> Sounds like arm should override SYM_TYPED_FUNC_START to add whatever
> alignment is needed?

The 8B alignment seems to be a requirement of fncopy() for 32b ARM.
But functions in ARM can have 4B or even 2B alignment w/ Thumb, I
think.

Doing such an override would add unnecessary padding for each use of
SYM_TYPED_FUNC_START where probably none of them have this fncopy()
constraint.

```
    #ifdef CONFIG_CFI

    #define SYM_TYPED_START_ALIGNED(name, linkage, align)        \
        linkage(name) ASM_NL                    \
        .balign align ASM_NL                    \
        .fill (align) - 4, 1, 0 ASM_NL                \
        __CFI_TYPE(name) ASM_NL                    \
        name:

    #else /* CONFIG_CFI */

    #define SYM_TYPED_START_ALIGNED(name, linkage, align)        \
        SYM_START(name, linkage, .balign align)

    #endif /* CONFIG_CFI */
    #define SYM_TYPED_FUNC_START_ALIGNED(name, align)        \
        SYM_TYPED_START_ALIGNED(name, SYM_L_GLOBAL, align)
```
then this driver could do
```
// fncopy needs 8B alignment; see arch/arm/include/asm/fncpy.h.
SYM_TYPED_FUNC_START_ALIGNED(imx6_suspend, 8)
```
-- 
Thanks,
~Nick Desaulniers

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

[Sami Tolvanen]

On Fri, Aug 28, 2026 at 1:19 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> The 8B alignment seems to be a requirement of fncopy() for 32b ARM.
> But functions in ARM can have 4B or even 2B alignment w/ Thumb, I
> think.
>
> Doing such an override would add unnecessary padding for each use of
> SYM_TYPED_FUNC_START where probably none of them have this fncopy()
> constraint.
>
> ```
>     #ifdef CONFIG_CFI
>
>     #define SYM_TYPED_START_ALIGNED(name, linkage, align)        \
>         linkage(name) ASM_NL                    \
>         .balign align ASM_NL                    \
>         .fill (align) - 4, 1, 0 ASM_NL                \
>         __CFI_TYPE(name) ASM_NL                    \
>         name:
>
>     #else /* CONFIG_CFI */
>
>     #define SYM_TYPED_START_ALIGNED(name, linkage, align)        \
>         SYM_START(name, linkage, .balign align)
>
>     #endif /* CONFIG_CFI */
>     #define SYM_TYPED_FUNC_START_ALIGNED(name, align)        \
>         SYM_TYPED_START_ALIGNED(name, SYM_L_GLOBAL, align)
> ```
> then this driver could do
> ```
> // fncopy needs 8B alignment; see arch/arm/include/asm/fncpy.h.
> SYM_TYPED_FUNC_START_ALIGNED(imx6_suspend, 8)

Looks reasonable, but this still feels ARM-specific to me. How about
adding the macro to arch/arm/include/asm/linkage.h unless you have
other users in mind?

Sami