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

[Alan Coopersmith]

On 04/17/15 04:59 AM, Daniel Stone wrote:
> 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.

Yeah, the XKB code could always use more work, but I'll leave that for
someone else to tackle later.   The inconsistent usage of the various
models for tracking array sizes drives me crazy too:

http://cgit.freedesktop.org/xorg/proto/kbproto/commit/?id=95ee49d90c28b15a3c3be54a233368fc69f3531a

> 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>

Thanks.


-- 
	-Alan Coopersmith-              alan.coopersmith at oracle.com
	 Oracle Solaris Engineering - http://blogs.oracle.com/alanc