public inbox for elfutils@sourceware.org
 help / color / mirror / Atom feed
From: Mark Wielaard <mark@klomp.org>
To: elfutils-devel@sourceware.org
Cc: Josh Stone <jistone@redhat.com>,	Mark Wielaard <mark@klomp.org>
Subject: [PATCH 1/2] libelf: Add ELF_E_INVALID_ELF error value.
Date: Wed, 04 Oct 2017 18:57:00 -0000	[thread overview]
Message-ID: <1507143449-19288-1-git-send-email-mark@klomp.org> (raw)
In-Reply-To: <1507143353.9961.25.camel@klomp.org>

Add ELF_E_INVALID_ELF which is set when the ELF file data is bad.
This is different from ELF_E_INVALID_FILE which is set when the file
could not be read.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 libelf/ChangeLog           | 12 +++++++++
 libelf/elf_begin.c         | 61 ++++++++++++++++++++++++++++++++++------------
 libelf/elf_error.c         |  7 +++++-
 libelf/elf_getshdrstrndx.c | 20 ++++++++++-----
 libelf/libelfP.h           |  1 +
 tests/ChangeLog            |  4 +++
 tests/msg_tst.c            |  1 +
 7 files changed, 83 insertions(+), 23 deletions(-)

diff --git a/libelf/ChangeLog b/libelf/ChangeLog
index 7bd9e1b..36b57dd 100644
--- a/libelf/ChangeLog
+++ b/libelf/ChangeLog
@@ -1,3 +1,15 @@
+2017-10-04  Mark Wielaard  <mark@klomp.org>
+
+	* libelfP.h: Add ELF_E_INVALID_ELF to error values enum.
+	* elf_error.c (ELF_E_INVALID_ELF_IDX): New define. Use it as value
+	for ELF_E_INVALID_ELF in msgidx.
+	* elf_getshdrstrndx.c (elf_getshdrstrndx): Distinquish between pread
+	failing and not having enough data.
+	* elf_begin.c (get_shnum): Likewise. Explicitly set libelf errno on
+	too large value.
+	(file_read_elf): Make sure to always set libelf errno when returning
+	NULL. Distinquish between i/o file and elf data errors.
+
 2017-08-18  Ulf Hermann  <ulf.hermann@qt.io>
 
 	* gelf_xlate.c: Use attribute_packed.
diff --git a/libelf/elf_begin.c b/libelf/elf_begin.c
index 6f85038..5545278 100644
--- a/libelf/elf_begin.c
+++ b/libelf/elf_begin.c
@@ -158,6 +158,7 @@ get_shnum (void *map_address, unsigned char *e_ident, int fildes, off_t offset,
 	  else
 	    {
 	      Elf32_Word size;
+	      ssize_t r;
 
 	      if (likely (map_address != NULL))
 		/* gcc will optimize the memcpy to a simple memory
@@ -167,11 +168,19 @@ get_shnum (void *map_address, unsigned char *e_ident, int fildes, off_t offset,
 						 + offset))->sh_size,
 			sizeof (Elf32_Word));
 	      else
-		if (unlikely (pread_retry (fildes, &size, sizeof (Elf32_Word),
-					   offset + ehdr.e32->e_shoff
-					   + offsetof (Elf32_Shdr, sh_size))
+		if (unlikely ((r = pread_retry (fildes, &size,
+						sizeof (Elf32_Word),
+						offset + ehdr.e32->e_shoff
+						+ offsetof (Elf32_Shdr,
+							    sh_size)))
 			      != sizeof (Elf32_Word)))
-		  return (size_t) -1l;
+		  {
+		    if (r < 0)
+		      __libelf_seterrno (ELF_E_INVALID_FILE);
+		    else
+		      __libelf_seterrno (ELF_E_INVALID_ELF);
+		    return (size_t) -1l;
+		  }
 
 	      if (e_ident[EI_DATA] != MY_ELFDATA)
 		CONVERT (size);
@@ -207,6 +216,7 @@ get_shnum (void *map_address, unsigned char *e_ident, int fildes, off_t offset,
 				    + offset))->sh_size;
 	  else
 	    {
+	      ssize_t r;
 	      if (likely (map_address != NULL))
 		/* gcc will optimize the memcpy to a simple memory
 		   access while taking care of alignment issues.  */
@@ -215,19 +225,30 @@ get_shnum (void *map_address, unsigned char *e_ident, int fildes, off_t offset,
 						 + offset))->sh_size,
 			sizeof (Elf64_Xword));
 	      else
