* [PATCH] module: validate string table section types
@ 2026-07-08 1:21 Thiébaud Weksteen
2026-07-08 1:33 ` sashiko-bot
2026-07-08 15:40 ` Petr Pavlu
0 siblings, 2 replies; 5+ messages in thread
From: Thiébaud Weksteen @ 2026-07-08 1:21 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen
Cc: Thiébaud Weksteen, Aaron Tomlin, Siddharth Nayyar,
linux-modules, linux-kernel
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.
Signed-off-by: Thiébaud Weksteen <tweek@google.com>
---
kernel/module/main.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 46dd8d25a605..7cbc8f0e28c6 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2011,6 +2011,7 @@ static int elf_validity_cache_sechdrs(struct load_info *info)
* Specifically checks:
*
* * Section name table index is inbounds of section headers
+ * * Section name table type is SHT_STRTAB
* * Section name table is not empty
* * Section name table is NUL terminated
* * All section name offsets are inbounds of the section
@@ -2038,6 +2039,11 @@ static int elf_validity_cache_secstrings(struct load_info *info)
strhdr = &info->sechdrs[info->hdr->e_shstrndx];
+ if (strhdr->sh_type != SHT_STRTAB) {
+ pr_err("Invalid ELF section name table type: %u\n", strhdr->sh_type);
+ return -ENOEXEC;
+ }
+
/*
* The section name table must be NUL-terminated, as required
* by the spec. This makes strcmp and pr_* calls that access
@@ -2204,7 +2210,7 @@ static int elf_validity_cache_index_sym(struct load_info *info)
* Must have &load_info->index.sym populated.
*
* Looks at the symbol table's associated string table, makes sure it is
- * in-bounds, and caches it.
+ * in-bounds and of type SHT_STRTAB, and caches it.
*
* Return: %0 if valid, %-ENOEXEC on failure.
*/
@@ -2218,6 +2224,12 @@ static int elf_validity_cache_index_str(struct load_info *info)
return -ENOEXEC;
}
+ if (info->sechdrs[str_idx].sh_type != SHT_STRTAB) {
+ pr_err("Invalid ELF symbol string table type: %u\n",
+ info->sechdrs[str_idx].sh_type);
+ return -ENOEXEC;
+ }
+
info->index.str = str_idx;
return 0;
}
--
2.55.0.795.g602f6c329a-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] module: validate string table section types
2026-07-08 1:21 [PATCH] module: validate string table section types Thiébaud Weksteen
@ 2026-07-08 1:33 ` sashiko-bot
2026-07-08 15:40 ` Petr Pavlu
1 sibling, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-07-08 1:33 UTC (permalink / raw)
To: Thiébaud Weksteen; +Cc: linux-modules
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] The patch fixes an out-of-bounds read vulnerability for string tables by ensuring they are of type SHT_STRTAB, but fails to address the exact same vulnerability class for several other ELF sections (e.g., .modinfo, .gnu.linkonce.this_module, __version_ext_names, __ksymtab).
- [High] Iterating over sections in lookup functions triggers an out-of-bounds string read for SHT_NULL sections due to missing sh_name bounds checking.
- [High] Validation logic for section headers accesses sechdrs[0] out of bounds if e_shnum is 0 and e_shoff is near the end of the file.
--
--- Patch [1]: [PATCH] module: validate string table section types ---
commit e8e815fedf365febe70f5870d807616d37946281
Author: Thiébaud Weksteen <tweek@google.com>
module: validate string table section types
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.
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index 46dd8d25a6058..7cbc8f0e28c61 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -2011,6 +2011,7 @@ static int elf_validity_cache_sechdrs(struct load_info *info)
[Severity: High]
This is a pre-existing issue, but does the validation logic in
elf_validity_cache_sechdrs() access sechdrs out of bounds if e_shnum is 0
and e_shoff is near the end of the file?
The overflow check evaluates to 0 > positive if e_shnum is 0 and e_shoff is
less than info->len, allowing the check to incorrectly pass:
elf_validity_cache_sechdrs() {
...
if (info->hdr->e_shoff >= info->len
|| (info->hdr->e_shnum * sizeof(Elf_Shdr) >
info->len - info->hdr->e_shoff)) {
...
}
The code then sets sechdrs and blindly accesses sechdrs[0], reading past the
end of the user-provided module buffer:
elf_validity_cache_sechdrs() {
...
if (sechdrs[0].sh_type != SHT_NULL
|| sechdrs[0].sh_size != 0
|| sechdrs[0].sh_addr != 0) {
...
}
> * Specifically checks:
> *
> * * Section name table index is inbounds of section headers
> + * * Section name table type is SHT_STRTAB
> * * Section name table is not empty
> * * Section name table is NUL terminated
[ ... ]
> @@ -2038,6 +2039,11 @@ static int elf_validity_cache_secstrings(struct load_info *info)
>
> strhdr = &info->sechdrs[info->hdr->e_shstrndx];
>
> + if (strhdr->sh_type != SHT_STRTAB) {
> + pr_err("Invalid ELF section name table type: %u\n", strhdr->sh_type);
> + return -ENOEXEC;
> + }
> +
[Severity: High]
This is a pre-existing issue, but does iterating over sections in lookup
functions trigger an out-of-bounds string read for SHT_NULL sections due to
missing sh_name bounds checking?
In elf_validity_cache_secstrings(), bounds validation of sh_name against the
string table size is explicitly skipped for SHT_NULL sections:
elf_validity_cache_secstrings() {
...
if (shdr->sh_type == SHT_NULL)
continue;
...
}
Later, section lookup functions like find_any_unique_sec() iterate over all
sections and blindly pass the unvalidated info->secstrings pointer to strcmp():
find_any_unique_sec() {
...
if (strcmp(info->secstrings + info->sechdrs[i].sh_name,
name) == 0) {
...
}
Could this cause an immediate out-of-bounds memory read when strcmp()
executes if an attacker sets a huge sh_name on a SHT_NULL section?
[ ... ]
> @@ -2218,6 +2224,12 @@ static int elf_validity_cache_index_str(struct load_info *info)
> return -ENOEXEC;
> }
>
> + if (info->sechdrs[str_idx].sh_type != SHT_STRTAB) {
> + pr_err("Invalid ELF symbol string table type: %u\n",
> + info->sechdrs[str_idx].sh_type);
> + return -ENOEXEC;
> + }
> +
[Severity: High]
This is a pre-existing issue, but while this patch fixes an out-of-bounds
read vulnerability for string tables by ensuring they are of type SHT_STRTAB,
does the same vulnerability class exist for other ELF sections?
Since elf_validity_cache_sechdrs() skips bounds checking of sh_offset and
sh_size for SHT_NULL and SHT_NOBITS sections, lookup functions can match
sections by name while ignoring sh_type.
For example, elf_validity_cache_index_info() looks up .modinfo by name and
caches it without validating its sh_type:
elf_validity_cache_index_info() {
...
info_idx = find_any_unique_sec(info, ".modinfo");
...
}
Similarly, in elf_validity_cache_index_mod(), the .gnu.linkonce.this_module
section rejects SHT_NOBITS but fails to reject SHT_NULL:
elf_validity_cache_index_mod() {
...
if (shdr->sh_type == SHT_NOBITS) {
...
}
And in move_module(), the loop skips SHT_NOBITS but calls memcpy for
SHT_NULL:
move_module() {
...
memcpy(dest, (void *)shdr->sh_addr, shdr->sh_size);
...
}
Could a malicious module supply these sections with type SHT_NULL and an
arbitrary, unvalidated sh_offset, leading to out-of-bounds reads or copies?
> info->index.str = str_idx;
> return 0;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260708012107.1621513-1-tweek@google.com?part=1
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] module: validate string table section types
2026-07-08 1:21 [PATCH] module: validate string table section types Thiébaud Weksteen
2026-07-08 1:33 ` sashiko-bot
@ 2026-07-08 15:40 ` Petr Pavlu
2026-07-09 1:56 ` Thiébaud Weksteen
1 sibling, 1 reply; 5+ messages in thread
From: Petr Pavlu @ 2026-07-08 15:40 UTC (permalink / raw)
To: Thiébaud Weksteen
Cc: Luis Chamberlain, Daniel Gomez, Sami Tolvanen, Aaron Tomlin,
Siddharth Nayyar, linux-modules, linux-kernel
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,
Petr
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] module: validate string table section types
2026-07-08 15:40 ` Petr Pavlu
@ 2026-07-09 1:56 ` Thiébaud Weksteen
2026-07-09 9:23 ` Petr Pavlu
0 siblings, 1 reply; 5+ messages in thread
From: Thiébaud Weksteen @ 2026-07-09 1:56 UTC (permalink / raw)
To: Petr Pavlu
Cc: Luis Chamberlain, Daniel Gomez, Sami Tolvanen, Aaron Tomlin,
Siddharth Nayyar, linux-modules, linux-kernel
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).
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] module: validate string table section types
2026-07-09 1:56 ` Thiébaud Weksteen
@ 2026-07-09 9:23 ` Petr Pavlu
0 siblings, 0 replies; 5+ messages in thread
From: Petr Pavlu @ 2026-07-09 9:23 UTC (permalink / raw)
To: Thiébaud Weksteen
Cc: Luis Chamberlain, Daniel Gomez, Sami Tolvanen, Aaron Tomlin,
Siddharth Nayyar, linux-modules, linux-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-07-09 9:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-08 1:21 [PATCH] module: validate string table section types Thiébaud Weksteen
2026-07-08 1:33 ` sashiko-bot
2026-07-08 15:40 ` Petr Pavlu
2026-07-09 1:56 ` Thiébaud Weksteen
2026-07-09 9:23 ` Petr Pavlu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox