[08/10] elf: Do not load libpthread for PTHREAD_IN_LIBC

Message ID e3ad0aceca81d9db786582382b0a3ce4a7b2221c.1621347402.git.fweimer@redhat.com
State Dropped
Headers
Series nptl: Complete libpthread removal |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Florian Weimer May 18, 2021, 2:25 p.m. UTC
  ---
 elf/dl-load.c    | 19 +++++++++++++++++--
 elf/dl-version.c |  9 +++++++++
 2 files changed, 26 insertions(+), 2 deletions(-)
  

Comments

Adhemerval Zanella May 20, 2021, 8:53 p.m. UTC | #1
I am not sure about this change, is this required to load old shared
libraries or binaries with libpthread as DT_NEEDED? 

I would prefer if we add an orthogonal way by providing a dummy 
libpthread.so in such cases instead of adding this ad-hoc solution 
to loader.  Maybe it would even be possible to hack a dummy libpthread.so
that results in no runtime overhead. 

On 18/05/2021 11:25, Florian Weimer via Libc-alpha wrote:
> ---
>  elf/dl-load.c    | 19 +++++++++++++++++--
>  elf/dl-version.c |  9 +++++++++
>  2 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index 918ec7546c..f97cb140f4 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -2029,12 +2029,26 @@ open_path (const char *name, size_t namelen, int mode,
>  
>  /* Map in the shared object file NAME.  */
>  
> +static inline const char *
> +object_real_name (const char *name)
> +{
> +  /* If libpthread is integrated into libc, treat a request to load
> +     libpthread as a request to load libc (because libc was a
> +     dependency of libpthread).  */
> +#if PTHREAD_IN_LIBC
> +  if (strcmp (name, LIBPTHREAD_SO) == 0)
> +    return LIBC_SO;
> +#endif
> +  return name;
> +}
> +
>  struct link_map *
>  _dl_map_object (struct link_map *loader, const char *name,
>  		int type, int trace_mode, int mode, Lmid_t nsid)
>  {
>    int fd;
>    const char *origname = NULL;
> +  const char *implementation_name = object_real_name (name);
>    char *realname;
>    char *name_copy;
>    struct link_map *l;
> @@ -2051,7 +2065,7 @@ _dl_map_object (struct link_map *loader, const char *name,
>  	 yet been opened.  */
>        if (__glibc_unlikely ((l->l_faked | l->l_removed) != 0))
>  	continue;
> -      if (!_dl_name_match_p (name, l))
> +      if (!_dl_name_match_p (implementation_name, l))
>  	{
>  	  const char *soname;
>  
> @@ -2061,7 +2075,7 @@ _dl_map_object (struct link_map *loader, const char *name,
>  
>  	  soname = ((const char *) D_PTR (l, l_info[DT_STRTAB])
>  		    + l->l_info[DT_SONAME]->d_un.d_val);
> -	  if (strcmp (name, soname) != 0)
> +	  if (strcmp (implementation_name, soname) != 0)
>  	    continue;
>  
>  	  /* We have a match on a new name -- cache it.  */
> @@ -2106,6 +2120,7 @@ _dl_map_object (struct link_map *loader, const char *name,
>  		  if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_FILES))
>  		    _dl_debug_printf ("audit changed filename %s -> %s\n",
>  				      before, name);
> +		  implementation_name = object_real_name (name);
>  
>  		  if (origname == NULL)
>  		    origname = before;
> diff --git a/elf/dl-version.c b/elf/dl-version.c
> index 914955c2a8..932cffda09 100644
> --- a/elf/dl-version.c
> +++ b/elf/dl-version.c
> @@ -24,6 +24,7 @@
>  #include <string.h>
>  #include <ldsodefs.h>
>  #include <_itoa.h>
> +#include <gnu/lib-names.h>
>  
>  #include <assert.h>
>  
> @@ -200,6 +201,14 @@ _dl_check_map_versions (struct link_map *map, int verbose, int trace_mode)
>  	  ElfW(Vernaux) *aux;
>  	  struct link_map *needed = find_needed (strtab + ent->vn_file, map);
>  
> +#if PTHREAD_IN_LIBC
> +	  /* With an integrated libpthread, check libpthread
> +	     references against libc instead.  */
> +	  if (needed == NULL
> +	      && strcmp (strtab + ent->vn_file, LIBPTHREAD_SO) == 0)
> +	    needed = find_needed (LIBC_SO, map);
> +#endif
> +
>  	  /* If NEEDED is NULL this means a dependency was not found
>  	     and no stub entry was created.  This should never happen.  */
>  	  assert (needed != NULL);
>
  
Florian Weimer May 21, 2021, 7:15 p.m. UTC | #2
* Adhemerval Zanella via Libc-alpha:

> I am not sure about this change, is this required to load old shared
> libraries or binaries with libpthread as DT_NEEDED? 
>
> I would prefer if we add an orthogonal way by providing a dummy 
> libpthread.so in such cases instead of adding this ad-hoc solution 
> to loader.  Maybe it would even be possible to hack a dummy libpthread.so
> that results in no runtime overhead.

Yes, Andreas and H.J. argued for the same thing.  The new series
implements that.

Thanks,
Florian
  

Patch

diff --git a/elf/dl-load.c b/elf/dl-load.c
index 918ec7546c..f97cb140f4 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -2029,12 +2029,26 @@  open_path (const char *name, size_t namelen, int mode,
 
 /* Map in the shared object file NAME.  */
 
+static inline const char *
+object_real_name (const char *name)
+{
+  /* If libpthread is integrated into libc, treat a request to load
+     libpthread as a request to load libc (because libc was a
+     dependency of libpthread).  */
+#if PTHREAD_IN_LIBC
+  if (strcmp (name, LIBPTHREAD_SO) == 0)
+    return LIBC_SO;
+#endif
+  return name;
+}
+
 struct link_map *
 _dl_map_object (struct link_map *loader, const char *name,
 		int type, int trace_mode, int mode, Lmid_t nsid)
 {
   int fd;
   const char *origname = NULL;
+  const char *implementation_name = object_real_name (name);
   char *realname;
   char *name_copy;
   struct link_map *l;
@@ -2051,7 +2065,7 @@  _dl_map_object (struct link_map *loader, const char *name,
 	 yet been opened.  */
       if (__glibc_unlikely ((l->l_faked | l->l_removed) != 0))
 	continue;
-      if (!_dl_name_match_p (name, l))
+      if (!_dl_name_match_p (implementation_name, l))
 	{
 	  const char *soname;
 
@@ -2061,7 +2075,7 @@  _dl_map_object (struct link_map *loader, const char *name,
 
 	  soname = ((const char *) D_PTR (l, l_info[DT_STRTAB])
 		    + l->l_info[DT_SONAME]->d_un.d_val);
-	  if (strcmp (name, soname) != 0)
+	  if (strcmp (implementation_name, soname) != 0)
 	    continue;
 
 	  /* We have a match on a new name -- cache it.  */
@@ -2106,6 +2120,7 @@  _dl_map_object (struct link_map *loader, const char *name,
 		  if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_FILES))
 		    _dl_debug_printf ("audit changed filename %s -> %s\n",
 				      before, name);
+		  implementation_name = object_real_name (name);
 
 		  if (origname == NULL)
 		    origname = before;
diff --git a/elf/dl-version.c b/elf/dl-version.c
index 914955c2a8..932cffda09 100644
--- a/elf/dl-version.c
+++ b/elf/dl-version.c
@@ -24,6 +24,7 @@ 
 #include <string.h>
 #include <ldsodefs.h>
 #include <_itoa.h>
+#include <gnu/lib-names.h>
 
 #include <assert.h>
 
@@ -200,6 +201,14 @@  _dl_check_map_versions (struct link_map *map, int verbose, int trace_mode)
 	  ElfW(Vernaux) *aux;
 	  struct link_map *needed = find_needed (strtab + ent->vn_file, map);
 
+#if PTHREAD_IN_LIBC
+	  /* With an integrated libpthread, check libpthread
+	     references against libc instead.  */
+	  if (needed == NULL
+	      && strcmp (strtab + ent->vn_file, LIBPTHREAD_SO) == 0)
+	    needed = find_needed (LIBC_SO, map);
+#endif
+
 	  /* If NEEDED is NULL this means a dependency was not found
 	     and no stub entry was created.  This should never happen.  */
 	  assert (needed != NULL);