arm64: Disable LLD linker ASSERT()s for the time being

[PATCH] arm64: Disable LLD linker ASSERT()s for the time being

Posted by Ard Biesheuvel 6 months, 2 weeks ago

From: Ard Biesheuvel <ardb@kernel.org>

It turns out that the way LLD handles ASSERT()s in the linker script can
result in spurious failures, so disable them for the newly introduced
BSS symbol export checks.

Link: https://github.com/ClangBuiltLinux/linux/issues/2094
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm64/kernel/image-vars.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index c5266430284b..86f088a16147 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -10,6 +10,10 @@
 #error This file should only be included in vmlinux.lds.S
 #endif
 
+#if defined(CONFIG_LD_IS_LLD) && CONFIG_LLD_VERSION < 210000
+#define ASSERT(...)
+#endif
+
 #define PI_EXPORT_SYM(sym)		\
 	__PI_EXPORT_SYM(sym, __pi_ ## sym, Cannot export BSS symbol sym to startup code)
 #define __PI_EXPORT_SYM(sym, pisym, msg)\
@@ -142,4 +146,6 @@ KVM_NVHE_ALIAS(kvm_protected_mode_initialized);
 _kernel_codesize = ABSOLUTE(__inittext_end - _text);
 #endif
 
+#undef ASSERT
+
 #endif /* __ARM64_KERNEL_IMAGE_VARS_H */
-- 
2.49.0.1238.gf8c92423fb-goog

Re: [PATCH] arm64: Disable LLD linker ASSERT()s for the time being

Posted by Will Deacon 6 months, 2 weeks ago

On Thu, 29 May 2025 09:35:08 +0200, Ard Biesheuvel wrote:
> It turns out that the way LLD handles ASSERT()s in the linker script can
> result in spurious failures, so disable them for the newly introduced
> BSS symbol export checks.
> 
> 

Applied to arm64 (for-next/core), thanks!

[1/1] arm64: Disable LLD linker ASSERT()s for the time being
      https://git.kernel.org/arm64/c/e21560b7d33c

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

Re: [PATCH] arm64: Disable LLD linker ASSERT()s for the time being

Posted by Arnd Bergmann 6 months, 2 weeks ago

On Thu, May 29, 2025, at 09:35, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
>
> It turns out that the way LLD handles ASSERT()s in the linker script can
> result in spurious failures, so disable them for the newly introduced
> BSS symbol export checks.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/2094
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

I have lld-21 from apt.llvm.org, though with a slightly
older version at the moment:

1:21~++20250418112422+c609cd2df981-1~exp1~20250418112440.1395

this version still hits the assertions on the bss symbols, but
I guess we don't really have to worry about pre-release builds.

I checked your patch with the latest ld.ldd-21 and an earlier ld.lld-20,
both work fine.

Tested-by: Arnd Bergmann <arnd@arndb.de>

Re: [PATCH] arm64: Disable LLD linker ASSERT()s for the time being

Posted by Will Deacon 6 months, 2 weeks ago

On Thu, May 29, 2025 at 09:35:08AM +0200, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
> 
> It turns out that the way LLD handles ASSERT()s in the linker script can
> result in spurious failures, so disable them for the newly introduced
> BSS symbol export checks.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/2094
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  arch/arm64/kernel/image-vars.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
> index c5266430284b..86f088a16147 100644
> --- a/arch/arm64/kernel/image-vars.h
> +++ b/arch/arm64/kernel/image-vars.h
> @@ -10,6 +10,10 @@
>  #error This file should only be included in vmlinux.lds.S
>  #endif
>  
> +#if defined(CONFIG_LD_IS_LLD) && CONFIG_LLD_VERSION < 210000
> +#define ASSERT(...)
> +#endif
> +
>  #define PI_EXPORT_SYM(sym)		\
>  	__PI_EXPORT_SYM(sym, __pi_ ## sym, Cannot export BSS symbol sym to startup code)
>  #define __PI_EXPORT_SYM(sym, pisym, msg)\
> @@ -142,4 +146,6 @@ KVM_NVHE_ALIAS(kvm_protected_mode_initialized);
>  _kernel_codesize = ABSOLUTE(__inittext_end - _text);
>  #endif
>  
> +#undef ASSERT

What about the ASSERT()s at the end of vmlinux.lds.S? Are they not
affected by the bug, for some reason?

Also, even with this patch applied, I still see a link failure:

  | ld.lld: error: assignment to symbol __init_end does not converge

with the .config you sent me off-list.

Will

Re: [PATCH] arm64: Disable LLD linker ASSERT()s for the time being

Posted by Ard Biesheuvel 6 months, 2 weeks ago

On Fri, 30 May 2025 at 15:38, Will Deacon <will@kernel.org> wrote:
>
> On Thu, May 29, 2025 at 09:35:08AM +0200, Ard Biesheuvel wrote:
> > From: Ard Biesheuvel <ardb@kernel.org>
> >
> > It turns out that the way LLD handles ASSERT()s in the linker script can
> > result in spurious failures, so disable them for the newly introduced
> > BSS symbol export checks.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/2094
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >  arch/arm64/kernel/image-vars.h | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
> > index c5266430284b..86f088a16147 100644
> > --- a/arch/arm64/kernel/image-vars.h
> > +++ b/arch/arm64/kernel/image-vars.h
> > @@ -10,6 +10,10 @@
> >  #error This file should only be included in vmlinux.lds.S
> >  #endif
> >
> > +#if defined(CONFIG_LD_IS_LLD) && CONFIG_LLD_VERSION < 210000
> > +#define ASSERT(...)
> > +#endif
> > +
> >  #define PI_EXPORT_SYM(sym)           \
> >       __PI_EXPORT_SYM(sym, __pi_ ## sym, Cannot export BSS symbol sym to startup code)
> >  #define __PI_EXPORT_SYM(sym, pisym, msg)\
> > @@ -142,4 +146,6 @@ KVM_NVHE_ALIAS(kvm_protected_mode_initialized);
> >  _kernel_codesize = ABSOLUTE(__inittext_end - _text);
> >  #endif
> >
> > +#undef ASSERT
>
> What about the ASSERT()s at the end of vmlinux.lds.S? Are they not
> affected by the bug, for some reason?
>
> Also, even with this patch applied, I still see a link failure:
>
>   | ld.lld: error: assignment to symbol __init_end does not converge
>
> with the .config you sent me off-list.
>

That is a different error that has been lurking for a while now; Arnd
occasionally hits it but I haven't seen any other reports of it. AIUI,
the issue is that INIT_IDMAP_DIR_PAGES and INIT_DIR_SIZE are defined
in terms of (_end - KIMAGE_VADDR), resulting in a circular dependency.

The config in the kernel test robot's report [0] appears to build fine
with this patch applied.


[0] https://lore.kernel.org/all/202505261019.OUlitN6m-lkp@intel.com/T/#u

Re: [PATCH] arm64: Disable LLD linker ASSERT()s for the time being

Posted by Will Deacon 6 months, 2 weeks ago

On Fri, May 30, 2025 at 04:23:16PM +0200, Ard Biesheuvel wrote:
> On Fri, 30 May 2025 at 15:38, Will Deacon <will@kernel.org> wrote:
> >
> > On Thu, May 29, 2025 at 09:35:08AM +0200, Ard Biesheuvel wrote:
> > > From: Ard Biesheuvel <ardb@kernel.org>
> > >
> > > It turns out that the way LLD handles ASSERT()s in the linker script can
> > > result in spurious failures, so disable them for the newly introduced
> > > BSS symbol export checks.
> > >
> > > Link: https://github.com/ClangBuiltLinux/linux/issues/2094
> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > ---
> > >  arch/arm64/kernel/image-vars.h | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
> > > index c5266430284b..86f088a16147 100644
> > > --- a/arch/arm64/kernel/image-vars.h
> > > +++ b/arch/arm64/kernel/image-vars.h
> > > @@ -10,6 +10,10 @@
> > >  #error This file should only be included in vmlinux.lds.S
> > >  #endif
> > >
> > > +#if defined(CONFIG_LD_IS_LLD) && CONFIG_LLD_VERSION < 210000
> > > +#define ASSERT(...)
> > > +#endif
> > > +
> > >  #define PI_EXPORT_SYM(sym)           \
> > >       __PI_EXPORT_SYM(sym, __pi_ ## sym, Cannot export BSS symbol sym to startup code)
> > >  #define __PI_EXPORT_SYM(sym, pisym, msg)\
> > > @@ -142,4 +146,6 @@ KVM_NVHE_ALIAS(kvm_protected_mode_initialized);
> > >  _kernel_codesize = ABSOLUTE(__inittext_end - _text);
> > >  #endif
> > >
> > > +#undef ASSERT
> >
> > What about the ASSERT()s at the end of vmlinux.lds.S? Are they not
> > affected by the bug, for some reason?
> >
> > Also, even with this patch applied, I still see a link failure:
> >
> >   | ld.lld: error: assignment to symbol __init_end does not converge
> >
> > with the .config you sent me off-list.
> >
> 
> That is a different error that has been lurking for a while now; Arnd
> occasionally hits it but I haven't seen any other reports of it. AIUI,
> the issue is that INIT_IDMAP_DIR_PAGES and INIT_DIR_SIZE are defined
> in terms of (_end - KIMAGE_VADDR), resulting in a circular dependency.

Ok, I'll ignore that one for the moment, then...

> The config in the kernel test robot's report [0] appears to build fine
> with this patch applied.
> 
> 
> [0] https://lore.kernel.org/all/202505261019.OUlitN6m-lkp@intel.com/T/#u

... but I'm still not sure why the ASSERT()s in vmlinux.lds.S are not
affected. Is it just that we've not hit a .config which breaks with
those yet, or is it something more fundamental than that? I'd have
thought we'd need to so something like below (on top of your patch) to
fix this issue properly.

Will

--->8

diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index 86f088a16147..c5266430284b 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -10,10 +10,6 @@
 #error This file should only be included in vmlinux.lds.S
 #endif
 
-#if defined(CONFIG_LD_IS_LLD) && CONFIG_LLD_VERSION < 210000
-#define ASSERT(...)
-#endif
-
 #define PI_EXPORT_SYM(sym)		\
 	__PI_EXPORT_SYM(sym, __pi_ ## sym, Cannot export BSS symbol sym to startup code)
 #define __PI_EXPORT_SYM(sym, pisym, msg)\
@@ -146,6 +142,4 @@ KVM_NVHE_ALIAS(kvm_protected_mode_initialized);
 _kernel_codesize = ABSOLUTE(__inittext_end - _text);
 #endif
 
-#undef ASSERT
-
 #endif /* __ARM64_KERNEL_IMAGE_VARS_H */
diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
index e4a525a865c1..3f7a365f7113 100644
--- a/arch/arm64/kernel/vmlinux.lds.S
+++ b/arch/arm64/kernel/vmlinux.lds.S
@@ -150,6 +150,10 @@ PECOFF_FILE_ALIGNMENT = 0x200;
 #define PECOFF_EDATA_PADDING
 #endif
 
+#if defined(CONFIG_LD_IS_LLD) && CONFIG_LLD_VERSION < 210000
+#define ASSERT(...)
+#endif
+
 SECTIONS
 {
 	/*

Re: [PATCH] arm64: Disable LLD linker ASSERT()s for the time being

Posted by Ard Biesheuvel 6 months, 2 weeks ago

On Mon, 2 Jun 2025 at 12:09, Will Deacon <will@kernel.org> wrote:
>
> On Fri, May 30, 2025 at 04:23:16PM +0200, Ard Biesheuvel wrote:
> > On Fri, 30 May 2025 at 15:38, Will Deacon <will@kernel.org> wrote:
> > >
> > > On Thu, May 29, 2025 at 09:35:08AM +0200, Ard Biesheuvel wrote:
> > > > From: Ard Biesheuvel <ardb@kernel.org>
> > > >
> > > > It turns out that the way LLD handles ASSERT()s in the linker script can
> > > > result in spurious failures, so disable them for the newly introduced
> > > > BSS symbol export checks.
> > > >
> > > > Link: https://github.com/ClangBuiltLinux/linux/issues/2094
> > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > > ---
> > > >  arch/arm64/kernel/image-vars.h | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
> > > > index c5266430284b..86f088a16147 100644
> > > > --- a/arch/arm64/kernel/image-vars.h
> > > > +++ b/arch/arm64/kernel/image-vars.h
> > > > @@ -10,6 +10,10 @@
> > > >  #error This file should only be included in vmlinux.lds.S
> > > >  #endif
> > > >
> > > > +#if defined(CONFIG_LD_IS_LLD) && CONFIG_LLD_VERSION < 210000
> > > > +#define ASSERT(...)
> > > > +#endif
> > > > +
> > > >  #define PI_EXPORT_SYM(sym)           \
> > > >       __PI_EXPORT_SYM(sym, __pi_ ## sym, Cannot export BSS symbol sym to startup code)
> > > >  #define __PI_EXPORT_SYM(sym, pisym, msg)\
> > > > @@ -142,4 +146,6 @@ KVM_NVHE_ALIAS(kvm_protected_mode_initialized);
> > > >  _kernel_codesize = ABSOLUTE(__inittext_end - _text);
> > > >  #endif
> > > >
> > > > +#undef ASSERT
> > >
> > > What about the ASSERT()s at the end of vmlinux.lds.S? Are they not
> > > affected by the bug, for some reason?
> > >
> > > Also, even with this patch applied, I still see a link failure:
> > >
> > >   | ld.lld: error: assignment to symbol __init_end does not converge
> > >
> > > with the .config you sent me off-list.
> > >
> >
> > That is a different error that has been lurking for a while now; Arnd
> > occasionally hits it but I haven't seen any other reports of it. AIUI,
> > the issue is that INIT_IDMAP_DIR_PAGES and INIT_DIR_SIZE are defined
> > in terms of (_end - KIMAGE_VADDR), resulting in a circular dependency.
>
> Ok, I'll ignore that one for the moment, then...
>
> > The config in the kernel test robot's report [0] appears to build fine
> > with this patch applied.
> >
> >
> > [0] https://lore.kernel.org/all/202505261019.OUlitN6m-lkp@intel.com/T/#u
>
> ... but I'm still not sure why the ASSERT()s in vmlinux.lds.S are not
> affected. Is it just that we've not hit a .config which breaks with
> those yet, or is it something more fundamental than that?

The former, as far as I can tell. The BSS patch just adds a fair
amount of ASSERT()s so the attack surface has become larger. And
perhaps those checks are more susceptible due to the fact that they
compare symbols living in different sections? But that is just
conjecture.

> I'd have
> thought we'd need to so something like below (on top of your patch) to
> fix this issue properly.
>

Yes, it is the more thorough fix, but we'd lose coverage for those
ASSERT()s which are arguably more important than the ones I added.

Re: [PATCH] arm64: Disable LLD linker ASSERT()s for the time being

Posted by Will Deacon 6 months, 2 weeks ago

On Mon, Jun 02, 2025 at 12:18:33PM +0200, Ard Biesheuvel wrote:
> On Mon, 2 Jun 2025 at 12:09, Will Deacon <will@kernel.org> wrote:
> >
> > On Fri, May 30, 2025 at 04:23:16PM +0200, Ard Biesheuvel wrote:
> > > On Fri, 30 May 2025 at 15:38, Will Deacon <will@kernel.org> wrote:
> > > >
> > > > On Thu, May 29, 2025 at 09:35:08AM +0200, Ard Biesheuvel wrote:
> > > > > From: Ard Biesheuvel <ardb@kernel.org>
> > > > >
> > > > > It turns out that the way LLD handles ASSERT()s in the linker script can
> > > > > result in spurious failures, so disable them for the newly introduced
> > > > > BSS symbol export checks.
> > > > >
> > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/2094
> > > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > > > ---
> > > > >  arch/arm64/kernel/image-vars.h | 6 ++++++
> > > > >  1 file changed, 6 insertions(+)
> > > > >
> > > > > diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
> > > > > index c5266430284b..86f088a16147 100644
> > > > > --- a/arch/arm64/kernel/image-vars.h
> > > > > +++ b/arch/arm64/kernel/image-vars.h
> > > > > @@ -10,6 +10,10 @@
> > > > >  #error This file should only be included in vmlinux.lds.S
> > > > >  #endif
> > > > >
> > > > > +#if defined(CONFIG_LD_IS_LLD) && CONFIG_LLD_VERSION < 210000
> > > > > +#define ASSERT(...)
> > > > > +#endif
> > > > > +
> > > > >  #define PI_EXPORT_SYM(sym)           \
> > > > >       __PI_EXPORT_SYM(sym, __pi_ ## sym, Cannot export BSS symbol sym to startup code)
> > > > >  #define __PI_EXPORT_SYM(sym, pisym, msg)\
> > > > > @@ -142,4 +146,6 @@ KVM_NVHE_ALIAS(kvm_protected_mode_initialized);
> > > > >  _kernel_codesize = ABSOLUTE(__inittext_end - _text);
> > > > >  #endif
> > > > >
> > > > > +#undef ASSERT
> > > >
> > > > What about the ASSERT()s at the end of vmlinux.lds.S? Are they not
> > > > affected by the bug, for some reason?
> > > >
> > > > Also, even with this patch applied, I still see a link failure:
> > > >
> > > >   | ld.lld: error: assignment to symbol __init_end does not converge
> > > >
> > > > with the .config you sent me off-list.
> > > >
> > >
> > > That is a different error that has been lurking for a while now; Arnd
> > > occasionally hits it but I haven't seen any other reports of it. AIUI,
> > > the issue is that INIT_IDMAP_DIR_PAGES and INIT_DIR_SIZE are defined
> > > in terms of (_end - KIMAGE_VADDR), resulting in a circular dependency.
> >
> > Ok, I'll ignore that one for the moment, then...
> >
> > > The config in the kernel test robot's report [0] appears to build fine
> > > with this patch applied.
> > >
> > >
> > > [0] https://lore.kernel.org/all/202505261019.OUlitN6m-lkp@intel.com/T/#u
> >
> > ... but I'm still not sure why the ASSERT()s in vmlinux.lds.S are not
> > affected. Is it just that we've not hit a .config which breaks with
> > those yet, or is it something more fundamental than that?
> 
> The former, as far as I can tell. The BSS patch just adds a fair
> amount of ASSERT()s so the attack surface has become larger. And
> perhaps those checks are more susceptible due to the fact that they
> compare symbols living in different sections? But that is just
> conjecture.
> 
> > I'd have
> > thought we'd need to so something like below (on top of your patch) to
> > fix this issue properly.
> >
> 
> Yes, it is the more thorough fix, but we'd lose coverage for those
> ASSERT()s which are arguably more important than the ones I added.

Alright, thanks. Let's go with what you have and I'll try to stick some
of this rationale in the commit message.

Will

Re: [PATCH] arm64: Disable LLD linker ASSERT()s for the time being

Posted by Nathan Chancellor 6 months, 2 weeks ago

On Thu, May 29, 2025 at 09:35:08AM +0200, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
> 
> It turns out that the way LLD handles ASSERT()s in the linker script can
> result in spurious failures, so disable them for the newly introduced
> BSS symbol export checks.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/2094
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

You could add Fangrui's commit as a reference to why LLD 21 is needed
for this to work but it is probably not worth a v2 by itself.

https://github.com/llvm/llvm-project/commit/5859863bab7fb1cd98b6028293cba6ba25f7d514

> ---
>  arch/arm64/kernel/image-vars.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
> index c5266430284b..86f088a16147 100644
> --- a/arch/arm64/kernel/image-vars.h
> +++ b/arch/arm64/kernel/image-vars.h
> @@ -10,6 +10,10 @@
>  #error This file should only be included in vmlinux.lds.S
>  #endif
>  
> +#if defined(CONFIG_LD_IS_LLD) && CONFIG_LLD_VERSION < 210000
> +#define ASSERT(...)
> +#endif
> +
>  #define PI_EXPORT_SYM(sym)		\
>  	__PI_EXPORT_SYM(sym, __pi_ ## sym, Cannot export BSS symbol sym to startup code)
>  #define __PI_EXPORT_SYM(sym, pisym, msg)\
> @@ -142,4 +146,6 @@ KVM_NVHE_ALIAS(kvm_protected_mode_initialized);
>  _kernel_codesize = ABSOLUTE(__inittext_end - _text);
>  #endif
>  
> +#undef ASSERT
> +
>  #endif /* __ARM64_KERNEL_IMAGE_VARS_H */
> -- 
> 2.49.0.1238.gf8c92423fb-goog
> 
>