[PATCH v3] readelf: Save and dump the original section header values

[Jan Beulich]

On 09.07.2026 08:32, H.J. Lu wrote:
> On Thu, Jul 9, 2026 at 2:06 PM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 04.07.2026 13:06, H.J. Lu wrote:
>>> On Sat, Jul 4, 2026 at 11:01 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> On Sat, Jul 4, 2026 at 8:39 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>> On Sat, Jul 4, 2026 at 7:49 AM Alan Modra <amodra@gmail.com> wrote:
>>>>>>
>>>>>> On Fri, Jul 03, 2026 at 10:30:14PM +0800, H.J. Lu wrote:
>>>>>>> validate_section_info clears the garbage values in the section header
>>>>>>> to avoid crash later.  Save and dump the original section header values
>>>>>>> to make the garbage values in the section header visible when dumping
>>>>>>> section headers.
>>>>>>
>>>>>> I think this would be better done the other way around.  ie. have a
>>>>>> Elf_Internal_Shdr **sane_section_headers that is initialised to point
>>>>>> at entries in section_headers, with sane_section_headers[i] allocated
>>>>>> as necessary when needing to correct a bogus header.
>>>>>>
>>>>>
>>>>> I have thought about something similar and decided against it
>>>>> since filedata->section_headers is used in many places.   However,
>>>>> I can change filedata->orig_section_headers to on demand.
>>
>> I find this a plausible argument, so ...
>>
>>>> Changes in v2:
>>>>
>>>> 1.  filedata->orig_section_headers is changed to on demand.
>>>
>>> Changes in v3:
>>>
>>> 1.  Clear filedata->orig_section_headers first.
>>
>> ... the patch is okay unless you get an objection from Alan within another
> 
> Will do.
> 
>> day or two. Just one thing though: calloc() + memset(, 0,) is redundant.
>> Please drop those memset()s.
> 
> It is cmalloc, not calloc:
> 
>   filedata->orig_section_headers = (Elf_Internal_Shdr **)
>     cmalloc (num, sizeof (Elf_Internal_Shdr *));
> 
> cmalloc doesn't clear memory.  memset is needed.

Seeing that cmalloc() uses xmalloc() (not malloc()), why don't you then
simply use xcmalloc2()?

Further, again seeing that cmalloc() uses xmalloc() - the NULL checks
that you add are then solely to cover the multiplication overflow case.
That doesn't match the use of malloc() elsewhere in the patch.

Taken together, why is it that calloc() isn't used here in the first
place (to match the use of malloc())? There's no truncation risk on the
first argument passed, and surely a sane implementation of calloc() has
to check for overflow of the multiplication when determining overall
size.

Jan