-		if (unlikely (pread_retry (fildes, &size, sizeof (Elf64_Xword),
-					   offset + ehdr.e64->e_shoff
-					   + offsetof (Elf64_Shdr, sh_size))
+		if (unlikely ((r = pread_retry (fildes, &size,
+						sizeof (Elf64_Xword),
+						offset + ehdr.e64->e_shoff
+						+ offsetof (Elf64_Shdr,
+							    sh_size)))
 			      != sizeof (Elf64_Xword)))
-		  return (size_t) -1l;
+		  {
+		    if (r < 0)
+		      __libelf_seterrno (ELF_E_INVALID_FILE);
+		    else
+		      __libelf_seterrno (ELF_E_INVALID_ELF);
+		    return (size_t) -1l;
+		  }
 
 	      if (e_ident[EI_DATA] != MY_ELFDATA)
 		CONVERT (size);
 	    }
 
 	  if (size > ~((GElf_Word) 0))
-	    /* Invalid value, it is too large.  */
-	    return (size_t) -1l;
+	    {
+	      /* Invalid value, it is too large.  */
+	      __libelf_seterrno (ELF_E_INVALID_ELF);
+	      return (size_t) -1l;
+	    }
 
 	  result = size;
 	}
@@ -255,11 +276,13 @@ file_read_elf (int fildes, void *map_address, unsigned char *e_ident,
 		    && e_ident[EI_DATA] != ELFDATA2MSB)))
     {
       /* Cannot handle this.  */
-      __libelf_seterrno (ELF_E_INVALID_FILE);
+      __libelf_seterrno (ELF_E_INVALID_ELF);
       return NULL;
     }
 
-  /* Determine the number of sections.  */
+  /* Determine the number of sections.  Returns -1 and sets libelf errno
+     if the file handle or elf file is invalid.  Returns zero if there
+     are no section headers (or they cannot be read).  */
   size_t scncnt = get_shnum (map_address, e_ident, fildes, offset, maxsize);
   if (scncnt == (size_t) -1l)
     /* Could not determine the number of sections.  */
