Re: [PATCH] scsi_transport_sas: Write outside array bounds

Jeff Garzik
2009-07-29T02:50:57Z

James Bottomley wrote:
> On Tue, 2009-07-28 at 16:10 -0400, Jeff Garzik wrote:
>> James Bottomley wrote:
>>> In this case, as you can see all of the SETUP_X are using count to step
>>> through an array and when we're finished we add a NULL to the end. For
>>> the case of SETUP_PORT_ATTR, the array is i->port_attrs, so the bug here
>>> is apparently that the NULL is going in the wrong array. Trying to put
>>> a NULL at the zero element is manifestly wrong because sas_internal is
>>> zero allocated.
>>>
>>> Actually, as a final weirdness, we have a duplicate initialisation of
>>> this anyway, so the correct patch is below.
>>>
>>> James
>>>
>>> ---
>>>
>>> diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c
>>> index 0895d3c..fd47cb1 100644
>>> --- a/drivers/scsi/scsi_transport_sas.c
>>> +++ b/drivers/scsi/scsi_transport_sas.c
>>> @@ -1692,10 +1692,6 @@ sas_attach_transport(struct sas_function_template *ft)
>>> i->f = ft;
>>>
>>> count = 0;
>>> - SETUP_PORT_ATTRIBUTE(num_phys);
>>> - i->host_attrs[count] = NULL;
>>> -
>>> - count = 0;
>>> SETUP_PHY_ATTRIBUTE(initiator_port_protocols);
>>> SETUP_PHY_ATTRIBUTE(target_port_protocols);
>>> SETUP_PHY_ATTRIBUTE(device_type);
>> er, eh?
>>
>> It's not a duplicate initialization, as port attributes != phy
>> attributes. Different macro, same counter variable.
>>
>> Your fix seems quite strange - num_phy attribute goes away?
>
> Look in the actual file. About 20 lines down you'll find a duplicate
> port setup for num_phys.

Indeed... sorry for the noise!

Jeff