Hi Aaron,

On Wed, 2026-01-07 at 18:43 -0500, Aaron Merey wrote:
> Currently gelf_getmove does not distinguish between ELFCLASS32 and
> ELFCLASS64 binaries.  This is assumes that Elf32_Move and Elf64_Move
> structs are the same size.
> 
> This assumption is false since the m_info and m_poffset fields have
> type uint32_t for Elf32_Move but uint64_t for Elf64_Move.
> 
> Fix this by handling ELFCLASS32 and ELFCLASS64 cases separately when
> copying Elfxx_Move fields to dst.

Nice catch. So even though this is defined in glibc's elf.h (which we
copy into libelf), this was obviously never used. Unsurprisingly
because Move Sections seem a Solaris only feature. This cleanup looks
good, cleanly separating out the 32 and 64 bit cases.

Thanks,

Mark

> Signed-off-by: Aaron Merey <[email protected]>
> ---
>  libelf/gelf_getmove.c | 49 +++++++++++++++++++++++++++++++------------
>  1 file changed, 36 insertions(+), 13 deletions(-)
> 
> diff --git a/libelf/gelf_getmove.c b/libelf/gelf_getmove.c
> index 18efedcc..20fb54d5 100644
> --- a/libelf/gelf_getmove.c
> +++ b/libelf/gelf_getmove.c
> @@ -53,27 +53,50 @@ gelf_getmove (Elf_Data *data, int ndx, GElf_Move *dst)
>        return NULL;
>      }
>  
> -  /* The types for 32 and 64 bit are the same.  Lucky us.  */
> -  assert (sizeof (GElf_Move) == sizeof (Elf32_Move));
> -  assert (sizeof (GElf_Move) == sizeof (Elf64_Move));
> +  elf = ((Elf_Data_Scn *) data)->s->elf;
> +  rwlock_rdlock (elf->lock);
>  
> -  /* The data is already in the correct form.  Just make sure the
> -     index is OK.  */
> -  if (INVALID_NDX (ndx, GElf_Move, data))
> +  if (elf->class == ELFCLASS32)
>      {
> -      __libelf_seterrno (ELF_E_INVALID_INDEX);
> -      goto out;
> +      Elf32_Move *src;
> +
> +      if (INVALID_NDX (ndx, Elf32_Move, data))
> +     {
> +       __libelf_seterrno (ELF_E_INVALID_INDEX);
> +       goto out;
> +     }
> +
> +      src = &((Elf32_Move *) data->d_buf)[ndx];
> +
> +      /* The following copies may perform zero-extension. m_info can be
> +      copied directly since ELF32_M_* and ELF64_M_* are the same.  */
> +#define COPY(name) \
> +      dst->name = src->name
> +      COPY (m_value);
> +      COPY (m_info);
> +      COPY (m_poffset);
> +      COPY (m_repeat);
> +      COPY (m_stride);
>      }
> +  else
> +    {
> +      eu_static_assert (sizeof (GElf_Move) == sizeof (Elf64_Move));
>  
> -  elf = ((Elf_Data_Scn *) data)->s->elf;
> -  rwlock_rdlock (elf->lock);
> -
> -  *dst = ((GElf_Move *) data->d_buf)[ndx];
> +      /* The data is already in the correct form.  Just make sure the
> +      index is OK.  */
> +      if (INVALID_NDX (ndx, GElf_Move, data))
> +     {
> +       __libelf_seterrno (ELF_E_INVALID_INDEX);
> +       goto out;
> +     }
>  
> -  rwlock_unlock (elf->lock);
> +      *dst = ((GElf_Move *) data->d_buf)[ndx];
> +    }
>  
>    result = dst;
>  
>   out:
> +  rwlock_unlock (elf->lock);
> +
>    return result;
>  }

Reply via email to