diff mbox series

[v5,15/22] elf: Run constructors if executable has a soname of a dependency

Message ID 20211109183347.2943786-16-adhemerval.zanella@linaro.org
State Superseded
Headers show
Series Multiple rtld-audit fixes | expand

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Adhemerval Zanella Nov. 9, 2021, 6:33 p.m. UTC
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(-)

Comments

Florian Weimer Nov. 11, 2021, 12:30 p.m. UTC | #1
* 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
Adhemerval Zanella Nov. 12, 2021, 7:02 p.m. UTC | #2
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 mbox series

Patch

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