Re: [PATCH] module: validate string table section types

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

On 7/9/26 3:56 AM, Thiébaud Weksteen wrote:
> On Thu, Jul 9, 2026 at 1:40 AM Petr Pavlu <petr.pavlu@suse.com> wrote:
>>
>> On 7/8/26 3:21 AM, Thiébaud Weksteen wrote:
>>> In elf_validity_cache_sechdrs, section sizes and offsets are validated,
>>> unless the section type is SHT_NULL or SHT_NOBITS.
>>>
>>> Later, elf_validity_cache_secstrings and elf_validity_cache_index_str
>>> access the section name table (.shstrtab) and symbol string table
>>> (.strtab) headers without first ensuring that their types are
>>> SHT_STRTAB. If a section type is SHT_NULL or SHT_NOBITS, sh_offset has
>>> not been validated and may reference out-of-bounds memory when
>>> dereferenced in elf_validity_cache_secstrings or
>>> elf_validity_cache_strtab.
>>>
>>> Validate that both string section headers are of type SHT_STRTAB before
>>> caching them.
>>
>> The module loader should normally at least get through the signature and
>> blacklist checks without crashing due to a corrupted module ELF file.
>> Failing to validate the offset+size of .shstrtab means the module loader
>> could crash before the blacklist check, so I believe it is useful to add
>> this validation.
>>
>> How did you run into this issue? Was it observed in practice with the
>> GNU or LLVM toolchain, or with some manually crafted module?
> 
> Thanks for the review Petr. I found the issue while reading the code.
> I am working on a separate commit that reuses some of the ELF parsing
> logic (in a different subsystem, with simpler assertions). I was able
> to confirm with a basic PoC (reusing a valid .ko and replacing the
> type and offset of .shstrtab).

Thanks for the explanation. The change looks ok to me.

Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>

-- Petr