_dl_map_object_deps: Use struct scratch_buffer [BZ #18023]
Commit Message
The function comment suggests that _dl_map_object_deps cannot use
malloc, but it already allocates the l_initfini array on the heap, so
the additional allocation should be acceptable.
2018-06-26 Florian Weimer <fweimer@redhat.com>
[BZ #18023]
* elf/dl-deps.c (_dl_map_object_deps): Use struct
scratch_buffer instead of extend_alloca.
Comments
On 06/26/2018 02:05 PM, Florian Weimer wrote:
> The function comment suggests that _dl_map_object_deps cannot use
> malloc, but it already allocates the l_initfini array on the heap, so
> the additional allocation should be acceptable.
>
> 2018-06-26 Florian Weimer <fweimer@redhat.com>
>
> [BZ #18023]
> * elf/dl-deps.c (_dl_map_object_deps): Use struct
> scratch_buffer instead of extend_alloca.
LGTM.
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
>
> diff --git a/elf/dl-deps.c b/elf/dl-deps.c
> index 20b8e94f2e..9d9b1ba7f2 100644
> --- a/elf/dl-deps.c
> +++ b/elf/dl-deps.c
> @@ -27,6 +27,7 @@
> #include <unistd.h>
> #include <sys/param.h>
> #include <ldsodefs.h>
> +#include <scratch_buffer.h>
>
> #include <dl-dst.h>
>
> @@ -181,9 +182,8 @@ _dl_map_object_deps (struct link_map *map,
> /* Pointer to last unique object. */
> tail = &known[nlist - 1];
>
> - /* No alloca'd space yet. */
> - struct link_map **needed_space = NULL;
> - size_t needed_space_bytes = 0;
> + struct scratch_buffer needed_space;
> + scratch_buffer_init (&needed_space);
OK.
>
> /* Process each element of the search list, loading each of its
> auxiliary objects and immediate dependencies. Auxiliary objects
> @@ -213,13 +213,12 @@ _dl_map_object_deps (struct link_map *map,
> if (l->l_searchlist.r_list == NULL && l->l_initfini == NULL
> && l != map && l->l_ldnum > 0)
> {
> - size_t new_size = l->l_ldnum * sizeof (struct link_map *);
> -
> - if (new_size > needed_space_bytes)
> - needed_space
> - = extend_alloca (needed_space, needed_space_bytes, new_size);
> -
> - needed = needed_space;
> + /* l->l_ldnum includes space for the terminating NULL. */
> + if (!scratch_buffer_set_array_size
> + (&needed_space, l->l_ldnum, sizeof (struct link_map *)))
> + _dl_signal_error (ENOMEM, map->l_name, NULL,
> + N_("cannot allocate dependency buffer"));
> + needed = needed_space.data;
OK.
> }
>
> if (l->l_info[DT_NEEDED] || l->l_info[AUXTAG] || l->l_info[FILTERTAG])
> @@ -438,8 +437,11 @@ _dl_map_object_deps (struct link_map *map,
> struct link_map **l_initfini = (struct link_map **)
> malloc ((2 * nneeded + 1) * sizeof needed[0]);
> if (l_initfini == NULL)
> - _dl_signal_error (ENOMEM, map->l_name, NULL,
> - N_("cannot allocate dependency list"));
> + {
> + scratch_buffer_free (&needed_space);
> + _dl_signal_error (ENOMEM, map->l_name, NULL,
> + N_("cannot allocate dependency list"));
OK.
> + }
> l_initfini[0] = l;
> memcpy (&l_initfini[1], needed, nneeded * sizeof needed[0]);
> memcpy (&l_initfini[nneeded + 1], l_initfini,
> @@ -457,6 +459,8 @@ _dl_map_object_deps (struct link_map *map,
> }
>
> out:
> + scratch_buffer_free (&needed_space);
OK. Verified flow in the function can't get around this.
> +
> if (errno == 0 && errno_saved != 0)
> __set_errno (errno_saved);
>
>
cheers,
Carlos.
@@ -27,6 +27,7 @@
#include <unistd.h>
#include <sys/param.h>
#include <ldsodefs.h>
+#include <scratch_buffer.h>
#include <dl-dst.h>
@@ -181,9 +182,8 @@ _dl_map_object_deps (struct link_map *map,
/* Pointer to last unique object. */
tail = &known[nlist - 1];
- /* No alloca'd space yet. */
- struct link_map **needed_space = NULL;
- size_t needed_space_bytes = 0;
+ struct scratch_buffer needed_space;
+ scratch_buffer_init (&needed_space);
/* Process each element of the search list, loading each of its
auxiliary objects and immediate dependencies. Auxiliary objects
@@ -213,13 +213,12 @@ _dl_map_object_deps (struct link_map *map,
if (l->l_searchlist.r_list == NULL && l->l_initfini == NULL
&& l != map && l->l_ldnum > 0)
{
- size_t new_size = l->l_ldnum * sizeof (struct link_map *);
-
- if (new_size > needed_space_bytes)
- needed_space
- = extend_alloca (needed_space, needed_space_bytes, new_size);
-
- needed = needed_space;
+ /* l->l_ldnum includes space for the terminating NULL. */
+ if (!scratch_buffer_set_array_size
+ (&needed_space, l->l_ldnum, sizeof (struct link_map *)))
+ _dl_signal_error (ENOMEM, map->l_name, NULL,
+ N_("cannot allocate dependency buffer"));
+ needed = needed_space.data;
}
if (l->l_info[DT_NEEDED] || l->l_info[AUXTAG] || l->l_info[FILTERTAG])
@@ -438,8 +437,11 @@ _dl_map_object_deps (struct link_map *map,
struct link_map **l_initfini = (struct link_map **)
malloc ((2 * nneeded + 1) * sizeof needed[0]);
if (l_initfini == NULL)
- _dl_signal_error (ENOMEM, map->l_name, NULL,
- N_("cannot allocate dependency list"));
+ {
+ scratch_buffer_free (&needed_space);
+ _dl_signal_error (ENOMEM, map->l_name, NULL,
+ N_("cannot allocate dependency list"));
+ }
l_initfini[0] = l;
memcpy (&l_initfini[1], needed, nneeded * sizeof needed[0]);
memcpy (&l_initfini[nneeded + 1], l_initfini,
@@ -457,6 +459,8 @@ _dl_map_object_deps (struct link_map *map,
}
out:
+ scratch_buffer_free (&needed_space);
+
if (errno == 0 && errno_saved != 0)
__set_errno (errno_saved);