Re: [PATCH 2/2] module: Validate the __version_ext_names section offset

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] module: allows reading arbitrary file data for SHT_NOBITS sections
--

commit 7826bfa932aee0181b2db001dad06d62e3bc7702
Author: Fang Xieyan <fangxy@xiaopeng.com>
module: Validate the __version_ext_names section offset

This commit bounds the offset of the __version_ext_names section to prevent
an out-of-bounds read when encountering a SHT_NOBITS section. It adds a call
to validate_section_offset() in elf_validity_cache_index_versions() before
dereferencing the section's offset.

> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index e36bfe4734d59..adaaaca5a4e68 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -2296,6 +2296,16 @@ static int elf_validity_cache_index_versions(struct load_info *info, int flags)
>  	 * number of entries in every section.
>  	 */
>  	if (vers_ext_crc) {
> +		/*
> +		 * The name section is walked by its sh_offset and sh_size
> +		 * below. elf_validity_check_sectionheaders() exempts SHT_NOBITS
> +		 * sections from validate_section_offset(), so a __version_ext_names
> +		 * section of that type reaches here with an unvalidated sh_offset.
> +		 * Bound it before dereferencing hdr + sh_offset.
> +		 */
> +		if (validate_section_offset(info, &info->sechdrs[vers_ext_name]))
> +			return -ENOEXEC;

[Severity: Low]
Does this allow in-bounds SHT_NOBITS sections to pass, reading arbitrary
in-bounds file data as extended version names?

This would appear to violate ELF semantics since SHT_NOBITS sections are
not supposed to hold file data.

> +
>  		crc_count = info->sechdrs[vers_ext_crc].sh_size / sizeof(u32);
>  		name = (void *)info->hdr +
>  			info->sechdrs[vers_ext_name].sh_offset;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916174753.14870-1-fangxy@xiaopeng.com?part=2