Bug 28868 - Dynamic loader DFS algorithm segfaults on missing libraries
Summary: Dynamic loader DFS algorithm segfaults on missing libraries
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: dynamic-link (show other bugs)
Version: 2.35
: P2 normal
Target Milestone: 2.36
Assignee: Adhemerval Zanella
URL:
Keywords:
: 28920 (view as bug list)
Depends on:
Blocks:
 
Reported: 2022-02-07 14:44 UTC by Gleb Fotengauer-Malinovskiy
Modified: 2022-04-27 16:43 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Gleb Fotengauer-Malinovskiy 2022-02-07 14:44:51 UTC
$ mkdir missing
$ gcc -shared -o missing/libmissing1.so -Wl,-soname,libmissing1.so
$ gcc -shared -o missing/libmissing2.so -Wl,-soname,libmissing2.so
$ gcc -Wl,--no-as-needed -shared missing/libmissing1.so missing/libmissing2.so -o libexists.so.0 -Wl,-soname,libexists.so.0
$ gcc -Wl,--no-as-needed -shared libexists.so.0 -o libtest.so -Wl,-soname,libtest.so
$ GLIBC_TUNABLES=glibc.rtld.dynamic_sort=1 LD_LIBRARY_PATH="$PWD" LD_TRACE_LOADED_OBJECTS=1 /lib64/ld-linux-x86-64.so.2 ./libtest.so
        linux-vdso.so.1 (0x00007fffc244d000)
        libexists.so.0 => /usr/src/libexists.so.0 (0x00007f9a59459000)
        libc.so.6 => /lib64/libc.so.6 (0x00007f9a59248000)
        /lib64/ld-linux-x86-64.so.2 (0x00007f9a59465000)
        libmissing1.so => not found
        libmissing2.so => not found
$ GLIBC_TUNABLES=glibc.rtld.dynamic_sort=2 LD_LIBRARY_PATH="$PWD" LD_TRACE_LOADED_OBJECTS=1 /lib64/ld-linux-x86-64.so.2 ./libtest.so
Segmentation fault
Comment 1 Florian Weimer 2022-02-07 16:29:17 UTC
Potentially related thread:

