[PATCH] libelf: Simplify gelf_update_phdr and gelf_getphdr ndx check

[Mark Wielaard]

The gelf_update_phdr and gelf_getphdr ndx check didn't reject negative
ndx. And gelf_update_phdr checked against ehdr->e_phnum and checked
for PN_XNUM, while __elf_getphdrnum_rdlock already handles that case.

Simplify the ndx check by rejecting negative indexes and then simply
check the ndx against the phnum __elf_getphdrnum_rdlock (as
gelf_getphdr already did). This makes the check class independent, so
we only need the check once.

        * libelf/gelf_getphdr.c (gelf_getphdr): Reject negative ndx.
        * libelf/gelf_update_phdr.c (gelf_update_phdr): Likewise and
        just call __elf_getphdrnum_rdlock directly.

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 libelf/gelf_getphdr.c     | 28 ++++++++++------------------
 libelf/gelf_update_phdr.c | 32 ++++++++++----------------------
 2 files changed, 20 insertions(+), 40 deletions(-)

diff --git a/libelf/gelf_getphdr.c b/libelf/gelf_getphdr.c
index c719e4be6966..6c85cecfe41b 100644
--- a/libelf/gelf_getphdr.c
+++ b/libelf/gelf_getphdr.c
@@ -60,6 +60,16 @@ gelf_getphdr (Elf *elf, int ndx, GElf_Phdr *dst)
 
   rwlock_rdlock (elf->lock);
 
+  /* Test whether the index is ok.  */
+  size_t phnum;
+  if (ndx < 0
+      || __elf_getphdrnum_chk_rdlock (elf, &phnum) != 0
+      || (size_t) ndx >= phnum)
+    {
+      __libelf_seterrno (ELF_E_INVALID_INDEX);
+      goto out;
+    }
+
   if (elf->class == ELFCLASS32)
     {
       /* Copy the elements one-by-one.  */
@@ -75,15 +85,6 @@ gelf_getphdr (Elf *elf, int ndx, GElf_Phdr *dst)
          rwlock_rdlock (elf->lock);
        }
 
-      /* Test whether the index is ok.  */
-      size_t phnum;
-      if (__elf_getphdrnum_chk_rdlock (elf, &phnum) != 0
-         || (size_t) ndx >= phnum)
-       {
-         __libelf_seterrno (ELF_E_INVALID_INDEX);
-         goto out;
-       }
-
       /* We know the result now.  */
       result = dst;
 
@@ -115,15 +116,6 @@ gelf_getphdr (Elf *elf, int ndx, GElf_Phdr *dst)
          rwlock_rdlock (elf->lock);
        }
 
-      /* Test whether the index is ok.  */
-      size_t phnum;
-      if (__elf_getphdrnum_chk_rdlock (elf, &phnum) != 0
-         || (size_t) ndx >= phnum)
-       {
-         __libelf_seterrno (ELF_E_INVALID_INDEX);
-         goto out;
-       }
-
       /* We only have to copy the data.  */
       result = memcpy (dst, phdr + ndx, sizeof (GElf_Phdr));
     }
diff --git a/libelf/gelf_update_phdr.c b/libelf/gelf_update_phdr.c
index a848677fa7fe..4566328f5aa7 100644
--- a/libelf/gelf_update_phdr.c
+++ b/libelf/gelf_update_phdr.c
@@ -53,6 +53,16 @@ gelf_update_phdr (Elf *elf, int ndx, GElf_Phdr *src)
 
   rwlock_wrlock (elf->lock);
 
+  /* Test whether the index is ok.  */
+  size_t phnum;
+  if (ndx < 0
+      || __elf_getphdrnum_chk_rdlock (elf, &phnum) != 0
+      || (size_t) ndx >= phnum)
+    {
+      __libelf_seterrno (ELF_E_INVALID_INDEX);
+      goto out;
+    }
+
   if (elf->class == ELFCLASS32)
     {
       Elf32_Phdr *phdr = elf->state.elf32.phdr;
@@ -79,17 +89,6 @@ gelf_update_phdr (Elf *elf, int ndx, GElf_Phdr *src)
            goto out;
        }
 
-      /* Test whether the index is ok.  */
-      size_t phnum;
-      if (ndx >= elf->state.elf32.ehdr->e_phnum
-         && (elf->state.elf32.ehdr->e_phnum != PN_XNUM
-             || __elf_getphdrnum_rdlock (elf, &phnum) != 0
-             || (size_t) ndx >= phnum))
-       {
-         __libelf_seterrno (ELF_E_INVALID_INDEX);
-         goto out;
-       }
-
       /* Now correct the pointer to point to the correct element.  */
       phdr += ndx;
 
@@ -116,17 +115,6 @@ gelf_update_phdr (Elf *elf, int ndx, GElf_Phdr *src)
            goto out;
        }
 
-      /* Test whether the index is ok.  */
-      size_t phnum;
-      if (ndx >= elf->state.elf64.ehdr->e_phnum
-         && (elf->state.elf64.ehdr->e_phnum != PN_XNUM
-             || __elf_getphdrnum_rdlock (elf, &phnum) != 0
-             || (size_t) ndx >= phnum))
-       {
-         __libelf_seterrno (ELF_E_INVALID_INDEX);
-         goto out;
-       }
-
       /* Just copy the data.  */
       memcpy (phdr + ndx, src, sizeof (Elf64_Phdr));
     }
-- 
2.56.0