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 06/13] elf: load elf hdr fully in open_verify()
Date: Sat, 18 Mar 2023 21:51:03 +0500	[thread overview]
Message-ID: <20230318165110.3672749-7-stsp2@yandex.ru> (raw)
In-Reply-To: <20230318165110.3672749-1-stsp2@yandex.ru>

open_verify() reads an elf header, putting only the ehdr part into
the filebuf, and reading the rest of the header into the alloca()
space. This requires _ld_map_object_1() (former
_ld_map_object_from_fd()) to read parts of an elf header again,
getting only ehdr from filebuf.
This patch makes filebuf sizeable and reads an entire elf header
there, avoiding the code duplication and getting rid of file reads
in _ld_map_object_1().

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 | 122 +++++++++++++++++++++++++++-----------------------
 1 file changed, 66 insertions(+), 56 deletions(-)

diff --git a/elf/dl-load.c b/elf/dl-load.c
index 353dc2aa13..45656d8fa5 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -55,7 +55,8 @@ struct filebuf
 #else
 # define FILEBUF_SIZE 832
 #endif
-  char buf[FILEBUF_SIZE] __attribute__ ((aligned (__alignof (ElfW(Ehdr)))));
+  ssize_t allocated;
+  char *buf;
 };
 
 #include "dynamic-link.h"
@@ -124,6 +125,29 @@ static const size_t system_dirs_len[] =
 };
 #define nsystem_dirs_len array_length (system_dirs_len)
 
