[RFC,v8,12/20] Use the new DSO finder helper function since we have it
Commit Message
---
elf/dl-load.c | 43 +++++++++++++++++--------------------------
1 file changed, 17 insertions(+), 26 deletions(-)
Comments
On 09/02/2021 14:18, Vivek Das Mohapatra via Libc-alpha wrote:
> ---
> elf/dl-load.c | 43 +++++++++++++++++--------------------------
> 1 file changed, 17 insertions(+), 26 deletions(-)
>
> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index 780bca99e8..13ac2053b8 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -2133,35 +2133,26 @@ _dl_map_object (struct link_map *loader, const char *name,
> #endif
>
> /* Look for this name among those already loaded. */
> - for (l = GL(dl_ns)[nsid]._ns_loaded; l; l = l->l_next)
> + l = _dl_find_dso (name, nsid);
> +
> + if (l != NULL)
> {
> - /* 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))
> - continue;
> - if (!_dl_name_match_p (name, l))
> - {
> - const char *soname;
> -
> - if (__glibc_likely (l->l_soname_added)
> - || l->l_info[DT_SONAME] == NULL)
> - continue;
> -
> - soname = ((const char *) D_PTR (l, l_info[DT_STRTAB])
> - + l->l_info[DT_SONAME]->d_un.d_val);
> - if (strcmp (name, soname) != 0)
> - continue;
> -
> - /* We have a match on a new name -- cache it. */
> - add_name_to_object (l, soname);
> - l->l_soname_added = 1;
> - }
> -
> - /* We have a match. */
> +#ifdef SHARED
> + /* If we are trying to load a DF_GNU_1_UNIQUE flagged DSO which WAS
> + already opened in the target NS but with RTLD_ISOLATE so it WAS NOT
> + created as a proxy we need to error out since we cannot satisfy the
> + DF_GNU_1_UNIQUE is-equivalent-to RTLD_SHARED semantics. */
> + if (!(mode & RTLD_ISOLATE) &&
> + (l->l_ns != LM_ID_BASE) &&
> + (l->l_gnu_flags_1 & DF_GNU_1_UNIQUE) &&
> + !l->l_proxy)
> + {
> + _dl_signal_error (EEXIST, name, NULL,
> + N_("object cannot be demoted to a proxy"));
> + }
> +#endif
> return l;
I think this patch should reorganized along with the previous, where
first you refactor the code to use find_dso and *then* add the
required l_proxy change in a subsequent patch.
(There are other patch organization that I think should be changed,
I will get back to them once I finish reviewing the set)
> }
> -
I usually avoid such spurious change.
> /* Display information if we are debugging. */
> if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_FILES)
> && loader != NULL)
>
On 19/02/2021 11:39, Adhemerval Zanella wrote:
>
>
> On 09/02/2021 14:18, Vivek Das Mohapatra via Libc-alpha wrote:
>> ---
>> elf/dl-load.c | 43 +++++++++++++++++--------------------------
>> 1 file changed, 17 insertions(+), 26 deletions(-)
>>
>> diff --git a/elf/dl-load.c b/elf/dl-load.c
>> index 780bca99e8..13ac2053b8 100644
>> --- a/elf/dl-load.c
>> +++ b/elf/dl-load.c
>> @@ -2133,35 +2133,26 @@ _dl_map_object (struct link_map *loader, const char *name,
>> #endif
>>
>> /* Look for this name among those already loaded. */
>> - for (l = GL(dl_ns)[nsid]._ns_loaded; l; l = l->l_next)
>> + l = _dl_find_dso (name, nsid);
>> +
>> + if (l != NULL)
>> {
>> - /* 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))
>> - continue;
>> - if (!_dl_name_match_p (name, l))
>> - {
>> - const char *soname;
>> -
>> - if (__glibc_likely (l->l_soname_added)
>> - || l->l_info[DT_SONAME] == NULL)
>> - continue;
>> -
>> - soname = ((const char *) D_PTR (l, l_info[DT_STRTAB])
>> - + l->l_info[DT_SONAME]->d_un.d_val);
>> - if (strcmp (name, soname) != 0)
>> - continue;
>> -
>> - /* We have a match on a new name -- cache it. */
>> - add_name_to_object (l, soname);
>> - l->l_soname_added = 1;
>> - }
>> -
>> - /* We have a match. */
>> +#ifdef SHARED
>> + /* If we are trying to load a DF_GNU_1_UNIQUE flagged DSO which WAS
>> + already opened in the target NS but with RTLD_ISOLATE so it WAS NOT
>> + created as a proxy we need to error out since we cannot satisfy the
>> + DF_GNU_1_UNIQUE is-equivalent-to RTLD_SHARED semantics. */
>> + if (!(mode & RTLD_ISOLATE) &&
Also the new RTLD_ISOLATE flag is only defined later in this set.
>> + (l->l_ns != LM_ID_BASE) &&
>> + (l->l_gnu_flags_1 & DF_GNU_1_UNIQUE) &&
>> + !l->l_proxy)
>> + {
>> + _dl_signal_error (EEXIST, name, NULL,
>> + N_("object cannot be demoted to a proxy"));
>> + }
>> +#endif
>> return l;
>
> I think this patch should reorganized along with the previous, where
> first you refactor the code to use find_dso and *then* add the
> required l_proxy change in a subsequent patch.
>
> (There are other patch organization that I think should be changed,
> I will get back to them once I finish reviewing the set)
>
>> }
>> -
>
> I usually avoid such spurious change.
>
>> /* Display information if we are debugging. */
>> if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_FILES)
>> && loader != NULL)
>>
> I think this patch should reorganized along with the previous, where
> first you refactor the code to use find_dso and *then* add the
> required l_proxy change in a subsequent patch.
Do you want this combined with the preceding patch or reorganised
in some other way?
On 22/02/2021 15:44, Vivek Das Mohapatra wrote:
>> I think this patch should reorganized along with the previous, where
>> first you refactor the code to use find_dso and *then* add the
>> required l_proxy change in a subsequent patch.
>
> Do you want this combined with the preceding patch or reorganised
> in some other way?
>
My plan is to finish the review and send a overall note of how I
think the set should be better organized.
>> Do you want this combined with the preceding patch or reorganised
>> in some other way?
>
> My plan is to finish the review and send a overall note of how I think the set
> should be better organized.
Ok, thanks.
@@ -2133,35 +2133,26 @@ _dl_map_object (struct link_map *loader, const char *name,
#endif
/* Look for this name among those already loaded. */
- for (l = GL(dl_ns)[nsid]._ns_loaded; l; l = l->l_next)
+ l = _dl_find_dso (name, nsid);
+
+ if (l != NULL)
{
- /* 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))
- continue;
- if (!_dl_name_match_p (name, l))
- {
- const char *soname;
-
- if (__glibc_likely (l->l_soname_added)
- || l->l_info[DT_SONAME] == NULL)
- continue;
-
- soname = ((const char *) D_PTR (l, l_info[DT_STRTAB])
- + l->l_info[DT_SONAME]->d_un.d_val);
- if (strcmp (name, soname) != 0)
- continue;
-
- /* We have a match on a new name -- cache it. */
- add_name_to_object (l, soname);
- l->l_soname_added = 1;
- }
-
- /* We have a match. */
+#ifdef SHARED
+ /* If we are trying to load a DF_GNU_1_UNIQUE flagged DSO which WAS
+ already opened in the target NS but with RTLD_ISOLATE so it WAS NOT
+ created as a proxy we need to error out since we cannot satisfy the
+ DF_GNU_1_UNIQUE is-equivalent-to RTLD_SHARED semantics. */
+ if (!(mode & RTLD_ISOLATE) &&
+ (l->l_ns != LM_ID_BASE) &&
+ (l->l_gnu_flags_1 & DF_GNU_1_UNIQUE) &&
+ !l->l_proxy)
+ {
+ _dl_signal_error (EEXIST, name, NULL,
+ N_("object cannot be demoted to a proxy"));
+ }
+#endif
return l;
}
-
/* Display information if we are debugging. */
if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_FILES)
&& loader != NULL)