[PATCH 3/5] Xi: avoid overrun of callback array.

[walter harms]

Am 20.10.2011 16:43, schrieb Jamey Sharp:
> On Wed, Oct 19, 2011 at 05:01:45PM +0100, Dave Airlie wrote:
>> From: Dave Airlie <airlied at redhat.com>
>>
>> This code had an off-by-one and would allow writing one past the end of
>> the callbacks array.
> 
> I think you mean "reading" one past the end? I don't see any bad pointer
> writes here, though I'm only looking at the patch context.
> 

Hi Jamey,
you are right a "write" is not visible in the patch. NTL any "access"
beyond  index >= ARRAY_SIZE(SProcIVector) is wrong. So the patch
is ok.

More interessing is: why do i see the same code two times ?
(1. ProcIVector, 1. SProcIVector; can this be merged in future ?)

re,
 wh

Reviewed-by: Walter Harms <wharms at bfs.de>

> 
>> Pointed out by coverity.
>>
>> Signed-off-by: Dave Airlie <airlied at redhat.com>
>> ---
>>  Xi/extinit.c |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/Xi/extinit.c b/Xi/extinit.c
>> index 7724f5f..1fbe0a2 100644
>> --- a/Xi/extinit.c
>> +++ b/Xi/extinit.c
>> @@ -409,7 +409,7 @@ static int
>>  ProcIDispatch(ClientPtr client)
>>  {
>>      REQUEST(xReq);
>> -    if (stuff->data > ARRAY_SIZE(ProcIVector) || !ProcIVector[stuff->data])
>> +    if (stuff->data >= ARRAY_SIZE(ProcIVector) || !ProcIVector[stuff->data])
>>          return BadRequest;
>>  
>>      return (*ProcIVector[stuff->data])(client);
>> @@ -428,7 +428,7 @@ static int
>>  SProcIDispatch(ClientPtr client)
>>  {
>>      REQUEST(xReq);
>> -    if (stuff->data > ARRAY_SIZE(SProcIVector) || !SProcIVector[stuff->data])
>> +    if (stuff->data >= ARRAY_SIZE(SProcIVector) || !SProcIVector[stuff->data])
>>          return BadRequest;
>>  
>>      return (*SProcIVector[stuff->data])(client);
>> -- 
>> 1.7.6.4
>>