All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Jim Cromie" <jim.cromie@gmail.com>
Cc: linux-modules@vger.kernel.org
Subject: Re: [PATCH v2 05/24] vmlinux.lds.h: Fix ALIGN(8) omission causing NULL ptr on i386
Date: Sat, 23 May 2026 07:42:29 +0000	[thread overview]
Message-ID: <20260523074230.4F5CF1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260523-dd-maint-2-v2-5-b937312aa083@gmail.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The implicit 8-byte alignment added to `BOUNDED_SECTION` macros corrupts the `.notes` ELF section by injecting padding at the start, breaking `PT_NOTE` parsing.
- [Low] The patch fails to apply the `_ALIGNED` variant to `.BTF_ids`, violating the commit message's stated refactoring rules.
--

commit 8b6f5abfb8521b62106505848b2663ec04bac566
Author: Jim Cromie <jim.cromie@gmail.com>
Subject: vmlinux.lds.h: Fix ALIGN(8) omission causing NULL ptr on i386

This commit explicitly adds 8-byte alignment to BOUNDED_SECTION macros to
prevent unaligned section pointers. It also introduces _ALIGNED variants
for manually aligned sections and updates existing macros to use them.

Link: https://lore.kernel.org/lkml/177402491426.6181.12855763650074831089.b4-review@b4/

> diff --git a/include/asm-generic/bounded_sections.lds.h b/include/asm-generic/bounded_sections.lds.h
> index 268cdc34389b0..8ff3e3420f60a 100644
> --- a/include/asm-generic/bounded_sections.lds.h
> +++ b/include/asm-generic/bounded_sections.lds.h
[ ... ]
> -#define BOUNDED_SECTION_PRE_LABEL(_sec_, _label_, _BEGIN_, _END_)	\
> +#define BOUNDED_SECTION_PRE_LABEL_ALIGNED(_sec_, _label_, _BEGIN_, _END_, _ALIGNED_) \
> +	. = ALIGN(_ALIGNED_);						\
>  	_BEGIN_##_label_ = .;						\
>  	KEEP(*(_sec_))							\
>  	_END_##_label_ = .;
>  
> -#define BOUNDED_SECTION_POST_LABEL(_sec_, _label_, _BEGIN_, _END_)	\
> +#define BOUNDED_SECTION_PRE_LABEL(_sec_, _label_, _BEGIN_, _END_)	\
> +	BOUNDED_SECTION_PRE_LABEL_ALIGNED(_sec_, _label_, _BEGIN_, _END_, 8)

[Severity: High]
Does this implicit 8-byte alignment break the .notes ELF section by
injecting padding?

The NOTES macro in vmlinux.lds.h defines the .notes output section, which
inherits a 4-byte alignment from its input *(.note.*) sections. It often
follows RO_EXCEPTION_TABLE, which can end on a 4-byte boundary.

If the output section VMA is not 8-byte aligned, this new explicit
. = ALIGN(8) inside the BOUNDED_SECTION_BY macro will inject 4 bytes of
zero padding at the beginning of the .notes section.

This padding can be misinterpreted as an empty note header by external
tools and the kernel, breaking PT_NOTE parsing. Should the NOTES macro be
updated to use BOUNDED_SECTION_BY_ALIGNED(.note.*, _notes, 4)?

> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index 9c61dd083f26a..477333bdf3d35 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
[ ... ]
> @@ -650,7 +650,7 @@
>  #define BTF								\
>  	. = ALIGN(PAGE_SIZE);						\
>  	.BTF : AT(ADDR(.BTF) - LOAD_OFFSET) {				\
> -		BOUNDED_SECTION_BY(.BTF, _BTF)				\
> +		BOUNDED_SECTION_BY_ALIGNED(.BTF, _BTF, PAGE_SIZE)	\
>  	}								\
>  	. = ALIGN(PAGE_SIZE);						\
>  	.BTF_ids : AT(ADDR(.BTF_ids) - LOAD_OFFSET) {			\

[Severity: Low]
The commit message states "update BOUNDED_SECTION uses inside . = ALIGN(x)
stanzas to use _ALIGNED variants".

Was .BTF_ids intentionally omitted from this rule?

It sits right below .BTF and also has an outer . = ALIGN(PAGE_SIZE), but
still uses the base BOUNDED_SECTION_BY macro. This doesn't cause functional
issues since it is already page-aligned, but it leaves the macro usage
inconsistent with the stated refactoring rules.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260523-dd-maint-2-v2-0-b937312aa083@gmail.com?part=5

  reply	other threads:[~2026-05-23  7:42 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-23  7:14 [PATCH v2 00/24] dynamic-debug cleanups refactors maintenance + alignment fix Jim Cromie
2026-05-23  7:14 ` [PATCH v2 01/24] docs/dyndbg: update examples \012 to \n Jim Cromie
2026-05-23  7:14 ` [PATCH v2 02/24] docs/dyndbg: explain flags parse 1st Jim Cromie
2026-05-23  7:14 ` [PATCH v2 03/24] vmlinux.lds.h: refactor BOUNDED_SECTION_* macros into bounded_sections.lds.h Jim Cromie
2026-05-23  7:14 ` [PATCH v2 04/24] vmlinux.lds.h: drop unused HEADERED_SECTION* macros Jim Cromie
2026-05-23  7:14 ` [PATCH v2 05/24] vmlinux.lds.h: Fix ALIGN(8) omission causing NULL ptr on i386 Jim Cromie
2026-05-23  7:42   ` sashiko-bot [this message]
2026-05-23  7:14 ` [PATCH v2 06/24] vmlinux.lds.h: remove redundant ALIGN(8) directives Jim Cromie
2026-05-23  7:14 ` [PATCH v2 07/24] dyndbg.lds.S: fix lost dyndbg sections in modules Jim Cromie
2026-05-23  7:14 ` [PATCH v2 08/24] dyndbg: factor ddebug_match_desc out from ddebug_change Jim Cromie
2026-05-23  7:14 ` [PATCH v2 09/24] dyndbg: add stub macro for DECLARE_DYNDBG_CLASSMAP Jim Cromie
2026-05-23  7:14 ` [PATCH v2 10/24] dyndbg: reword "class unknown," to "class:_UNKNOWN_" Jim Cromie
2026-05-23  7:14 ` [PATCH v2 11/24] dyndbg-API: remove DD_CLASS_TYPE_(DISJOINT|LEVEL)_NAMES and code Jim Cromie
2026-05-23  7:33   ` sashiko-bot
2026-05-23  7:14 ` [PATCH v2 12/24] dyndbg: drop NUM_TYPE_ARGS Jim Cromie
2026-05-23  7:32   ` sashiko-bot
2026-05-23  7:14 ` [PATCH v2 13/24] dyndbg: reduce verbose/debug clutter Jim Cromie
2026-05-23  7:30   ` sashiko-bot
2026-05-23  7:14 ` [PATCH v2 14/24] dyndbg: refactor param_set_dyndbg_classes and below Jim Cromie
2026-05-23  7:14 ` [PATCH v2 15/24] dyndbg: tighten fn-sig of ddebug_apply_class_bitmap Jim Cromie
2026-05-23  7:14 ` [PATCH v2 16/24] dyndbg: replace classmap list with an array-slice Jim Cromie
2026-05-23  7:41   ` sashiko-bot
2026-05-23  7:14 ` [PATCH v2 17/24] dyndbg: macrofy a 2-index for-loop pattern Jim Cromie
2026-05-23  7:14 ` [PATCH v2 18/24] dyndbg: Upgrade class param storage to u64 for 64-bit classmaps Jim Cromie
2026-05-23  7:42   ` sashiko-bot
2026-05-23  7:14 ` [PATCH v2 19/24] dyndbg,module: make proper substructs in _ddebug_info Jim Cromie
2026-05-23  7:45   ` sashiko-bot
2026-05-25  9:24   ` Petr Pavlu
2026-05-23  7:14 ` [PATCH v2 20/24] dyndbg: move mod_name down from struct ddebug_table to _ddebug_info Jim Cromie
2026-05-23  7:14 ` [PATCH v2 21/24] dyndbg: hoist classmap-filter-by-modname up to ddebug_add_module Jim Cromie
2026-05-23  7:45   ` sashiko-bot
2026-05-23  7:14 ` [PATCH v2 22/24] selftests-dyndbg: add a dynamic_debug run_tests target Jim Cromie
2026-05-23  7:37   ` sashiko-bot
2026-05-23  7:14 ` [PATCH v2 23/24] dyndbg: change __dynamic_func_call_cls* macros into expressions Jim Cromie
2026-05-23  7:14 ` [PATCH v2 24/24] dyndbg: improve section names Jim Cromie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260523074230.4F5CF1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=jim.cromie@gmail.com \
    --cc=linux-modules@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.