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

On Fri, Aug 28, 2026 at 11:45 AM Sami Tolvanen [off-list ref] wrote:

On Fri, Aug 28, 2026 at 11:29 AM Nick Desaulniers
[off-list ref] wrote:

quoted

On Thu, Aug 27, 2026 at 12:46 PM Yo'av Moshe [off-list ref] wrote:

quoted

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/ (local)

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

quoted

quoted

-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