[PATCH v2 42/62] kbuild,x86: Fix module permissions for __jump_table and __bug_table

Josh Poimboeuf posted 62 patches 7 months, 1 week ago
There is a newer version of this series
[PATCH v2 42/62] kbuild,x86: Fix module permissions for __jump_table and __bug_table
Posted by Josh Poimboeuf 7 months, 1 week ago
An upcoming patch will add the SHF_MERGE flag to x86 __jump_table and
__bug_table so their entry sizes can be defined in inline asm.

However, those sections have SHF_WRITE, which the Clang linker (lld)
explicitly forbids combining with SHF_MERGE.

Those sections are modified at runtime and must remain writable.  While
SHF_WRITE is ignored by vmlinux, it's still needed for modules.

To work around the linker interference, remove SHF_WRITE during
compilation and restore it after linking the module.

Cc: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 arch/Kconfig                      |  3 +++
 arch/x86/Kconfig                  |  1 +
 arch/x86/include/asm/bug.h        |  4 ++--
 arch/x86/include/asm/jump_label.h |  2 +-
 scripts/Makefile.modfinal         | 18 +++++++++++++-----
 5 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index b0adb665041f..a413cd86f87c 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -1314,6 +1314,9 @@ config HAVE_NOINSTR_HACK
 config HAVE_NOINSTR_VALIDATION
 	bool
 
+config NEED_MODULE_PERMISSIONS_FIX
+	bool
+
 config HAVE_UACCESS_VALIDATION
 	bool
 	select OBJTOOL
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 4c33c644b92d..996d59e59e5d 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -309,6 +309,7 @@ config X86
 	select HOTPLUG_SPLIT_STARTUP		if SMP && X86_32
 	select IRQ_FORCED_THREADING
 	select LOCK_MM_AND_FIND_VMA
+	select NEED_MODULE_PERMISSIONS_FIX
 	select NEED_PER_CPU_EMBED_FIRST_CHUNK
 	select NEED_PER_CPU_PAGE_FIRST_CHUNK
 	select NEED_SG_DMA_LENGTH
diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index f0e9acf72547..fb3534ddbea2 100644
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -42,7 +42,7 @@
 #define _BUG_FLAGS(ins, flags, extra)					\
 do {									\
 	asm_inline volatile("1:\t" ins "\n"				\
-		     ".pushsection __bug_table,\"aw\"\n"		\
+		     ".pushsection __bug_table,\"a\"\n"			\
 		     "2:\t" __BUG_REL(1b) "\t# bug_entry::bug_addr\n"	\
 		     "\t"  __BUG_REL(%c0) "\t# bug_entry::file\n"	\
 		     "\t.word %c1"        "\t# bug_entry::line\n"	\
@@ -60,7 +60,7 @@ do {									\
 #define _BUG_FLAGS(ins, flags, extra)					\
 do {									\
 	asm_inline volatile("1:\t" ins "\n"				\
-		     ".pushsection __bug_table,\"aw\"\n"		\
+		     ".pushsection __bug_table,\"a\"\n"			\
 		     "2:\t" __BUG_REL(1b) "\t# bug_entry::bug_addr\n"	\
 		     "\t.word %c0"        "\t# bug_entry::flags\n"	\
 		     "\t.org 2b+%c1\n"					\
diff --git a/arch/x86/include/asm/jump_label.h b/arch/x86/include/asm/jump_label.h
index 61dd1dee7812..cd21554b3675 100644
--- a/arch/x86/include/asm/jump_label.h
+++ b/arch/x86/include/asm/jump_label.h
@@ -13,7 +13,7 @@
 #include <linux/types.h>
 
 #define JUMP_TABLE_ENTRY(key, label)			\
-	".pushsection __jump_table,  \"aw\" \n\t"	\
+	".pushsection __jump_table,  \"a\"\n\t"		\
 	_ASM_ALIGN "\n\t"				\
 	".long 1b - . \n\t"				\
 	".long " label " - . \n\t"			\
diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
index 542ba462ed3e..878d0d25a461 100644
--- a/scripts/Makefile.modfinal
+++ b/scripts/Makefile.modfinal
@@ -28,12 +28,23 @@ ccflags-remove-y := $(CC_FLAGS_CFI)
 .module-common.o: $(srctree)/scripts/module-common.c FORCE
 	$(call if_changed_rule,cc_o_c)
 
+ifdef CONFIG_NEED_MODULE_PERMISSIONS_FIX
+cmd_fix_mod_permissions =						\
+	$(OBJCOPY) --set-section-flags __jump_table=alloc,data		\
+		   --set-section-flags __bug_table=alloc,data $@
+endif
+
 quiet_cmd_ld_ko_o = LD [M]  $@
       cmd_ld_ko_o =							\
 	$(LD) -r $(KBUILD_LDFLAGS)					\
 		$(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE)		\
 		-T $(objtree)/scripts/module.lds -o $@ $(filter %.o, $^)
 
+define rule_ld_ko_o
+	$(call cmd_and_savecmd,ld_ko_o)
+	$(call cmd,fix_mod_permissions)
+endef
+
 quiet_cmd_btf_ko = BTF [M] $@
       cmd_btf_ko = 							\
 	if [ ! -f $(objtree)/vmlinux ]; then				\
@@ -46,14 +57,11 @@ quiet_cmd_btf_ko = BTF [M] $@
 # Same as newer-prereqs, but allows to exclude specified extra dependencies
 newer_prereqs_except = $(filter-out $(PHONY) $(1),$?)
 
-# Same as if_changed, but allows to exclude specified extra dependencies
-if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check),      \
-	$(cmd);                                                              \
-	printf '%s\n' 'savedcmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:)
+if_changed_rule_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check),$(rule_$(1)),@:)
 
 # Re-generate module BTFs if either module's .ko or vmlinux changed
 %.ko: %.o %.mod.o .module-common.o $(objtree)/scripts/module.lds $(and $(CONFIG_DEBUG_INFO_BTF_MODULES),$(KBUILD_BUILTIN),$(objtree)/vmlinux) FORCE
-	+$(call if_changed_except,ld_ko_o,$(objtree)/vmlinux)
+	+$(call if_changed_rule_except,ld_ko_o,$(objtree)/vmlinux)
 ifdef CONFIG_DEBUG_INFO_BTF_MODULES
 	+$(if $(newer-prereqs),$(call cmd,btf_ko))
 endif
-- 
2.49.0
Re: [PATCH v2 42/62] kbuild,x86: Fix module permissions for __jump_table and __bug_table
Posted by Peter Zijlstra 6 months, 3 weeks ago
On Fri, May 09, 2025 at 01:17:06PM -0700, Josh Poimboeuf wrote:
> An upcoming patch will add the SHF_MERGE flag to x86 __jump_table and
> __bug_table so their entry sizes can be defined in inline asm.
> 
> However, those sections have SHF_WRITE, which the Clang linker (lld)
> explicitly forbids combining with SHF_MERGE.
> 
> Those sections are modified at runtime and must remain writable.  While
> SHF_WRITE is ignored by vmlinux, it's still needed for modules.
> 
> To work around the linker interference, remove SHF_WRITE during
> compilation and restore it after linking the module.

*groan*

This and the following patches marking a whole bunch of sections M,
seems to suggest you're going to rely on sh_entsize actually working.

There was an ld.lld bug, and IIRC you need to enforce llvm-20 or later
if you want this to be so.
Re: [PATCH v2 42/62] kbuild,x86: Fix module permissions for __jump_table and __bug_table
Posted by Josh Poimboeuf 6 months, 2 weeks ago
On Mon, May 26, 2025 at 01:06:34PM +0200, Peter Zijlstra wrote:
> On Fri, May 09, 2025 at 01:17:06PM -0700, Josh Poimboeuf wrote:
> > An upcoming patch will add the SHF_MERGE flag to x86 __jump_table and
> > __bug_table so their entry sizes can be defined in inline asm.
> > 
> > However, those sections have SHF_WRITE, which the Clang linker (lld)
> > explicitly forbids combining with SHF_MERGE.
> > 
> > Those sections are modified at runtime and must remain writable.  While
> > SHF_WRITE is ignored by vmlinux, it's still needed for modules.
> > 
> > To work around the linker interference, remove SHF_WRITE during
> > compilation and restore it after linking the module.
> 
> *groan*
> 
> This and the following patches marking a whole bunch of sections M,
> seems to suggest you're going to rely on sh_entsize actually working.
> 
> There was an ld.lld bug, and IIRC you need to enforce llvm-20 or later
> if you want this to be so.

Hm, ISTR this working with clang 18, I'll go test that again.

-- 
Josh
Re: [PATCH v2 42/62] kbuild,x86: Fix module permissions for __jump_table and __bug_table
Posted by Josh Poimboeuf 6 months, 2 weeks ago
On Wed, Jun 04, 2025 at 05:22:15PM -0700, Josh Poimboeuf wrote:
> On Mon, May 26, 2025 at 01:06:34PM +0200, Peter Zijlstra wrote:
> > On Fri, May 09, 2025 at 01:17:06PM -0700, Josh Poimboeuf wrote:
> > > An upcoming patch will add the SHF_MERGE flag to x86 __jump_table and
> > > __bug_table so their entry sizes can be defined in inline asm.
> > > 
> > > However, those sections have SHF_WRITE, which the Clang linker (lld)
> > > explicitly forbids combining with SHF_MERGE.
> > > 
> > > Those sections are modified at runtime and must remain writable.  While
> > > SHF_WRITE is ignored by vmlinux, it's still needed for modules.
> > > 
> > > To work around the linker interference, remove SHF_WRITE during
> > > compilation and restore it after linking the module.
> > 
> > *groan*
> > 
> > This and the following patches marking a whole bunch of sections M,
> > seems to suggest you're going to rely on sh_entsize actually working.
> > 
> > There was an ld.lld bug, and IIRC you need to enforce llvm-20 or later
> > if you want this to be so.
> 
> Hm, ISTR this working with clang 18, I'll go test that again.

You're right, looks like sh_entsize is getting cleared by the linker
with my Clang 18.  I guess I tested with newer Clang.

"objtool klp diff" fails with:

  vmlinux.o: error: objtool: .discard.annotate_insn: unknown entry size

So yeah, non-buggy linker is already being enforced, though I should
probably make the error more human friendly.

-- 
Josh