Home » Mailing list archives » linux-kernel-janitors » 2011 » January » 4

RE: [patch] [SCSI] scsi_dh: potential null dereference in scsi_dh_activate()

Previous thread: [patch 2/2] Staging: tm6000: silence Sparse warning "dubious: !x | !y" by Dan Carpenter on Sunday, January 2, 2011 - 10:47 pm. (1 message)

Next thread: [patch -next] ocfs2/cluster: dereferencing before checking in nst_seq_show() by Dan Carpenter on Sunday, January 2, 2011 - 11:00 pm. (2 messages)
[view original message]
From: Dan Carpenter
Subject: [patch] [SCSI] scsi_dh: potential null dereference in scsi_dh_activate()
Date: Sunday, January 2, 2011 - 10:48 pm

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);
 
--


[ message continues ]
[view original message]
From: Moger, Babu
Subject: RE: [patch] [SCSI] scsi_dh: potential null dereference in scsi_dh_activate()
Date: Tuesday, January 4, 2011 - 9:13 am

--


[ message continues ]
[view original message]
From: James Bottomley
Subject: RE: [patch] [SCSI] scsi_dh: potential null dereference in scsi_dh_activate()
Date: Tuesday, January 4, 2011 - 9:24 am

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


--


[ message continues ]
[view original message]
From: Moger, Babu
Subject: RE: [patch] [SCSI] scsi_dh: potential null dereference in scsi_dh_activate()
Date: Tuesday, January 4, 2011 - 12:50 pm

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;
}
 

[ message continues ]
[view original message]
From: James Bottomley
Subject: RE: [patch] [SCSI] scsi_dh: potential null dereference in scsi_dh_activate()
Date: Tuesday, January 4, 2011 - 12:58 pm

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


--


[ message continues ]
Previous thread: [patch 2/2] Staging: tm6000: silence Sparse warning "dubious: !x | !y" by Dan Carpenter on Sunday, January 2, 2011 - 10:47 pm. (1 message)

Next thread: [patch -next] ocfs2/cluster: dereferencing before checking in nst_seq_show() by Dan Carpenter on Sunday, January 2, 2011 - 11:00 pm. (2 messages)