Re: [PATCH] Simplify note processing

["Dmitry V. Levin" <ldv at altlinux dot org>]

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.

I think there should be an explanation e.g. in the commit message why
"Some PT_NOTE segment may have alignment value of 0 or 1" statement
is no longer true.

Besides that, there is a "free (abi_note_malloced)" right after the "if"
statement what looks suspicious: if free() is a no-op in this context,
why bother?  If free() is not a no-op, then it's a chance of double free.


-- 
ldv

Attachment: signature.asc
Description: PGP signature