Debugging ld.so in gdb
https://sourceware.org/pipermail/gdb/2022-February/049884.html
Comment 2 Adhemerval Zanella 2022-02-07 18:51:29 UTC
I think the issue is _dl_map_object_deps ignores l_faked objects (set if the underlying file can't be opened at _dl_map_object):

490   for (nlist = 0, runp = known; runp; runp = runp->next)
491     {
492       if (__builtin_expect (trace_mode, 0) && runp->map->l_faked)         
493         /* This can happen when we trace the loading.  */                 
494         --map->l_searchlist.r_nlist;                                      
495       else
496         {                                                                 
497           if (runp->map == map)
498             map_index = nlist;                                            
499           map->l_searchlist.r_list[nlist++] = runp->map;                  
500         } 
501         
502       /* Now clear all the mark bits we set in the objects on the search list
503          to avoid duplicates, so the next call starts fresh.  */
504       runp->map->l_reserved = 0;
505     }    

Using the example the 'nlist' prior the call it set to 6, however after it will be 4 because of the libmissing1.so and libmissing2.so. The 'nlist' is used then on:

614   /* If libc.so.6 is the main map, it participates in the sort, so
615      that the relocation order is correct regarding libc.so.6.  */
616   _dl_sort_maps (l_initfini, nlist,
617                  (l_initfini[0] != GL (dl_ns)[l_initfini[0]->l_ns].libc_map),
618                  false);

To indicate the maximum number of link_map objects on l_initfini.  It then used on _dl_sort_maps_dfs on the stack allocated working maps:

222   /* Array to hold RPO sorting results, before we copy back to maps[].  */
223   struct link_map *rpo[nmaps];
224 
225   /* The 'head' position during each DFS iteration. Note that we start at
226      one past the last element due to first-decrement-then-store (see the
227      bottom of above dfs_traversal() routine).  */
228   struct link_map **rpo_head = &rpo[nmaps];

However while transversing the 'l_initfini' on dfs_traversal it will still considere the l_faked maps and thus update rpo more times than the allocated working 'rpo'.  I think the straighforward solution is just to ignore l_faked link_maps on new algorithm and there is no need to check if audit mode is set because l_faked in only set on this mode:

---
diff --git a/elf/dl-sort-maps.c b/elf/dl-sort-maps.c
index 9e9d53ec47..f049178ac3 100644
--- a/elf/dl-sort-maps.c
+++ b/elf/dl-sort-maps.c
@@ -140,7 +140,7 @@ static void
 dfs_traversal (struct link_map ***rpo, struct link_map *map,
               bool *do_reldeps)
 {
-  if (map->l_visited)
+  if (map->l_visited || map->l_faked)
     return;
 
   map->l_visited = 1;
---

With the patch applied:

$ GLIBC_TUNABLES=glibc.rtld.dynamic_sort=1 LD_LIBRARY_PATH="$PWD" LD_TRACE_LOADED_OBJECTS=1 ./elf//ld-linux-x86-64.so.2 --library-path . ./libtest.so
        linux-vdso.so.1 (0x00007ffcdad3c000)
        libexists.so.0 => ./libexists.so.0 (0x00007f371960a000)
        libc.so.6 => ./libc.so.6 (0x00007f37193eb000)
        ./elf//ld-linux-x86-64.so.2 (0x00007f3719616000)
        libmissing1.so => not found
        libmissing2.so => not found

$ GLIBC_TUNABLES=glibc.rtld.dynamic_sort=2 LD_LIBRARY_PATH="$PWD" LD_TRACE_LOADED_OBJECTS=1 ./elf//ld-linux-x86-64.so.2 --library-path . ./libtest.so
        linux-vdso.so.1 (0x00007ffd0bd51000)
        libexists.so.0 => ./libexists.so.0 (0x00007f2645d7a000)
        libc.so.6 => ./libc.so.6 (0x00007f2645b5b000)
        ./elf//ld-linux-x86-64.so.2 (0x00007f2645d86000)
        libmissing1.so => not found
        libmissing2.so => not found
Comment 3 Florian Weimer 2022-02-07 19:05:36 UTC
(In reply to Adhemerval Zanella from comment #2)
> I think the issue is _dl_map_object_deps ignores l_faked objects (set if the
> underlying file can't be opened at _dl_map_object):

That's what I concluded as well.

I suppose we need the dependency sorting in trace mode for DL_DEBUG_UNUSED support? But maybe we can skip the sorting (and shrinking of the dependency list) altogether in tracing mode.
Comment 4 Adhemerval Zanella 2022-02-07 19:31:09 UTC
(In reply to Florian Weimer from comment #3)
> (In reply to Adhemerval Zanella from comment #2)
> > I think the issue is _dl_map_object_deps ignores l_faked objects (set if the
> > underlying file can't be opened at _dl_map_object):
> 
> That's what I concluded as well.
> 
> I suppose we need the dependency sorting in trace mode for DL_DEBUG_UNUSED
> support? But maybe we can skip the sorting (and shrinking of the dependency
> list) altogether in tracing mode.

I think so since loader realocated the objects in this case.  And I think it shold be ok to skip sorting for trace, I am not sure if it would be better to handle l_faked maps on _dl_sort_maps_dfs or just skip sorting for trace mode.
Comment 5 Florian Weimer 2022-02-07 19:40:21 UTC
(In reply to Adhemerval Zanella from comment #4)
> I think so since loader realocated the objects in this case.  And I think it
> shold be ok to skip sorting for trace, I am not sure if it would be better
> to handle l_faked maps on _dl_sort_maps_dfs or just skip sorting for trace
> mode.

Are you going to send a patch? I planned to put something together tomorrow.
Comment 6 Adhemerval Zanella 2022-02-07 19:53:35 UTC
(In reply to Florian Weimer from comment #5)
> (In reply to Adhemerval Zanella from comment #4)
> > I think so since loader realocated the objects in this case.  And I think it
> > shold be ok to skip sorting for trace, I am not sure if it would be better
> > to handle l_faked maps on _dl_sort_maps_dfs or just skip sorting for trace
> > mode.
> 
> Are you going to send a patch? I planned to put something together tomorrow.

I will send the above fix, I think ignoring l_faked on sorting still make sense. We can discuss on maillist if this is the best approach.
Comment 7 Adhemerval Zanella 2022-02-23 17:31:15 UTC
*** Bug 28920 has been marked as a duplicate of this bug. ***
Comment 8 Adhemerval Zanella 2022-04-27 16:43:52 UTC
Fixed on master.