[v4,06/14] elf: Disambiguate some failures in _dl_load_cache_lookup
Checks
| Context |
Check |
Description |
| redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
| linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_glibc_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_glibc_check--master-arm |
success
|
Test passed
|
Commit Message
Failure to allocate a copy of the string is now distinct from
a cache lookup failure. Some infrastructure failures in
_dl_sysdep_read_whole_file are still treated as cache lookup
failures, though.
---
elf/dl-cache.c | 42 ++++++++++++++++++++++++++------------
elf/dl-load.c | 5 ++++-
sysdeps/generic/ldsodefs.h | 6 +++---
3 files changed, 36 insertions(+), 17 deletions(-)
Comments
On 02/02/25 18:13, Florian Weimer wrote:
> Failure to allocate a copy of the string is now distinct from
> a cache lookup failure. Some infrastructure failures in
> _dl_sysdep_read_whole_file are still treated as cache lookup
> failures, though.
LGTM, thanks.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> elf/dl-cache.c | 42 ++++++++++++++++++++++++++------------
> elf/dl-load.c | 5 ++++-
> sysdeps/generic/ldsodefs.h | 6 +++---
> 3 files changed, 36 insertions(+), 17 deletions(-)
>
> diff --git a/elf/dl-cache.c b/elf/dl-cache.c
> index 300aa1b6dd..c9c5bf549a 100644
> --- a/elf/dl-cache.c
> +++ b/elf/dl-cache.c
> @@ -375,15 +375,21 @@ _dl_cache_libcmp (const char *p1, const char *p2)
> }
>
>
> -/* Look up NAME in ld.so.cache and return the file name stored there, or null
> - if none is found. The cache is loaded if it was not already. If loading
> - the cache previously failed there will be no more attempts to load it.
> - The caller is responsible for freeing the returned string. The ld.so.cache
> - may be unmapped at any time by a completing recursive dlopen and
> - this function must take care that it does not return references to
> - any data in the mapping. */
> -char *
> -_dl_load_cache_lookup (const char *name)
> +/* Look up NAME in ld.so.cache and write the file name stored there to
> + *REALNAME, or null if none is found, and return true. In this case,
> + the caller is responsible for freeing the string in *REALNAME. If
> + there is an error condition that causes the lookup to fail (such as
> + a failure to allocate memory), the function returns false, and
> + *REALNAME is unchanged.
> +
> + The cache is loaded if it was not already. If loading the cache
> + previously failed there will be no more attempts to load it.
> +
> + The ld.so.cache may be unmapped at any time by a completing
> + recursive dlopen and this function must take care that it does not
> + return references to any data in the mapping. */
> +bool
> +_dl_load_cache_lookup (const char *name, char **realname)
> {
> /* Print a message if the loading of libs is traced. */
> if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_LIBS))
> @@ -459,8 +465,11 @@ _dl_load_cache_lookup (const char *name)
> }
>
> if (cache == (void *) -1)
> - /* Previously looked for the cache file and didn't find it. */
> - return NULL;
> + {
> + /* Previously looked for the cache file and didn't find it. */
> + *realname = NULL;
> + return true;
> + }
>
> const char *best;
> if (cache_new != (void *) -1)
> @@ -486,7 +495,10 @@ _dl_load_cache_lookup (const char *name)
> _dl_debug_printf (" trying file=%s\n", best);
>
> if (best == NULL)
> - return NULL;
> + {
> + *realname = NULL;
> + return true;
> + }
>
> /* The double copy is *required* since malloc may be interposed
> and call dlopen itself whose completion would unmap the data
> @@ -496,7 +508,11 @@ _dl_load_cache_lookup (const char *name)
> size_t best_len = strlen (best) + 1;
> temp = alloca (best_len);
> memcpy (temp, best, best_len);
> - return __strdup (temp);
> + char *copy = __strdup (temp);
> + if (copy == NULL)
> + return false;
> + *realname = copy;
> + return true;
> }
>
> #ifndef MAP_COPY
> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index e328d678b4..b583714c96 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -2046,7 +2046,10 @@ _dl_map_new_object (struct link_map *loader, const char *name,
> {
> /* Check the list of libraries in the file /etc/ld.so.cache,
> for compatibility with Linux's ldconfig program. */
> - char *cached = _dl_load_cache_lookup (name);
> + char *cached;
> + if (!_dl_load_cache_lookup (name, &cached))
> + _dl_signal_error (ENOMEM, NULL, NULL,
> + N_("cannot allocate library name"));
>
> if (cached != NULL)
> {
> diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
> index 8465cbaa9b..c0785cba04 100644
> --- a/sysdeps/generic/ldsodefs.h
> +++ b/sysdeps/generic/ldsodefs.h
> @@ -1117,9 +1117,9 @@ const struct r_strlenpair *_dl_important_hwcaps (const char *prepend,
> size_t *max_capstrlen)
> attribute_hidden;
>
> -/* Look up NAME in ld.so.cache and return the file name stored there,
> - or null if none is found. Caller must free returned string. */
> -extern char *_dl_load_cache_lookup (const char *name) attribute_hidden;
> +/* Look up NAME in ld.so.cache. */
> +bool _dl_load_cache_lookup (const char *name, char **realname)
> + attribute_hidden __nonnull ((1, 2)) __attribute__ ((warn_unused_result));
>
> /* If the system does not support MAP_COPY we cannot leave the file open
> all the time since this would create problems when the file is replaced.
@@ -375,15 +375,21 @@ _dl_cache_libcmp (const char *p1, const char *p2)
}
-/* Look up NAME in ld.so.cache and return the file name stored there, or null
- if none is found. The cache is loaded if it was not already. If loading
- the cache previously failed there will be no more attempts to load it.
- The caller is responsible for freeing the returned string. The ld.so.cache
- may be unmapped at any time by a completing recursive dlopen and
- this function must take care that it does not return references to
- any data in the mapping. */
-char *
-_dl_load_cache_lookup (const char *name)
+/* Look up NAME in ld.so.cache and write the file name stored there to
+ *REALNAME, or null if none is found, and return true. In this case,
+ the caller is responsible for freeing the string in *REALNAME. If
+ there is an error condition that causes the lookup to fail (such as
+ a failure to allocate memory), the function returns false, and
+ *REALNAME is unchanged.
+
+ The cache is loaded if it was not already. If loading the cache
+ previously failed there will be no more attempts to load it.
+
+ The ld.so.cache may be unmapped at any time by a completing
+ recursive dlopen and this function must take care that it does not
+ return references to any data in the mapping. */
+bool
+_dl_load_cache_lookup (const char *name, char **realname)
{
/* Print a message if the loading of libs is traced. */
if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_LIBS))
@@ -459,8 +465,11 @@ _dl_load_cache_lookup (const char *name)
}
if (cache == (void *) -1)
- /* Previously looked for the cache file and didn't find it. */
- return NULL;
+ {
+ /* Previously looked for the cache file and didn't find it. */
+ *realname = NULL;
+ return true;
+ }
const char *best;
if (cache_new != (void *) -1)
@@ -486,7 +495,10 @@ _dl_load_cache_lookup (const char *name)
_dl_debug_printf (" trying file=%s\n", best);
if (best == NULL)
- return NULL;
+ {
+ *realname = NULL;
+ return true;
+ }
/* The double copy is *required* since malloc may be interposed
and call dlopen itself whose completion would unmap the data
@@ -496,7 +508,11 @@ _dl_load_cache_lookup (const char *name)
size_t best_len = strlen (best) + 1;
temp = alloca (best_len);
memcpy (temp, best, best_len);
- return __strdup (temp);
+ char *copy = __strdup (temp);
+ if (copy == NULL)
+ return false;
+ *realname = copy;
+ return true;
}
#ifndef MAP_COPY
@@ -2046,7 +2046,10 @@ _dl_map_new_object (struct link_map *loader, const char *name,
{
/* Check the list of libraries in the file /etc/ld.so.cache,
for compatibility with Linux's ldconfig program. */
- char *cached = _dl_load_cache_lookup (name);
+ char *cached;
+ if (!_dl_load_cache_lookup (name, &cached))
+ _dl_signal_error (ENOMEM, NULL, NULL,
+ N_("cannot allocate library name"));
if (cached != NULL)
{
@@ -1117,9 +1117,9 @@ const struct r_strlenpair *_dl_important_hwcaps (const char *prepend,
size_t *max_capstrlen)
attribute_hidden;
-/* Look up NAME in ld.so.cache and return the file name stored there,
- or null if none is found. Caller must free returned string. */
-extern char *_dl_load_cache_lookup (const char *name) attribute_hidden;
+/* Look up NAME in ld.so.cache. */
+bool _dl_load_cache_lookup (const char *name, char **realname)
+ attribute_hidden __nonnull ((1, 2)) __attribute__ ((warn_unused_result));
/* If the system does not support MAP_COPY we cannot leave the file open
all the time since this would create problems when the file is replaced.