Re: [PATCH] scsi_transport_sas: Write outside array bounds

[James Bottomley <James.Bottomley@HansenPartnership.com>]

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.

James