On Tue, 2011-01-04 at 12:50 -0700, Moger, Babu wrote: > > -----Original Message----- > > From: James Bottomley [mailto:James.Bottomley@xxxxxxx] > > Sent: Tuesday, January 04, 2011 10:25 AM > > To: Moger, Babu > > Cc: Dan Carpenter; Mike Snitzer; Menny Hamburger; linux- > > scsi@xxxxxxxxxxxxxxx; kernel-janitors@xxxxxxxxxxxxxxx > > Subject: RE: [patch] [SCSI] scsi_dh: potential null dereference in > > scsi_dh_activate() > > > > On Tue, 2011-01-04 at 09:13 -0700, Moger, Babu wrote: > > > Looks good to me. > > > > It does? The first check is the bogus one, surely. The queue is > > created and destroyed by scsi_alloc_sdev(), so queuedata can never be > > NULL for a SCSI queue. There's no check anywhere in the rest of SCSI, > > so there shouldn't be one here, should there? > > You are right. This check may not be required. > > But I am not sure why there is a check in scsi_device_from_queue. > Is there a possibility of request_fn other than scsi_request_fn for scsi device? I donât know. Here is the code.. > > struct scsi_device *scsi_device_from_queue(struct request_queue *q) > { > struct scsi_device *sdev = NULL; > > if (q->request_fn == scsi_request_fn) > sdev = q->queuedata; > > return sdev; It can be called for any queue so it returns NULL for a non-SCSI queue. If you think the queue scsi_dh_activate() is called on may be a non-scsi one, then you need to use scsi_device_from_queue() and check the result. Checking q->queuedata for NULL isn't sufficient because other devices are perfectly entitled to use q->queuedata for their own purposes. If you know you have a SCSI queue, you can just pick sdev out of q->queuedata. James -- 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
