[PATCH v2 10/17] Convert XKB to new *allocarray functions

[Daniel Stone]

Hi,

On 17 April 2015 at 02:49, Alan Coopersmith <alan.coopersmith at oracle.com> wrote:
> diff --git a/xkb/xkbActions.c b/xkb/xkbActions.c
> index 2a196f1..9dd1cbd 100644
> --- a/xkb/xkbActions.c
> +++ b/xkb/xkbActions.c
> @@ -1103,8 +1103,8 @@ _XkbNextFreeFilter(XkbSrvInfoPtr xkbi)
>          }
>      }
>      xkbi->szFilters *= 2;
> -    xkbi->filters = realloc(xkbi->filters,
> -                            xkbi->szFilters * sizeof(XkbFilterRec));
> +    xkbi->filters = reallocarray(xkbi->filters,
> +                                 xkbi->szFilters, sizeof(XkbFilterRec));
>      /* 6/21/93 (ef) -- XXX! deal with allocation failure */
>      memset(&xkbi->filters[xkbi->szFilters / 2], 0,
>             (xkbi->szFilters / 2) * sizeof(XkbFilterRec));

This stuck out as being potentially unsafe, but seems to just be a bug
(in that filters disappear) rather than a security vulnerability,
because the access is always bounded by szFilters. I can see it being
an issue in other similar codepaths though, where the pattern is to
have szFoo (size of list) and numFoo (actual elements in the list). In
the case where we naïvely increase the list and szFoo overflows, then
anything using numFoo to iterate over the list suddenly becomes
unsafe.

My brain started leaking out my face halfway through reviewing this
diff though, and auditing all sz/num users would be a bit of a
nightmare. So, on the grounds that this makes things better rather
than in any way worse:
Acked-by: Daniel Stone <daniels at collabora.com>

Cheers,
Daniel