* [patch] [SCSI] scsi_dh: potential null dereference in
@ 2011-01-03 5:48 Dan Carpenter
2011-01-04 16:13 ` Moger, Babu
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2011-01-03 5:48 UTC (permalink / raw)
To: James E.J. Bottomley
Cc: Mike Snitzer, Menny Hamburger, Babu Moger, linux-scsi,
kernel-janitors
We assumed "sdev" could be NULL ealier, so lets check it here as well.
Signed-off-by: Dan Carpenter <error27@gmail.com>
diff --git a/drivers/scsi/device_handler/scsi_dh.c b/drivers/scsi/device_handler/scsi_dh.c
index b837c5b..ff340e3 100644
--- a/drivers/scsi/device_handler/scsi_dh.c
+++ b/drivers/scsi/device_handler/scsi_dh.c
@@ -446,7 +446,7 @@ int scsi_dh_activate(struct request_queue *q, activate_complete fn, void *data)
sdev->sdev_state = SDEV_CANCEL ||
sdev->sdev_state = SDEV_DEL)
err = SCSI_DH_NOSYS;
- if (sdev->sdev_state = SDEV_OFFLINE)
+ if (sdev && sdev->sdev_state = SDEV_OFFLINE)
err = SCSI_DH_DEV_OFFLINED;
spin_unlock_irqrestore(q->queue_lock, flags);
^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: [patch] [SCSI] scsi_dh: potential null dereference in
2011-01-03 5:48 [patch] [SCSI] scsi_dh: potential null dereference in Dan Carpenter
@ 2011-01-04 16:13 ` Moger, Babu
2011-01-04 16:24 ` James Bottomley
0 siblings, 1 reply; 5+ messages in thread
From: Moger, Babu @ 2011-01-04 16:13 UTC (permalink / raw)
To: Dan Carpenter, James E.J. Bottomley
Cc: Mike Snitzer, Menny Hamburger, linux-scsi@vger.kernel.org,
kernel-janitors@vger.kernel.org
Looks good to me.
> -----Original Message-----
> From: Dan Carpenter [mailto:error27@gmail.com]
> Sent: Sunday, January 02, 2011 11:49 PM
> To: James E.J. Bottomley
> Cc: Mike Snitzer; Menny Hamburger; Moger, Babu; linux-
> scsi@vger.kernel.org; kernel-janitors@vger.kernel.org
> Subject: [patch] [SCSI] scsi_dh: potential null dereference in
> scsi_dh_activate()
>
> We assumed "sdev" could be NULL ealier, so lets check it here as well.
>
> Signed-off-by: Dan Carpenter <error27@gmail.com>
>
> diff --git a/drivers/scsi/device_handler/scsi_dh.c
> b/drivers/scsi/device_handler/scsi_dh.c
> index b837c5b..ff340e3 100644
> --- a/drivers/scsi/device_handler/scsi_dh.c
> +++ b/drivers/scsi/device_handler/scsi_dh.c
> @@ -446,7 +446,7 @@ int scsi_dh_activate(struct request_queue *q,
> activate_complete fn, void *data)
> sdev->sdev_state = SDEV_CANCEL ||
> sdev->sdev_state = SDEV_DEL)
> err = SCSI_DH_NOSYS;
> - if (sdev->sdev_state = SDEV_OFFLINE)
> + if (sdev && sdev->sdev_state = SDEV_OFFLINE)
> err = SCSI_DH_DEV_OFFLINED;
> spin_unlock_irqrestore(q->queue_lock, flags);
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [patch] [SCSI] scsi_dh: potential null dereference in
2011-01-04 16:13 ` Moger, Babu
@ 2011-01-04 16:24 ` James Bottomley
2011-01-04 19:50 ` Moger, Babu
0 siblings, 1 reply; 5+ messages in thread
From: James Bottomley @ 2011-01-04 16:24 UTC (permalink / raw)
To: Moger, Babu
Cc: Dan Carpenter, Mike Snitzer, Menny Hamburger,
linux-scsi@vger.kernel.org, kernel-janitors@vger.kernel.org
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?
James
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [patch] [SCSI] scsi_dh: potential null dereference in
2011-01-04 16:24 ` James Bottomley
@ 2011-01-04 19:50 ` Moger, Babu
2011-01-04 19:58 ` James Bottomley
0 siblings, 1 reply; 5+ messages in thread
From: Moger, Babu @ 2011-01-04 19:50 UTC (permalink / raw)
To: James Bottomley
Cc: Dan Carpenter, Mike Snitzer, Menny Hamburger,
linux-scsi@vger.kernel.org, kernel-janitors@vger.kernel.org
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 1283 bytes --]
> -----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;
}
ÿôèº{.nÇ+‰·Ÿ®‰†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¤z¹Þ—øÚž+h®ÏâžØ^n‡r¡ö¦zË\x1aëh™¨èÚ&£ûàz¿äz¹Þ—ú+€Ê+zf£¢·hšˆ§~††Ûiÿÿïêÿ‘êçz_è®\x0fæj:+v‰¨þ)ߣøm
^ permalink raw reply [flat|nested] 5+ messages in thread* RE: [patch] [SCSI] scsi_dh: potential null dereference in
2011-01-04 19:50 ` Moger, Babu
@ 2011-01-04 19:58 ` James Bottomley
0 siblings, 0 replies; 5+ messages in thread
From: James Bottomley @ 2011-01-04 19:58 UTC (permalink / raw)
To: Moger, Babu
Cc: Dan Carpenter, Mike Snitzer, Menny Hamburger,
linux-scsi@vger.kernel.org, kernel-janitors@vger.kernel.org
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-01-04 19:58 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-01-03 5:48 [patch] [SCSI] scsi_dh: potential null dereference in Dan Carpenter
2011-01-04 16:13 ` Moger, Babu
2011-01-04 16:24 ` James Bottomley
2011-01-04 19:50 ` Moger, Babu
2011-01-04 19:58 ` James Bottomley
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox