[PATCH] module: validate string table section types

[PATCH] module: validate string table section types

[Thiébaud Weksteen]

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

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

[Petr Pavlu]

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

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

[Thiébaud Weksteen]

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).

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

[Petr Pavlu]

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

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

[Markus Elfring]

…
> Validate that both string section headers are of type SHT_STRTAB before
> caching them.

How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?

Regards,
Markus

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

[Aaron Tomlin]

On Wed, Jul 08, 2026 at 11:21:07AM +1000, 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.
> 
> 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
> 

Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>

-- 
Aaron Tomlin

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

[Aaron Tomlin]

On Fri, Jul 10, 2026 at 10:23:00AM +0200, Markus Elfring wrote:
> …
> > Validate that both string section headers are of type SHT_STRTAB before
> > caching them.
> 
> How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?

+1

-- 
Aaron Tomlin

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

[Thiébaud Weksteen]

On Mon, Jul 13, 2026 at 10:14 PM Aaron Tomlin <atomlin@atomlin.com> wrote:
>
> On Fri, Jul 10, 2026 at 10:23:00AM +0200, Markus Elfring wrote:
> > …
> > > Validate that both string section headers are of type SHT_STRTAB before
> > > caching them.
> >
> > How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?

Thanks for the suggestion. I can't seem to find any relevant commit
for Fixes. There are
ec2a29593c83e ("module: harden ELF info handling") and potentially
3c5700aeabd87 ("module: Factor out elf_validity_cache_secstrings")
Both of these commits already intended to harden the parsing. I don't
think there is any regression here.

As for Cc:, I can't think of anyone who should be explicitly added. If
you have any suggestions, please chime in.

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

[Petr Pavlu]

On 7/14/26 1:37 AM, Thiébaud Weksteen wrote:
> On Mon, Jul 13, 2026 at 10:14 PM Aaron Tomlin <atomlin@atomlin.com> wrote:
>>
>> On Fri, Jul 10, 2026 at 10:23:00AM +0200, Markus Elfring wrote:
>>> …
>>>> Validate that both string section headers are of type SHT_STRTAB before
>>>> caching them.
>>>
>>> How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?
> 
> Thanks for the suggestion. I can't seem to find any relevant commit
> for Fixes. There are
> ec2a29593c83e ("module: harden ELF info handling") and potentially
> 3c5700aeabd87 ("module: Factor out elf_validity_cache_secstrings")
> Both of these commits already intended to harden the parsing. I don't
> think there is any regression here.

I agree.

> As for Cc:, I can't think of anyone who should be explicitly added. If
> you have any suggestions, please chime in.

I'll add 'Cc: stable@vger.kernel.org' when queuing the patch.

-- 
Thanks,
Petr