public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Stas Sergeev <stsp2@yandex.ru>
To: libc-alpha@sourceware.org
Cc: Stas Sergeev <stsp2@yandex.ru>
Subject: [PATCH 05/13] elf: split open_verify() into reusable parts
Date: Sat, 18 Mar 2023 21:51:02 +0500	[thread overview]
Message-ID: <20230318165110.3672749-6-stsp2@yandex.ru> (raw)
In-Reply-To: <20230318165110.3672749-1-stsp2@yandex.ru>

This is almost a mechanical refactoring that splits open_verify()
into 2 parts.

The test-suite was run on x86_64/64 and showed no regressions.

Signed-off-by: Stas Sergeev <stsp2@yandex.ru>
---
 elf/dl-load.c | 337 ++++++++++++++++++++++++++------------------------
 1 file changed, 175 insertions(+), 162 deletions(-)

diff --git a/elf/dl-load.c b/elf/dl-load.c
index 9e0e63b9a3..353dc2aa13 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -1598,19 +1598,11 @@ print_search_path (struct r_search_path_elem **list,
   else
     _dl_debug_printf_c ("\t\t(%s)\n", what);
 }
-\f
-/* Open a file and verify it is an ELF file for this architecture.  We
-   ignore only ELF files for other architectures.  Non-ELF files and
-   ELF files with different header information cause fatal errors since
-   this could mean there is something wrong in the installation and the
-   user might want to know about this.
 
-   If FD is not -1, then the file is already open and FD refers to it.
-   In that case, FD is consumed for both successful and error returns.  */
 static int
