Thanks for taking a look!
On Wed, Aug 19, 2026 at 1:42 AM Mathieu Poirier
<[email protected]> wrote:
> On Thu, 23 Jul 2026 at 20:53, HyeongJun An <[email protected]> wrote:
> > + /* find_table() reads the header at shstrndx even with no sections
> > */
>
> Right, but if there is no sections, @shnum in find_tables is 0 and not
> arm is done.
The loop is skipped, but there is one load before it:
:266 name_table_shdr = shdr + (shstrndx * elf_shdr_get_size);
:268 name_table = elf_data + elf_shdr_get_sh_offset(class, name_table_shdr);
:270 for (i = 0; i < shnum; i++, shdr += elf_shdr_get_size) {
:268 reads sh_offset out of the header at index shstrndx, before shnum is
tested at :270. Both callers, :336 and :380, reach find_table() without
checking shnum. The value is unused when shnum is 0, so it is a 4 or 8
byte read past the buffer and nothing worse.
> > + shend = size_add(size_mul(elf_shdr_get_size, (size_t)shstrndx + 1),
> > shoff);
>
> Why the shstrndx + 1?
To hold the header at index shstrndx the table needs shstrndx + 1 entries.
It is keyed on shstrndx and not shnum because nothing here requires
e_shstrndx < e_shnum.
> Also, there is no point in doing this check if @shnum is 0. Please
> move this block in the "if (shnum)".
The check it replaces, :95, sits outside any shnum test today, so moving
it in loses what master already has.
If you want it inside, the way there is to drop the read:
if (!shnum)
return NULL;
at the top of find_table(). The loop already falls through to return NULL
at :313, so it changes nothing today, and the bound can then go inside
"if (shnum)".
I am happy to do either. Let me know which you would prefer for v2.
Thanks a lot!
