Re: [PATCH] integrity: avoid using __weak functions

On Fri, 2026-03-06 at 15:56 -0700, Nathan Chancellor wrote:
> On Fri, Mar 06, 2026 at 04:03:24PM +0100, Arnd Bergmann wrote:
> > From: Arnd Bergmann <[email protected]>
> > 
> > The security/integrity/secure_boot.c file containing only a __weak function
> > leads to a build failure with clang:
> > 
> > Cannot find symbol for section 2: .text.
> > security/integrity/secure_boot.o: failed
> > 
> > Moving the function into another file that has at least one non-__weak
> > symbol would solve this, but this is always fragile.
> > 
> > Avoid __weak definitions entirely and instead move the stub helper into
> > an asm-generic header that gets used by default on architectures that
> > do not provide their own version. This is consistent with how a lot
> > of other architecture specific functionality works, and is more reliable.
> > 
> > Fixes: a0f87ede3bf4 ("integrity: Make arch_ima_get_secureboot integrity-wide")
> > Signed-off-by: Arnd Bergmann <[email protected]>
> > ---
> > This is a larger change than I had hoped for.
> > 
> > If you prefer a different way to address the build failure, please
> > treat this as a Reported-by when you apply your own fix
> > ---
> >  arch/powerpc/include/asm/secure_boot.h        |  6 +++
> >  arch/powerpc/kernel/secure_boot.c             |  1 -
> >  arch/s390/include/asm/secure_boot.h           |  9 +++++
> >  include/asm-generic/Kbuild                    |  1 +
> >  include/asm-generic/secure_boot.h             | 37 +++++++++++++++++++
> >  include/linux/secure_boot.h                   |  8 +---
> >  security/integrity/Makefile                   |  2 +-
> >  .../integrity/platform_certs/load_powerpc.c   |  2 +-
> >  security/integrity/secure_boot.c              | 16 --------
> >  9 files changed, 56 insertions(+), 26 deletions(-)
> >  create mode 100644 arch/s390/include/asm/secure_boot.h
> >  create mode 100644 include/asm-generic/secure_boot.h
> >  delete mode 100644 security/integrity/secure_boot.c
> 
> Thanks, I noticed this as well. The version I came up with and have been
> locally testing is the following, which is a little bit more compact.

Thanks Arnd, Nathan. LGTM.  Nathan, could you send a patch with a proper patch
description.

Mimi

> 
>  arch/Kconfig                     |  3 +++
>  arch/powerpc/Kconfig             |  1 +
>  arch/s390/Kconfig                |  1 +
>  arch/s390/kernel/ipl.c           | 10 +++++-----
>  include/linux/secure_boot.h      |  4 ++++
>  security/integrity/Makefile      |  2 +-
>  security/integrity/secure_boot.c | 16 ----------------
>  7 files changed, 15 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 102ddbd4298e..a6d1c8cc1d64 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -1841,4 +1841,7 @@ config ARCH_WANTS_PRE_LINK_VMLINUX
>  config ARCH_HAS_CPU_ATTACK_VECTORS
>  	bool
>  
> +config HAVE_ARCH_GET_SECUREBOOT
> +	def_bool EFI
> +
>  endmenu
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index c28776660246..e76d6cf0c403 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -1062,6 +1062,7 @@ config PPC_SECURE_BOOT
>  	depends on IMA_ARCH_POLICY
>  	imply IMA_SECURE_AND_OR_TRUSTED_BOOT
>  	select PSERIES_PLPKS if PPC_PSERIES
> +	select HAVE_ARCH_GET_SECUREBOOT
>  	help
>  	  Systems with firmware secure boot enabled need to define security
>  	  policies to extend secure boot to the OS. This config allows a user
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index 24695ea29d5b..76f191dd208b 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -181,6 +181,7 @@ config S390
>  	select GENERIC_IOREMAP if PCI
>  	select HAVE_ALIGNED_STRUCT_PAGE
>  	select HAVE_ARCH_AUDITSYSCALL
> +	select HAVE_ARCH_GET_SECUREBOOT
>  	select HAVE_ARCH_JUMP_LABEL
>  	select HAVE_ARCH_JUMP_LABEL_RELATIVE
>  	select HAVE_ARCH_KASAN
> diff --git a/arch/s390/kernel/ipl.c b/arch/s390/kernel/ipl.c
> index 2d01a1713938..3c346b02ceb9 100644
> --- a/arch/s390/kernel/ipl.c
> +++ b/arch/s390/kernel/ipl.c
> @@ -2388,6 +2388,11 @@ void __no_stack_protector s390_reset_system(void)
>  	diag_amode31_ops.diag308_reset();
>  }
>  
> +bool arch_get_secureboot(void)
> +{
> +	return ipl_secure_flag;
> +}
> +
>  #ifdef CONFIG_KEXEC_FILE
>  
>  int ipl_report_add_component(struct ipl_report *report, struct kexec_buf *kbuf,
> @@ -2505,11 +2510,6 @@ void *ipl_report_finish(struct ipl_report *report)
>  	return buf;
>  }
>  
> -bool arch_get_secureboot(void)
> -{
> -	return ipl_secure_flag;
> -}
> -
>  int ipl_report_free(struct ipl_report *report)
>  {
>  	struct ipl_report_component *comp, *ncomp;
> diff --git a/include/linux/secure_boot.h b/include/linux/secure_boot.h
> index 3ded3f03655c..d17e92351567 100644
> --- a/include/linux/secure_boot.h
> +++ b/include/linux/secure_boot.h
> @@ -10,10 +10,14 @@
>  
>  #include <linux/types.h>
>  
> +#ifdef CONFIG_HAVE_ARCH_GET_SECUREBOOT
>  /*
>   * Returns true if the platform secure boot is enabled.
>   * Returns false if disabled or not supported.
>   */
>  bool arch_get_secureboot(void);
> +#else
> +static inline bool arch_get_secureboot(void) { return false; }
> +#endif
>  
>  #endif /* _LINUX_SECURE_BOOT_H */
> diff --git a/security/integrity/Makefile b/security/integrity/Makefile
> index 548665e2b702..45dfdedbdad4 100644
> --- a/security/integrity/Makefile
> +++ b/security/integrity/Makefile
> @@ -5,7 +5,7 @@
>  
>  obj-$(CONFIG_INTEGRITY) += integrity.o
>  
> -integrity-y := iint.o secure_boot.o
> +integrity-y := iint.o
>  integrity-$(CONFIG_INTEGRITY_AUDIT) += integrity_audit.o
>  integrity-$(CONFIG_INTEGRITY_SIGNATURE) += digsig.o
>  integrity-$(CONFIG_INTEGRITY_ASYMMETRIC_KEYS) += digsig_asymmetric.o
> diff --git a/security/integrity/secure_boot.c b/security/integrity/secure_boot.c
> deleted file mode 100644
> index fc2693c286f8..000000000000
> --- a/security/integrity/secure_boot.c
> +++ /dev/null
> @@ -1,16 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-only
> -/*
> - * Copyright (C) 2026 Red Hat, Inc. All Rights Reserved.
> - *
> - * Author: Coiby Xu <[email protected]>
> - */
> -#include <linux/secure_boot.h>
> -
> -/*
> - * Default weak implementation.
> - * Architectures that support secure boot must override this.
> - */
> -__weak bool arch_get_secureboot(void)
> -{
> -	return false;
> -}