@@ -269,10 +292,16 @@ file_read_elf (int fildes, void *map_address, unsigned char *e_ident,
   if (e_ident[EI_CLASS] == ELFCLASS32)
     {
       if (scncnt > SIZE_MAX / (sizeof (Elf_Scn) + sizeof (Elf32_Shdr)))
-	return NULL;
+	{
+	  __libelf_seterrno (ELF_E_INVALID_ELF);
+	  return NULL;
+	}
     }
   else if (scncnt > SIZE_MAX / (sizeof (Elf_Scn) + sizeof (Elf64_Shdr)))
-    return NULL;
+    {
+      __libelf_seterrno (ELF_E_INVALID_ELF);
+      return NULL;
+    }
 
   /* We can now allocate the memory.  Even if there are no section headers,
      we allocate space for a zeroth section in case we need it later.  */
@@ -281,7 +310,7 @@ file_read_elf (int fildes, void *map_address, unsigned char *e_ident,
   Elf *elf = allocate_elf (fildes, map_address, offset, maxsize, cmd, parent,
 			   ELF_K_ELF, scnmax * sizeof (Elf_Scn));
   if (elf == NULL)
-    /* Not enough memory.  */
+    /* Not enough memory.  allocate_elf will have set libelf errno.  */
     return NULL;
 
   assert ((unsigned int) scncnt == scncnt);
@@ -350,7 +379,7 @@ file_read_elf (int fildes, void *map_address, unsigned char *e_ident,
 	    {
 	    free_and_out:
 	      free (elf);
-	      __libelf_seterrno (ELF_E_INVALID_FILE);
+	      __libelf_seterrno (ELF_E_INVALID_ELF);
 	      return NULL;
 	    }
 	  elf->state.elf32.shdr
diff --git a/libelf/elf_error.c b/libelf/elf_error.c
index 888b389..5364e68 100644
--- a/libelf/elf_error.c
+++ b/libelf/elf_error.c
@@ -94,8 +94,12 @@ static const char msgstr[] =
   (ELF_E_NOMEM_IDX + sizeof "out of memory")
   N_("invalid file descriptor")
   "\0"
-#define ELF_E_INVALID_OP_IDX \
+#define ELF_E_INVALID_ELF_IDX \
   (ELF_E_INVALID_FILE_IDX + sizeof "invalid file descriptor")
+  N_("invalid ELF file data")
+  "\0"
+#define ELF_E_INVALID_OP_IDX \
+  (ELF_E_INVALID_ELF_IDX + sizeof "invalid ELF file data")
   N_("invalid operation")
   "\0"
 #define ELF_E_NO_VERSION_IDX \
@@ -280,6 +284,7 @@ static const uint_fast16_t msgidx[ELF_E_NUM] =
   [ELF_E_INVALID_ENCODING] = ELF_E_INVALID_ENCODING_IDX,
   [ELF_E_NOMEM] = ELF_E_NOMEM_IDX,
   [ELF_E_INVALID_FILE] = ELF_E_INVALID_FILE_IDX,
+  [ELF_E_INVALID_ELF] = ELF_E_INVALID_ELF_IDX,
   [ELF_E_INVALID_OP] = ELF_E_INVALID_OP_IDX,
   [ELF_E_NO_VERSION] = ELF_E_NO_VERSION_IDX,
   [ELF_E_INVALID_CMD] = ELF_E_INVALID_CMD_IDX,
diff --git a/libelf/elf_getshdrstrndx.c b/libelf/elf_getshdrstrndx.c
index aead2fe..ad884fd 100644
--- a/libelf/elf_getshdrstrndx.c
+++ b/libelf/elf_getshdrstrndx.c
@@ -133,13 +133,17 @@ elf_getshdrstrndx (Elf *elf, size_t *dst)
 		  /* We avoid reading in all the section headers.  Just read
 		     the first one.  */
 		  Elf32_Shdr shdr_mem;
+		  ssize_t r;
 
-		  if (unlikely (pread_retry (elf->fildes, &shdr_mem,
-					     sizeof (Elf32_Shdr), offset)
+		  if (unlikely ((r = pread_retry (elf->fildes, &shdr_mem,
+						  sizeof (Elf32_Shdr), offset))
 				!= sizeof (Elf32_Shdr)))
 		    {
 		      /* We must be able to read this ELF section header.  */
-		      __libelf_seterrno (ELF_E_INVALID_FILE);
+		      if (r < 0)
+			__libelf_seterrno (ELF_E_INVALID_FILE);
+		      else
+			__libelf_seterrno (ELF_E_INVALID_ELF);
 		      result = -1;
 		      goto out;
 		    }
@@ -194,13 +198,17 @@ elf_getshdrstrndx (Elf *elf, size_t *dst)
 		  /* We avoid reading in all the section headers.  Just read
 		     the first one.  */
 		  Elf64_Shdr shdr_mem;
+		  ssize_t r;
 
-		  if (unlikely (pread_retry (elf->fildes, &shdr_mem,
-					     sizeof (Elf64_Shdr), offset)
+		  if (unlikely ((r = pread_retry (elf->fildes, &shdr_mem,
+						  sizeof (Elf64_Shdr), offset))
 				!= sizeof (Elf64_Shdr)))
 		    {
 		      /* We must be able to read this ELF section header.  */
-		      __libelf_seterrno (ELF_E_INVALID_FILE);
+		      if (r < 0)
+			__libelf_seterrno (ELF_E_INVALID_FILE);
+		      else
+			__libelf_seterrno (ELF_E_INVALID_ELF);
 		      result = -1;
 		      goto out;
 		    }
diff --git a/libelf/libelfP.h b/libelf/libelfP.h
index a4a0a3a..ca805ac 100644
--- a/libelf/libelfP.h
+++ b/libelf/libelfP.h
@@ -102,6 +102,7 @@ enum
   ELF_E_INVALID_ENCODING,
   ELF_E_NOMEM,
   ELF_E_INVALID_FILE,
+  ELF_E_INVALID_ELF,
   ELF_E_INVALID_OP,
   ELF_E_NO_VERSION,
   ELF_E_INVALID_CMD,
diff --git a/tests/ChangeLog b/tests/ChangeLog
index 7b6bf30..35688dc 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,7 @@
+2017-10-04  Mark Wielaard  <mark@klomp.org>
+
+	* msg_tst.c: Handle ELF_E_INVALID_ELF.
+
 2017-09-10  Mark Wielaard  <mark@klomp.org>
 
 	* run-ar.sh: New test.
diff --git a/tests/msg_tst.c b/tests/msg_tst.c
index 7baea0a..aa974d0 100644
--- a/tests/msg_tst.c
+++ b/tests/msg_tst.c
@@ -39,6 +39,7 @@ static struct
     { ELF_E_INVALID_ENCODING, "invalid encoding" },
     { ELF_E_NOMEM, "out of memory" },
     { ELF_E_INVALID_FILE, "invalid file descriptor" },
+    { ELF_E_INVALID_ELF, "invalid ELF file data" },
     { ELF_E_INVALID_OP, "invalid operation" },
     { ELF_E_NO_VERSION, "ELF version not set" },
     { ELF_E_INVALID_CMD, "invalid command" },
-- 
1.8.3.1

  reply	other threads:[~2017-10-04 18:57 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-28 19:06 failed reading "Smallest x86 ELF Hello World" Josh Stone
2017-10-04 18:55 ` Mark Wielaard
2017-10-04 18:57   ` Mark Wielaard [this message]
2017-10-04 18:57     ` [PATCH 2/2] libelf: Don't error out when sanity checking e_shoff if scncnt is zero Mark Wielaard
2017-10-13 15:09   ` failed reading "Smallest x86 ELF Hello World" Mark Wielaard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1507143449-19288-1-git-send-email-mark@klomp.org \
    --to=mark@klomp.org \
    --cc=elfutils-devel@sourceware.org \
    --cc=jistone@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).