[RFC,v8,07/20] elf/dl-open.c: Don't try libc linit in namespaces with no libc mapping

Message ID 20210209171839.7911-8-vivek@collabora.com
State Superseded
Delegated to: Adhemerval Zanella Netto
Headers
Series Implementation of RTLD_SHARED for dlmopen |

Commit Message

Vivek Dasmohapatra Feb. 9, 2021, 5:18 p.m. UTC
  Secondary namespaces which share their libc mapping with the main
namespace cannot (and should not) have _dl_call_libc_early_init
called for them by dl_open_worker.
---
 elf/dl-open.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
  

Comments

Adhemerval Zanella Feb. 15, 2021, 5:54 p.m. UTC | #1
On 09/02/2021 14:18, Vivek Das Mohapatra via Libc-alpha wrote:
> Secondary namespaces which share their libc mapping with the main
> namespace cannot (and should not) have _dl_call_libc_early_init
> called for them by dl_open_worker.
> ---
>  elf/dl-open.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/elf/dl-open.c b/elf/dl-open.c
> index 4cb90bfe19..dc4b386559 100644
> --- a/elf/dl-open.c
> +++ b/elf/dl-open.c
> @@ -792,16 +792,21 @@ dl_open_worker (void *a)
>       namespace.  */
>    if (!args->libc_already_loaded)
>      {
> +      /* If this is a secondary (nsid != LM_ID_BASE) namespace then
> +         it is POSSIBLE there's no libc_map at all - We use the one
> +         shared with LM_ID_BASE instead (which MUST already be
> +         initialised for us to even reach here).  */
>        struct link_map *libc_map = GL(dl_ns)[args->nsid].libc_map;
>  #ifdef SHARED
> -      bool initial = libc_map->l_ns == LM_ID_BASE;
> +      bool initial = libc_map && (libc_map->l_real->l_ns == LM_ID_BASE);

No implicit checks.

>  #else
>        /* In the static case, there is only one namespace, but it
>  	 contains a secondary libc (the primary libc is statically
>  	 linked).  */
>        bool initial = false;
>  #endif
> -      _dl_call_libc_early_init (libc_map, initial);
> +      if (libc_map != NULL)
> +        _dl_call_libc_early_init (libc_map, initial);
>      }
>  
>  #ifndef SHARED
> 

Ok.
  
Vivek Dasmohapatra Feb. 17, 2021, 3:39 p.m. UTC | #2
>>  #ifdef SHARED
>> -      bool initial = libc_map->l_ns == LM_ID_BASE;
>> +      bool initial = libc_map && (libc_map->l_real->l_ns == LM_ID_BASE);
>
> No implicit checks.

Sorry - It's not clear to me what change you want here.
  
Adhemerval Zanella Feb. 17, 2021, 4:17 p.m. UTC | #3
On 17/02/2021 12:39, Vivek Das Mohapatra wrote:
>>>  #ifdef SHARED
>>> -      bool initial = libc_map->l_ns == LM_ID_BASE;
>>> +      bool initial = libc_map && (libc_map->l_real->l_ns == LM_ID_BASE);
>>
>> No implicit checks.
> 
> Sorry - It's not clear to me what change you want here.

The libc_map is a pointer here, so:

 bool initial = libc_map != NULL && libc_map->l_real->l_ns == LM_ID_BASE;

(is libc_map->l_real always valid here btw?)
  
Vivek Dasmohapatra Feb. 17, 2021, 6:32 p.m. UTC | #4
> The libc_map is a pointer here, so:
>
> bool initial = libc_map != NULL && libc_map->l_real->l_ns == LM_ID_BASE;

Ah, got it. Will do,

> (is libc_map->l_real always valid here btw?)

Yes: _dl_new_object assigns it immediately after calloc'ing the
link map entry struct. _dl_new_proxy does similar and the only
other case is ld.so itself which also has l_real assigned
early.

_dl_init asserts that ->l_real-><something> is non NULL too
so the assumption that l_real is dereferencable is pretty
widespread and fundamental.
  
Adhemerval Zanella Feb. 17, 2021, 8:52 p.m. UTC | #5
On 17/02/2021 15:32, Vivek Das Mohapatra wrote:
>> The libc_map is a pointer here, so:
>>
>> bool initial = libc_map != NULL && libc_map->l_real->l_ns == LM_ID_BASE;
> 
> Ah, got it. Will do,

Thanks, I will try to finish the set review by the end of the week.

> 
>> (is libc_map->l_real always valid here btw?)
> 
> Yes: _dl_new_object assigns it immediately after calloc'ing the
> link map entry struct. _dl_new_proxy does similar and the only
> other case is ld.so itself which also has l_real assigned
> early.
> 
> _dl_init asserts that ->l_real-><something> is non NULL too
> so the assumption that l_real is dereferencable is pretty
> widespread and fundamental.
> 

Ack.
  

Patch

diff --git a/elf/dl-open.c b/elf/dl-open.c
index 4cb90bfe19..dc4b386559 100644
--- a/elf/dl-open.c
+++ b/elf/dl-open.c
@@ -792,16 +792,21 @@  dl_open_worker (void *a)
      namespace.  */
   if (!args->libc_already_loaded)
     {
+      /* If this is a secondary (nsid != LM_ID_BASE) namespace then
+         it is POSSIBLE there's no libc_map at all - We use the one
+         shared with LM_ID_BASE instead (which MUST already be
+         initialised for us to even reach here).  */
       struct link_map *libc_map = GL(dl_ns)[args->nsid].libc_map;
 #ifdef SHARED
-      bool initial = libc_map->l_ns == LM_ID_BASE;
+      bool initial = libc_map && (libc_map->l_real->l_ns == LM_ID_BASE);
 #else
       /* In the static case, there is only one namespace, but it
 	 contains a secondary libc (the primary libc is statically
 	 linked).  */
       bool initial = false;
 #endif
-      _dl_call_libc_early_init (libc_map, initial);
+      if (libc_map != NULL)
+        _dl_call_libc_early_init (libc_map, initial);
     }
 
 #ifndef SHARED