Re: [PATCH v5 02/11] perf tools: Check ELF section type with name

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf symbol-elf: uninitialized memory read on gelf_getshdr failure
--

commit f2750bf0f2a1c49778b5a9d0173f02f48eb9e193
Author: Namhyung Kim <namhyung@kernel.org>

perf tools: Check ELF section type with name

This commit updates filename__has_section() to verify that a section
has the SHT_PROGBITS type instead of just checking its name. This prevents
empty sections with SHT_NOBITS from being incorrectly identified as present.

> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -228,7 +228,8 @@ bool filename__has_section(const char *filename, const char *sec)
>  	if (gelf_getehdr(elf, &ehdr) == NULL)
>  		goto elf_out;
>  
> -	found = !!elf_section_by_name(elf, &ehdr, &shdr, sec, NULL);
> +	found = !!elf_section_by_name(elf, &ehdr, &shdr, sec, NULL) &&
> +		shdr.sh_type == SHT_PROGBITS;

[Severity: Medium]
Can this lead to an uninitialized memory read of shdr.sh_type if the ELF
file is malformed?

Looking at elf_section_by_name(), it does not check the return value of
gelf_getshdr():

tools/perf/util/symbol-elf.c:elf_section_by_name() {
    ...
    gelf_getshdr(sec, shp);
    str = elf_strptr(elf, ep->e_shstrndx, shp->sh_name);
    ...
}

If gelf_getshdr() fails, it returns NULL and leaves the shp struct unmodified.
Because shp points to the stack-allocated shdr in filename__has_section(),
a failure in gelf_getshdr() leaves shdr potentially uninitialized.

Since this patch adds a direct read of shdr.sh_type after elf_section_by_name()
returns, does this new check risk reading uninitialized stack memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013172155.160869-1-namhyung@kernel.org?part=2