diff mbox series

integrity: avoid using __weak functions

Message ID 20260306150421.270124-1-arnd@kernel.org (mailing list archive)
State New
Headers show
Series integrity: avoid using __weak functions | expand

Commit Message

Arnd Bergmann March 6, 2026, 3:03 p.m. UTC 
From: Arnd Bergmann <arnd@arndb.de>

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 <arnd@arndb.de>
---
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

Comments

Nathan Chancellor March 6, 2026, 10:56 p.m. UTC | #1 
On Fri, Mar 06, 2026 at 04:03:24PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> 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 <arnd@arndb.de>
> ---
> 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.

 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 <coxu@redhat.com>
- */
-#include <linux/secure_boot.h>
-
-/*
- * Default weak implementation.
- * Architectures that support secure boot must override this.
- */
-__weak bool arch_get_secureboot(void)
-{
-	return false;
-}
Arnd Bergmann March 6, 2026, 11:28 p.m. UTC | #2 
On Fri, Mar 6, 2026, at 23:56, Nathan Chancellor wrote:
> On Fri, Mar 06, 2026 at 04:03:24PM +0100, Arnd Bergmann wrote:
>
> 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.
>
>  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(-)
>

Right, your version looks good to me as well.

       Arnd
Mimi Zohar March 9, 2026, 1:01 a.m. UTC | #3 
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 <arnd@arndb.de>
> > 
> > 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 <arnd@arndb.de>
> > ---
> > 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 <coxu@redhat.com>
> - */
> -#include <linux/secure_boot.h>
> -
> -/*
> - * Default weak implementation.
> - * Architectures that support secure boot must override this.
> - */
> -__weak bool arch_get_secureboot(void)
> -{
> -	return false;
> -}
Nathan Chancellor March 9, 2026, 9:29 p.m. UTC | #4 
On Sun, Mar 08, 2026 at 09:01:58PM -0400, Mimi Zohar wrote:
> Thanks Arnd, Nathan. LGTM.  Nathan, could you send a patch with a proper patch
> description.

Done, thanks for the input!

  https://lore.kernel.org/20260309-integrity-drop-weak-arch-get-secureboot-v1-1-6460d5c4bb89@kernel.org/

Cheers,
Nathan
diff mbox series

Patch

diff --git a/arch/powerpc/include/asm/secure_boot.h b/arch/powerpc/include/asm/secure_boot.h
index a2ff556916c6..db72dcdf5bb3 100644
--- a/arch/powerpc/include/asm/secure_boot.h
+++ b/arch/powerpc/include/asm/secure_boot.h
@@ -10,11 +10,17 @@ 
 
 #ifdef CONFIG_PPC_SECURE_BOOT
 
+bool arch_get_secureboot(void);
 bool is_ppc_secureboot_enabled(void);
 bool is_ppc_trustedboot_enabled(void);
 
 #else
 
+static inline bool arch_get_secureboot(void)
+{
+	return false;
+}
+
 static inline bool is_ppc_secureboot_enabled(void)
 {
 	return false;
diff --git a/arch/powerpc/kernel/secure_boot.c b/arch/powerpc/kernel/secure_boot.c
index 28436c1599e0..e3ea46124180 100644
--- a/arch/powerpc/kernel/secure_boot.c
+++ b/arch/powerpc/kernel/secure_boot.c
@@ -7,7 +7,6 @@ 
 #include <linux/of.h>
 #include <linux/secure_boot.h>
 #include <linux/string_choices.h>
-#include <asm/secure_boot.h>
 
 static struct device_node *get_ppc_fw_sb_node(void)
 {
diff --git a/arch/s390/include/asm/secure_boot.h b/arch/s390/include/asm/secure_boot.h
new file mode 100644
index 000000000000..4086fdfb9e5c
--- /dev/null
+++ b/arch/s390/include/asm/secure_boot.h
@@ -0,0 +1,9 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_S390_SECURE_BOOT_H
+#define _ASM_S390_SECURE_BOOT_H
+
+#include <linux/types.h
+
+bool arch_get_secureboot(void);
+
+#endif
diff --git a/include/asm-generic/Kbuild b/include/asm-generic/Kbuild
index 0f97f7b594c3..8c0a499141fb 100644
--- a/include/asm-generic/Kbuild
+++ b/include/asm-generic/Kbuild
@@ -51,6 +51,7 @@  mandatory-y += rqspinlock.h
 mandatory-y += runtime-const.h
 mandatory-y += rwonce.h
 mandatory-y += sections.h
+mandatory-y += secure_boot.h
 mandatory-y += serial.h
 mandatory-y += shmparam.h
 mandatory-y += simd.h
diff --git a/include/asm-generic/secure_boot.h b/include/asm-generic/secure_boot.h
new file mode 100644
index 000000000000..08d8e294576c
--- /dev/null
+++ b/include/asm-generic/secure_boot.h
@@ -0,0 +1,37 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2026 Red Hat, Inc. All Rights Reserved.
+ *
+ * Author: Coiby Xu <coxu@redhat.com>
+ */
+#ifndef _ASM_SECURE_BOOT_H
+#define _ASM_SECURE_BOOT_H
+
+
+#include <linux/types.h>
+
+#ifdef CONFIG_EFI
+
+/*
+ * Default implementation.
+ * Architectures that support secure boot must override this.
+ *
+ * Returns true if the platform secure boot is enabled.
+ * Returns false if disabled or not supported.
+ */
+bool arch_get_secureboot(void);
+
+#else
+
+/*
+ * Default implementation.
+ * Architectures that support secure boot must override this.
+ */
+static inline bool arch_get_secureboot(void)
+{
+	return false;
+}
+
+#endif
+
+#endif
diff --git a/include/linux/secure_boot.h b/include/linux/secure_boot.h
index 3ded3f03655c..9ddfbe109b1d 100644
--- a/include/linux/secure_boot.h
+++ b/include/linux/secure_boot.h
@@ -8,12 +8,6 @@ 
 #ifndef _LINUX_SECURE_BOOT_H
 #define _LINUX_SECURE_BOOT_H
 
-#include <linux/types.h>
-
-/*
- * Returns true if the platform secure boot is enabled.
- * Returns false if disabled or not supported.
- */
-bool arch_get_secureboot(void);
+#include <asm/secure_boot.h>
 
 #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/platform_certs/load_powerpc.c b/security/integrity/platform_certs/load_powerpc.c
index 714c961a00f5..ab74e947a8bc 100644
--- a/security/integrity/platform_certs/load_powerpc.c
+++ b/security/integrity/platform_certs/load_powerpc.c
@@ -10,7 +10,7 @@ 
 #include <linux/cred.h>
 #include <linux/err.h>
 #include <linux/slab.h>
-#include <asm/secure_boot.h>
+#include <linux/secure_boot.h>
 #include <asm/secvar.h>
 #include "keyring_handler.h"
 #include "../integrity.h"
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 <coxu@redhat.com>
- */
-#include <linux/secure_boot.h>
-
-/*
- * Default weak implementation.
- * Architectures that support secure boot must override this.
- */
-__weak bool arch_get_secureboot(void)
-{
-	return false;
-}