stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 6.1&6.6 0/3] kbuild: Avoid weak external linkage where possible
@ 2024-12-06  8:58 Huacai Chen
  2024-12-06  8:58 ` [PATCH 6.1&6.6 1/3] kallsyms: Avoid weak references for kallsyms symbols Huacai Chen
                   ` (3 more replies)
  0 siblings, 4 replies; 20+ messages in thread
From: Huacai Chen @ 2024-12-06  8:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Sasha Levin, Huacai Chen
  Cc: Xuerui Wang, Masahiro Yamada, Nathan Chancellor, Nick Desaulniers,
	Nicolas Schier, stable, linux-kbuild, linux-kernel, loongarch,
	Huacai Chen, Ard Biesheuvel

Backport this series to 6.1&6.6 because LoongArch gets build errors with
latest binutils which has commit 599df6e2db17d1c4 ("ld, LoongArch: print
error about linking without -fPIC or -fPIE flag in more detail").

  CC      .vmlinux.export.o
  UPD     include/generated/utsversion.h
  CC      init/version-timestamp.o
  LD      .tmp_vmlinux.kallsyms1
loongarch64-unknown-linux-gnu-ld: kernel/kallsyms.o:(.text+0): relocation R_LARCH_PCALA_HI20 against `kallsyms_markers` can not be used when making a PIE object; recompile with -fPIE
loongarch64-unknown-linux-gnu-ld: kernel/crash_core.o:(.init.text+0x984): relocation R_LARCH_PCALA_HI20 against `kallsyms_names` can not be used when making a PIE object; recompile with -fPIE
loongarch64-unknown-linux-gnu-ld: kernel/bpf/btf.o:(.text+0xcc7c): relocation R_LARCH_PCALA_HI20 against `__start_BTF` can not be used when making a PIE object; recompile with -fPIE
loongarch64-unknown-linux-gnu-ld: BFD (GNU Binutils) 2.43.50.20241126 assertion fail ../../bfd/elfnn-loongarch.c:2673

In theory 5.10&5.15 also need this, but since LoongArch get upstream at
5.19, so I just ignore them because there is no error report about other
archs now.

Weak external linkage is intended for cases where a symbol reference
can remain unsatisfied in the final link. Taking the address of such a
symbol should yield NULL if the reference was not satisfied.

Given that ordinary RIP or PC relative references cannot produce NULL,
some kind of indirection is always needed in such cases, and in position
independent code, this results in a GOT entry. In ordinary code, it is
arch specific but amounts to the same thing.

While unavoidable in some cases, weak references are currently also used
to declare symbols that are always defined in the final link, but not in
the first linker pass. This means we end up with worse codegen for no
good reason. So let's clean this up, by providing preliminary
definitions that are only used as a fallback.

Ard Biesheuvel (3):
  kallsyms: Avoid weak references for kallsyms symbols
  vmlinux: Avoid weak reference to notes section
  btf: Avoid weak external references

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
---
 include/asm-generic/vmlinux.lds.h | 28 ++++++++++++++++++
 kernel/bpf/btf.c                  |  7 +++--
 kernel/bpf/sysfs_btf.c            |  6 ++--
 kernel/kallsyms.c                 |  6 ----
 kernel/kallsyms_internal.h        | 30 ++++++++------------
 kernel/ksysfs.c                   |  4 +--
 lib/buildid.c                     |  4 +--
 7 files changed, 52 insertions(+), 33 deletions(-)
---
2.27.0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 6.1&6.6 1/3] kallsyms: Avoid weak references for kallsyms symbols
  2024-12-06  8:58 [PATCH 6.1&6.6 0/3] kbuild: Avoid weak external linkage where possible Huacai Chen
@ 2024-12-06  8:58 ` Huacai Chen
  2024-12-06 17:11   ` Sasha Levin
  2025-02-07 22:50   ` Sasha Levin
  2024-12-06  8:58 ` [PATCH 6.1&6.6 2/3] vmlinux: Avoid weak reference to notes section Huacai Chen
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 20+ messages in thread
From: Huacai Chen @ 2024-12-06  8:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Sasha Levin, Huacai Chen
  Cc: Xuerui Wang, Masahiro Yamada, Nathan Chancellor, Nick Desaulniers,
	Nicolas Schier, stable, linux-kbuild, linux-kernel, loongarch,
	Ard Biesheuvel, Kees Cook, Arnd Bergmann, Huacai Chen

From: Ard Biesheuvel <ardb@kernel.org>

[ Upstream commit 951bcae6c5a0bfaa55b27c5f16178204988f0379 ]

kallsyms is a directory of all the symbols in the vmlinux binary, and so
creating it is somewhat of a chicken-and-egg problem, as its non-zero
size affects the layout of the binary, and therefore the values of the
symbols.

For this reason, the kernel is linked more than once, and the first pass
does not include any kallsyms data at all. For the linker to accept
this, the symbol declarations describing the kallsyms metadata are
emitted as having weak linkage, so they can remain unsatisfied. During
the subsequent passes, the weak references are satisfied by the kallsyms
metadata that was constructed based on information gathered from the
preceding passes.

Weak references lead to somewhat worse codegen, because taking their
address may need to produce NULL (if the reference was unsatisfied), and
this is not usually supported by RIP or PC relative symbol references.

Given that these references are ultimately always satisfied in the final
link, let's drop the weak annotation, and instead, provide fallback
definitions in the linker script that are only emitted if an unsatisfied
reference exists.

While at it, drop the FRV specific annotation that these symbols reside
in .rodata - FRV is long gone.

Tested-by: Nick Desaulniers <ndesaulniers@google.com> # Boot
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Link: https://lkml.kernel.org/r/20230504174320.3930345-1-ardb%40kernel.org
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
---
 include/asm-generic/vmlinux.lds.h | 19 +++++++++++++++++++
 kernel/kallsyms.c                 |  6 ------
 kernel/kallsyms_internal.h        | 30 ++++++++++++------------------
 3 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 63029bc7c9dd..64a80059d7fa 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -447,11 +447,30 @@
 #endif
 #endif
 
+/*
+ * Some symbol definitions will not exist yet during the first pass of the
+ * link, but are guaranteed to exist in the final link. Provide preliminary
+ * definitions that will be superseded in the final link to avoid having to
+ * rely on weak external linkage, which requires a GOT when used in position
+ * independent code.
+ */
+#define PRELIMINARY_SYMBOL_DEFINITIONS					\
+	PROVIDE(kallsyms_addresses = .);				\
+	PROVIDE(kallsyms_offsets = .);					\
+	PROVIDE(kallsyms_names = .);					\
+	PROVIDE(kallsyms_num_syms = .);					\
+	PROVIDE(kallsyms_relative_base = .);				\
+	PROVIDE(kallsyms_token_table = .);				\
+	PROVIDE(kallsyms_token_index = .);				\
+	PROVIDE(kallsyms_markers = .);					\
+	PROVIDE(kallsyms_seqs_of_names = .);
+
 /*
  * Read only Data
  */
 #define RO_DATA(align)							\
 	. = ALIGN((align));						\
