RE: [patch] [SCSI] scsi_dh: potential null dereference in

[James Bottomley <James.Bottomley@suse.de>]

On Tue, 2011-01-04 at 12:50 -0700, Moger, Babu wrote:
> > -----Original Message-----
> > From: James Bottomley [mailto:James.Bottomley@suse.de]
> > Sent: Tuesday, January 04, 2011 10:25 AM
> > To: Moger, Babu
> > Cc: Dan Carpenter; Mike Snitzer; Menny Hamburger; linux-
> > scsi@vger.kernel.org; kernel-janitors@vger.kernel.org
> > 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 kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html