[PATCH] libelf: Use GElf_Ehdr instead of Elf handle in __libelf_data_type

[PATCH] libelf: Use GElf_Ehdr instead of Elf handle in __libelf_data_type

[Mark Wielaard]

GCC with -flto detects some (very) unlikely issues in error paths.
In theory getting the Ehdr from an Elf handle might fail. But in
most cases where __libelf_data_type is used, we already have the
Ehdr anyway. So simply pass that to __libelf_data_type. In the one
place where we don't have it yet, get it and check for failure
before calling __libelf_data_type.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 libelf/ChangeLog          |  9 +++++++++
 libelf/elf_compress.c     |  2 +-
 libelf/elf_compress_gnu.c |  3 ++-
 libelf/elf_getdata.c      | 14 +++++++++-----
 libelf/libelfP.h          |  5 +++--
 5 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/libelf/ChangeLog b/libelf/ChangeLog
index b15508f2..7bd15a28 100644
--- a/libelf/ChangeLog
+++ b/libelf/ChangeLog
@@ -1,3 +1,12 @@
+2020-11-01  Mark Wielaard  <mark@klomp.org>
+
+	* libelfP.h (__libelf_data_type): Take an GElf_Ehdr instead of an
+	Elf handle.
+	* elf_getdata.c (__libelf_data_type): Likewise. And check ehdr
+	directly instead of fetching a new one.
+	(__libelf_set_rawdata_wrlock): Fetch Ehdr, report an error when that
+	fails, otherwise call __libelf_data_type.
+
 2020-10-28  Mark Wielaard  <mark@klomp.org>
 
 	* elf.h: Update from glibc.
diff --git a/libelf/elf_compress.c b/libelf/elf_compress.c
index e5d3d2e0..9d6f15c0 100644
--- a/libelf/elf_compress.c
+++ b/libelf/elf_compress.c
@@ -522,7 +522,7 @@ elf_compress (Elf_Scn *scn, int type, unsigned int flags)
 
       __libelf_reset_rawdata (scn, scn->zdata_base,
 			      scn->zdata_size, scn->zdata_align,
-			      __libelf_data_type (elf, sh_type,
+			      __libelf_data_type (&ehdr, sh_type,
 						  scn->zdata_align));
 
       return 1;
diff --git a/libelf/elf_compress_gnu.c b/libelf/elf_compress_gnu.c
index 1ecd6a08..7aed4640 100644
--- a/libelf/elf_compress_gnu.c
+++ b/libelf/elf_compress_gnu.c
@@ -197,7 +197,8 @@ elf_compress_gnu (Elf_Scn *scn, int inflate, unsigned int flags)
 	}
 
       __libelf_reset_rawdata (scn, buf_out, size, sh_addralign,
-			      __libelf_data_type (elf, sh_type, sh_addralign));
+			      __libelf_data_type (&ehdr, sh_type,
+						  sh_addralign));
 
       scn->zdata_base = buf_out;
 
diff --git a/libelf/elf_getdata.c b/libelf/elf_getdata.c
index 0d8f8d2e..6ed44504 100644
--- a/libelf/elf_getdata.c
+++ b/libelf/elf_getdata.c
@@ -113,14 +113,12 @@ const uint_fast8_t __libelf_type_aligns[ELFCLASSNUM - 1][ELF_T_NUM] =
 
 Elf_Type
 internal_function
-__libelf_data_type (Elf *elf, int sh_type, GElf_Xword align)
+__libelf_data_type (GElf_Ehdr *ehdr, int sh_type, GElf_Xword align)
 {
   /* Some broken ELF ABI for 64-bit machines use the wrong hash table
      entry size.  See elf-knowledge.h for more information.  */
-  if (sh_type == SHT_HASH && elf->class == ELFCLASS64)
+  if (sh_type == SHT_HASH && ehdr->e_ident[EI_CLASS] == ELFCLASS64)
     {
-      GElf_Ehdr ehdr_mem;
-      GElf_Ehdr *ehdr = __gelf_getehdr_rdlock (elf, &ehdr_mem);
       return (SH_ENTSIZE_HASH (ehdr) == 4 ? ELF_T_WORD : ELF_T_XWORD);
     }
   else
@@ -365,7 +363,13 @@ __libelf_set_rawdata_wrlock (Elf_Scn *scn)
   if ((flags & SHF_COMPRESSED) != 0)
     scn->rawdata.d.d_type = ELF_T_CHDR;
   else
-    scn->rawdata.d.d_type = __libelf_data_type (elf, type, align);
+    {
+      GElf_Ehdr ehdr_mem;
+      GElf_Ehdr *ehdr = __gelf_getehdr_rdlock (elf, &ehdr_mem);
+      if (unlikely (ehdr == NULL))
+	return 1;
+      scn->rawdata.d.d_type = __libelf_data_type (ehdr, type, align);
+    }
   scn->rawdata.d.d_off = 0;
 
   /* Make sure the alignment makes sense.  d_align should be aligned both
diff --git a/libelf/libelfP.h b/libelf/libelfP.h
index b55d5c48..6a3243de 100644
--- a/libelf/libelfP.h
+++ b/libelf/libelfP.h
@@ -436,10 +436,11 @@ extern const uint_fast8_t __libelf_type_aligns[ELFCLASSNUM - 1][ELF_T_NUM]
 # define __libelf_type_align(class, type)	\
     (__libelf_type_aligns[class - 1][type] ?: 1)
 
-/* Given an Elf handle and a section type returns the Elf_Data d_type.
+/* Given an GElf_Ehdr handle and a section type returns the Elf_Data d_type.
    Should not be called when SHF_COMPRESSED is set, the d_type should
    be ELF_T_BYTE.  */
-extern Elf_Type __libelf_data_type (Elf *elf, int sh_type, GElf_Xword align)
+extern Elf_Type __libelf_data_type (GElf_Ehdr *ehdr,
+				    int sh_type, GElf_Xword align)
   internal_function;
 
 
-- 
2.18.4

Re: [PATCH] libelf: Use GElf_Ehdr instead of Elf handle in __libelf_data_type

[Mark Wielaard]

On Sun, 2020-11-01 at 19:36 +0100, Mark Wielaard wrote:
> GCC with -flto detects some (very) unlikely issues in error paths.
> In theory getting the Ehdr from an Elf handle might fail. But in
> most cases where __libelf_data_type is used, we already have the
> Ehdr anyway. So simply pass that to __libelf_data_type. In the one
> place where we don't have it yet, get it and check for failure
> before calling __libelf_data_type.

Pushed.