| Message ID | LO6P265MB63473097FD2F2A726C7565398BE32@LO6P265MB6347.GBRP265.PROD.OUTLOOK.COM |
|---|---|
| State | New |
| Headers | show |
| Series | elf: Fix LD_DEBUG misattributing R(UN)PATH list (bug 31739) | expand |
On 14/05/24 02:13, Tobiasz Laskowski wrote: > If the first RUNPATH entry for the current binary has encountered > previously, then `this_dir->where` gives the name of the first binary > which had this RUNPATH entry. This means that the entire RUNPATH list > for the current binary is misattributed to another file by the > `LD_DEBUG=libs` output. > > This patch solves this by instead using the `l_name` field of the loader > object from where the RUNPATH list was read. > > NB: This all applies to RPATHs as well as RUNPATHs > > Signed-off-by: Tobiasz Laskowski <tobil4sk@outlook.com> LGTM, thanks. How hard would to add a regression test for it? Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > --- > elf/dl-load.c | 34 +++++++++++++++++----------------- > 1 file changed, 17 insertions(+), 17 deletions(-) > > diff --git a/elf/dl-load.c b/elf/dl-load.c > index a34cb3559c..2362a4aa99 100644 > --- a/elf/dl-load.c > +++ b/elf/dl-load.c > @@ -1786,9 +1786,9 @@ open_verify (const char *name, int fd, > > static int > open_path (const char *name, size_t namelen, int mode, > - struct r_search_path_struct *sps, char **realname, > - struct filebuf *fbp, struct link_map *loader, int whatcode, > - bool *found_other_class) > + struct r_search_path_struct *sps, const char* search_path_source, > + char **realname, struct filebuf *fbp, struct link_map *loader, > + int whatcode, bool *found_other_class) > { > struct r_search_path_elem **dirs = sps->dirs; > char *buf; > @@ -1816,7 +1816,7 @@ open_path (const char *name, size_t namelen, int mode, > && current_what != this_dir->what) > { > current_what = this_dir->what; > - print_search_path (dirs, current_what, this_dir->where); > + print_search_path (dirs, current_what, search_path_source); > } > > edp = (char *) __mempcpy (buf, this_dir->dirname, this_dir->dirnamelen); > @@ -2038,10 +2038,9 @@ _dl_map_object (struct link_map *loader, const char *name, > for (l = loader; l; l = l->l_loader) > if (cache_rpath (l, &l->l_rpath_dirs, DT_RPATH, "RPATH")) > { > - fd = open_path (name, namelen, mode, > - &l->l_rpath_dirs, > - &realname, &fb, loader, LA_SER_RUNPATH, > - &found_other_class); > + fd = open_path (name, namelen, mode, &l->l_rpath_dirs, > + l->l_name, &realname, &fb, loader, > + LA_SER_RUNPATH, &found_other_class); > if (fd != -1) > break; > > @@ -2054,9 +2053,9 @@ _dl_map_object (struct link_map *loader, const char *name, > && 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, > + fd = open_path (name, namelen, mode, &main_map->l_rpath_dirs, > + main_map->l_name, &realname, &fb, > + loader ?: main_map, LA_SER_RUNPATH, > &found_other_class); > > /* Also try DT_RUNPATH in the executable for LD_AUDIT dlopen > @@ -2070,14 +2069,15 @@ _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, > - LA_SER_RUNPATH, &found_other_class); > + main_map->l_name, &realname, &fb, > + loader ?: main_map, LA_SER_RUNPATH, > + &found_other_class); > } > } > > /* 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, > + fd = open_path (name, namelen, mode, &__rtld_env_path_list, NULL, > &realname, &fb, > loader ?: GL(dl_ns)[LM_ID_BASE]._ns_loaded, > LA_SER_LIBPATH, &found_other_class); > @@ -2086,8 +2086,8 @@ _dl_map_object (struct link_map *loader, const char *name, > if (fd == -1 && loader != NULL > && cache_rpath (loader, &loader->l_runpath_dirs, > DT_RUNPATH, "RUNPATH")) > - fd = open_path (name, namelen, mode, > - &loader->l_runpath_dirs, &realname, &fb, loader, > + fd = open_path (name, namelen, mode, &loader->l_runpath_dirs, > + loader->l_name, &realname, &fb, loader, > LA_SER_RUNPATH, &found_other_class); > > #ifdef USE_LDCONFIG > @@ -2153,7 +2153,7 @@ _dl_map_object (struct link_map *loader, const char *name, > && ((l = loader ?: GL(dl_ns)[nsid]._ns_loaded) == NULL > || __glibc_likely (!(l->l_flags_1 & DF_1_NODEFLIB))) > && __rtld_search_dirs.dirs != (void *) -1) > - fd = open_path (name, namelen, mode, &__rtld_search_dirs, > + fd = open_path (name, namelen, mode, &__rtld_search_dirs, NULL, > &realname, &fb, l, LA_SER_DEFAULT, &found_other_class); > > /* Add another newline when we are tracing the library loading. */
On 5/16/24 3:23 AM, Adhemerval Zanella Netto wrote: > > > On 14/05/24 02:13, Tobiasz Laskowski wrote: >> If the first RUNPATH entry for the current binary has encountered >> previously, then `this_dir->where` gives the name of the first binary >> which had this RUNPATH entry. This means that the entire RUNPATH list >> for the current binary is misattributed to another file by the >> `LD_DEBUG=libs` output. >> >> This patch solves this by instead using the `l_name` field of the loader >> object from where the RUNPATH list was read. >> >> NB: This all applies to RPATHs as well as RUNPATHs >> >> Signed-off-by: Tobiasz Laskowski <tobil4sk@outlook.com> > > LGTM, thanks. How hard would to add a regression test for it? My guidance for a regression test would be: Build two binaries that exhibit this behaviour via RPATH and RUNPATH. Run a top-level test via support_capture_subprocess() and run the sub-process, and then look for the result to contain what you need. Examples: elf/tst-pldd.c (container test), elf/tst-audit18.c (normal test) etc. I would avoid containerizing the test so it can run in as many configurations as possible.
diff --git a/elf/dl-load.c b/elf/dl-load.c index a34cb3559c..2362a4aa99 100644 --- a/elf/dl-load.c +++ b/elf/dl-load.c @@ -1786,9 +1786,9 @@ open_verify (const char *name, int fd, static int open_path (const char *name, size_t namelen, int mode, - struct r_search_path_struct *sps, char **realname, - struct filebuf *fbp, struct link_map *loader, int whatcode, - bool *found_other_class) + struct r_search_path_struct *sps, const char* search_path_source, + char **realname, struct filebuf *fbp, struct link_map *loader, + int whatcode, bool *found_other_class) { struct r_search_path_elem **dirs = sps->dirs; char *buf; @@ -1816,7 +1816,7 @@ open_path (const char *name, size_t namelen, int mode, && current_what != this_dir->what) { current_what = this_dir->what; - print_search_path (dirs, current_what, this_dir->where); + print_search_path (dirs, current_what, search_path_source); } edp = (char *) __mempcpy (buf, this_dir->dirname, this_dir->dirnamelen); @@ -2038,10 +2038,9 @@ _dl_map_object (struct link_map *loader, const char *name, for (l = loader; l; l = l->l_loader) if (cache_rpath (l, &l->l_rpath_dirs, DT_RPATH, "RPATH")) { - fd = open_path (name, namelen, mode, - &l->l_rpath_dirs, - &realname, &fb, loader, LA_SER_RUNPATH, - &found_other_class); + fd = open_path (name, namelen, mode, &l->l_rpath_dirs, + l->l_name, &realname, &fb, loader, + LA_SER_RUNPATH, &found_other_class); if (fd != -1) break; @@ -2054,9 +2053,9 @@ _dl_map_object (struct link_map *loader, const char *name, && 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, + fd = open_path (name, namelen, mode, &main_map->l_rpath_dirs, + main_map->l_name, &realname, &fb, + loader ?: main_map, LA_SER_RUNPATH, &found_other_class); /* Also try DT_RUNPATH in the executable for LD_AUDIT dlopen @@ -2070,14 +2069,15 @@ _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, - LA_SER_RUNPATH, &found_other_class); + main_map->l_name, &realname, &fb, + loader ?: main_map, LA_SER_RUNPATH, + &found_other_class); } } /* 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, + fd = open_path (name, namelen, mode, &__rtld_env_path_list, NULL, &realname, &fb, loader ?: GL(dl_ns)[LM_ID_BASE]._ns_loaded, LA_SER_LIBPATH, &found_other_class); @@ -2086,8 +2086,8 @@ _dl_map_object (struct link_map *loader, const char *name, if (fd == -1 && loader != NULL && cache_rpath (loader, &loader->l_runpath_dirs, DT_RUNPATH, "RUNPATH")) - fd = open_path (name, namelen, mode, - &loader->l_runpath_dirs, &realname, &fb, loader, + fd = open_path (name, namelen, mode, &loader->l_runpath_dirs, + loader->l_name, &realname, &fb, loader, LA_SER_RUNPATH, &found_other_class); #ifdef USE_LDCONFIG @@ -2153,7 +2153,7 @@ _dl_map_object (struct link_map *loader, const char *name, && ((l = loader ?: GL(dl_ns)[nsid]._ns_loaded) == NULL || __glibc_likely (!(l->l_flags_1 & DF_1_NODEFLIB))) && __rtld_search_dirs.dirs != (void *) -1) - fd = open_path (name, namelen, mode, &__rtld_search_dirs, + fd = open_path (name, namelen, mode, &__rtld_search_dirs, NULL, &realname, &fb, l, LA_SER_DEFAULT, &found_other_class); /* Add another newline when we are tracing the library loading. */
If the first RUNPATH entry for the current binary has encountered previously, then `this_dir->where` gives the name of the first binary which had this RUNPATH entry. This means that the entire RUNPATH list for the current binary is misattributed to another file by the `LD_DEBUG=libs` output. This patch solves this by instead using the `l_name` field of the loader object from where the RUNPATH list was read. NB: This all applies to RPATHs as well as RUNPATHs Signed-off-by: Tobiasz Laskowski <tobil4sk@outlook.com> --- elf/dl-load.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-)