Re: [PATCH] Simplify note processing

[Florian Weimer <fweimer at redhat dot com>]

* Dmitry V. Levin:

> On Thu, Oct 10, 2019 at 12:26:37PM +0200, Andreas Schwab wrote:
>> On Okt 10 2019, "Dmitry V. Levin" <ldv@altlinux.org> wrote:
>> 
>> > On Thu, Oct 10, 2019 at 10:48:29AM +0200, Andreas Schwab wrote:
>> >> This removes dead code during note processing.
>> >> 
>> >> * elf/dl-load.c (open_verify): Remove dead code.
>> >> ---
>> >>  elf/dl-load.c | 17 +++--------------
>> >>  1 file changed, 3 insertions(+), 14 deletions(-)
>> >> 
>> >> diff --git a/elf/dl-load.c b/elf/dl-load.c
>> >> index 24e2819345..1ed7a7bbd6 100644
>> >> --- a/elf/dl-load.c
>> >> +++ b/elf/dl-load.c
>> >> @@ -1682,21 +1682,10 @@ open_verify (const char *name, int fd,
>> >>  
>> >>        /* Check .note.ABI-tag if present.  */
>> >>        for (ph = phdr; ph < &phdr[ehdr->e_phnum]; ++ph)
>> >> -	if (ph->p_type == PT_NOTE && ph->p_filesz >= 32 && ph->p_align >= 4)
>> >> +	if (ph->p_type == PT_NOTE && ph->p_filesz >= 32
>> >> +	    && (ph->p_align == 4 || ph->p_align == 8))
>> >>  	  {
>> >>  	    ElfW(Addr) size = ph->p_filesz;
>> >> -	    /* NB: Some PT_NOTE segment may have alignment value of 0
>> >> -	       or 1.  gABI specifies that PT_NOTE segments should be
>> >> -	       aligned to 4 bytes in 32-bit objects and to 8 bytes in
>> >> -	       64-bit objects.  As a Linux extension, we also support
>> >> -	       4 byte alignment in 64-bit objects.  If p_align is less
>> >> -	       than 4, we treate alignment as 4 bytes since some note
>> >> -	       segments have 0 or 1 byte alignment.   */
>> >> -	    ElfW(Addr) align = ph->p_align;
>> >> -	    if (align < 4)
>> >> -	      align = 4;
>> >> -	    else if (align != 4 && align != 8)
>> >> -	      continue;
>> >
>> > This effectively removes support of ph->p_align < 4.
>> 
>> No.  It was never supported.  The condition ph->p_align >= 4 has been
>> there from the beginning.
>
> Indeed, and the "free (abi_note_malloced)" is actually outside the loop,
> I must've completely misread the code, sorry for the noise.
>
> The change is fine then.

I agree.  I believe alignment of 0 and 1 is only encountered in Linux
core files, which does not matter to this note parser.

Thanks,
Florian