| Message ID | 20211109183347.2943786-16-adhemerval.zanella@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Multiple rtld-audit fixes | expand |
* Adhemerval Zanella: > The DSO constructor should not be ignored if the main executable > has the SONAME set to a dependency. It fixes the case where > (using the scripts/dso-ordering-test.py definition): > > {}->a->b->c;soname({})=c > > Where the constructors should return > > c>b>a>{}<a<b<c > > Checked on x86_64-linux-gnu. > --- > elf/dl-load.c | 9 +++++++-- > elf/dso-sort-tests-1.def | 5 ++++- > 2 files changed, 11 insertions(+), 3 deletions(-) > > diff --git a/elf/dl-load.c b/elf/dl-load.c > index 4a0ff9d010..d585e1795d 100644 > --- a/elf/dl-load.c > +++ b/elf/dl-load.c > @@ -1998,13 +1998,18 @@ _dl_map_object (struct link_map *loader, const char *name, > assert (nsid >= 0); > assert (nsid < GL(dl_nns)); > > + /* Special case: trying to map itself. */ > + if (name[0] == '\0') > + return GL(dl_ns)[nsid]._ns_loaded; Can you expand the comment a bit? As far as I can see, "" corresponds to a NULL argument for dlopen, and we interpret that as returning the head object of the namespace. The head link map must exist because the dlopen call with NULL/"" has to come from some. Do you know if we have explict tests for this behavior? The change itself looks okay. The reference count is updated by the caller in elf/dl-open.c. Thanks, Florian
On 11/11/2021 09:30, Florian Weimer wrote: > * Adhemerval Zanella: > >> The DSO constructor should not be ignored if the main executable >> has the SONAME set to a dependency. It fixes the case where >> (using the scripts/dso-ordering-test.py definition): >> >> {}->a->b->c;soname({})=c >> >> Where the constructors should return >> >> c>b>a>{}<a<b<c >> >> Checked on x86_64-linux-gnu. >> --- >> elf/dl-load.c | 9 +++++++-- >> elf/dso-sort-tests-1.def | 5 ++++- >> 2 files changed, 11 insertions(+), 3 deletions(-) >> >> diff --git a/elf/dl-load.c b/elf/dl-load.c >> index 4a0ff9d010..d585e1795d 100644 >> --- a/elf/dl-load.c >> +++ b/elf/dl-load.c >> @@ -1998,13 +1998,18 @@ _dl_map_object (struct link_map *loader, const char *name, >> assert (nsid >= 0); >> assert (nsid < GL(dl_nns)); >> >> + /* Special case: trying to map itself. */ >> + if (name[0] == '\0') >> + return GL(dl_ns)[nsid]._ns_loaded; > > Can you expand the comment a bit? As far as I can see, "" corresponds > to a NULL argument for dlopen, and we interpret that as returning the > head object of the namespace. The head link map must exist because the > dlopen call with NULL/"" has to come from some. I have changed to: /* Special case: trying to map itself. An empty name correspond to a NULL or "" argument for dlopen(), and it returns the head object of the namespace. */ > > Do you know if we have explict tests for this behavior? Yes, dlfcn/tststatic4 (I have to add this special case for this). > > The change itself looks okay. The reference count is updated by the > caller in elf/dl-open.c. > > Thanks, > Florian >
diff --git a/elf/dl-load.c b/elf/dl-load.c index 4a0ff9d010..d585e1795d 100644 --- a/elf/dl-load.c +++ b/elf/dl-load.c @@ -1998,13 +1998,18 @@ _dl_map_object (struct link_map *loader, const char *name, assert (nsid >= 0); assert (nsid < GL(dl_nns)); + /* Special case: trying to map itself. */ + if (name[0] == '\0') + return GL(dl_ns)[nsid]._ns_loaded; + /* Look for this name among those already loaded. */ for (l = GL(dl_ns)[nsid]._ns_loaded; l; l = l->l_next) { /* If the requested name matches the soname of a loaded object, use that object. Elide this check for names that have not - yet been opened. */ - if (__glibc_unlikely ((l->l_faked | l->l_removed) != 0)) + yet been opened or the executable itself. */ + if (__glibc_unlikely ((l->l_faked | l->l_removed) != 0 + || l->l_type == lt_executable)) continue; if (!_dl_name_match_p (name, l)) { diff --git a/elf/dso-sort-tests-1.def b/elf/dso-sort-tests-1.def index 5f7f18ef27..2228910c0d 100644 --- a/elf/dso-sort-tests-1.def +++ b/elf/dso-sort-tests-1.def @@ -50,7 +50,10 @@ output: e>d>c>b>a>{}<a<b<c<d<e # Test if init/fini ordering behavior is proper, despite main program with # an soname that may cause confusion tst-dso-ordering10: {}->a->b->c;soname({})=c -output: b>a>{}<a<b +output: c>b>a>{}<a<b<c +# Same as before, but setting the soname as a direct dependency. +tst-dso-ordering11: {}->a->b->c;soname({})=a +output: c>b>a>{}<a<b<c # Complex example from Bugzilla #15311, under-linked and with circular # relocation(dynamic) dependencies. While this is technically unspecified, the
The DSO constructor should not be ignored if the main executable has the SONAME set to a dependency. It fixes the case where (using the scripts/dso-ordering-test.py definition): {}->a->b->c;soname({})=c Where the constructors should return c>b>a>{}<a<b<c Checked on x86_64-linux-gnu. --- elf/dl-load.c | 9 +++++++-- elf/dso-sort-tests-1.def | 5 ++++- 2 files changed, 11 insertions(+), 3 deletions(-)