elf: Also try DT_RUNPATH for internal dlopen [BZ #28455]
Checks
| Context |
Check |
Description |
| dj/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
| dj/TryBot-32bit |
success
|
Build for i686
|
Commit Message
DT_RUNPATH is only used to find the immediate dependencies of the
executable or shared object containing the DT_RUNPATH entry. The
glibc internal dlopen call should try the DT_RUNPATH of the executable.
This partially fixes BZ #28455.
---
elf/Makefile | 9 +++++++--
elf/dl-load.c | 30 ++++++++++++++++++++++--------
elf/tst-audit14a.c | 1 +
nss/Makefile | 3 +++
nss/tst-nss-test1a.c | 1 +
5 files changed, 34 insertions(+), 10 deletions(-)
create mode 100644 elf/tst-audit14a.c
create mode 100644 nss/tst-nss-test1a.c
Comments
On Fri, Oct 15, 2021 at 1:07 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> DT_RUNPATH is only used to find the immediate dependencies of the
> executable or shared object containing the DT_RUNPATH entry. The
> glibc internal dlopen call should try the DT_RUNPATH of the executable.
> This partially fixes BZ #28455.
> ---
> elf/Makefile | 9 +++++++--
> elf/dl-load.c | 30 ++++++++++++++++++++++--------
> elf/tst-audit14a.c | 1 +
> nss/Makefile | 3 +++
> nss/tst-nss-test1a.c | 1 +
> 5 files changed, 34 insertions(+), 10 deletions(-)
> create mode 100644 elf/tst-audit14a.c
> create mode 100644 nss/tst-nss-test1a.c
>
> diff --git a/elf/Makefile b/elf/Makefile
> index eeef71b82a..739cd6c8ef 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -239,10 +239,10 @@ ifneq ($(selinux-enabled),1)
> tests-execstack-yes = tst-execstack tst-execstack-needed tst-execstack-prog
> endif
> ifeq ($(have-depaudit),yes)
> -tests += tst-audit14 tst-audit15 tst-audit16
> +tests += tst-audit14 tst-audit15 tst-audit16 tst-audit14a
> ifeq ($(run-built-tests),yes)
> tests-special += $(objpfx)tst-audit14-cmp.out $(objpfx)tst-audit15-cmp.out \
> - $(objpfx)tst-audit16-cmp.out
> + $(objpfx)tst-audit16-cmp.out $(objpfx)tst-audit14a-cmp.out
> endif
> endif
> endif
> @@ -1479,6 +1479,8 @@ tst-auditmany-ENV = \
> LDFLAGS-tst-audit14 = -Wl,--audit=tst-auditlogmod-1.so
> $(objpfx)tst-auditlogmod-1.so: $(libsupport)
> $(objpfx)tst-audit14.out: $(objpfx)tst-auditlogmod-1.so
> +LDFLAGS-tst-audit14a = -Wl,--audit=tst-auditlogmod-1.so,--enable-new-dtags
> +$(objpfx)tst-audit14a.out: $(objpfx)tst-auditlogmod-1.so
> LDFLAGS-tst-audit15 = \
> -Wl,--audit=tst-auditlogmod-1.so,--depaudit=tst-auditlogmod-2.so
> $(objpfx)tst-auditlogmod-2.so: $(libsupport)
> @@ -1505,6 +1507,9 @@ tst-audit17-ENV = LD_AUDIT=$(objpfx)tst-auditmod17.so
> $(objpfx)tst-audit14-cmp.out: tst-audit14.exp $(objpfx)tst-audit14.out
> cmp $^ > $@; \
> $(evaluate-test)
> +$(objpfx)tst-audit14a-cmp.out: tst-audit14.exp $(objpfx)tst-audit14a.out
> + cmp $^ > $@; \
> + $(evaluate-test)
> $(objpfx)tst-audit15-cmp.out: tst-audit15.exp $(objpfx)tst-audit15.out
> cmp $^ > $@; \
> $(evaluate-test)
> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index 18d3e8fe64..95f4d13c12 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -2162,15 +2162,29 @@ _dl_map_object (struct link_map *loader, const char *name,
> }
>
> /* If dynamically linked, try the DT_RPATH of the executable
> - itself. NB: we do this for lookups in any namespace. */
> + itself. NB: we do this for lookups in any namespace.
> +
> + Also try DT_RUNPATH for glibc internal dlopen call. */
> if (fd == -1 && !did_main_map
> - && main_map != NULL && main_map->l_type != lt_loaded
> - && cache_rpath (main_map, &main_map->l_rpath_dirs, DT_RPATH,
> - "RPATH"))
> - fd = open_path (name, namelen, mode,
> - &main_map->l_rpath_dirs,
> - &realname, &fb, loader ?: main_map, LA_SER_RUNPATH,
> - &found_other_class);
> + && main_map != NULL && main_map->l_type != lt_loaded)
> + {
> + struct r_search_path_struct l_rpath_dirs;
> + struct r_search_path_struct *l_rpath_dirs_p = NULL;
> + if (cache_rpath (main_map, &main_map->l_rpath_dirs,
> + DT_RPATH, "RPATH"))
> + l_rpath_dirs_p = &main_map->l_rpath_dirs;
> + else if (__glibc_unlikely (mode & __RTLD_DLOPEN))
> + {
> + l_rpath_dirs.dirs = NULL;
> + if (cache_rpath (main_map, &l_rpath_dirs, DT_RUNPATH,
> + "RUNPATH"))
> + l_rpath_dirs_p = &l_rpath_dirs;
> + }
> + if (l_rpath_dirs_p)
> + fd = open_path (name, namelen, mode, l_rpath_dirs_p,
> + &realname, &fb, loader ?: main_map,
> + LA_SER_RUNPATH, &found_other_class);
> + }
> }
>
> /* Try the LD_LIBRARY_PATH environment variable. */
> diff --git a/elf/tst-audit14a.c b/elf/tst-audit14a.c
> new file mode 100644
> index 0000000000..c6232eacf2
> --- /dev/null
> +++ b/elf/tst-audit14a.c
> @@ -0,0 +1 @@
> +#include "tst-audit14.c"
> diff --git a/nss/Makefile b/nss/Makefile
> index bccf9f2806..cd99e04732 100644
> --- a/nss/Makefile
> +++ b/nss/Makefile
> @@ -58,6 +58,7 @@ tests-static = tst-field
> tests-internal = tst-field
> tests = test-netdb test-digits-dots tst-nss-getpwent bug17079 \
> tst-nss-test1 \
> + tst-nss-test1a \
> tst-nss-test2 \
> tst-nss-test4 \
> tst-nss-test5
> @@ -189,3 +190,5 @@ endif
>
> $(objpfx)tst-nss-files-alias-leak.out: $(objpfx)/libnss_files.so
> $(objpfx)tst-nss-files-alias-truncated.out: $(objpfx)/libnss_files.so
> +
> +LDFLAGS-tst-nss-test1a = -Wl,--enable-new-dtags
> diff --git a/nss/tst-nss-test1a.c b/nss/tst-nss-test1a.c
> new file mode 100644
> index 0000000000..f1428259c8
> --- /dev/null
> +++ b/nss/tst-nss-test1a.c
> @@ -0,0 +1 @@
> +#include "tst-nss-test1.c"
> --
> 2.31.1
>
PING.
On 10/15/21 16:07, H.J. Lu via Libc-alpha wrote:
> DT_RUNPATH is only used to find the immediate dependencies of the
> executable or shared object containing the DT_RUNPATH entry. The
> glibc internal dlopen call should try the DT_RUNPATH of the executable.
> This partially fixes BZ #28455.
This needs some changes.
High level changes:
(1) Split DT_RUNPATH handling into distinct section away from DT_RPATH handling.
(2) Should be specific for audit modules in object being loaded. That is to say
if this is an audit module then the object that caused the loading should
be inspected for DT_RUNPATH. I think the current check is too broad.
(3) Why does NSS fail? The modules have the correct DT_RUNPATH set.
When the loader itself calls dlopen it does so with __RTLD_DLOPEN to indicate
that this is an indirect dlopen of an object based on some other semantic
requirement e.g. LD_AUDIT.
It is clear to me that an executable with a recorded audit module should use
the exectuables DT_RUNPATH to search for the audit module. How we achieve that
in the code needs a little bit of restructuring.
This patch places the DT_RUNPATH lookup within the DT_RPATH lookup, and that
is not logically correct, it should be a distinct lookup *after* LD_LIBRARY_PATH
lookup and around line 2187 with the current RUNPATH lookup.
This lookup is a *unique* lookup for audit modules IMO since they are the only
lookup that is __RTLD_DLOPEN that would need to also lookup via DT_RUNPATH
from the executable.
I think generally doing this lookup for __RTDL_DLOPEN is too broad and could
cause other problems for which we don't have test coverage.
> ---
> elf/Makefile | 9 +++++++--
> elf/dl-load.c | 30 ++++++++++++++++++++++--------
> elf/tst-audit14a.c | 1 +
> nss/Makefile | 3 +++
> nss/tst-nss-test1a.c | 1 +
> 5 files changed, 34 insertions(+), 10 deletions(-)
> create mode 100644 elf/tst-audit14a.c
> create mode 100644 nss/tst-nss-test1a.c
>
> diff --git a/elf/Makefile b/elf/Makefile
> index eeef71b82a..739cd6c8ef 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -239,10 +239,10 @@ ifneq ($(selinux-enabled),1)
> tests-execstack-yes = tst-execstack tst-execstack-needed tst-execstack-prog
> endif
> ifeq ($(have-depaudit),yes)
> -tests += tst-audit14 tst-audit15 tst-audit16
> +tests += tst-audit14 tst-audit15 tst-audit16 tst-audit14a
OK. Add new tst-audit14a test.
> ifeq ($(run-built-tests),yes)
> tests-special += $(objpfx)tst-audit14-cmp.out $(objpfx)tst-audit15-cmp.out \
> - $(objpfx)tst-audit16-cmp.out
> + $(objpfx)tst-audit16-cmp.out $(objpfx)tst-audit14a-cmp.out
OK. Add comparison output for special test.
> endif
> endif
> endif
> @@ -1479,6 +1479,8 @@ tst-auditmany-ENV = \
> LDFLAGS-tst-audit14 = -Wl,--audit=tst-auditlogmod-1.so
> $(objpfx)tst-auditlogmod-1.so: $(libsupport)
> $(objpfx)tst-audit14.out: $(objpfx)tst-auditlogmod-1.so
> +LDFLAGS-tst-audit14a = -Wl,--audit=tst-auditlogmod-1.so,--enable-new-dtags
> +$(objpfx)tst-audit14a.out: $(objpfx)tst-auditlogmod-1.so
OK. Adds new test with --enable-new-dtags to catch differences in DT_RPATH vs. DT_RUNPATH.
> LDFLAGS-tst-audit15 = \
> -Wl,--audit=tst-auditlogmod-1.so,--depaudit=tst-auditlogmod-2.so
> $(objpfx)tst-auditlogmod-2.so: $(libsupport)
> @@ -1505,6 +1507,9 @@ tst-audit17-ENV = LD_AUDIT=$(objpfx)tst-auditmod17.so
> $(objpfx)tst-audit14-cmp.out: tst-audit14.exp $(objpfx)tst-audit14.out
> cmp $^ > $@; \
> $(evaluate-test)
> +$(objpfx)tst-audit14a-cmp.out: tst-audit14.exp $(objpfx)tst-audit14a.out
> + cmp $^ > $@; \
> + $(evaluate-test)
OK. Do comparison.
> $(objpfx)tst-audit15-cmp.out: tst-audit15.exp $(objpfx)tst-audit15.out
> cmp $^ > $@; \
> $(evaluate-test)
> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index 18d3e8fe64..95f4d13c12 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -2162,15 +2162,29 @@ _dl_map_object (struct link_map *loader, const char *name,
> }
>
> /* If dynamically linked, try the DT_RPATH of the executable
> - itself. NB: we do this for lookups in any namespace. */
> + itself. NB: we do this for lookups in any namespace.
> +
> + Also try DT_RUNPATH for glibc internal dlopen call. */
OK. Agreed, we should be looking at DT_RPATH and DT_RUNPATH.
The entire sequence here in _dl_map_object contains the logic for handling:
* DT_RPATH
* LD_LIBRARY_PATH
* DT_RUNPATH
etc.
> if (fd == -1 && !did_main_map
> - && main_map != NULL && main_map->l_type != lt_loaded
> - && cache_rpath (main_map, &main_map->l_rpath_dirs, DT_RPATH,
> - "RPATH"))
> - fd = open_path (name, namelen, mode,
> - &main_map->l_rpath_dirs,
> - &realname, &fb, loader ?: main_map, LA_SER_RUNPATH,
> - &found_other_class);
> + && main_map != NULL && main_map->l_type != lt_loaded)
OK. Removed the cache_rpath() success call from the conditional since we need to take
different actions depending on the success or failure of the call.
This specifically is the logic for handling DT_RPATH in the object that caused the
current object to be loaded.
loader == parent object that needed name to be mapped
name == object to be loaded
The load path ends at the top-most shared object, but if we are dynamically
linked we should look at DT_RPATH for the exectuable. The rules for DT_RPATH
imply that all lookups should be using the DT_RPATH of the executable for lookups.
> + {
> + struct r_search_path_struct l_rpath_dirs;
> + struct r_search_path_struct *l_rpath_dirs_p = NULL;
> + if (cache_rpath (main_map, &main_map->l_rpath_dirs,
> + DT_RPATH, "RPATH"))
> + l_rpath_dirs_p = &main_map->l_rpath_dirs;
OK. If cach_rpath() via DT_RPATH works then use that.
> + else if (__glibc_unlikely (mode & __RTLD_DLOPEN))
> + {
> + l_rpath_dirs.dirs = NULL;
> + if (cache_rpath (main_map, &l_rpath_dirs, DT_RUNPATH,
> + "RUNPATH"))
> + l_rpath_dirs_p = &l_rpath_dirs;
Else we need to look at DT_RUNPATH. Always using main_map as the link map.
I don't think this is correct, I think we should split this out and be more
specific about this lookup, restricting the scope to audit modules.
If this breaks for NSS modules we need to have a discussion about this and
it means NSS modules may need changing how they are built.
Today the libnss_test1.so.2 has large DT_RUNPATH that I expect should get
used for lookup. Why it fails isn't clearly explained in your commit message
and it should be.
> + }
> + if (l_rpath_dirs_p)
> + fd = open_path (name, namelen, mode, l_rpath_dirs_p,
> + &realname, &fb, loader ?: main_map,
> + LA_SER_RUNPATH, &found_other_class);
If we found a path then use that for open_path.
> + }
> }
>
> /* Try the LD_LIBRARY_PATH environment variable. */
> diff --git a/elf/tst-audit14a.c b/elf/tst-audit14a.c
> new file mode 100644
> index 0000000000..c6232eacf2
> --- /dev/null
> +++ b/elf/tst-audit14a.c
> @@ -0,0 +1 @@
> +#include "tst-audit14.c"
> diff --git a/nss/Makefile b/nss/Makefile
> index bccf9f2806..cd99e04732 100644
> --- a/nss/Makefile
> +++ b/nss/Makefile
> @@ -58,6 +58,7 @@ tests-static = tst-field
> tests-internal = tst-field
> tests = test-netdb test-digits-dots tst-nss-getpwent bug17079 \
> tst-nss-test1 \
> + tst-nss-test1a \
OK. New test.
> tst-nss-test2 \
> tst-nss-test4 \
> tst-nss-test5
> @@ -189,3 +190,5 @@ endif
>
> $(objpfx)tst-nss-files-alias-leak.out: $(objpfx)/libnss_files.so
> $(objpfx)tst-nss-files-alias-truncated.out: $(objpfx)/libnss_files.so
> +
> +LDFLAGS-tst-nss-test1a = -Wl,--enable-new-dtags
OK. Duplicate tst-nss-test1 as tst-nss-test1a with --enable-new-dtags to increase coverage.
> diff --git a/nss/tst-nss-test1a.c b/nss/tst-nss-test1a.c
> new file mode 100644
> index 0000000000..f1428259c8
> --- /dev/null
> +++ b/nss/tst-nss-test1a.c
> @@ -0,0 +1 @@
> +#include "tst-nss-test1.c"
OK.
>
On Mon, Nov 29, 2021 at 7:30 AM Carlos O'Donell <carlos@redhat.com> wrote:
>
> On 10/15/21 16:07, H.J. Lu via Libc-alpha wrote:
> > DT_RUNPATH is only used to find the immediate dependencies of the
> > executable or shared object containing the DT_RUNPATH entry. The
> > glibc internal dlopen call should try the DT_RUNPATH of the executable.
> > This partially fixes BZ #28455.
>
> This needs some changes.
>
> High level changes:
> (1) Split DT_RUNPATH handling into distinct section away from DT_RPATH handling.
> (2) Should be specific for audit modules in object being loaded. That is to say
> if this is an audit module then the object that caused the loading should
> be inspected for DT_RUNPATH. I think the current check is too broad.
This is how audit works with LD_LIBRARY_PATH and DT_RPATH. My
patch simply extends it to DT_RUNPATH as shown in the newly added
audit testcase.
> (3) Why does NSS fail? The modules have the correct DT_RUNPATH set.
DT_RUNPATH only applies to the module, not libc.so.
> When the loader itself calls dlopen it does so with __RTLD_DLOPEN to indicate
> that this is an indirect dlopen of an object based on some other semantic
> requirement e.g. LD_AUDIT.
>
> It is clear to me that an executable with a recorded audit module should use
> the exectuables DT_RUNPATH to search for the audit module. How we achieve that
> in the code needs a little bit of restructuring.
>
> This patch places the DT_RUNPATH lookup within the DT_RPATH lookup, and that
> is not logically correct, it should be a distinct lookup *after* LD_LIBRARY_PATH
> lookup and around line 2187 with the current RUNPATH lookup.
>
> This lookup is a *unique* lookup for audit modules IMO since they are the only
> lookup that is __RTLD_DLOPEN that would need to also lookup via DT_RUNPATH
> from the executable.
>
> I think generally doing this lookup for __RTDL_DLOPEN is too broad and could
> cause other problems for which we don't have test coverage.
Do we have 100% test coverage for __RTDL_DLOPEN usages? I can
modify the testcase to show the DT_RUNPATH problem with __RTDL_DLOPEN.
> > ---
> > elf/Makefile | 9 +++++++--
> > elf/dl-load.c | 30 ++++++++++++++++++++++--------
> > elf/tst-audit14a.c | 1 +
> > nss/Makefile | 3 +++
> > nss/tst-nss-test1a.c | 1 +
> > 5 files changed, 34 insertions(+), 10 deletions(-)
> > create mode 100644 elf/tst-audit14a.c
> > create mode 100644 nss/tst-nss-test1a.c
> >
> > diff --git a/elf/Makefile b/elf/Makefile
> > index eeef71b82a..739cd6c8ef 100644
> > --- a/elf/Makefile
> > +++ b/elf/Makefile
> > @@ -239,10 +239,10 @@ ifneq ($(selinux-enabled),1)
> > tests-execstack-yes = tst-execstack tst-execstack-needed tst-execstack-prog
> > endif
> > ifeq ($(have-depaudit),yes)
> > -tests += tst-audit14 tst-audit15 tst-audit16
> > +tests += tst-audit14 tst-audit15 tst-audit16 tst-audit14a
>
> OK. Add new tst-audit14a test.
>
> > ifeq ($(run-built-tests),yes)
> > tests-special += $(objpfx)tst-audit14-cmp.out $(objpfx)tst-audit15-cmp.out \
> > - $(objpfx)tst-audit16-cmp.out
> > + $(objpfx)tst-audit16-cmp.out $(objpfx)tst-audit14a-cmp.out
>
> OK. Add comparison output for special test.
>
> > endif
> > endif
> > endif
> > @@ -1479,6 +1479,8 @@ tst-auditmany-ENV = \
> > LDFLAGS-tst-audit14 = -Wl,--audit=tst-auditlogmod-1.so
> > $(objpfx)tst-auditlogmod-1.so: $(libsupport)
> > $(objpfx)tst-audit14.out: $(objpfx)tst-auditlogmod-1.so
> > +LDFLAGS-tst-audit14a = -Wl,--audit=tst-auditlogmod-1.so,--enable-new-dtags
> > +$(objpfx)tst-audit14a.out: $(objpfx)tst-auditlogmod-1.so
>
> OK. Adds new test with --enable-new-dtags to catch differences in DT_RPATH vs. DT_RUNPATH.
>
> > LDFLAGS-tst-audit15 = \
> > -Wl,--audit=tst-auditlogmod-1.so,--depaudit=tst-auditlogmod-2.so
> > $(objpfx)tst-auditlogmod-2.so: $(libsupport)
> > @@ -1505,6 +1507,9 @@ tst-audit17-ENV = LD_AUDIT=$(objpfx)tst-auditmod17.so
> > $(objpfx)tst-audit14-cmp.out: tst-audit14.exp $(objpfx)tst-audit14.out
> > cmp $^ > $@; \
> > $(evaluate-test)
> > +$(objpfx)tst-audit14a-cmp.out: tst-audit14.exp $(objpfx)tst-audit14a.out
> > + cmp $^ > $@; \
> > + $(evaluate-test)
>
> OK. Do comparison.
>
> > $(objpfx)tst-audit15-cmp.out: tst-audit15.exp $(objpfx)tst-audit15.out
> > cmp $^ > $@; \
> > $(evaluate-test)
> > diff --git a/elf/dl-load.c b/elf/dl-load.c
> > index 18d3e8fe64..95f4d13c12 100644
> > --- a/elf/dl-load.c
> > +++ b/elf/dl-load.c
> > @@ -2162,15 +2162,29 @@ _dl_map_object (struct link_map *loader, const char *name,
> > }
> >
> > /* If dynamically linked, try the DT_RPATH of the executable
> > - itself. NB: we do this for lookups in any namespace. */
> > + itself. NB: we do this for lookups in any namespace.
> > +
> > + Also try DT_RUNPATH for glibc internal dlopen call. */
>
> OK. Agreed, we should be looking at DT_RPATH and DT_RUNPATH.
>
> The entire sequence here in _dl_map_object contains the logic for handling:
> * DT_RPATH
> * LD_LIBRARY_PATH
> * DT_RUNPATH
> etc.
>
> > if (fd == -1 && !did_main_map
> > - && main_map != NULL && main_map->l_type != lt_loaded
> > - && cache_rpath (main_map, &main_map->l_rpath_dirs, DT_RPATH,
> > - "RPATH"))
> > - fd = open_path (name, namelen, mode,
> > - &main_map->l_rpath_dirs,
> > - &realname, &fb, loader ?: main_map, LA_SER_RUNPATH,
> > - &found_other_class);
> > + && main_map != NULL && main_map->l_type != lt_loaded)
>
> OK. Removed the cache_rpath() success call from the conditional since we need to take
> different actions depending on the success or failure of the call.
>
> This specifically is the logic for handling DT_RPATH in the object that caused the
> current object to be loaded.
>
> loader == parent object that needed name to be mapped
> name == object to be loaded
>
> The load path ends at the top-most shared object, but if we are dynamically
> linked we should look at DT_RPATH for the exectuable. The rules for DT_RPATH
> imply that all lookups should be using the DT_RPATH of the executable for lookups.
>
> > + {
> > + struct r_search_path_struct l_rpath_dirs;
> > + struct r_search_path_struct *l_rpath_dirs_p = NULL;
> > + if (cache_rpath (main_map, &main_map->l_rpath_dirs,
> > + DT_RPATH, "RPATH"))
> > + l_rpath_dirs_p = &main_map->l_rpath_dirs;
>
> OK. If cach_rpath() via DT_RPATH works then use that.
>
> > + else if (__glibc_unlikely (mode & __RTLD_DLOPEN))
> > + {
> > + l_rpath_dirs.dirs = NULL;
> > + if (cache_rpath (main_map, &l_rpath_dirs, DT_RUNPATH,
> > + "RUNPATH"))
> > + l_rpath_dirs_p = &l_rpath_dirs;
>
> Else we need to look at DT_RUNPATH. Always using main_map as the link map.
>
> I don't think this is correct, I think we should split this out and be more
> specific about this lookup, restricting the scope to audit modules.
It won't work with NSS.
> If this breaks for NSS modules we need to have a discussion about this and
> it means NSS modules may need changing how they are built.
>
> Today the libnss_test1.so.2 has large DT_RUNPATH that I expect should get
> used for lookup. Why it fails isn't clearly explained in your commit message
> and it should be.
NSS in libc.so tries to dlopen libnss_test1.so.2 whose DT_RUNPATH has
no impact on dlopen in libc.so. Since we can't add DT_RUNPATH to libc.so,
my solution handles audit and NSS properly. I can do
diff --git a/elf/dl-load.c b/elf/dl-load.c
index bf8957e73c..b51f18dc8b 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -2175,6 +2175,21 @@ _dl_map_object (struct link_map *loader, const
char *name,
&main_map->l_rpath_dirs,
&realname, &fb, loader ?: main_map, LA_SER_RUNPATH,
&found_other_class);
+
+ /* Also try DT_RUNPATH in the executable for glibc internal
+ dlopen call. */
+ if (__glibc_unlikely (mode & __RTLD_DLOPEN)
+ && fd == -1 && !did_main_map
+ && main_map != NULL && main_map->l_type != lt_loaded)
+ {
+ struct r_search_path_struct l_rpath_dirs;
+ l_rpath_dirs.dirs = NULL;
+ 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,
+ LA_SER_RUNPATH, &found_other_class);
+ }
}
/* Try the LD_LIBRARY_PATH environment variable. */
> > + }
> > + if (l_rpath_dirs_p)
> > + fd = open_path (name, namelen, mode, l_rpath_dirs_p,
> > + &realname, &fb, loader ?: main_map,
> > + LA_SER_RUNPATH, &found_other_class);
>
> If we found a path then use that for open_path.
>
> > + }
> > }
> >
> > /* Try the LD_LIBRARY_PATH environment variable. */
> > diff --git a/elf/tst-audit14a.c b/elf/tst-audit14a.c
> > new file mode 100644
> > index 0000000000..c6232eacf2
> > --- /dev/null
> > +++ b/elf/tst-audit14a.c
> > @@ -0,0 +1 @@
> > +#include "tst-audit14.c"
> > diff --git a/nss/Makefile b/nss/Makefile
> > index bccf9f2806..cd99e04732 100644
> > --- a/nss/Makefile
> > +++ b/nss/Makefile
> > @@ -58,6 +58,7 @@ tests-static = tst-field
> > tests-internal = tst-field
> > tests = test-netdb test-digits-dots tst-nss-getpwent bug17079 \
> > tst-nss-test1 \
> > + tst-nss-test1a \
>
> OK. New test.
>
> > tst-nss-test2 \
> > tst-nss-test4 \
> > tst-nss-test5
> > @@ -189,3 +190,5 @@ endif
> >
> > $(objpfx)tst-nss-files-alias-leak.out: $(objpfx)/libnss_files.so
> > $(objpfx)tst-nss-files-alias-truncated.out: $(objpfx)/libnss_files.so
> > +
> > +LDFLAGS-tst-nss-test1a = -Wl,--enable-new-dtags
>
> OK. Duplicate tst-nss-test1 as tst-nss-test1a with --enable-new-dtags to increase coverage.
>
> > diff --git a/nss/tst-nss-test1a.c b/nss/tst-nss-test1a.c
> > new file mode 100644
> > index 0000000000..f1428259c8
> > --- /dev/null
> > +++ b/nss/tst-nss-test1a.c
> > @@ -0,0 +1 @@
> > +#include "tst-nss-test1.c"
>
> OK.
>
> >
>
>
> --
> Cheers,
> Carlos.
>
@@ -239,10 +239,10 @@ ifneq ($(selinux-enabled),1)
tests-execstack-yes = tst-execstack tst-execstack-needed tst-execstack-prog
endif
ifeq ($(have-depaudit),yes)
-tests += tst-audit14 tst-audit15 tst-audit16
+tests += tst-audit14 tst-audit15 tst-audit16 tst-audit14a
ifeq ($(run-built-tests),yes)
tests-special += $(objpfx)tst-audit14-cmp.out $(objpfx)tst-audit15-cmp.out \
- $(objpfx)tst-audit16-cmp.out
+ $(objpfx)tst-audit16-cmp.out $(objpfx)tst-audit14a-cmp.out
endif
endif
endif
@@ -1479,6 +1479,8 @@ tst-auditmany-ENV = \
LDFLAGS-tst-audit14 = -Wl,--audit=tst-auditlogmod-1.so
$(objpfx)tst-auditlogmod-1.so: $(libsupport)
$(objpfx)tst-audit14.out: $(objpfx)tst-auditlogmod-1.so
+LDFLAGS-tst-audit14a = -Wl,--audit=tst-auditlogmod-1.so,--enable-new-dtags
+$(objpfx)tst-audit14a.out: $(objpfx)tst-auditlogmod-1.so
LDFLAGS-tst-audit15 = \
-Wl,--audit=tst-auditlogmod-1.so,--depaudit=tst-auditlogmod-2.so
$(objpfx)tst-auditlogmod-2.so: $(libsupport)
@@ -1505,6 +1507,9 @@ tst-audit17-ENV = LD_AUDIT=$(objpfx)tst-auditmod17.so
$(objpfx)tst-audit14-cmp.out: tst-audit14.exp $(objpfx)tst-audit14.out
cmp $^ > $@; \
$(evaluate-test)
+$(objpfx)tst-audit14a-cmp.out: tst-audit14.exp $(objpfx)tst-audit14a.out
+ cmp $^ > $@; \
+ $(evaluate-test)
$(objpfx)tst-audit15-cmp.out: tst-audit15.exp $(objpfx)tst-audit15.out
cmp $^ > $@; \
$(evaluate-test)
@@ -2162,15 +2162,29 @@ _dl_map_object (struct link_map *loader, const char *name,
}
/* If dynamically linked, try the DT_RPATH of the executable
- itself. NB: we do this for lookups in any namespace. */
+ itself. NB: we do this for lookups in any namespace.
+
+ Also try DT_RUNPATH for glibc internal dlopen call. */
if (fd == -1 && !did_main_map
- && main_map != NULL && main_map->l_type != lt_loaded
- && cache_rpath (main_map, &main_map->l_rpath_dirs, DT_RPATH,
- "RPATH"))
- fd = open_path (name, namelen, mode,
- &main_map->l_rpath_dirs,
- &realname, &fb, loader ?: main_map, LA_SER_RUNPATH,
- &found_other_class);
+ && main_map != NULL && main_map->l_type != lt_loaded)
+ {
+ struct r_search_path_struct l_rpath_dirs;
+ struct r_search_path_struct *l_rpath_dirs_p = NULL;
+ if (cache_rpath (main_map, &main_map->l_rpath_dirs,
+ DT_RPATH, "RPATH"))
+ l_rpath_dirs_p = &main_map->l_rpath_dirs;
+ else if (__glibc_unlikely (mode & __RTLD_DLOPEN))
+ {
+ l_rpath_dirs.dirs = NULL;
+ if (cache_rpath (main_map, &l_rpath_dirs, DT_RUNPATH,
+ "RUNPATH"))
+ l_rpath_dirs_p = &l_rpath_dirs;
+ }
+ if (l_rpath_dirs_p)
+ fd = open_path (name, namelen, mode, l_rpath_dirs_p,
+ &realname, &fb, loader ?: main_map,
+ LA_SER_RUNPATH, &found_other_class);
+ }
}
/* Try the LD_LIBRARY_PATH environment variable. */
new file mode 100644
@@ -0,0 +1 @@
+#include "tst-audit14.c"
@@ -58,6 +58,7 @@ tests-static = tst-field
tests-internal = tst-field
tests = test-netdb test-digits-dots tst-nss-getpwent bug17079 \
tst-nss-test1 \
+ tst-nss-test1a \
tst-nss-test2 \
tst-nss-test4 \
tst-nss-test5
@@ -189,3 +190,5 @@ endif
$(objpfx)tst-nss-files-alias-leak.out: $(objpfx)/libnss_files.so
$(objpfx)tst-nss-files-alias-truncated.out: $(objpfx)/libnss_files.so
+
+LDFLAGS-tst-nss-test1a = -Wl,--enable-new-dtags
new file mode 100644
@@ -0,0 +1 @@
+#include "tst-nss-test1.c"