[PATCH] rtld: Add --no-default-paths option

[PATCH] rtld: Add --no-default-paths option

[Fergus Dall]

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

This option causes the default library search path to be skipped,
using only the paths in DT_RPATH, LD_LIBRARY_PATH, and
DT_RUNPATH. This option implies --inhibit-cache, as there is no point
in searching a cache of system libraries when we are not using the
system libraries at all.

This is necessary to preserve negative search results when isolating
applications from the system libraries. This can be important when an
application uses dlopen at run time to load optional libraries.

When a shared library is required by the application, it can be
isolated by putting appropriate versions of the libraries in
directories specified in LD_LIBRARY_PATH, because the library search
will always terminate before potentially loading any system libraries.

On the other hand, if the application should be run without an
optional library, the search will proceed past the LD_LIBRARY_PATH
directories into the default system libraries, potentially causing an
incorrect library to be linked.

[-- Attachment #2: 0001-rtld-Add-no-default-paths-option.patch --]
[-- Type: text/x-patch, Size: 4954 bytes --]

From bf76dfcdd411a0394957b7a7ce8ee7c47d997036 Mon Sep 17 00:00:00 2001
From: Fergus Dall <sidereal@google.com>
Date: Fri, 21 May 2021 17:16:38 +1000
Subject: [PATCH] rtld: Add --no-default-paths option
To: libc-alpha@sourceware.org
Cc: chromeos-toolchain@google.com,
    vapier@google.com,
    clumptini@google.com

This option causes the default library search path to be skipped,
using only the paths in DT_RPATH, LD_LIBRARY_PATH, and
DT_RUNPATH. This option implies --inhibit-cache, as there is no point
in searching a cache of system libraries when we are not using the
system libraries at all.

This is necessary to preserve negative search results when isolating
applications from the system libraries. This can be important when an
application uses dlopen at run time to load optional libraries.

When a shared library is required by the application, it can be
isolated by putting appropriate versions of the libraries in
directories specified in LD_LIBRARY_PATH, because the library search
will always terminate before potentially loading any system libraries.

On the other hand, if the application should be run without an
optional library, the search will proceed past the LD_LIBRARY_PATH
directories into the default system libraries, potentially causing an
incorrect library to be linked.
---
 elf/dl-load.c              |  6 ++++--
 elf/dl-support.c           |  2 ++
 elf/dl-usage.c             |  2 ++
 elf/rtld.c                 | 10 ++++++++++
 sysdeps/generic/ldsodefs.h |  3 +++
 5 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/elf/dl-load.c b/elf/dl-load.c
index 918ec7546c..4ed0d1767c 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -2258,7 +2258,8 @@ _dl_map_object (struct link_map *loader, const char *name,
       if (fd == -1
 	  && ((l = loader ?: GL(dl_ns)[nsid]._ns_loaded) == NULL
 	      || __glibc_likely (!(l->l_flags_1 & DF_1_NODEFLIB)))
-	  && __rtld_search_dirs.dirs != (void *) -1)
+	  && __rtld_search_dirs.dirs != (void *) -1
+	  && __glibc_likely (GLRO(dl_no_default_paths) == 0))
 	fd = open_path (name, namelen, mode, &__rtld_search_dirs,
 			&realname, &fb, l, LA_SER_DEFAULT, &found_other_class);
 
@@ -2438,7 +2439,8 @@ _dl_rtld_di_serinfo (struct link_map *loader, Dl_serinfo *si, bool counting)
      a way to indicate that in the results for Dl_serinfo.  */
 
   /* Finally, try the default path.  */
-  if (!(loader->l_flags_1 & DF_1_NODEFLIB))
+  if (!(loader->l_flags_1 & DF_1_NODEFLIB)
+      && __glibc_likely (GLRO(dl_no_default_paths) == 0))
     add_path (&p, &__rtld_search_dirs, XXX_default);
 
   if (counting)
diff --git a/elf/dl-support.c b/elf/dl-support.c
index dfc9ab760e..36261ed080 100644
--- a/elf/dl-support.c
+++ b/elf/dl-support.c
@@ -144,6 +144,8 @@ size_t _dl_minsigstacksize = CONSTANT_MINSIGSTKSZ;
 
 int _dl_inhibit_cache;
 
+int _dl_no_default_paths;
+
 unsigned int _dl_osversion;
 
 /* All known directories in sorted order.  */
diff --git a/elf/dl-usage.c b/elf/dl-usage.c
index 5ad3a72559..f0c1a38eab 100644
--- a/elf/dl-usage.c
+++ b/elf/dl-usage.c
@@ -247,6 +247,8 @@ setting environment variables (which would be inherited by subprocesses).\n\
   --inhibit-cache       Do not use " LD_SO_CACHE "\n\
   --library-path PATH   use given PATH instead of content of the environment\n\
                         variable LD_LIBRARY_PATH\n\
+  --no-default-paths    Do not use the default library search path\n\
+                        This option implies --inhibit-cache\n\
   --glibc-hwcaps-prepend LIST\n\
                         search glibc-hwcaps subdirectories in LIST\n\
   --glibc-hwcaps-mask LIST\n\
diff --git a/elf/rtld.c b/elf/rtld.c
index fbbd60b446..89183107e4 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -360,6 +360,7 @@ struct rtld_global_ro _rtld_global_ro attribute_relro =
     ._dl_fpu_control = _FPU_DEFAULT,
     ._dl_pagesize = EXEC_PAGESIZE,
     ._dl_inhibit_cache = 0,
+    ._dl_no_default_paths = 0,
 
     /* Function pointers.  */
     ._dl_debug_printf = _dl_debug_printf,
@@ -1204,6 +1205,15 @@ dl_main (const ElfW(Phdr) *phdr,
 	    _dl_argc -= 2;
 	    _dl_argv += 2;
 	  }
+        else if (! strcmp (_dl_argv[1], "--no-default-paths"))
+          {
+            GLRO(dl_no_default_paths) = 1;
+            GLRO(dl_inhibit_cache) = 1;
+
+	    ++_dl_skip_args;
+	    --_dl_argc;
+	    ++_dl_argv;
+          }
 	else if (! strcmp (_dl_argv[1], "--inhibit-rpath")
 		 && _dl_argc > 2)
 	  {
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index e383aa1dc3..e01ddd7b4b 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -551,6 +551,9 @@ struct rtld_global_ro
   /* Do we read from ld.so.cache?  */
   EXTERN int _dl_inhibit_cache;
 
+  /* Do we search the default system paths? */
+  EXTERN int _dl_no_default_paths;
+
   /* Copy of the content of `_dl_main_searchlist' at startup time.  */
   EXTERN struct r_scope_elem _dl_initial_searchlist;
 
-- 
2.31.1.818.g46aad6cb9e-goog

Re: [PATCH] rtld: Add --no-default-paths option

[Mike Frysinger]

On 25 May 2021 12:25, Fergus Dall via Libc-alpha wrote:
> This option implies --inhibit-cache, as there is no point
> in searching a cache of system libraries when we are not using the
> system libraries at all.

i'm on the fence about this.  i don't disagree with your logic, but it
still seems like we have a dedicated --inhibit-cache option, so might as
well leave it at that.

logic-wise, code seems OK.  would be nice to add a test, but this is a bit
hard as it relies on the installed / paths.

> +int _dl_no_default_paths;

i try to avoid negative variables as it makes reading code harder.
how about "_dl_search_default_paths" ?

> --- a/elf/dl-usage.c
> +++ b/elf/dl-usage.c
>
> +  --no-default-paths    Do not use the default library search path\n\a
> +                        This option implies --inhibit-cache\n\

prob should read:
  --no-default-paths    do not use the default library search path\n\a
                        (this option implies --inhibit-cache)\n\
-mike

Re: [PATCH] rtld: Add --no-default-paths option

[Carlos O'Donell]

On 5/24/21 11:22 PM, Mike Frysinger via Libc-alpha wrote:
> On 25 May 2021 12:25, Fergus Dall via Libc-alpha wrote:
>> This option implies --inhibit-cache, as there is no point
>> in searching a cache of system libraries when we are not using the
>> system libraries at all.
> 
> i'm on the fence about this.  i don't disagree with your logic, but it
> still seems like we have a dedicated --inhibit-cache option, so might as
> well leave it at that.
> 
> logic-wise, code seems OK.  would be nice to add a test, but this is a bit
> hard as it relies on the installed / paths.

We have containerized tests now that can use installed paths.

We have tests for corrupted cache, NSS, localedef invocation etc. all that
depend on installed paths to trigger the specific code.

We *should* be creating tests in all of the cases where we lacked them
before because of installed path problems.

> 
>> +int _dl_no_default_paths;
> 
> i try to avoid negative variables as it makes reading code harder.
> how about "_dl_search_default_paths" ?
> 
>> --- a/elf/dl-usage.c
>> +++ b/elf/dl-usage.c
>>
>> +  --no-default-paths    Do not use the default library search path\n\a
>> +                        This option implies --inhibit-cache\n\
> 
> prob should read:
>   --no-default-paths    do not use the default library search path\n\a
>                         (this option implies --inhibit-cache)\n\
> -mike
> 


-- 
Cheers,
Carlos.

Re: [PATCH] rtld: Add --no-default-paths option

[Fergus Dall]

On Tue, May 25, 2021 at 10:01 PM Carlos O'Donell <carlos@redhat.com> wrote:
>
> On 5/24/21 11:22 PM, Mike Frysinger via Libc-alpha wrote:
> > On 25 May 2021 12:25, Fergus Dall via Libc-alpha wrote:
> >> This option implies --inhibit-cache, as there is no point
> >> in searching a cache of system libraries when we are not using the
> >> system libraries at all.
> >
> > i'm on the fence about this.  i don't disagree with your logic, but it
> > still seems like we have a dedicated --inhibit-cache option, so might as
> > well leave it at that.
> >
> > logic-wise, code seems OK.  would be nice to add a test, but this is a bit
> > hard as it relies on the installed / paths.
>
> We have containerized tests now that can use installed paths.
>
> We have tests for corrupted cache, NSS, localedef invocation etc. all that
> depend on installed paths to trigger the specific code.
>
> We *should* be creating tests in all of the cases where we lacked them
> before because of installed path problems.

Could you point me to an example?

Re: [PATCH] rtld: Add --no-default-paths option

[Joseph Myers]

As a new user-visible feature, the patch should add a NEWS entry (under 
"Major new features" for 2.34).

In principle, it would be good to use this option for running all tests in 
the glibc testsuite.  But given the complication that libgcc_s and 
libstdc++ shared libraries would then always need copying to the build 
directory before running tests (at present that's only needed if they're 
in directories the dynamic linker doesn't search by default, I think), and 
that in some configurations it would make sense for the glibc build system 
to copy those libraries but in other cases it may be more appropriate to 
copy them from outside the build system, it's probably best not to include 
that change in this patch.

-- 
Joseph S. Myers
joseph@codesourcery.com

[PATCH v3 1/2] rtld: Add --no-default-paths option

[Fergus Dall]

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

gnu formatting fixes have been applied to the test files, and the SONAMEs
for libc and libdl now use <gnu/lib-names.h>

-- 8< --
This option causes the default library search path to be skipped,
using only the paths in DT_RPATH, LD_LIBRARY_PATH, and
DT_RUNPATH. This option implies --inhibit-cache, as there is no point
in searching a cache of system libraries when we are not using the
system libraries at all.

This is necessary to preserve negative search results when isolating
applications from the system libraries. This can be important when an
application uses dlopen at run time to load optional libraries.

When a shared library is required by the application, it can be
isolated by putting appropriate versions of the libraries in
directories specified in LD_LIBRARY_PATH, because the library search
will always terminate before potentially loading any system libraries.

On the other hand, if the application should be run without an
optional library, the search will proceed past the LD_LIBRARY_PATH
directories into the default system libraries, potentially causing an
incorrect library to be linked.

[-- Attachment #2: v3-0001-rtld-Add-no-default-paths-option.patch --]
[-- Type: application/x-patch, Size: 5604 bytes --]

Re: [PATCH v3 1/2] rtld: Add --no-default-paths option

[Mike Frysinger]

On 07 Jun 2021 20:28, Fergus Dall via Libc-alpha wrote:
> This option causes the default library search path to be skipped,
> using only the paths in DT_RPATH, LD_LIBRARY_PATH, and
> DT_RUNPATH. This option implies --inhibit-cache, as there is no point
> in searching a cache of system libraries when we are not using the
> system libraries at all.

in docs, should we be overly pedantic for clarity sake ?  for example:
-search the default system paths
+search the default (compiled in) system paths

> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -1204,6 +1205,15 @@ dl_main (const ElfW(Phdr) *phdr,
>  	    _dl_argc -= 2;
>  	    _dl_argv += 2;
>  	  }
> +        else if (! strcmp (_dl_argv[1], "--no-default-paths"))
> +          {
> +            GLRO(dl_search_default_paths) = 0;
> +            GLRO(dl_inhibit_cache) = 1;

bad indentation

> --- a/sysdeps/generic/ldsodefs.h
> +++ b/sysdeps/generic/ldsodefs.h
> @@ -551,6 +551,9 @@ struct rtld_global_ro
>    /* Do we read from ld.so.cache?  */
>    EXTERN int _dl_inhibit_cache;
>  
> +  /* Do we search the default system paths? */

two spaces before */
-mike