+static void
+filebuf_done (struct filebuf *fb)
+{
+  free (fb->buf);
+  fb->buf = NULL;
+  fb->allocated = 0;
+}
+
+static bool
+filebuf_ensure (struct filebuf *fb, size_t size)
+{
+  bool ret = false;
+
+  if (size > fb->allocated)
+    {
+      size_t new_len = size + FILEBUF_SIZE;
+      fb->buf = realloc (fb->buf, new_len);
+      fb->allocated = new_len;
+      ret = true;
+    }
+  return ret;
+}
+
 static bool
 is_trusted_path_normalize (const char *path, size_t len)
 {
@@ -955,18 +979,8 @@ _ld_map_object_1 (struct link_map *l, int fd,
   l->l_phnum = header->e_phnum;
 
   maplength = header->e_phnum * sizeof (ElfW(Phdr));
-  if (header->e_phoff + maplength <= (size_t) fbp->len)
-    phdr = (void *) (fbp->buf + header->e_phoff);
-  else
-    {
-      phdr = alloca (maplength);
-      if ((size_t) __pread64_nocancel (fd, (void *) phdr, maplength,
-				       header->e_phoff) != maplength)
-	{
-	  errstring = N_("cannot read file data");
-	  goto lose_errno;
-	}
-    }
+  assert (header->e_phoff + maplength <= (size_t) fbp->len);
+  phdr = (void *) (fbp->buf + header->e_phoff);
 
    /* On most platforms presume that PT_GNU_STACK is absent and the stack is
     * executable.  Other platforms default to a nonexecutable stack and don't
@@ -1629,31 +1643,16 @@ do_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(Ehdr) _ehdr;
+  ElfW(Ehdr) *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))))
+  if (__pread64_nocancel (fd, &_ehdr, sizeof(_ehdr), 0) != sizeof(_ehdr))
     {
       errval = errno;
       errstring = (errval == 0
@@ -1754,19 +1753,17 @@ do_open_verify (const char *name, int fd,
     }
 
   maplength = ehdr->e_phnum * sizeof (ElfW(Phdr));
-  if (ehdr->e_phoff + maplength <= (size_t) fbp->len)
-    phdr = (void *) (fbp->buf + ehdr->e_phoff);
-  else
+  filebuf_ensure (fbp, maplength + ehdr->e_phoff);
+  if ((size_t) __pread64_nocancel (fd, fbp->buf, maplength +
+				   ehdr->e_phoff, 0) != maplength +
+				   ehdr->e_phoff)
     {
-      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;
-        }
+      errval = errno;
+      errstring = N_("cannot read file data");
+      goto lose;
     }
+  fbp->len = maplength + ehdr->e_phoff;
+  phdr = (void *) (fbp->buf + ehdr->e_phoff);
 
   if (__glibc_unlikely (elf_machine_reject_phdr_p
 		       (phdr, ehdr->e_phnum, fbp->buf, fbp->len,
@@ -1992,16 +1989,16 @@ open_path (const char *name, size_t namelen, int mode,
 
 /* Map in the shared object file NAME.  */
 
-struct link_map *
-_dl_map_object (struct link_map *loader, const char *name,
-		int type, int trace_mode, int mode, Lmid_t nsid)
+static struct link_map *
+___dl_map_object (struct link_map *loader, const char *name,
+		  int type, int trace_mode, int mode, Lmid_t nsid,
+		  struct filebuf *fbp)
 {
   int fd;
   const char *origname = NULL;
   char *realname;
   char *name_copy;
   struct link_map *l;
-  struct filebuf fb;
 
   assert (nsid >= 0);
   assert (nsid < GL(dl_nns));
@@ -2091,7 +2088,7 @@ _dl_map_object (struct link_map *loader, const char *name,
 	      {
 		fd = open_path (name, namelen, mode,
 				&l->l_rpath_dirs,
-				&realname, &fb, loader, LA_SER_RUNPATH,
+				&realname, fbp, loader, LA_SER_RUNPATH,
 				&found_other_class);
 		if (fd != -1)
 		  break;
@@ -2107,7 +2104,7 @@ _dl_map_object (struct link_map *loader, const char *name,
 			      "RPATH"))
 	    fd = open_path (name, namelen, mode,
 			    &main_map->l_rpath_dirs,
-			    &realname, &fb, loader ?: main_map, LA_SER_RUNPATH,
+			    &realname, fbp, loader ?: main_map, LA_SER_RUNPATH,
 			    &found_other_class);
 
 	  /* Also try DT_RUNPATH in the executable for LD_AUDIT dlopen
@@ -2121,7 +2118,7 @@ _dl_map_object (struct link_map *loader, const char *name,
 	      if (cache_rpath (main_map, &l_rpath_dirs,
 			       DT_RUNPATH, "RUNPATH"))
 		fd = open_path (name, namelen, mode, &l_rpath_dirs,
-				&realname, &fb, loader ?: main_map,
+				&realname, fbp, loader ?: main_map,
 				LA_SER_RUNPATH, &found_other_class);
 	    }
 	}
@@ -2129,7 +2126,7 @@ _dl_map_object (struct link_map *loader, const char *name,
       /* Try the LD_LIBRARY_PATH environment variable.  */
       if (fd == -1 && __rtld_env_path_list.dirs != (void *) -1)
 	fd = open_path (name, namelen, mode, &__rtld_env_path_list,
-			&realname, &fb,
+			&realname, fbp,
 			loader ?: GL(dl_ns)[LM_ID_BASE]._ns_loaded,
 			LA_SER_LIBPATH, &found_other_class);
 
@@ -2138,7 +2135,7 @@ _dl_map_object (struct link_map *loader, const char *name,
 	  && cache_rpath (loader, &loader->l_runpath_dirs,
 			  DT_RUNPATH, "RUNPATH"))
 	fd = open_path (name, namelen, mode,
-			&loader->l_runpath_dirs, &realname, &fb, loader,
+			&loader->l_runpath_dirs, &realname, fbp, loader,
 			LA_SER_RUNPATH, &found_other_class);
 
       if (fd == -1)
@@ -2147,7 +2144,7 @@ _dl_map_object (struct link_map *loader, const char *name,
           if (realname != NULL)
             {
               fd = open_verify (realname, fd,
-                                &fb, loader ?: GL(dl_ns)[nsid]._ns_loaded,
+                                fbp, loader ?: GL(dl_ns)[nsid]._ns_loaded,
                                 LA_SER_CONFIG, mode, &found_other_class,
                                 false);
               if (fd == -1)
@@ -2201,7 +2198,7 @@ _dl_map_object (struct link_map *loader, const char *name,
 	      if (cached != NULL)
 		{
 		  fd = open_verify (cached, -1,
-				    &fb, loader ?: GL(dl_ns)[nsid]._ns_loaded,
+				    fbp, loader ?: GL(dl_ns)[nsid]._ns_loaded,
 				    LA_SER_CONFIG, mode, &found_other_class,
 				    false);
 		  if (__glibc_likely (fd != -1))
@@ -2219,7 +2216,7 @@ _dl_map_object (struct link_map *loader, const char *name,
 	      || __glibc_likely (!(l->l_flags_1 & DF_1_NODEFLIB)))
 	  && __rtld_search_dirs.dirs != (void *) -1)
 	fd = open_path (name, namelen, mode, &__rtld_search_dirs,
-			&realname, &fb, l, LA_SER_DEFAULT, &found_other_class);
+			&realname, fbp, l, LA_SER_DEFAULT, &found_other_class);
 
       /* Add another newline when we are tracing the library loading.  */
       if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_LIBS))
@@ -2235,7 +2232,7 @@ _dl_map_object (struct link_map *loader, const char *name,
 	fd = -1;
       else
 	{
-	  fd = open_verify (realname, -1, &fb,
+	  fd = open_verify (realname, -1, fbp,
 			    loader ?: GL(dl_ns)[nsid]._ns_loaded, 0, mode,
 			    &found_other_class, true);
 	  if (__glibc_unlikely (fd == -1))
@@ -2296,10 +2293,23 @@ _dl_map_object (struct link_map *loader, const char *name,
     }
 
   void *stack_end = __libc_stack_end;
-  return _dl_map_object_from_fd (name, origname, fd, &fb, realname, loader,
+  return _dl_map_object_from_fd (name, origname, fd, fbp, realname, loader,
 				 type, mode, &stack_end, nsid);
 }
 
+struct link_map *
+_dl_map_object (struct link_map *loader, const char *name,
+                 int type, int trace_mode,
+                 int mode, Lmid_t nsid)
+{
+  struct link_map *ret;
+  struct filebuf fb = {};
+
+  ret = ___dl_map_object (loader, name, type, trace_mode, mode, nsid, &fb);
+  filebuf_done (&fb);
+  return ret;
+}
+
 struct add_path_state
 {
   bool counting;
-- 
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 ` [PATCH 05/13] elf: split open_verify() " Stas Sergeev
2023-03-18 16:51 ` Stas Sergeev [this message]
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-7-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).