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 <[email protected]>
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/[email protected]?part=2

Reply via email to