linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] alloc_tag: remove empty module tag section
@ 2025-06-10 16:22 Casey Chen
  2025-06-17  9:27 ` Petr Pavlu
  0 siblings, 1 reply; 3+ messages in thread
From: Casey Chen @ 2025-06-10 16:22 UTC (permalink / raw)
  To: akpm
  Cc: linux-mm, linux-kernel, linux-modules, linux-arch, surenb,
	kent.overstreet, arnd, mcgrof, pasha.tatashin, yzhong, Casey Chen

The empty MOD_CODETAG_SECTIONS() macro added an incomplete .data
section in module linker script, which caused symbol lookup tools
like gdb to misinterpret symbol addresses e.g., __ib_process_cq
incorrectly mapping to unrelated functions like below.

  (gdb) disas __ib_process_cq
  Dump of assembler code for function trace_event_fields_cq_schedule:

Removing the empty section restores proper symbol resolution and
layout, ensuring .data placement behaves as expected.

Fixes: 0db6f8d7820a ("alloc_tag: load module tags into separate contiguous memory")
       22d407b164ff ("lib: add allocation tagging support for memory allocation profiling")
Signed-off-by: Casey Chen <cachen@purestorage.com>
Reviewed-by: Yuanyuan Zhong <yzhong@purestorage.com>
Acked-by: Suren Baghdasaryan <surenb@google.com>
---
 include/asm-generic/codetag.lds.h | 6 ------
 scripts/module.lds.S              | 5 -----
 2 files changed, 11 deletions(-)

diff --git a/include/asm-generic/codetag.lds.h b/include/asm-generic/codetag.lds.h
index 372c320c5043..a45fe3d141a1 100644
--- a/include/asm-generic/codetag.lds.h
+++ b/include/asm-generic/codetag.lds.h
@@ -11,12 +11,6 @@
 #define CODETAG_SECTIONS()		\
 	SECTION_WITH_BOUNDARIES(alloc_tags)
 
-/*
- * Module codetags which aren't used after module unload, therefore have the
- * same lifespan as the module and can be safely unloaded with the module.
- */
-#define MOD_CODETAG_SECTIONS()
-
 #define MOD_SEPARATE_CODETAG_SECTION(_name)	\
 	.codetag.##_name : {			\
 		SECTION_WITH_BOUNDARIES(_name)	\
diff --git a/scripts/module.lds.S b/scripts/module.lds.S
index 711c6e029936..c071ca4beedd 100644
--- a/scripts/module.lds.S
+++ b/scripts/module.lds.S
@@ -50,17 +50,12 @@ SECTIONS {
 	.data : {
 		*(.data .data.[0-9a-zA-Z_]*)
 		*(.data..L*)
-		MOD_CODETAG_SECTIONS()
 	}
 
 	.rodata : {
 		*(.rodata .rodata.[0-9a-zA-Z_]*)
 		*(.rodata..L*)
 	}
-#else
-	.data : {
-		MOD_CODETAG_SECTIONS()
-	}
 #endif
 	MOD_SEPARATE_CODETAG_SECTIONS()
 }
-- 
2.34.1


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

* Re: [PATCH] alloc_tag: remove empty module tag section
  2025-06-10 16:22 [PATCH] alloc_tag: remove empty module tag section Casey Chen
@ 2025-06-17  9:27 ` Petr Pavlu
  2025-06-17 15:05   ` Suren Baghdasaryan
  0 siblings, 1 reply; 3+ messages in thread
From: Petr Pavlu @ 2025-06-17  9:27 UTC (permalink / raw)
  To: Casey Chen
  Cc: akpm, linux-mm, linux-kernel, linux-modules, linux-arch, surenb,
	kent.overstreet, arnd, mcgrof, pasha.tatashin, yzhong

On 6/10/25 6:22 PM, Casey Chen wrote:
> The empty MOD_CODETAG_SECTIONS() macro added an incomplete .data
> section in module linker script, which caused symbol lookup tools
> like gdb to misinterpret symbol addresses e.g., __ib_process_cq
> incorrectly mapping to unrelated functions like below.
> 
>   (gdb) disas __ib_process_cq
>   Dump of assembler code for function trace_event_fields_cq_schedule:
> 
> Removing the empty section restores proper symbol resolution and
> layout, ensuring .data placement behaves as expected.

The patch looks ok me, but I'm somewhat confused about the problem.
I think a linker should not add an empty output section if it doesn't
contain anything, or if .data actually contains something then the extra
dummy definition should be also harmless?

This also reminds me of my previous related fix "codetag: Avoid unused
alloc_tags sections/symbols" [1] which fell through the cracks. I can
rebase it on top of this patch.

[1] https://lore.kernel.org/all/20250313143002.9118-1-petr.pavlu@suse.com/

-- 
Thanks,
Petr

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

* Re: [PATCH] alloc_tag: remove empty module tag section
  2025-06-17  9:27 ` Petr Pavlu
@ 2025-06-17 15:05   ` Suren Baghdasaryan
  0 siblings, 0 replies; 3+ messages in thread
From: Suren Baghdasaryan @ 2025-06-17 15:05 UTC (permalink / raw)
  To: Petr Pavlu
  Cc: Casey Chen, akpm, linux-mm, linux-kernel, linux-modules,
	linux-arch, kent.overstreet, arnd, mcgrof, pasha.tatashin, yzhong

On Tue, Jun 17, 2025 at 2:27 AM Petr Pavlu <petr.pavlu@suse.com> wrote:
>
> On 6/10/25 6:22 PM, Casey Chen wrote:
> > The empty MOD_CODETAG_SECTIONS() macro added an incomplete .data
> > section in module linker script, which caused symbol lookup tools
> > like gdb to misinterpret symbol addresses e.g., __ib_process_cq
> > incorrectly mapping to unrelated functions like below.
> >
> >   (gdb) disas __ib_process_cq
> >   Dump of assembler code for function trace_event_fields_cq_schedule:
> >
> > Removing the empty section restores proper symbol resolution and
> > layout, ensuring .data placement behaves as expected.
>
> The patch looks ok me, but I'm somewhat confused about the problem.
> I think a linker should not add an empty output section if it doesn't
> contain anything, or if .data actually contains something then the extra
> dummy definition should be also harmless?

I also assumed so but apparently this is not entirely harmless.

>
> This also reminds me of my previous related fix "codetag: Avoid unused
> alloc_tags sections/symbols" [1] which fell through the cracks. I can
> rebase it on top of this patch.
>
> [1] https://lore.kernel.org/all/20250313143002.9118-1-petr.pavlu@suse.com/

Yes please.

>
> --
> Thanks,
> Petr

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

end of thread, other threads:[~2025-06-17 15:05 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-10 16:22 [PATCH] alloc_tag: remove empty module tag section Casey Chen
2025-06-17  9:27 ` Petr Pavlu
2025-06-17 15:05   ` Suren Baghdasaryan

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).