Re: [PATCH] module: reject malformed .modinfo sections

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

Pre-existing issues:
- [High] The `.modinfo` section bypasses bounds validation if its type is s=
et to `SHT_NOBITS`, allowing arbitrary `sh_offset` and `sh_size` values tha=
t lead to out-of-bounds kernel memory reads.
- [High] The patch fails to protect against out-of-bounds reads if the `.mo=
dinfo` section is completely omitted, falling back to an unvalidated `SHT_N=
ULL` section offset.
--

--- Patch [1]: [PATCH] module: reject malformed .modinfo sections ---
commit 7548fa1ced4767f35fb298eeab3978cae911d641
Author: Laxman Acharya Padhya <[email protected]>

module: reject malformed .modinfo sections

This commit attempts to prevent loaders from reading past the end of the
.modinfo section by rejecting empty sections and those that are not
NUL-terminated before parsing them.

> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index 528690ba160bd..91e20cd1e6ac9 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -2082,6 +2082,8 @@ static int elf_validity_cache_secstrings(struct loa=
d_info *info)
>  static int elf_validity_cache_index_info(struct load_info *info)
>  {
>  	int info_idx;
> +	Elf_Shdr *info_shdr;
> +	char *modinfo;
> =20
>  	info_idx =3D find_any_unique_sec(info, ".modinfo");
> =20

[Severity: High]
This is a pre-existing issue, but does the early return when info_idx is 0
leave us vulnerable if a module omits the .modinfo section completely?

When the subsequent early return for info_idx =3D=3D 0 is taken, info->inde=
x.info
remains 0. Subsequent calls to get_modinfo() will use info->sechdrs[0].
While elf_validity_cache_sechdrs() validates the size and addr of section 0,
it doesn't validate sh_offset:

kernel/module/main.c:elf_validity_cache_sechdrs()
        if (sechdrs[0].sh_type !=3D SHT_NULL
            || sechdrs[0].sh_size !=3D 0
            || sechdrs[0].sh_addr !=3D 0) {

When get_next_modinfo() processes this, it uses this unvalidated sh_offset
to calculate modinfo, which is immediately passed to strncmp():

kernel/module/main.c:get_next_modinfo()
        char *modinfo =3D (char *)info->hdr + infosec->sh_offset;
        ...
        for (p =3D modinfo; p; p =3D module_next_tag_pair(p, &size)) {
                if (strncmp(p, tag, taglen) =3D=3D 0 && p[taglen] =3D=3D '=
=3D')

Could this lead to an out-of-bounds read if sh_offset points outside valid
memory?

> @@ -2095,6 +2097,18 @@ static int elf_validity_cache_index_info(struct lo=
ad_info *info)
>  	}
> =20
>  	info->index.info =3D info_idx;
> +	info_shdr =3D &info->sechdrs[info_idx];
> +	modinfo =3D (char *)info->hdr + info_shdr->sh_offset;
> +
> +	if (info_shdr->sh_size =3D=3D 0) {
> +		pr_err("empty .modinfo section\n");
> +		return -ENOEXEC;
> +	}
> +	if (modinfo[info_shdr->sh_size - 1] !=3D '\0') {

[Severity: High]
This isn't a bug introduced by this patch, but does accessing modinfo here
pose a risk if the .modinfo section type is set to SHT_NOBITS?

In elf_validity_cache_sechdrs(), offset validation is explicitly skipped
for SHT_NOBITS sections:

kernel/module/main.c:elf_validity_cache_sechdrs()
        case SHT_NOBITS:
                /* No contents, offset/size don't mean anything */
                continue;

Since info_shdr->sh_size and sh_offset are unvalidated for these sections,
could modinfo[info_shdr->sh_size - 1] read from unmapped or arbitrary memor=
y?

> +		pr_err(".modinfo section isn't NUL terminated\n");
> +		return -ENOEXEC;
> +	}
> +
>  	/* Try to find a name early so we can log errors with a module name */
>  	info->name =3D get_modinfo(info, "name");
>

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260728173318.6373=
[email protected]?part=3D1