[v3,07/32] 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
|
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 | 22 ++++++++++++++++------
elf/dl-load.c | 5 ++++-
sysdeps/generic/ldsodefs.h | 10 +++++++---
3 files changed, 27 insertions(+), 10 deletions(-)
Comments
On Thu, 7 Dec 2023, Florian Weimer wrote:
> diff --git a/elf/dl-cache.c b/elf/dl-cache.c
> index 804bf23222..a3eb960dac 100644
> --- a/elf/dl-cache.c
> +++ b/elf/dl-cache.c
> @@ -398,8 +398,8 @@ _dl_cache_libcmp (const char *p1, const char *p2)
> 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)
> +bool
> +_dl_load_cache_lookup (const char *name, char **realname)
The comment above this function definition needs updating to describe the
new semantics for return value and result stored in *REALNAME.
> -/* 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. Return false on memory allocation
> + failure and do not change *REALNAME. If lookup fails, return true
> + and write a null pointer to *REALNAME. If lookup suceeds, write a
"succeeds"
> + copy of the full name to *REALNAME (which has to be freed by the
> + caller). */
> +bool _dl_load_cache_lookup (const char *name, char **realname)
> + attribute_hidden __nonnull ((1, 2)) __attribute__ ((warn_unused_result));
Something like the comment here - though the two comments give different
information, so don't lose the extra information on the definition unless
you're confident it's no longer relevant. However, the comment here
doesn't actually say it returns true when lookup and allocation both
succeed.
@@ -398,8 +398,8 @@ _dl_cache_libcmp (const char *p1, const char *p2)
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)
+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))
@@ -475,8 +475,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)
@@ -502,7 +505,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
@@ -512,7 +518,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
@@ -2082,7 +2082,10 @@ _dl_map_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)
{
@@ -1111,9 +1111,13 @@ 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. Return false on memory allocation
+ failure and do not change *REALNAME. If lookup fails, return true
+ and write a null pointer to *REALNAME. If lookup suceeds, write a
+ copy of the full name to *REALNAME (which has to be freed by the
+ caller). */
+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.