bug#43491: Fakechroot execution engine can fail to find libraries

["Ludovic Courtès" <ludo@gnu.org>]

[-- Attachment #1: Type: text/plain, Size: 640 bytes --]

Ludovic Courtès <ludo@gnu.org> skribis:

> Ludovic Courtès <ludovic.courtes@inria.fr> skribis:
>
>> Indeed, we can see ‘stat’ calls passed raw /gnu/store file names from
>> RUNPATH entries (instead of /tmp/fakechroot-test/gnu/store), suggesting
>> that ‘la_objsearch’ didn’t have a chance to rewrite them:
>
> This is probably an ld.so bug:
>
>   https://sourceware.org/bugzilla/show_bug.cgi?id=26634

The patch below provides a fix/workaround for glibc, confirming the
hypothesis above.  (I don’t think we should apply this patch though,
rather we’ll work around the issue in ‘guix pack’.)

Ludo’.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 5825 bytes --]

diff --git a/gnu/packages/base.scm b/gnu/packages/base.scm
index c83775d8ee..fa4da801af 100644
--- a/gnu/packages/base.scm
+++ b/gnu/packages/base.scm
@@ -685,6 +685,7 @@ the store.")
   (package
    (name "glibc")
    (version "2.31")
+   (replacement glibc-2.31/fixed)
    (source (origin
             (method url-fetch)
             (uri (string-append "mirror://gnu/glibc/glibc-" version ".tar.xz"))
@@ -933,6 +934,15 @@ with the Linux kernel.")
    (license lgpl2.0+)
    (home-page "https://www.gnu.org/software/libc/")))
 
+(define-public glibc-2.31/fixed
+  (package
+    (inherit glibc)
+    (version "2.3A")
+    (source (origin
+              (inherit (package-source glibc))
+              (patches (append (search-patches "glibc-audit-stat.patch")
+                               (origin-patches (package-source glibc))))))))
+
 ;; Below are old libc versions, which we use mostly to build locale data in
 ;; the old format (which the new libc cannot cope with.)
 
--- /dev/null
+++ b/gnu/packages/patches/glibc-audit-stat.patch
@@ -0,0 +1,126 @@
+diff --git a/elf/dl-load.c b/elf/dl-load.c
+index a6b80f9395..9daa32f76b 100644
+--- a/elf/dl-load.c
++++ b/elf/dl-load.c
+@@ -1461,11 +1461,15 @@ print_search_path (struct r_search_path_elem **list,
+    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.  */
++   In that case, FD is consumed for both successful and error returns.
++
++   Set *CHANGED_BY_AUDIT to true if the audit module provided a file name
++   different from NAME.  */
+ 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)
++	     int whatcode, int mode, bool *found_other_class, bool free_name,
++	     bool *changed_by_audit)
+ {
+   /* This is the expected ELF header.  */
+ #define ELF32_CLASS ELFCLASS32
+@@ -1500,6 +1504,8 @@ open_verify (const char *name, int fd,
+   const char *errstring = NULL;
+   int errval = 0;
+ 
++  *changed_by_audit = false;
++
+ #ifdef SHARED
+   /* Give the auditing libraries a chance.  */
+   if (__glibc_unlikely (GLRO(dl_naudit) > 0) && whatcode != 0
+@@ -1521,12 +1527,19 @@ open_verify (const char *name, int fd,
+ 	  afct = afct->next;
+ 	}
+ 
+-      if (fd != -1 && name != original_name && strcmp (name, original_name))
++      if (name != original_name && strcmp (name, original_name))
+         {
+-          /* An audit library changed what we're supposed to open,
+-             so FD no longer matches it.  */
+-          __close_nocancel (fd);
+-          fd = -1;
++	  /* Tell the caller we're looking at something different from
++	     ORIGINAL_NAME.  */
++	  *changed_by_audit = true;
++
++	  if (fd != -1)
++	    {
++	      /* An audit library changed what we're supposed to open, so FD
++		 no longer matches it.  */
++	      __close_nocancel (fd);
++	      fd = -1;
++	    }
+         }
+     }
+ #endif
+@@ -1782,6 +1795,7 @@ open_path (const char *name, size_t namelen, int mode,
+       char *edp;
+       int here_any = 0;
+       int err;
++      bool changed_by_audit;
+ 
+       /* If we are debugging the search for libraries print the path
+ 	 now if it hasn't happened now.  */
+@@ -1810,16 +1824,19 @@ open_path (const char *name, size_t namelen, int mode,
+ 	    _dl_debug_printf ("  trying file=%s\n", buf);
+ 
+ 	  fd = open_verify (buf, -1, fbp, loader, whatcode, mode,
+-			    found_other_class, false);
++			    found_other_class, false,
++			    &changed_by_audit);
+ 	  if (this_dir->status[cnt] == unknown)
+ 	    {
+ 	      if (fd != -1)
+ 		this_dir->status[cnt] = existing;
+ 	      /* Do not update the directory information when loading
+ 		 auditing code.  We must try to disturb the program as
+-		 little as possible.  */
+-	      else if (loader == NULL
+-		       || GL(dl_ns)[loader->l_ns]._ns_loaded->l_auditing == 0)
++		 little as possible.  Additionally, if the audit module
++		 change the file name, keep directory information as is.  */
++	      else if ((loader == NULL
++			|| GL(dl_ns)[loader->l_ns]._ns_loaded->l_auditing == 0)
++		       && !changed_by_audit)
+ 		{
+ 		  /* We failed to open machine dependent library.  Let's
+ 		     test whether there is any directory at all.  */
+@@ -2064,10 +2081,11 @@ _dl_map_object (struct link_map *loader, const char *name,
+           realname = _dl_sysdep_open_object (name, namelen, &fd);
+           if (realname != NULL)
+             {
++	      bool changed_by_audit;
+               fd = open_verify (realname, fd,
+                                 &fb, loader ?: GL(dl_ns)[nsid]._ns_loaded,
+                                 LA_SER_CONFIG, mode, &found_other_class,
+-                                false);
++                                false, &changed_by_audit);
+               if (fd == -1)
+                 free (realname);
+             }
+@@ -2118,10 +2136,11 @@ _dl_map_object (struct link_map *loader, const char *name,
+ 
+ 	      if (cached != NULL)
+ 		{
++		  bool changed_by_audit;
+ 		  fd = open_verify (cached, -1,
+ 				    &fb, loader ?: GL(dl_ns)[nsid]._ns_loaded,
+ 				    LA_SER_CONFIG, mode, &found_other_class,
+-				    false);
++				    false, &changed_by_audit);
+ 		  if (__glibc_likely (fd != -1))
+ 		    realname = cached;
+ 		  else
+@@ -2153,9 +2172,10 @@ _dl_map_object (struct link_map *loader, const char *name,
+ 	fd = -1;
+       else
+ 	{
++	  bool changed_by_audit;
+ 	  fd = open_verify (realname, -1, &fb,
+ 			    loader ?: GL(dl_ns)[nsid]._ns_loaded, 0, mode,
+-			    &found_other_class, true);
++			    &found_other_class, true, &changed_by_audit);
+ 	  if (__glibc_unlikely (fd == -1))
+ 	    free (realname);
+ 	}