-open_verify (const char *name, int fd,
-             struct filebuf *fbp, struct link_map *loader,
-	     int whatcode, int mode, bool *found_other_class, bool free_name)
+do_open_verify (const char *name, int fd,
+                struct filebuf *fbp, struct link_map *loader,
+                bool *found_other_class, bool free_name)
 {
   /* This is the expected ELF header.  */
 #define ELF32_CLASS ELFCLASS32
@@ -1637,7 +1629,170 @@ open_verify (const char *name, int fd,
   /* Initialize it to make the compiler happy.  */
   const char *errstring = NULL;
   int errval = 0;
+  ElfW(Ehdr) *ehdr;
+  ElfW(Phdr) *phdr;
+  size_t maplength;
+
+  /* We successfully opened the file.  Now verify it is a file
+   we can use.  */
+  __set_errno (0);
+  fbp->len = 0;
+  assert (sizeof (fbp->buf) > sizeof (ElfW(Ehdr)));
+  /* Read in the header.  */
+  do
+    {
+      ssize_t retlen = __read_nocancel (fd, fbp->buf + fbp->len,
+					sizeof (fbp->buf) - fbp->len);
+      if (retlen <= 0)
+        break;
+      fbp->len += retlen;
+    }
+  while (__glibc_unlikely (fbp->len < sizeof (ElfW(Ehdr))));
+
+  /* This is where the ELF header is loaded.  */
+  ehdr = (ElfW(Ehdr) *) fbp->buf;
+
+  /* Now run the tests.  */
+  if (__glibc_unlikely (fbp->len < (ssize_t) sizeof (ElfW(Ehdr))))
+    {
+      errval = errno;
+      errstring = (errval == 0
+	       ? N_("file too short") : N_("cannot read file data"));
+    lose:
+      if (free_name)
+        {
+          char *realname = (char *) name;
+          name = strdupa (realname);
+          free (realname);
+        }
+      _dl_signal_error (errval, name, NULL, errstring);
+      return -1;
+    }
+
+  /* See whether the ELF header is what we expect.  */
+  if (__glibc_unlikely (! VALID_ELF_HEADER (ehdr->e_ident, expected,
+					    EI_ABIVERSION)
+			|| !VALID_ELF_ABIVERSION (ehdr->e_ident[EI_OSABI],
+						  ehdr->e_ident[EI_ABIVERSION])
+			|| memcmp (&ehdr->e_ident[EI_PAD],
+				   &expected[EI_PAD],
+				    EI_NIDENT - EI_PAD) != 0))
+    {
+      /* Something is wrong.  */
+      const Elf32_Word *magp = (const void *) ehdr->e_ident;
+      if (*magp !=
+#if BYTE_ORDER == LITTLE_ENDIAN
+          ((ELFMAG0 << (EI_MAG0 * 8))
+           | (ELFMAG1 << (EI_MAG1 * 8))
+           | (ELFMAG2 << (EI_MAG2 * 8))
+           | (ELFMAG3 << (EI_MAG3 * 8)))
+#else
+          ((ELFMAG0 << (EI_MAG3 * 8))
+           | (ELFMAG1 << (EI_MAG2 * 8))
+           | (ELFMAG2 << (EI_MAG1 * 8))
+           | (ELFMAG3 << (EI_MAG0 * 8)))
+#endif
+          )
+        errstring = N_("invalid ELF header");
+
+      else if (ehdr->e_ident[EI_CLASS] != ELFW(CLASS))
+        {
+          /* This is not a fatal error.  On architectures where
+             32-bit and 64-bit binaries can be run this might
+             happen.  */
+          *found_other_class = true;
+          __set_errno (ENOENT);
+          return -1;
+        }
+      else if (ehdr->e_ident[EI_DATA] != byteorder)
+        {
+          if (BYTE_ORDER == BIG_ENDIAN)
+            errstring = N_("ELF file data encoding not big-endian");
+          else
+            errstring = N_("ELF file data encoding not little-endian");
+        }
+      else if (ehdr->e_ident[EI_VERSION] != EV_CURRENT)
+        errstring
+          = N_("ELF file version ident does not match current one");
+      /* XXX We should be able so set system specific versions which are
+         allowed here.  */
+      else if (!VALID_ELF_OSABI (ehdr->e_ident[EI_OSABI]))
+        errstring = N_("ELF file OS ABI invalid");
+      else if (!VALID_ELF_ABIVERSION (ehdr->e_ident[EI_OSABI],
+				      ehdr->e_ident[EI_ABIVERSION]))
+        errstring = N_("ELF file ABI version invalid");
+      else if (memcmp (&ehdr->e_ident[EI_PAD], &expected[EI_PAD],
+		       EI_NIDENT - EI_PAD) != 0)
+        errstring = N_("nonzero padding in e_ident");
+      else
+        /* Otherwise we don't know what went wrong.  */
+        errstring = N_("internal error");
+
+      goto lose;
+    }
+
+  if (__glibc_unlikely (ehdr->e_version != EV_CURRENT))
+    {
+      errstring = N_("ELF file version does not match current one");
+      goto lose;
+    }
+  if (! __glibc_likely (elf_machine_matches_host (ehdr)))
+    {
+      __set_errno (ENOENT);
+      return -1;
+    }
+  else if (__glibc_unlikely (ehdr->e_type != ET_DYN
+				 && ehdr->e_type != ET_EXEC))
+    {
+      errstring = N_("only ET_DYN and ET_EXEC can be loaded");
+      goto lose;
+    }
+  else if (__glibc_unlikely (ehdr->e_phentsize != sizeof (ElfW(Phdr))))
+    {
+      errstring = N_("ELF file's phentsize not the expected size");
+      goto lose;
+    }
+
+  maplength = ehdr->e_phnum * sizeof (ElfW(Phdr));
+  if (ehdr->e_phoff + maplength <= (size_t) fbp->len)
+    phdr = (void *) (fbp->buf + ehdr->e_phoff);
+  else
+    {
+      phdr = alloca (maplength);
+      if ((size_t) __pread64_nocancel (fd, (void *) phdr, maplength,
+					   ehdr->e_phoff) != maplength)
+        {
+          errval = errno;
+          errstring = N_("cannot read file data");
+          goto lose;
+        }
+    }
+
+  if (__glibc_unlikely (elf_machine_reject_phdr_p
+		       (phdr, ehdr->e_phnum, fbp->buf, fbp->len,
+			loader, -1)))
+    {
+      __set_errno (ENOENT);
+      return -1;
+    }
+
+  return 0;
+}
+
+
+/* Open a file and verify it is an ELF file for this architecture.  We
+   ignore only ELF files for other architectures.  Non-ELF files and
+   ELF files with different header information cause fatal errors since
+   this could mean there is something wrong in the installation and the
+   user might want to know about this.
 
+   If FD is not -1, then the file is already open and FD refers to it.
+   In that case, FD is consumed for both successful and error returns.  */
+static int
+open_verify (const char *name, int fd,
+             struct filebuf *fbp, struct link_map *loader,
+	     int whatcode, int mode, bool *found_other_class, bool free_name)
+{
 #ifdef SHARED
   /* Give the auditing libraries a chance.  */
   if (__glibc_unlikely (GLRO(dl_naudit) > 0))
@@ -1663,161 +1818,19 @@ open_verify (const char *name, int fd,
 
   if (fd != -1)
     {
-      ElfW(Ehdr) *ehdr;
-      ElfW(Phdr) *phdr;
-      size_t maplength;
-
-      /* We successfully opened the file.  Now verify it is a file
-	 we can use.  */
-      __set_errno (0);
-      fbp->len = 0;
-      assert (sizeof (fbp->buf) > sizeof (ElfW(Ehdr)));
-      /* Read in the header.  */
-      do
-	{
-	  ssize_t retlen = __read_nocancel (fd, fbp->buf + fbp->len,
-					    sizeof (fbp->buf) - fbp->len);
-	  if (retlen <= 0)
-	    break;
-	  fbp->len += retlen;
-	}
-      while (__glibc_unlikely (fbp->len < sizeof (ElfW(Ehdr))));
-
-      /* This is where the ELF header is loaded.  */
-      ehdr = (ElfW(Ehdr) *) fbp->buf;
-
-      /* Now run the tests.  */
-      if (__glibc_unlikely (fbp->len < (ssize_t) sizeof (ElfW(Ehdr))))
-	{
-	  errval = errno;
-	  errstring = (errval == 0
-		       ? N_("file too short") : N_("cannot read file data"));
-	lose:
-	  if (free_name)
-	    {
-	      char *realname = (char *) name;
-	      name = strdupa (realname);
-	      free (realname);
-	    }
-	  __close_nocancel (fd);
-	  _dl_signal_error (errval, name, NULL, errstring);
-	}
-
-      /* See whether the ELF header is what we expect.  */
-      if (__glibc_unlikely (! VALID_ELF_HEADER (ehdr->e_ident, expected,
-						EI_ABIVERSION)
-			    || !VALID_ELF_ABIVERSION (ehdr->e_ident[EI_OSABI],
-						      ehdr->e_ident[EI_ABIVERSION])
-			    || memcmp (&ehdr->e_ident[EI_PAD],
-				       &expected[EI_PAD],
-				       EI_NIDENT - EI_PAD) != 0))
-	{
-	  /* Something is wrong.  */
-	  const Elf32_Word *magp = (const void *) ehdr->e_ident;
-	  if (*magp !=
-#if BYTE_ORDER == LITTLE_ENDIAN
-	      ((ELFMAG0 << (EI_MAG0 * 8))
-	       | (ELFMAG1 << (EI_MAG1 * 8))
-	       | (ELFMAG2 << (EI_MAG2 * 8))
-	       | (ELFMAG3 << (EI_MAG3 * 8)))
-#else
-	      ((ELFMAG0 << (EI_MAG3 * 8))
-	       | (ELFMAG1 << (EI_MAG2 * 8))
-	       | (ELFMAG2 << (EI_MAG1 * 8))
-	       | (ELFMAG3 << (EI_MAG0 * 8)))
-#endif
-	      )
-	    errstring = N_("invalid ELF header");
-
-	  else if (ehdr->e_ident[EI_CLASS] != ELFW(CLASS))
-	    {
-	      /* This is not a fatal error.  On architectures where
-		 32-bit and 64-bit binaries can be run this might
-		 happen.  */
-	      *found_other_class = true;
-	      __close_nocancel (fd);
-	      __set_errno (ENOENT);
-	      return -1;
-	    }
-	  else if (ehdr->e_ident[EI_DATA] != byteorder)
-	    {
-	      if (BYTE_ORDER == BIG_ENDIAN)
-		errstring = N_("ELF file data encoding not big-endian");
-	      else
-		errstring = N_("ELF file data encoding not little-endian");
-	    }
-	  else if (ehdr->e_ident[EI_VERSION] != EV_CURRENT)
-	    errstring
-	      = N_("ELF file version ident does not match current one");
-	  /* XXX We should be able so set system specific versions which are
-	     allowed here.  */
-	  else if (!VALID_ELF_OSABI (ehdr->e_ident[EI_OSABI]))
-	    errstring = N_("ELF file OS ABI invalid");
-	  else if (!VALID_ELF_ABIVERSION (ehdr->e_ident[EI_OSABI],
-					  ehdr->e_ident[EI_ABIVERSION]))
-	    errstring = N_("ELF file ABI version invalid");
-	  else if (memcmp (&ehdr->e_ident[EI_PAD], &expected[EI_PAD],
-			   EI_NIDENT - EI_PAD) != 0)
-	    errstring = N_("nonzero padding in e_ident");
-	  else
-	    /* Otherwise we don't know what went wrong.  */
-	    errstring = N_("internal error");
-
-	  goto lose;
-	}
-
-      if (__glibc_unlikely (ehdr->e_version != EV_CURRENT))
-	{
-	  errstring = N_("ELF file version does not match current one");
-	  goto lose;
-	}
-      if (! __glibc_likely (elf_machine_matches_host (ehdr)))
-	{
-	  __close_nocancel (fd);
-	  __set_errno (ENOENT);
-	  return -1;
-	}
-      else if (__glibc_unlikely (ehdr->e_type != ET_DYN
-				 && ehdr->e_type != ET_EXEC))
-	{
-	  errstring = N_("only ET_DYN and ET_EXEC can be loaded");
-	  goto lose;
-	}
-      else if (__glibc_unlikely (ehdr->e_phentsize != sizeof (ElfW(Phdr))))
-	{
-	  errstring = N_("ELF file's phentsize not the expected size");
-	  goto lose;
-	}
-
-      maplength = ehdr->e_phnum * sizeof (ElfW(Phdr));
-      if (ehdr->e_phoff + maplength <= (size_t) fbp->len)
-	phdr = (void *) (fbp->buf + ehdr->e_phoff);
-      else
-	{
-	  phdr = alloca (maplength);
-	  if ((size_t) __pread64_nocancel (fd, (void *) phdr, maplength,
-					   ehdr->e_phoff) != maplength)
-	    {
-	      errval = errno;
-	      errstring = N_("cannot read file data");
-	      goto lose;
-	    }
-	}
-
-      if (__glibc_unlikely (elf_machine_reject_phdr_p
-			    (phdr, ehdr->e_phnum, fbp->buf, fbp->len,
-			     loader, fd)))
-	{
-	  __close_nocancel (fd);
-	  __set_errno (ENOENT);
-	  return -1;
-	}
-
+      int err = do_open_verify (name, fd, fbp, loader,
+                                found_other_class,
+                                free_name);
+      if (err)
+        {
+          __close_nocancel (fd);
+          return -1;
+        }
     }
 
   return fd;
 }
-\f
+
 /* Try to open NAME in one of the directories in *DIRSP.
    Return the fd, or -1.  If successful, fill in *REALNAME
    with the malloc'd full directory name.  If it turns out
-- 
2.37.2


  parent reply	other threads:[~2023-03-18 16:51 UTC|newest]

Thread overview: 107+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-18 16:50 [PATCH v9 0/13] implement dlmem() function Stas Sergeev
2023-03-18 16:50 ` [PATCH 01/13] elf: strdup() l_name if no realname [BZ #30100] Stas Sergeev
2023-03-29 13:54   ` Adhemerval Zanella Netto
2023-03-29 14:12     ` stsp
2023-03-29 14:19       ` Adhemerval Zanella Netto
2023-03-29 14:28         ` stsp
2023-03-29 14:30           ` Adhemerval Zanella Netto
2023-03-29 14:33             ` stsp
2023-03-18 16:50 ` [PATCH 02/13] elf: switch _dl_map_segment() to anonymous mapping Stas Sergeev
2023-03-29 17:01   ` Adhemerval Zanella Netto
2023-03-29 18:00     ` stsp
2023-03-29 18:29       ` Adhemerval Zanella Netto
2023-03-29 18:46         ` stsp
2023-03-29 19:17           ` Adhemerval Zanella Netto
2023-03-29 19:43             ` stsp
2023-03-18 16:51 ` [PATCH 03/13] elf: dont pass fd to _dl_process_pt_xx Stas Sergeev
2023-03-29 17:10   ` Adhemerval Zanella Netto
2023-03-30 16:08     ` stsp
2023-03-30 20:46       ` Adhemerval Zanella Netto
2023-03-31 12:02         ` Szabolcs Nagy
2023-03-31 12:54           ` Adhemerval Zanella Netto
2023-03-31 14:04             ` stsp
2023-03-18 16:51 ` [PATCH 04/13] elf: split _dl_map_object_from_fd() into reusable parts Stas Sergeev
2023-03-18 16:51 ` Stas Sergeev [this message]
2023-03-18 16:51 ` [PATCH 06/13] elf: load elf hdr fully in open_verify() Stas Sergeev
2023-03-18 16:51 ` [PATCH 07/13] elf: convert pread64 to callback in do_open_verify() Stas Sergeev
2023-03-18 16:51 ` [PATCH 08/13] elf: convert _dl_map_segments's mmap() to a callback Stas Sergeev
2023-03-18 16:51 ` [PATCH 09/13] elf: call _dl_map_segment() via premap callback Stas Sergeev
2023-03-18 16:51 ` [PATCH 10/13] elf: convert _dl_map_object to a callback Stas Sergeev
2023-03-18 16:51 ` [PATCH 11/13] elf: split _dl_check_loaded() from _dl_map_object Stas Sergeev
2023-03-18 16:51 ` [PATCH 12/13] dlfcn,elf: implement dlmem() [BZ #11767] Stas Sergeev
2023-03-29 13:45   ` Carlos O'Donell
2023-03-29 13:51     ` stsp
2023-03-29 14:10       ` Jonathon Anderson
2023-03-29 14:20         ` stsp
2023-03-29 14:31           ` Adhemerval Zanella Netto
2023-03-29 15:01             ` stsp
2023-03-29 14:35           ` Carlos O'Donell
2023-03-29 14:50             ` stsp
2023-03-29 15:20               ` Carlos O'Donell
2023-03-29 15:34                 ` stsp
2023-03-30  8:09         ` stsp
2023-03-18 16:51 ` [PATCH 13/13] dlfcn,elf: impl DLMEM_DONTREPLACE dlmem() flag Stas Sergeev
2023-03-29 12:32 ` [PATCH v9 0/13] implement dlmem() function Adhemerval Zanella Netto
2023-03-29 13:10   ` stsp
2023-03-29 13:18   ` stsp
2023-03-31 12:20     ` Szabolcs Nagy
2023-03-31 13:51       ` stsp
2023-03-31 14:49         ` Rich Felker
2023-03-31 14:56           ` stsp
2023-03-31 14:58             ` Rich Felker
2023-03-31 15:03               ` stsp
2023-03-31 14:44       ` stsp
2023-03-31 15:12       ` stsp
2023-03-31 17:12         ` Szabolcs Nagy
2023-03-31 17:36           ` stsp
2023-04-01  9:28             ` stsp
2023-04-03 10:04             ` Szabolcs Nagy
2023-04-03 10:43               ` stsp
2023-04-03 12:01                 ` Szabolcs Nagy
2023-04-03 13:07                   ` stsp
2023-04-05  7:29                   ` stsp
2023-04-05  8:51                     ` Szabolcs Nagy
2023-04-05  9:26                       ` stsp
2023-04-05  9:31                       ` Florian Weimer
2023-04-12 17:23                       ` stsp
2023-04-12 18:00                         ` stsp
2023-04-12 18:20                           ` Rich Felker
2023-04-12 18:46                             ` stsp
2023-04-12 19:52                               ` Zack Weinberg
2023-04-12 19:07                             ` stsp
2023-04-13 10:01                             ` stsp
2023-04-13 12:38                               ` Szabolcs Nagy
2023-04-13 15:59                                 ` stsp
2023-04-13 18:09                                   ` Adhemerval Zanella Netto
2023-04-13 18:59                                     ` stsp
2023-04-13 19:12                                       ` Adhemerval Zanella Netto
2023-04-13 19:29                                         ` stsp
2023-04-13 20:02                                           ` Adhemerval Zanella Netto
2023-04-13 20:21                                             ` stsp
2023-04-13 20:57                                             ` stsp
2023-04-14  7:07                                             ` stsp
2023-04-14  7:36                                             ` stsp
2023-04-14 11:30                                             ` stsp
2023-04-14 19:04                                             ` proof for dlmem() (Re: [PATCH v9 0/13] implement dlmem() function) stsp
2023-05-01 23:11                                               ` Zack Weinberg
2023-05-02  5:48                                                 ` stsp
2023-05-08 16:00                                                   ` stsp
2023-05-02  6:24                                                 ` stsp
2023-05-08 15:10                                 ` [PATCH v9 0/13] implement dlmem() function stsp
2023-03-31 18:47           ` stsp
2023-03-31 19:00             ` stsp
2023-03-29 13:17 ` Carlos O'Donell
2023-03-29 13:26   ` stsp
2023-03-29 17:03   ` stsp
2023-03-29 18:13     ` Carlos O'Donell
2023-03-29 18:29       ` stsp
2023-03-31 11:04       ` stsp
2023-04-13 21:17         ` Carlos O'Donell
2023-04-13 21:58           ` stsp
2023-04-13 22:08           ` stsp
2023-04-13 22:50           ` stsp
2023-04-14 16:15           ` Autoconf maintenance (extremely tangential to Re: [PATCH v9 0/13] implement dlmem() function) Zack Weinberg
2023-04-14 20:24             ` Carlos O'Donell
2023-04-14 20:40               ` Zack Weinberg
2023-05-08 15:05           ` [PATCH v9 0/13] implement dlmem() function stsp
2023-05-19  7:26           ` stsp

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=20230318165110.3672749-6-stsp2@yandex.ru \
    --to=stsp2@yandex.ru \
    --cc=libc-alpha@sourceware.org \
    /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).