+	PRELIMINARY_SYMBOL_DEFINITIONS					\
 	.rodata           : AT(ADDR(.rodata) - LOAD_OFFSET) {		\
 		__start_rodata = .;					\
 		*(.rodata) *(.rodata.*)					\
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 18edd57b5fe8..22ea19a36e6e 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -325,12 +325,6 @@ static unsigned long get_symbol_pos(unsigned long addr,
 	unsigned long symbol_start = 0, symbol_end = 0;
 	unsigned long i, low, high, mid;
 
-	/* This kernel should never had been booted. */
-	if (!IS_ENABLED(CONFIG_KALLSYMS_BASE_RELATIVE))
-		BUG_ON(!kallsyms_addresses);
-	else
-		BUG_ON(!kallsyms_offsets);
-
 	/* Do a binary search on the sorted kallsyms_addresses array. */
 	low = 0;
 	high = kallsyms_num_syms;
diff --git a/kernel/kallsyms_internal.h b/kernel/kallsyms_internal.h
index 27fabdcc40f5..85480274fc8f 100644
--- a/kernel/kallsyms_internal.h
+++ b/kernel/kallsyms_internal.h
@@ -5,27 +5,21 @@
 #include <linux/types.h>
 
 /*
- * These will be re-linked against their real values
- * during the second link stage.
+ * These will be re-linked against their real values during the second link
+ * stage. Preliminary values must be provided in the linker script using the
+ * PROVIDE() directive so that the first link stage can complete successfully.
  */
-extern const unsigned long kallsyms_addresses[] __weak;
-extern const int kallsyms_offsets[] __weak;
-extern const u8 kallsyms_names[] __weak;
+extern const unsigned long kallsyms_addresses[];
+extern const int kallsyms_offsets[];
+extern const u8 kallsyms_names[];
 
-/*
- * Tell the compiler that the count isn't in the small data section if the arch
- * has one (eg: FRV).
- */
-extern const unsigned int kallsyms_num_syms
-__section(".rodata") __attribute__((weak));
-
-extern const unsigned long kallsyms_relative_base
-__section(".rodata") __attribute__((weak));
+extern const unsigned int kallsyms_num_syms;
+extern const unsigned long kallsyms_relative_base;
 
-extern const char kallsyms_token_table[] __weak;
-extern const u16 kallsyms_token_index[] __weak;
+extern const char kallsyms_token_table[];
+extern const u16 kallsyms_token_index[];
 
-extern const unsigned int kallsyms_markers[] __weak;
-extern const u8 kallsyms_seqs_of_names[] __weak;
+extern const unsigned int kallsyms_markers[];
+extern const u8 kallsyms_seqs_of_names[];
 
 #endif // LINUX_KALLSYMS_INTERNAL_H_
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 6.1&6.6 2/3] vmlinux: Avoid weak reference to notes section
  2024-12-06  8:58 [PATCH 6.1&6.6 0/3] kbuild: Avoid weak external linkage where possible Huacai Chen
  2024-12-06  8:58 ` [PATCH 6.1&6.6 1/3] kallsyms: Avoid weak references for kallsyms symbols Huacai Chen
@ 2024-12-06  8:58 ` Huacai Chen
  2024-12-06 17:11   ` Sasha Levin
  2024-12-06  8:58 ` [PATCH 6.1&6.6 3/3] btf: Avoid weak external references Huacai Chen
  2024-12-06 13:04 ` [PATCH 6.1&6.6 0/3] kbuild: Avoid weak external linkage where possible Greg Kroah-Hartman
  3 siblings, 1 reply; 20+ messages in thread
From: Huacai Chen @ 2024-12-06  8:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Sasha Levin, Huacai Chen
  Cc: Xuerui Wang, Masahiro Yamada, Nathan Chancellor, Nick Desaulniers,
	Nicolas Schier, stable, linux-kbuild, linux-kernel, loongarch,
	Ard Biesheuvel, Arnd Bergmann, Huacai Chen

From: Ard Biesheuvel <ardb@kernel.org>

[ Upstream commit 377d9095117c084b835e38c020faf5a78e386f01 ]

Weak references are references that are permitted to remain unsatisfied
in the final link. This means they cannot be implemented using place
relative relocations, resulting in GOT entries when using position
independent code generation.

The notes section should always exist, so the weak annotations can be
omitted.

Acked-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
---
 kernel/ksysfs.c | 4 ++--
 lib/buildid.c   | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/ksysfs.c b/kernel/ksysfs.c
index 1d4bc493b2f4..347beb763c59 100644
--- a/kernel/ksysfs.c
+++ b/kernel/ksysfs.c
@@ -226,8 +226,8 @@ KERNEL_ATTR_RW(rcu_normal);
 /*
  * Make /sys/kernel/notes give the raw contents of our kernel .notes section.
  */
-extern const void __start_notes __weak;
-extern const void __stop_notes __weak;
+extern const void __start_notes;
+extern const void __stop_notes;
 #define	notes_size (&__stop_notes - &__start_notes)
 
 static ssize_t notes_read(struct file *filp, struct kobject *kobj,
diff --git a/lib/buildid.c b/lib/buildid.c
index 9fc46366597e..ebfc8a28e46a 100644
--- a/lib/buildid.c
+++ b/lib/buildid.c
@@ -208,8 +208,8 @@ unsigned char vmlinux_build_id[BUILD_ID_SIZE_MAX] __ro_after_init;
  */
 void __init init_vmlinux_build_id(void)
 {
-	extern const void __start_notes __weak;
-	extern const void __stop_notes __weak;
+	extern const void __start_notes;
+	extern const void __stop_notes;
 	unsigned int size = &__stop_notes - &__start_notes;
 
 	build_id_parse_buf(&__start_notes, vmlinux_build_id, size);
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 6.1&6.6 3/3] btf: Avoid weak external references
  2024-12-06  8:58 [PATCH 6.1&6.6 0/3] kbuild: Avoid weak external linkage where possible Huacai Chen
  2024-12-06  8:58 ` [PATCH 6.1&6.6 1/3] kallsyms: Avoid weak references for kallsyms symbols Huacai Chen
  2024-12-06  8:58 ` [PATCH 6.1&6.6 2/3] vmlinux: Avoid weak reference to notes section Huacai Chen
@ 2024-12-06  8:58 ` Huacai Chen
  2024-12-06 17:11   ` Sasha Levin
  2024-12-06 13:04 ` [PATCH 6.1&6.6 0/3] kbuild: Avoid weak external linkage where possible Greg Kroah-Hartman
  3 siblings, 1 reply; 20+ messages in thread
From: Huacai Chen @ 2024-12-06  8:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Sasha Levin, Huacai Chen
  Cc: Xuerui Wang, Masahiro Yamada, Nathan Chancellor, Nick Desaulniers,
	Nicolas Schier, stable, linux-kbuild, linux-kernel, loongarch,
	Ard Biesheuvel, Daniel Borkmann, Andrii Nakryiko, Arnd Bergmann,
	Jiri Olsa, Huacai Chen

From: Ard Biesheuvel <ardb@kernel.org>

[ Upstream commit fc5eb4a84e4c063e75a6a6e92308e9533c0f19b5 ]

If the BTF code is enabled in the build configuration, the start/stop
BTF markers are guaranteed to exist. Only when CONFIG_DEBUG_INFO_BTF=n,
the references in btf_parse_vmlinux() will remain unsatisfied, relying
on the weak linkage of the external references to avoid breaking the
build.

Avoid GOT based relocations to these markers in the final executable by
dropping the weak attribute and instead, make btf_parse_vmlinux() return
ERR_PTR(-ENOENT) directly if CONFIG_DEBUG_INFO_BTF is not enabled to
begin with.  The compiler will drop any subsequent references to
__start_BTF and __stop_BTF in that case, allowing the link to succeed.

Note that Clang will notice that taking the address of __start_BTF can
no longer yield NULL, so testing for that condition becomes unnecessary.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Link: https://lore.kernel.org/bpf/20240415162041.2491523-8-ardb+git@google.com
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
---
 kernel/bpf/btf.c       | 7 +++++--
 kernel/bpf/sysfs_btf.c | 6 +++---
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index c8828016a66f..f231e2a273a1 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -5574,8 +5574,8 @@ static struct btf *btf_parse(const union bpf_attr *attr, bpfptr_t uattr, u32 uat
 	return ERR_PTR(err);
 }
 
-extern char __weak __start_BTF[];
-extern char __weak __stop_BTF[];
+extern char __start_BTF[];
+extern char __stop_BTF[];
 extern struct btf *btf_vmlinux;
 
 #define BPF_MAP_TYPE(_id, _ops)
@@ -5724,6 +5724,9 @@ struct btf *btf_parse_vmlinux(void)
 	struct btf *btf = NULL;
 	int err;
 
+	if (!IS_ENABLED(CONFIG_DEBUG_INFO_BTF))
+		return ERR_PTR(-ENOENT);
+
 	env = kzalloc(sizeof(*env), GFP_KERNEL | __GFP_NOWARN);
 	if (!env)
 		return ERR_PTR(-ENOMEM);
diff --git a/kernel/bpf/sysfs_btf.c b/kernel/bpf/sysfs_btf.c
index ef6911aee3bb..fedb54c94cdb 100644
--- a/kernel/bpf/sysfs_btf.c
+++ b/kernel/bpf/sysfs_btf.c
@@ -9,8 +9,8 @@
 #include <linux/sysfs.h>
 
 /* See scripts/link-vmlinux.sh, gen_btf() func for details */
-extern char __weak __start_BTF[];
-extern char __weak __stop_BTF[];
+extern char __start_BTF[];
+extern char __stop_BTF[];
 
 static ssize_t
 btf_vmlinux_read(struct file *file, struct kobject *kobj,
@@ -32,7 +32,7 @@ static int __init btf_vmlinux_init(void)
 {
 	bin_attr_btf_vmlinux.size = __stop_BTF - __start_BTF;
 
-	if (!__start_BTF || bin_attr_btf_vmlinux.size == 0)
+	if (bin_attr_btf_vmlinux.size == 0)
 		return 0;
 
 	btf_kobj = kobject_create_and_add("btf", kernel_kobj);
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 6.1&6.6 0/3] kbuild: Avoid weak external linkage where possible
  2024-12-06  8:58 [PATCH 6.1&6.6 0/3] kbuild: Avoid weak external linkage where possible Huacai Chen
                   ` (2 preceding siblings ...)
  2024-12-06  8:58 ` [PATCH 6.1&6.6 3/3] btf: Avoid weak external references Huacai Chen
@ 2024-12-06 13:04 ` Greg Kroah-Hartman
  2024-12-07  9:21   ` Huacai Chen
  2025-02-06  8:37   ` WangYuli
  3 siblings, 2 replies; 20+ messages in thread
From: Greg Kroah-Hartman @ 2024-12-06 13:04 UTC (permalink / raw)
  To: Huacai Chen
  Cc: Sasha Levin, Huacai Chen, Xuerui Wang, Masahiro Yamada,
	Nathan Chancellor, Nick Desaulniers, Nicolas Schier, stable,
	linux-kbuild, linux-kernel, loongarch, Ard Biesheuvel

On Fri, Dec 06, 2024 at 04:58:07PM +0800, Huacai Chen wrote:
> Backport this series to 6.1&6.6 because LoongArch gets build errors with
> latest binutils which has commit 599df6e2db17d1c4 ("ld, LoongArch: print
> error about linking without -fPIC or -fPIE flag in more detail").
> 
>   CC      .vmlinux.export.o
>   UPD     include/generated/utsversion.h
>   CC      init/version-timestamp.o
>   LD      .tmp_vmlinux.kallsyms1
> loongarch64-unknown-linux-gnu-ld: kernel/kallsyms.o:(.text+0): relocation R_LARCH_PCALA_HI20 against `kallsyms_markers` can not be used when making a PIE object; recompile with -fPIE
> loongarch64-unknown-linux-gnu-ld: kernel/crash_core.o:(.init.text+0x984): relocation R_LARCH_PCALA_HI20 against `kallsyms_names` can not be used when making a PIE object; recompile with -fPIE
> loongarch64-unknown-linux-gnu-ld: kernel/bpf/btf.o:(.text+0xcc7c): relocation R_LARCH_PCALA_HI20 against `__start_BTF` can not be used when making a PIE object; recompile with -fPIE
> loongarch64-unknown-linux-gnu-ld: BFD (GNU Binutils) 2.43.50.20241126 assertion fail ../../bfd/elfnn-loongarch.c:2673
> 
> In theory 5.10&5.15 also need this, but since LoongArch get upstream at
> 5.19, so I just ignore them because there is no error report about other
> archs now.

Odd, why doesn't this affect other arches as well using new binutils?  I
hate to have to backport all of this just for one arch, as that feels
odd.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 6.1&6.6 1/3] kallsyms: Avoid weak references for kallsyms symbols
  2024-12-06  8:58 ` [PATCH 6.1&6.6 1/3] kallsyms: Avoid weak references for kallsyms symbols Huacai Chen
@ 2024-12-06 17:11   ` Sasha Levin
  2025-02-07 22:50   ` Sasha Levin
  1 sibling, 0 replies; 20+ messages in thread
From: Sasha Levin @ 2024-12-06 17:11 UTC (permalink / raw)
  To: stable; +Cc: Huacai Chen, Sasha Levin

[ Sasha's backport helper bot ]

Hi,

The upstream commit SHA1 provided is correct: 951bcae6c5a0bfaa55b27c5f16178204988f0379

WARNING: Author mismatch between patch and upstream commit:
Backport author: Huacai Chen <chenhuacai@loongson.cn>
Commit author: Ard Biesheuvel <ardb@kernel.org>


Status in newer kernel trees:
6.12.y | Present (exact SHA1)
6.6.y | Not found

Note: The patch differs from the upstream commit:
---
1:  951bcae6c5a0b ! 1:  a89be9ce6914a kallsyms: Avoid weak references for kallsyms symbols
    @@ Metadata
      ## Commit message ##
         kallsyms: Avoid weak references for kallsyms symbols
     
    +    [ Upstream commit 951bcae6c5a0bfaa55b27c5f16178204988f0379 ]
    +
         kallsyms is a directory of all the symbols in the vmlinux binary, and so
         creating it is somewhat of a chicken-and-egg problem, as its non-zero
         size affects the layout of the binary, and therefore the values of the
    @@ Commit message
         Link: https://lkml.kernel.org/r/20230504174320.3930345-1-ardb%40kernel.org
         Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
         Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
    +    Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
     
      ## include/asm-generic/vmlinux.lds.h ##
     @@
---

Results of testing on various branches:

| Branch                    | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-6.6.y        |  Success    |  Success   |
| stable/linux-6.1.y        |  Success    |  Success   |

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 6.1&6.6 3/3] btf: Avoid weak external references
  2024-12-06  8:58 ` [PATCH 6.1&6.6 3/3] btf: Avoid weak external references Huacai Chen
@ 2024-12-06 17:11   ` Sasha Levin
  0 siblings, 0 replies; 20+ messages in thread
From: Sasha Levin @ 2024-12-06 17:11 UTC (permalink / raw)
  To: stable; +Cc: Huacai Chen, Sasha Levin

[ Sasha's backport helper bot ]

Hi,

The upstream commit SHA1 provided is correct: fc5eb4a84e4c063e75a6a6e92308e9533c0f19b5

WARNING: Author mismatch between patch and upstream commit:
Backport author: Huacai Chen <chenhuacai@loongson.cn>
Commit author: Ard Biesheuvel <ardb@kernel.org>


Status in newer kernel trees:
6.12.y | Present (exact SHA1)
6.6.y | Not found

Note: The patch differs from the upstream commit:
---
1:  fc5eb4a84e4c0 ! 1:  0637e4a3f4e0a btf: Avoid weak external references
    @@ Metadata
      ## Commit message ##
         btf: Avoid weak external references
     
    +    [ Upstream commit fc5eb4a84e4c063e75a6a6e92308e9533c0f19b5 ]
    +
         If the BTF code is enabled in the build configuration, the start/stop
         BTF markers are guaranteed to exist. Only when CONFIG_DEBUG_INFO_BTF=n,
         the references in btf_parse_vmlinux() will remain unsatisfied, relying
    @@ Commit message
         Acked-by: Arnd Bergmann <arnd@arndb.de>
         Acked-by: Jiri Olsa <jolsa@kernel.org>
         Link: https://lore.kernel.org/bpf/20240415162041.2491523-8-ardb+git@google.com
    +    Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
     
      ## kernel/bpf/btf.c ##
     @@ kernel/bpf/btf.c: static struct btf *btf_parse(const union bpf_attr *attr, bpfptr_t uattr, u32 uat
---

Results of testing on various branches:

| Branch                    | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-6.6.y        |  Success    |  Success   |
| stable/linux-6.1.y        |  Success    |  Success   |

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 6.1&6.6 2/3] vmlinux: Avoid weak reference to notes section
  2024-12-06  8:58 ` [PATCH 6.1&6.6 2/3] vmlinux: Avoid weak reference to notes section Huacai Chen
@ 2024-12-06 17:11   ` Sasha Levin
  0 siblings, 0 replies; 20+ messages in thread
From: Sasha Levin @ 2024-12-06 17:11 UTC (permalink / raw)
  To: stable; +Cc: Huacai Chen, Sasha Levin

[ Sasha's backport helper bot ]

Hi,

The upstream commit SHA1 provided is correct: 377d9095117c084b835e38c020faf5a78e386f01

WARNING: Author mismatch between patch and upstream commit:
Backport author: Huacai Chen <chenhuacai@loongson.cn>
Commit author: Ard Biesheuvel <ardb@kernel.org>


Status in newer kernel trees:
6.12.y | Present (exact SHA1)
6.6.y | Not found

Note: The patch differs from the upstream commit:
---
1:  377d9095117c0 ! 1:  c735ac7f41f90 vmlinux: Avoid weak reference to notes section
    @@ Metadata
      ## Commit message ##
         vmlinux: Avoid weak reference to notes section
     
    +    [ Upstream commit 377d9095117c084b835e38c020faf5a78e386f01 ]
    +
         Weak references are references that are permitted to remain unsatisfied
         in the final link. This means they cannot be implemented using place
         relative relocations, resulting in GOT entries when using position
    @@ Commit message
         Acked-by: Arnd Bergmann <arnd@arndb.de>
         Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
         Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
    +    Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
     
      ## kernel/ksysfs.c ##
     @@ kernel/ksysfs.c: KERNEL_ATTR_RW(rcu_normal);
---

Results of testing on various branches:

| Branch                    | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-6.6.y        |  Success    |  Success   |
| stable/linux-6.1.y        |  Success    |  Success   |

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 6.1&6.6 0/3] kbuild: Avoid weak external linkage where possible
  2024-12-06 13:04 ` [PATCH 6.1&6.6 0/3] kbuild: Avoid weak external linkage where possible Greg Kroah-Hartman
@ 2024-12-07  9:21   ` Huacai Chen
  2024-12-07  9:32     ` Greg Kroah-Hartman
  2025-02-06  8:37   ` WangYuli
  1 sibling, 1 reply; 20+ messages in thread
From: Huacai Chen @ 2024-12-07  9:21 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Huacai Chen, Sasha Levin, Xuerui Wang, Masahiro Yamada,
	Nathan Chancellor, Nick Desaulniers, Nicolas Schier, stable,
	linux-kbuild, linux-kernel, loongarch, Ard Biesheuvel

Hi, Greg,

On Fri, Dec 6, 2024 at 9:04 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Fri, Dec 06, 2024 at 04:58:07PM +0800, Huacai Chen wrote:
> > Backport this series to 6.1&6.6 because LoongArch gets build errors with
> > latest binutils which has commit 599df6e2db17d1c4 ("ld, LoongArch: print
> > error about linking without -fPIC or -fPIE flag in more detail").
> >
> >   CC      .vmlinux.export.o
> >   UPD     include/generated/utsversion.h
> >   CC      init/version-timestamp.o
> >   LD      .tmp_vmlinux.kallsyms1
> > loongarch64-unknown-linux-gnu-ld: kernel/kallsyms.o:(.text+0): relocation R_LARCH_PCALA_HI20 against `kallsyms_markers` can not be used when making a PIE object; recompile with -fPIE
> > loongarch64-unknown-linux-gnu-ld: kernel/crash_core.o:(.init.text+0x984): relocation R_LARCH_PCALA_HI20 against `kallsyms_names` can not be used when making a PIE object; recompile with -fPIE
> > loongarch64-unknown-linux-gnu-ld: kernel/bpf/btf.o:(.text+0xcc7c): relocation R_LARCH_PCALA_HI20 against `__start_BTF` can not be used when making a PIE object; recompile with -fPIE
> > loongarch64-unknown-linux-gnu-ld: BFD (GNU Binutils) 2.43.50.20241126 assertion fail ../../bfd/elfnn-loongarch.c:2673
> >
> > In theory 5.10&5.15 also need this, but since LoongArch get upstream at
> > 5.19, so I just ignore them because there is no error report about other
> > archs now.
>
> Odd, why doesn't this affect other arches as well using new binutils?  I
> hate to have to backport all of this just for one arch, as that feels
> odd.
The related binutils commit is only for LoongArch, so build errors
only occured on LoongArch. I don't know why other archs have no
problem exactly, but may be related to their CFLAGS (for example, if
we disable CONFIG_RELOCATABLE, LoongArch also has no build errors
because CFLAGS changes).

On the other hand, Ard's original patches are not for LoongArch only,
so I think backport to stable branches is also not for LoongArch only.

Huacai

>
> thanks,
>
> greg k-h

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 6.1&6.6 0/3] kbuild: Avoid weak external linkage where possible
  2024-12-07  9:21   ` Huacai Chen
@ 2024-12-07  9:32     ` Greg Kroah-Hartman
  2024-12-07 10:46       ` Xi Ruoyao
  0 siblings, 1 reply; 20+ messages in thread
From: Greg Kroah-Hartman @ 2024-12-07  9:32 UTC (permalink / raw)
  To: Huacai Chen
  Cc: Huacai Chen, Sasha Levin, Xuerui Wang, Masahiro Yamada,
	Nathan Chancellor, Nick Desaulniers, Nicolas Schier, stable,
	linux-kbuild, linux-kernel, loongarch, Ard Biesheuvel

On Sat, Dec 07, 2024 at 05:21:00PM +0800, Huacai Chen wrote:
> Hi, Greg,
> 
> On Fri, Dec 6, 2024 at 9:04 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Fri, Dec 06, 2024 at 04:58:07PM +0800, Huacai Chen wrote:
> > > Backport this series to 6.1&6.6 because LoongArch gets build errors with
> > > latest binutils which has commit 599df6e2db17d1c4 ("ld, LoongArch: print
> > > error about linking without -fPIC or -fPIE flag in more detail").
> > >
> > >   CC      .vmlinux.export.o
> > >   UPD     include/generated/utsversion.h
> > >   CC      init/version-timestamp.o
> > >   LD      .tmp_vmlinux.kallsyms1
> > > loongarch64-unknown-linux-gnu-ld: kernel/kallsyms.o:(.text+0): relocation R_LARCH_PCALA_HI20 against `kallsyms_markers` can not be used when making a PIE object; recompile with -fPIE
> > > loongarch64-unknown-linux-gnu-ld: kernel/crash_core.o:(.init.text+0x984): relocation R_LARCH_PCALA_HI20 against `kallsyms_names` can not be used when making a PIE object; recompile with -fPIE
> > > loongarch64-unknown-linux-gnu-ld: kernel/bpf/btf.o:(.text+0xcc7c): relocation R_LARCH_PCALA_HI20 against `__start_BTF` can not be used when making a PIE object; recompile with -fPIE
> > > loongarch64-unknown-linux-gnu-ld: BFD (GNU Binutils) 2.43.50.20241126 assertion fail ../../bfd/elfnn-loongarch.c:2673
> > >
> > > In theory 5.10&5.15 also need this, but since LoongArch get upstream at
> > > 5.19, so I just ignore them because there is no error report about other
> > > archs now.
> >
> > Odd, why doesn't this affect other arches as well using new binutils?  I
> > hate to have to backport all of this just for one arch, as that feels
> > odd.
> The related binutils commit is only for LoongArch, so build errors
> only occured on LoongArch. I don't know why other archs have no
> problem exactly, but may be related to their CFLAGS (for example, if
> we disable CONFIG_RELOCATABLE, LoongArch also has no build errors
> because CFLAGS changes).

does LoongArch depend on that option?  What happens if it is enabled for
other arches?  Why doesn't it break them?

> On the other hand, Ard's original patches are not for LoongArch only,
> so I think backport to stable branches is also not for LoongArch only.

Maybe Ard can answer that.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 6.1&6.6 0/3] kbuild: Avoid weak external linkage where possible
  2024-12-07  9:32     ` Greg Kroah-Hartman
@ 2024-12-07 10:46       ` Xi Ruoyao
  2024-12-09  8:31         ` Ard Biesheuvel
  0 siblings, 1 reply; 20+ messages in thread
From: Xi Ruoyao @ 2024-12-07 10:46 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Huacai Chen
  Cc: Huacai Chen, Sasha Levin, Xuerui Wang, Masahiro Yamada,
	Nathan Chancellor, Nick Desaulniers, Nicolas Schier, stable,
	linux-kbuild, linux-kernel, loongarch, Ard Biesheuvel

On Sat, 2024-12-07 at 10:32 +0100, Greg Kroah-Hartman wrote:
> On Sat, Dec 07, 2024 at 05:21:00PM +0800, Huacai Chen wrote:
> > Hi, Greg,
> > 
> > On Fri, Dec 6, 2024 at 9:04 PM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > > 
> > > On Fri, Dec 06, 2024 at 04:58:07PM +0800, Huacai Chen wrote:
> > > > Backport this series to 6.1&6.6 because LoongArch gets build errors with
> > > > latest binutils which has commit 599df6e2db17d1c4 ("ld, LoongArch: print
> > > > error about linking without -fPIC or -fPIE flag in more detail").
> > > > 
> > > >   CC      .vmlinux.export.o
> > > >   UPD     include/generated/utsversion.h
> > > >   CC      init/version-timestamp.o
> > > >   LD      .tmp_vmlinux.kallsyms1
> > > > loongarch64-unknown-linux-gnu-ld: kernel/kallsyms.o:(.text+0): relocation R_LARCH_PCALA_HI20 against `kallsyms_markers` can not be used when making a PIE object; recompile with -fPIE
> > > > loongarch64-unknown-linux-gnu-ld: kernel/crash_core.o:(.init.text+0x984): relocation R_LARCH_PCALA_HI20 against `kallsyms_names` can not be used when making a PIE object; recompile with -fPIE
> > > > loongarch64-unknown-linux-gnu-ld: kernel/bpf/btf.o:(.text+0xcc7c): relocation R_LARCH_PCALA_HI20 against `__start_BTF` can not be used when making a PIE object; recompile with -fPIE
> > > > loongarch64-unknown-linux-gnu-ld: BFD (GNU Binutils) 2.43.50.20241126 assertion fail ../../bfd/elfnn-loongarch.c:2673
> > > > 
> > > > In theory 5.10&5.15 also need this, but since LoongArch get upstream at
> > > > 5.19, so I just ignore them because there is no error report about other
> > > > archs now.
> > > 
> > > Odd, why doesn't this affect other arches as well using new binutils?  I
> > > hate to have to backport all of this just for one arch, as that feels
> > > odd.
> > The related binutils commit is only for LoongArch, so build errors
> > only occured on LoongArch. I don't know why other archs have no
> > problem exactly, but may be related to their CFLAGS (for example, if
> > we disable CONFIG_RELOCATABLE, LoongArch also has no build errors
> > because CFLAGS changes).
> 
> does LoongArch depend on that option?

"That option" is -mdirect-extern-access.  Without it we'll use GOT in
the kernel image to address anything out of the current TU, bloating the
kernel size and making it slower.

The problem is the linker failed to handle a direct access to undefined
weak symbol on LoongArch.  With GCC 14.2 and Binutils 2.43:

$ cat t.c
extern int x __attribute__ ((weak));

int main()
{
	__builtin_printf("%p\n", &x);
}
$ cc t.c -mdirect-extern-access -static-pie -fPIE
$ ./a.out
0x7ffff27ac000

The output should be (nil) instead, as an undefined weak symbol should
be resolved to address 0.  I'm not sure why the kernel was not blown up
by this issue.

With Binutils trunk, an error is emitted instead of silently producing
buggy executable.  Still I don't think emitting an error is correct when
linking a static PIE (our vmlinux is a static PIE).  Instead the linker
should just rewrite

    pcalau12i rd, %pc_hi20(undef_weak)

to

    move rd, $zero

Also the "recompile with -fPIE" suggestion in the error message is
completely misleading.  We are *already* compiling relocatable kernel
with -fPIE.

I'm making some Binutils patches to implement the rewrite and reword the
error message (for instances where emitting an error is the correct
thing, e.g. someone attempts to build a dynamically linked program with
-mdirect-extern-access).

> What happens if it is enabled for other arches?  Why doesn't it break
> them?

The other arches have copy relocation, so their -mdirect-extern-access
is intended to work with dynamically linked executable, thus it's the
default and not as strong as ours.  On them -mdirect-extern-access still
uses GOT to address weak symbols.

We don't have copy relocation, thus our default is -mno-direct-extern-
access, and -mdirect-extern-access is only intended for static
executables (including OS kernel, embedded firmware, etc).  So it's
designed to be stronger, unfortunately the toolchain failed to implement
it correctly.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 6.1&6.6 0/3] kbuild: Avoid weak external linkage where possible
  2024-12-07 10:46       ` Xi Ruoyao
@ 2024-12-09  8:31         ` Ard Biesheuvel
  2024-12-09 10:01           ` Xi Ruoyao
  0 siblings, 1 reply; 20+ messages in thread
From: Ard Biesheuvel @ 2024-12-09  8:31 UTC (permalink / raw)
  To: Xi Ruoyao
  Cc: Greg Kroah-Hartman, Huacai Chen, Huacai Chen, Sasha Levin,
	Xuerui Wang, Masahiro Yamada, Nathan Chancellor, Nick Desaulniers,
	Nicolas Schier, stable, linux-kbuild, linux-kernel, loongarch

On Sat, 7 Dec 2024 at 11:46, Xi Ruoyao <xry111@xry111.site> wrote:
>
> On Sat, 2024-12-07 at 10:32 +0100, Greg Kroah-Hartman wrote:
> > On Sat, Dec 07, 2024 at 05:21:00PM +0800, Huacai Chen wrote:
> > > Hi, Greg,
> > >
> > > On Fri, Dec 6, 2024 at 9:04 PM Greg Kroah-Hartman
> > > <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Fri, Dec 06, 2024 at 04:58:07PM +0800, Huacai Chen wrote:
> > > > > Backport this series to 6.1&6.6 because LoongArch gets build errors with
> > > > > latest binutils which has commit 599df6e2db17d1c4 ("ld, LoongArch: print
> > > > > error about linking without -fPIC or -fPIE flag in more detail").
> > > > >
> > > > >   CC      .vmlinux.export.o
> > > > >   UPD     include/generated/utsversion.h
> > > > >   CC      init/version-timestamp.o
> > > > >   LD      .tmp_vmlinux.kallsyms1
> > > > > loongarch64-unknown-linux-gnu-ld: kernel/kallsyms.o:(.text+0): relocation R_LARCH_PCALA_HI20 against `kallsyms_markers` can not be used when making a PIE object; recompile with -fPIE
> > > > > loongarch64-unknown-linux-gnu-ld: kernel/crash_core.o:(.init.text+0x984): relocation R_LARCH_PCALA_HI20 against `kallsyms_names` can not be used when making a PIE object; recompile with -fPIE
> > > > > loongarch64-unknown-linux-gnu-ld: kernel/bpf/btf.o:(.text+0xcc7c): relocation R_LARCH_PCALA_HI20 against `__start_BTF` can not be used when making a PIE object; recompile with -fPIE
> > > > > loongarch64-unknown-linux-gnu-ld: BFD (GNU Binutils) 2.43.50.20241126 assertion fail ../../bfd/elfnn-loongarch.c:2673
> > > > >
> > > > > In theory 5.10&5.15 also need this, but since LoongArch get upstream at
> > > > > 5.19, so I just ignore them because there is no error report about other
> > > > > archs now.
> > > >
> > > > Odd, why doesn't this affect other arches as well using new binutils?  I
> > > > hate to have to backport all of this just for one arch, as that feels
> > > > odd.
> > > The related binutils commit is only for LoongArch, so build errors
> > > only occured on LoongArch. I don't know why other archs have no
> > > problem exactly, but may be related to their CFLAGS (for example, if
> > > we disable CONFIG_RELOCATABLE, LoongArch also has no build errors
> > > because CFLAGS changes).
> >
> > does LoongArch depend on that option?
>
> "That option" is -mdirect-extern-access.  Without it we'll use GOT in
> the kernel image to address anything out of the current TU, bloating the
> kernel size and making it slower.
>

An alternative to this might be to add

-include $(srctree)/include/linux/hidden.h

to KBUILD_CFLAGS_KERNEL, so that the compiler understands that all
external references are resolved at link time, not at load/run time.

> The problem is the linker failed to handle a direct access to undefined
> weak symbol on LoongArch.
...
> With Binutils trunk, an error is emitted instead of silently producing
> buggy executable.  Still I don't think emitting an error is correct when
> linking a static PIE (our vmlinux is a static PIE).  Instead the linker
> should just rewrite
>
>     pcalau12i rd, %pc_hi20(undef_weak)
>
> to
>
>     move rd, $zero
>

Is that transformation even possible at link time? Isn't pc_hi20 part of a pair?

> Also the "recompile with -fPIE" suggestion in the error message is
> completely misleading.  We are *already* compiling relocatable kernel
> with -fPIE.
>

And this is the most important difference between LoongArch and the
other arches - LoongArch already uses PIC code explicitly. Other
architectures use ordinary position dependent codegen and linking, or
-in the case of arm64- use position dependent codegen and PIE linking,
where the fact that this is even possible is a happy accident.

...
> > What happens if it is enabled for other arches?  Why doesn't it break
> > them?
>
> The other arches have copy relocation, so their -mdirect-extern-access
> is intended to work with dynamically linked executable, thus it's the
> default and not as strong as ours.  On them -mdirect-extern-access still
> uses GOT to address weak symbols.
>
> We don't have copy relocation, thus our default is -mno-direct-extern-
> access, and -mdirect-extern-access is only intended for static
> executables (including OS kernel, embedded firmware, etc).  So it's
> designed to be stronger, unfortunately the toolchain failed to implement
> it correctly.
>

This has nothing to do with copy relocations - those are only relevant
when shared libraries come into play.

Other architectures don't break because they either a) use position
dependent codegen with absolute addressing, and simply resolve
undefined weak references as 0x0, or b) use GOT indirection, where the
reference is a GOT load and the address in the GOT is set to 0x0.

So the issue here appears to be that the compiler fails to emit a GOT
entry for this reference, even though it is performing PIC codegen.
This is probably due to -mdirect-extern-access being taken into
account too strictly. The upshot is that a relative reference is
emitted to an undefined symbol, and it is impossible for a relative
reference to [reliably] yield NULL, and so the reference produces a
bogus non-NULL address.

As these patches deal with symbols that are only undefined in the
preliminary first linker pass, and are guaranteed to exist afterwards,
silently emitting a bogus relative reference was not a problem in
these cases. Obviously, throwing an error is.

The patches should be rather harmless in practice, but I know Masahiro
did not like the approach for the kallsyms markers, and made some
subsequent modifications to it.

Given that this is relatively new toolchain behavior, I'd suggest
fixing the compiler to emit weak external references via GOT entries
even when  -mdirect-extern-access is in effect.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 6.1&6.6 0/3] kbuild: Avoid weak external linkage where possible
  2024-12-09  8:31         ` Ard Biesheuvel
@ 2024-12-09 10:01           ` Xi Ruoyao
  0 siblings, 0 replies; 20+ messages in thread
From: Xi Ruoyao @ 2024-12-09 10:01 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Greg Kroah-Hartman, Huacai Chen, Huacai Chen, Sasha Levin,
	Xuerui Wang, Masahiro Yamada, Nathan Chancellor, Nick Desaulniers,
	Nicolas Schier, stable, linux-kbuild, linux-kernel, loongarch

On Mon, 2024-12-09 at 09:31 +0100, Ard Biesheuvel wrote:
> Given that this is relatively new toolchain behavior, I'd suggest
> fixing the compiler to emit weak external references via GOT entries
> even when  -mdirect-extern-access is in effect.

I'm working on an approach in the linker instead.  A PC-relative address
in +/- 2GiB range is

pcalau12i.d $a0, %pc_hi20(sym + addend)
addi.d $a0, $a0, %pc_lo12(sym + addend)

If doing a static linking, when sym is weak undefined, we should just
load addend.  The compiler already guarantees addend is in [-2**31,
2**31) range, so we just need to rewrite the pair to

lu12i.w $a0, ((addend + 0x800) & ~0x7ff)
addi.d $a0, $a0, (addend & 0x7ff)

OTOH if not doing a static linking, the user shouldn't use -mdirect-
extern-access at all [this rule is the thing related to copy relocation:
if copy relocation was available it would be possibly valid to use -
mdirect-extern-access w/o static linking] and the linker is correct to
report an error (but the error message is unclear and I need to fix it
anyway).

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Re: [PATCH 6.1&6.6 0/3] kbuild: Avoid weak external linkage where possible
  2024-12-06 13:04 ` [PATCH 6.1&6.6 0/3] kbuild: Avoid weak external linkage where possible Greg Kroah-Hartman
  2024-12-07  9:21   ` Huacai Chen
@ 2025-02-06  8:37   ` WangYuli
  2025-02-06  9:31     ` Ard Biesheuvel
  2025-02-06 10:03     ` Greg KH
  1 sibling, 2 replies; 20+ messages in thread
From: WangYuli @ 2025-02-06  8:37 UTC (permalink / raw)
  To: gregkh
  Cc: ardb, chenhuacai, chenhuacai, kernel, linux-kbuild, linux-kernel,
	loongarch, masahiroy, nathan, ndesaulniers, nicolas, sashal,
	stable

Hi, Greg,

It's rather unfortunate that currently, almost all Linux distributions
supporting LoongArch are using LTS kernels version v6.6 or older, such as
openEuler and deepin. [1][2]

If this bugfix isn't merged into linux-stable, then every single distro
kernel team will have to waste time fixing the same darn bug over and
over, even though it's already fixed in later kernels.

This would really make LTS look like it's failing to serve its intended
purpose. And I'm sure all of us do not want to see something so terrible
happen.

On Fri, Dec 6, 2024 at 9:04 PM Greg Kroah-Hartman wrote:
> Odd, why doesn't this affect other arches as well using new binutils?  I
> hate to have to backport all of this just for one arch, as that feels
> odd.

Could you help me understand why you expressed that you "hate" to have
to backport something for only one arch?
Given that we've historically done quite a bit of similar backporting for
architectures such as arm, powerpc, and x86...It's not exactly unprecedented.
I just want to grasp the rationale, as it all seems perfectly justified
and necessary.

Moreover, with all the active and strict code reviews by all developers,
such occurrences are not frequent on LoongArch. You could be not exactly
"always" backporting something like this just for LoongArch, so perhaps
that might make you and your colleagues feel a little less "hate" :-)

As for your questions on the root cause of the issue and the effectiveness
of this fix, I reckon Xi Ruoyao's explanation and Ard Biesheuvel's
supplementary points have already provided ample details. [3][4][5]

If, after your feedback, you still have any lingering doubts regarding the
issue itself or the LoongArch architecture, I believe that Xi Ruoyao,
Ard Biesheuvel, and Huacai Chen would all be more than willing to elaborate
further.

I'm bringing this up because we've encountered concrete issues in the
process of maintaining distributions. Furthermore, as an upstream resource,
linux-stable can help us more effectively drive forward community
development efforts.
Plus, we realize this benefits all Linux community developers just the same.

Hoping you could spare a moment from your busy schedule to take another look
at this patch series and perhaps reconsider the LTS inclusion of this bugfix.

[1]. https://gitee.com/openeuler/kernel/blob/openEuler-25.03/Makefile#L3
[2]. https://github.com/deepin-community/kernel/blob/linux-6.6.y/Makefile#L3
[3]. https://lore.kernel.org/all/ccb1fa9034b177042db8fcbe7a95a2a5b466dc30.camel@xry111.site/
[4]. https://lore.kernel.org/all/CAMj1kXEV+HC+2HMLhDaLfAufQLrXRs2J7akMNr1mjejDYc7kdw@mail.gmail.com/#t
[5]. https://lore.kernel.org/all/c9a43e5da01ee2215393c0f3c50956171fe660ab.camel@xry111.site/

Best Regards,
--
WangYuli

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Re: [PATCH 6.1&6.6 0/3] kbuild: Avoid weak external linkage where possible
  2025-02-06  8:37   ` WangYuli
@ 2025-02-06  9:31     ` Ard Biesheuvel
  2025-02-06 10:03     ` Greg KH
  1 sibling, 0 replies; 20+ messages in thread
From: Ard Biesheuvel @ 2025-02-06  9:31 UTC (permalink / raw)
  To: WangYuli
  Cc: gregkh, chenhuacai, chenhuacai, kernel, linux-kbuild,
	linux-kernel, loongarch, masahiroy, nathan, ndesaulniers, nicolas,
	sashal, stable

On Thu, 6 Feb 2025 at 09:53, WangYuli <wangyuli@uniontech.com> wrote:
>
> Hi, Greg,
>
> It's rather unfortunate that currently, almost all Linux distributions
> supporting LoongArch are using LTS kernels version v6.6 or older, such as
> openEuler and deepin. [1][2]
>
> If this bugfix isn't merged into linux-stable, then every single distro
> kernel team will have to waste time fixing the same darn bug over and
> over, even though it's already fixed in later kernels.
>
> This would really make LTS look like it's failing to serve its intended
> purpose. And I'm sure all of us do not want to see something so terrible
> happen.
>
> On Fri, Dec 6, 2024 at 9:04 PM Greg Kroah-Hartman wrote:
> > Odd, why doesn't this affect other arches as well using new binutils?  I
> > hate to have to backport all of this just for one arch, as that feels
> > odd.
>
> Could you help me understand why you expressed that you "hate" to have
> to backport something for only one arch?
> Given that we've historically done quite a bit of similar backporting for
> architectures such as arm, powerpc, and x86...It's not exactly unprecedented.
> I just want to grasp the rationale, as it all seems perfectly justified
> and necessary.
>
> Moreover, with all the active and strict code reviews by all developers,
> such occurrences are not frequent on LoongArch. You could be not exactly
> "always" backporting something like this just for LoongArch, so perhaps
> that might make you and your colleagues feel a little less "hate" :-)
>
> As for your questions on the root cause of the issue and the effectiveness
> of this fix, I reckon Xi Ruoyao's explanation and Ard Biesheuvel's
> supplementary points have already provided ample details. [3][4][5]
>
> If, after your feedback, you still have any lingering doubts regarding the
> issue itself or the LoongArch architecture, I believe that Xi Ruoyao,
> Ard Biesheuvel, and Huacai Chen would all be more than willing to elaborate
> further.
>
> I'm bringing this up because we've encountered concrete issues in the
> process of maintaining distributions. Furthermore, as an upstream resource,
> linux-stable can help us more effectively drive forward community
> development efforts.
> Plus, we realize this benefits all Linux community developers just the same.
>
> Hoping you could spare a moment from your busy schedule to take another look
> at this patch series and perhaps reconsider the LTS inclusion of this bugfix.
>
> [1]. https://gitee.com/openeuler/kernel/blob/openEuler-25.03/Makefile#L3
> [2]. https://github.com/deepin-community/kernel/blob/linux-6.6.y/Makefile#L3
> [3]. https://lore.kernel.org/all/ccb1fa9034b177042db8fcbe7a95a2a5b466dc30.camel@xry111.site/
> [4]. https://lore.kernel.org/all/CAMj1kXEV+HC+2HMLhDaLfAufQLrXRs2J7akMNr1mjejDYc7kdw@mail.gmail.com/#t
> [5]. https://lore.kernel.org/all/c9a43e5da01ee2215393c0f3c50956171fe660ab.camel@xry111.site/
>

You might consider sending a Loongarch-only patch for mainline that
adds weak definitions of these symbols, and backport that to -stable
once it hits Linus's tree. That way, the weak references are always
satisfied, even during the first linker pass.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Re: [PATCH 6.1&6.6 0/3] kbuild: Avoid weak external linkage where possible
  2025-02-06  8:37   ` WangYuli
  2025-02-06  9:31     ` Ard Biesheuvel
@ 2025-02-06 10:03     ` Greg KH
  2025-09-05  6:49       ` Ming Wang
  1 sibling, 1 reply; 20+ messages in thread
From: Greg KH @ 2025-02-06 10:03 UTC (permalink / raw)
  To: WangYuli
  Cc: ardb, chenhuacai, chenhuacai, kernel, linux-kbuild, linux-kernel,
	loongarch, masahiroy, nathan, ndesaulniers, nicolas, sashal,
	stable

On Thu, Feb 06, 2025 at 04:37:02PM +0800, WangYuli wrote:
> Hi, Greg,
> 
> It's rather unfortunate that currently, almost all Linux distributions
> supporting LoongArch are using LTS kernels version v6.6 or older, such as
> openEuler and deepin. [1][2]
> 
> If this bugfix isn't merged into linux-stable, then every single distro
> kernel team will have to waste time fixing the same darn bug over and
> over, even though it's already fixed in later kernels.
> 
> This would really make LTS look like it's failing to serve its intended
> purpose. And I'm sure all of us do not want to see something so terrible
> happen.

LTS is here to ensure that the original release of these branches, keeps
working for that branch.  Adding support for newer toolchains sometimes
happens, but is not a requirement or a normal thing to do as that really
isn't a "regression", right?

Most of the time, fixing things up for newer compilers is simple.
Sometimes it is not simple.  The "not simple" ones we usually just do
not backport as that causes extra work for everyone over time.

As for the distros like openEuler, and deepin, they are free to add
these patches there, on top of their other non-LTS patches, right?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 6.1&6.6 1/3] kallsyms: Avoid weak references for kallsyms symbols
  2024-12-06  8:58 ` [PATCH 6.1&6.6 1/3] kallsyms: Avoid weak references for kallsyms symbols Huacai Chen
  2024-12-06 17:11   ` Sasha Levin
@ 2025-02-07 22:50   ` Sasha Levin
  1 sibling, 0 replies; 20+ messages in thread
From: Sasha Levin @ 2025-02-07 22:50 UTC (permalink / raw)
  To: stable; +Cc: Huacai Chen, Sasha Levin

[ Sasha's backport helper bot ]

Hi,

The upstream commit SHA1 provided is correct: 951bcae6c5a0bfaa55b27c5f16178204988f0379

WARNING: Author mismatch between patch and upstream commit:
Backport author: Huacai Chen<chenhuacai@loongson.cn>
Commit author: Ard Biesheuvel<ardb@kernel.org>


Status in newer kernel trees:
6.13.y | Present (exact SHA1)
6.12.y | Present (exact SHA1)
6.6.y | Not found

Note: The patch differs from the upstream commit:
---
1:  951bcae6c5a0b ! 1:  d6ccfcaa675df kallsyms: Avoid weak references for kallsyms symbols
    @@ Metadata
      ## Commit message ##
         kallsyms: Avoid weak references for kallsyms symbols
     
    +    [ Upstream commit 951bcae6c5a0bfaa55b27c5f16178204988f0379 ]
    +
         kallsyms is a directory of all the symbols in the vmlinux binary, and so
         creating it is somewhat of a chicken-and-egg problem, as its non-zero
         size affects the layout of the binary, and therefore the values of the
    @@ Commit message
         Link: https://lkml.kernel.org/r/20230504174320.3930345-1-ardb%40kernel.org
         Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
         Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
    +    Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
     
      ## include/asm-generic/vmlinux.lds.h ##
     @@
---

Results of testing on various branches:

| Branch                    | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-6.6.y        |  Success    |  Failed    |
| stable/linux-6.1.y        |  Success    |  Failed    |

Build Errors:
Build error for stable/linux-6.6.y:
    ssh: connect to host 192.168.1.58 port 22: No route to host

Build error for stable/linux-6.1.y:
    ssh: connect to host 192.168.1.58 port 22: No route to host

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 6.1&6.6 0/3] kbuild: Avoid weak external linkage where possible
  2025-02-06 10:03     ` Greg KH
@ 2025-09-05  6:49       ` Ming Wang
  2025-09-05  7:09         ` Greg KH
  0 siblings, 1 reply; 20+ messages in thread
From: Ming Wang @ 2025-09-05  6:49 UTC (permalink / raw)
  To: Greg KH, WangYuli
  Cc: ardb, chenhuacai, chenhuacai, kernel, linux-kbuild, linux-kernel,
	loongarch, masahiroy, nathan, ndesaulniers, nicolas, sashal,
	stable

Hi Greg, all,

On 2/6/25 18:03, Greg KH wrote:
> On Thu, Feb 06, 2025 at 04:37:02PM +0800, WangYuli wrote:
>> Hi, Greg,
>>
>> It's rather unfortunate that currently, almost all Linux distributions
>> supporting LoongArch are using LTS kernels version v6.6 or older, such as
>> openEuler and deepin. [1][2]
>>
>> If this bugfix isn't merged into linux-stable, then every single distro
>> kernel team will have to waste time fixing the same darn bug over and
>> over, even though it's already fixed in later kernels.
>>
>> This would really make LTS look like it's failing to serve its intended
>> purpose. And I'm sure all of us do not want to see something so terrible
>> happen.
> 
> LTS is here to ensure that the original release of these branches, keeps
> working for that branch.  Adding support for newer toolchains sometimes
> happens, but is not a requirement or a normal thing to do as that really
> isn't a "regression", right?
> 
> Most of the time, fixing things up for newer compilers is simple.
> Sometimes it is not simple.  The "not simple" ones we usually just do
> not backport as that causes extra work for everyone over time.
> 
> As for the distros like openEuler, and deepin, they are free to add
> these patches there, on top of their other non-LTS patches, right?
> 
> thanks,
> 
> greg k-h

I'm writing to follow up on this important discussion. I have carefully
read the entire thread, including your explanation of the LTS philosophy
regarding support for new toolchains. I understand and respect the
principle that LTS aims to maintain stability for the environment in
which it was released, and that adapting to future toolchains is
primarily a distributor's responsibility.

However, I would like to respectfully ask for a reconsideration by
framing this issue from a slightly different perspective, based on the
excellent technical analysis provided by Xi Ruoyao and Ard Biesheuvel.

This situation appears to be more than just an incompatibility with a
"newer" toolchain. As Xi Ruoyao detailed, the older toolchains did not
"work correctly" but instead had a silent bug that produced incorrect
code for undefined weak symbols on LoongArch. The new binutils version
did not introduce a regression, but rather, it correctly started
erroring out on this problematic code pattern, thus exposing a
pre-existing, latent issue.

 From this viewpoint, this patch series is less about "adding support for
a new toolchain" and more about "fixing a latent bug that was previously
hidden by silent toolchain defects."

Furthermore, the patches themselves, originally authored by Ard, 
represent a clean, correct, and low-risk improvement. They were accepted 
into the mainline not just as a workaround, but as a superior way to 
handle these symbols, improving codegen for all architectures. 
Backporting this series would therefore be applying a high-quality, 
vetted bug fix that also has the fortunate side effect of resolving this 
build failure.

While the build failure currently only manifests on LoongArch, the
underlying code improvement is generic. For a relatively new 
architecture like LoongArch, ensuring that the primary LTS kernels are 
usable with modern, widely-adopted toolchains is crucial for the health 
and growth of its ecosystem within the broader Linux community. As 
WangYuli pointed out, this would prevent fragmented efforts across 
multiple distributions.

In summary, we believe this case is exceptional because the patch fixes
a latent issue exposed by a toolchain correction and represents a clean,
mainline-accepted improvement. We would be very grateful if you could
take another look at this series from this perspective.

Thank you for your time.

Best Regards,
Robin


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 6.1&6.6 0/3] kbuild: Avoid weak external linkage where possible
  2025-09-05  6:49       ` Ming Wang
@ 2025-09-05  7:09         ` Greg KH
  2025-09-05  8:29           ` Ming Wang
  0 siblings, 1 reply; 20+ messages in thread
From: Greg KH @ 2025-09-05  7:09 UTC (permalink / raw)
  To: Ming Wang
  Cc: WangYuli, ardb, chenhuacai, chenhuacai, kernel, linux-kbuild,
	linux-kernel, loongarch, masahiroy, nathan, ndesaulniers, nicolas,
	sashal, stable

On Fri, Sep 05, 2025 at 02:49:44PM +0800, Ming Wang wrote:
> Hi Greg, all,
> 
> On 2/6/25 18:03, Greg KH wrote:
> > On Thu, Feb 06, 2025 at 04:37:02PM +0800, WangYuli wrote:
> > > Hi, Greg,
> > > 
> > > It's rather unfortunate that currently, almost all Linux distributions
> > > supporting LoongArch are using LTS kernels version v6.6 or older, such as
> > > openEuler and deepin. [1][2]
> > > 
> > > If this bugfix isn't merged into linux-stable, then every single distro
> > > kernel team will have to waste time fixing the same darn bug over and
> > > over, even though it's already fixed in later kernels.
> > > 
> > > This would really make LTS look like it's failing to serve its intended
> > > purpose. And I'm sure all of us do not want to see something so terrible
> > > happen.
> > 
> > LTS is here to ensure that the original release of these branches, keeps
> > working for that branch.  Adding support for newer toolchains sometimes
> > happens, but is not a requirement or a normal thing to do as that really
> > isn't a "regression", right?
> > 
> > Most of the time, fixing things up for newer compilers is simple.
> > Sometimes it is not simple.  The "not simple" ones we usually just do
> > not backport as that causes extra work for everyone over time.
> > 
> > As for the distros like openEuler, and deepin, they are free to add
> > these patches there, on top of their other non-LTS patches, right?
> > 
> > thanks,
> > 
> > greg k-h
> 
> I'm writing to follow up on this important discussion. I have carefully
> read the entire thread, including your explanation of the LTS philosophy
> regarding support for new toolchains. I understand and respect the
> principle that LTS aims to maintain stability for the environment in
> which it was released, and that adapting to future toolchains is
> primarily a distributor's responsibility.
> 
> However, I would like to respectfully ask for a reconsideration by
> framing this issue from a slightly different perspective, based on the
> excellent technical analysis provided by Xi Ruoyao and Ard Biesheuvel.

<snip>

i'm sorry, but for an email thread that happened 6+ months ago, it's a
bit hard to try to remember anything involved in it.

Heck, I can't remember an email thread from last week.

Remember, some of us get 1000+ emails a day to deal with.

If you feel a patch set should be applied to a stable tree, and it has
been rejected in the past, feel free to resubmit it with all of the new
information about why the previous rejection was wrong and why it really
should be applied this time.  Otherwise, there's really nothing I could
possibly do here as the patches are long gone from everyone's review
queues.

Also, why aren't you just using 6.12.y now?  :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 6.1&6.6 0/3] kbuild: Avoid weak external linkage where possible
  2025-09-05  7:09         ` Greg KH
@ 2025-09-05  8:29           ` Ming Wang
  0 siblings, 0 replies; 20+ messages in thread
From: Ming Wang @ 2025-09-05  8:29 UTC (permalink / raw)
  To: Greg KH
  Cc: WangYuli, ardb, chenhuacai, chenhuacai, kernel, linux-kbuild,
	linux-kernel, loongarch, masahiroy, nathan, ndesaulniers, nicolas,
	sashal, stable

Hi Greg,

On 9/5/25 15:09, Greg KH wrote:
> On Fri, Sep 05, 2025 at 02:49:44PM +0800, Ming Wang wrote:
>> Hi Greg, all,
>>
>> On 2/6/25 18:03, Greg KH wrote:
>>> On Thu, Feb 06, 2025 at 04:37:02PM +0800, WangYuli wrote:
>>>> Hi, Greg,
>>>>
>>>> It's rather unfortunate that currently, almost all Linux distributions
>>>> supporting LoongArch are using LTS kernels version v6.6 or older, such as
>>>> openEuler and deepin. [1][2]
>>>>
>>>> If this bugfix isn't merged into linux-stable, then every single distro
>>>> kernel team will have to waste time fixing the same darn bug over and
>>>> over, even though it's already fixed in later kernels.
>>>>
>>>> This would really make LTS look like it's failing to serve its intended
>>>> purpose. And I'm sure all of us do not want to see something so terrible
>>>> happen.
>>>
>>> LTS is here to ensure that the original release of these branches, keeps
>>> working for that branch.  Adding support for newer toolchains sometimes
>>> happens, but is not a requirement or a normal thing to do as that really
>>> isn't a "regression", right?
>>>
>>> Most of the time, fixing things up for newer compilers is simple.
>>> Sometimes it is not simple.  The "not simple" ones we usually just do
>>> not backport as that causes extra work for everyone over time.
>>>
>>> As for the distros like openEuler, and deepin, they are free to add
>>> these patches there, on top of their other non-LTS patches, right?
>>>
>>> thanks,
>>>
>>> greg k-h
>>
>> I'm writing to follow up on this important discussion. I have carefully
>> read the entire thread, including your explanation of the LTS philosophy
>> regarding support for new toolchains. I understand and respect the
>> principle that LTS aims to maintain stability for the environment in
>> which it was released, and that adapting to future toolchains is
>> primarily a distributor's responsibility.
>>
>> However, I would like to respectfully ask for a reconsideration by
>> framing this issue from a slightly different perspective, based on the
>> excellent technical analysis provided by Xi Ruoyao and Ard Biesheuvel.
> 
> <snip>
> 
> i'm sorry, but for an email thread that happened 6+ months ago, it's a
> bit hard to try to remember anything involved in it.
> 
> Heck, I can't remember an email thread from last week.
> 
> Remember, some of us get 1000+ emails a day to deal with.
> 
> If you feel a patch set should be applied to a stable tree, and it has
> been rejected in the past, feel free to resubmit it with all of the new
> information about why the previous rejection was wrong and why it really
> should be applied this time.  Otherwise, there's really nothing I could
> possibly do here as the patches are long gone from everyone's review
> queues.
> 
> Also, why aren't you just using 6.12.y now?  :)
> 
> thanks,
> 
> greg k-h

Thank you for the quick reply.You've raised a very fair point about 
moving to a newer kernel. Thanks again for your time and the guidance.

Best Regards,
Robin


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2025-09-05  8:29 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-06  8:58 [PATCH 6.1&6.6 0/3] kbuild: Avoid weak external linkage where possible Huacai Chen
2024-12-06  8:58 ` [PATCH 6.1&6.6 1/3] kallsyms: Avoid weak references for kallsyms symbols Huacai Chen
2024-12-06 17:11   ` Sasha Levin
2025-02-07 22:50   ` Sasha Levin
2024-12-06  8:58 ` [PATCH 6.1&6.6 2/3] vmlinux: Avoid weak reference to notes section Huacai Chen
2024-12-06 17:11   ` Sasha Levin
2024-12-06  8:58 ` [PATCH 6.1&6.6 3/3] btf: Avoid weak external references Huacai Chen
2024-12-06 17:11   ` Sasha Levin
2024-12-06 13:04 ` [PATCH 6.1&6.6 0/3] kbuild: Avoid weak external linkage where possible Greg Kroah-Hartman
2024-12-07  9:21   ` Huacai Chen
2024-12-07  9:32     ` Greg Kroah-Hartman
2024-12-07 10:46       ` Xi Ruoyao
2024-12-09  8:31         ` Ard Biesheuvel
2024-12-09 10:01           ` Xi Ruoyao
2025-02-06  8:37   ` WangYuli
2025-02-06  9:31     ` Ard Biesheuvel
2025-02-06 10:03     ` Greg KH
2025-09-05  6:49       ` Ming Wang
2025-09-05  7:09         ` Greg KH
2025-09-05  8:29           ` Ming Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).