Re: [PATCH v3] ARM: module.lds: fix unwind metadata for merged .text sections

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing NULL check on the return value of `unwind_table_add()` in `module_finalize()` causes a kernel panic on memory allocation failure.
--

commit 0098f68f3e89e8e569da9c5eaaa9486505d4ea3d
Author: Xiao Junzhe <egg12138@foxmail.com>

ARM: module.lds: fix unwind metadata for merged .text sections

Commit 1ba9f8979426 ("vmlinux.lds: Unify TEXT_MAIN, DATA_MAIN,
and related macros") made scripts/module.lds.S merge module input
.text.* sections into the output .text section. On ARM, the paired
unwind input sections keep their original names. This commit ensures
the ARM module unwind output names stay in sync with the text sections.

> diff --git a/arch/arm/include/asm/module.lds.h b/arch/arm/include/asm/module.lds.h
> index 0e7cb4e314b48..1d56c0375c18b 100644
> --- a/arch/arm/include/asm/module.lds.h
> +++ b/arch/arm/include/asm/module.lds.h
> @@ -1,4 +1,16 @@
>  /* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifdef CONFIG_ARM_UNWIND
> +SECTIONS {
> +	.ARM.extab		0 : {
> +		*(.ARM.extab .ARM.extab.text .ARM.extab.text.[0-9a-zA-Z_]*)
> +	}
> +	.ARM.exidx		0 : {
> +		*(.ARM.exidx .ARM.exidx.text .ARM.exidx.text.[0-9a-zA-Z_]*)
> +	}
> +}
> +#endif

[Severity: High]
This is a pre-existing issue and was not introduced by the patch under review,
but while reviewing how ARM module unwind tables are registered, I noticed a
potential concern in arch/arm/kernel/module.c:

arch/arm/kernel/module.c:module_finalize() {
    ...
		if (txt_sec) {
			struct unwind_table *table =
				unwind_table_add(s->sh_addr,
						s->sh_size,
						txt_sec->sh_addr,
						txt_sec->sh_size);

			list_add(&table->mod_list, unwind_list);
    ...
}

Does this code safely handle a null return from unwind_table_add?

If unwind_table_add fails to allocate memory, it returns a null pointer.
Passing that directly to list_add causes an offset-based dereference of null.

Could this trigger a kernel panic during module loading if the system is out
of memory?

> +
>  #ifdef CONFIG_ARM_MODULE_PLTS
>  SECTIONS {
>  	.plt : { BYTE(0) }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_08845B64E5F38EB7FA1779982A071AB4A607@qq.com?part=1