On Wed, Aug 19, 2026 at 09:15:27PM +0900, HyeongJun An wrote: > 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.
You are correct. I would assume that if e_shnum is 0, then e_shstrndx would also be 0. If it isn't the case then your patch is valid. I will queue it when rc1 comes out. Thanks, Mathieu > 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!
