On Tue, 2009-07-28 at 12:20 +0200, Roel Kluin wrote: > SETUP_PORT_ATTRIBUTE increments count, making the write out of bounds (array of > size 1) > > Signed-off-by: Roel Kluin <roel.kluin@xxxxxxxxx> > --- > Credits to Parfait (http://research.sun.com/projects/parfait/) > > I suspect this isn't the only location where count shouldn't be incremented, > Somebody should review this function. > > diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c > index 0895d3c..c784ae4 100644 > --- a/drivers/scsi/scsi_transport_sas.c > +++ b/drivers/scsi/scsi_transport_sas.c > @@ -1693,9 +1693,10 @@ sas_attach_transport(struct sas_function_template *ft) > > count = 0; > SETUP_PORT_ATTRIBUTE(num_phys); > - i->host_attrs[count] = NULL; > > count = 0; > + i->host_attrs[count] = NULL; > + > SETUP_PHY_ATTRIBUTE(initiator_port_protocols); > SETUP_PHY_ATTRIBUTE(target_port_protocols); > SETUP_PHY_ATTRIBUTE(device_type); If you're going to use mechanical checkers for this type of thing, it would really help me if you'd actually think about what the code is trying to do before sending a patch. 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); -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
