Message ID | 20180626180542.5AAAD43994575@oldenburg.str.redhat.com |
---|---|
State | New |
Headers | show |
Series | _dl_map_object_deps: Use struct scratch_buffer [BZ #18023] | expand |
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.
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); /* 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);