Re: [PATCH] lib/bug: ignore non-allocated module bug tables

[Petr Pavlu <petr.pavlu@suse.com>]

On 7/29/26 7:42 PM, Laxman Acharya Padhya wrote:
> rewrite_section_headers() initially sets sh_addr for every section to its
> address in the temporary module image. Only SHF_ALLOC sections are later
> copied to module memory and have sh_addr updated.
> 
> module_bug_finalize() accepts any section named __bug_table. A malformed
> module can therefore leave mod->bug_table pointing into the temporary
> image, which is freed after loading. A later BUG/WARN lookup would then
> scan freed memory.
> 
> Require the bug table section to be allocated before retaining its
> address. Valid modules are unchanged because __bug_table is allocated.
> 
> Fixes: 7664c5a1da47 ("[PATCH] Generic BUG implementation")
> Signed-off-by: Laxman Acharya Padhya <acharyalaxman8848@gmail.com>
> ---
>  lib/bug.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/bug.c b/lib/bug.c
> index 7c1c2c27f..44966ead1 100644
> --- a/lib/bug.c
> +++ b/lib/bug.c
> @@ -94,7 +94,8 @@ void module_bug_finalize(const Elf_Ehdr *hdr, const Elf_Shdr *sechdrs,
>  	/* Find the __bug_table section, if present */
>  	secstrings = (char *)hdr + sechdrs[hdr->e_shstrndx].sh_offset;
>  	for (i = 1; i < hdr->e_shnum; i++) {
> -		if (strcmp(secstrings+sechdrs[i].sh_name, "__bug_table"))
> +		if (strcmp(secstrings + sechdrs[i].sh_name, "__bug_table") ||
> +		    !(sechdrs[i].sh_flags & SHF_ALLOC))
>  			continue;
>  		mod->bug_table = (void *) sechdrs[i].sh_addr;
>  		mod->num_bugs = sechdrs[i].sh_size / sizeof(struct bug_entry);

This change looks ok to me but I think it is not strictly necessary.

Loading a module should normally at least get through the signature and
blacklist checks without crashing due to a corrupted module ELF file.
After that point, I believe the ELF data should be trusted, similar to
how the actual module code is expected to be correct.

module_bug_finalize() is called fairly late in the module-loading
process, after the signature and blacklist checks.

-- 
Thanks,
Petr