Re: [PATCH] remoteproc: elf_loader: bound the section header table

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!