On 06/01/2012 11:47 AM, Rustad, Mark D wrote:
> Mike,
>
> On May 31, 2012, at 8:16 PM, Mike Christie wrote:
>
>> On 05/31/2012 07:14 PM, Mark Rustad wrote:
>>> Signed-off-by: Mark Rustad <mark.d.rus...@intel.com>
>>> Tested-by: Marcus Dennis <marcusx.e.den...@intel.com>
>>> ---
>>>
>>> include/scsi/scsi_cmnd.h | 8 +++++++-
>>> 1 files changed, 7 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
>>> index 1e11985..ac06cc5 100644
>>> --- a/include/scsi/scsi_cmnd.h
>>> +++ b/include/scsi/scsi_cmnd.h
>>> @@ -134,10 +134,16 @@ struct scsi_cmnd {
>>>
>>> static inline struct scsi_driver *scsi_cmd_to_driver(struct scsi_cmnd *cmd)
>>> {
>>> + struct scsi_driver **sdp;
>>> +
>>> if (!cmd->request->rq_disk)
>>> return NULL;
>>>
>>> - return *(struct scsi_driver **)cmd->request->rq_disk->private_data;
>>> + sdp = (struct scsi_driver **)cmd->request->rq_disk->private_data;
>>> + if (!sdp)
>>> + return NULL;
>>> +
>>> + return *sdp;
>>> }
>>
>> Upstream commit:
>>
>> Author: Martin K. Petersen <martin.peter...@oracle.com>
>> Date: Sat Apr 14 23:01:28 2012 -0400
>>
>> SCSI: Fix error handling when no ULD is attached
>>
>> should fix this.
>
>
> If you look closely, you will see that this patch applies on top of that
> patch. Martin's patch addressed the possibility of rq_disk being NULL. This
> patch adds checking private_data for NULL before dereferencing it.
>
Ah funny. I guess things got messed up in that thread. I do not think
everyone was talking about the same thing. In it, I was saying the
problem was in the driver being null
http://marc.info/?l=linux-usb&m=133407600206379&w=2
I thought we added the null driver check in that patch for that case,
but did not handle scsi_cmd_to_driver's dereference.
It does not make sense to me why we are doing a ** instead of just a
normal old pointer. Was
sd.c:
gd->private_data = &sdkp->driver;
supposed to just be
gd->private_data = sdkp->driver;
and then in
scsi_cmd_to_driver we just return like in the attached patch (patch not
even compile tested). It seems more clear in my patch what we are doing
and wanted.
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 5ba5c2a..6a90d3f 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2597,7 +2597,7 @@ static void sd_probe_async(void *data, async_cookie_t
cookie)
gd->minors = SD_MINORS;
gd->fops = &sd_fops;
- gd->private_data = &sdkp->driver;
+ gd->private_data = sdkp->driver;
gd->queue = sdkp->device->request_queue;
/* defaults, until the device tells us otherwise */
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 5fc97d2..6750ba9 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -705,7 +705,7 @@ static int sr_probe(struct device *dev)
disk->driverfs_dev = &sdev->sdev_gendev;
set_capacity(disk, cd->capacity);
- disk->private_data = &cd->driver;
+ disk->private_data = cd->driver;
disk->queue = sdev->request_queue;
cd->cdi.disk = disk;
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index e41998c..3ee1809 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -4075,7 +4075,7 @@ static int st_probe(struct device *dev)
kref_init(&tpnt->kref);
tpnt->disk = disk;
sprintf(disk->disk_name, "st%d", i);
- disk->private_data = &tpnt->driver;
+ disk->private_data = tpnt->driver;
disk->queue = SDp->request_queue;
tpnt->driver = &st_template;
scsi_tapes[i] = tpnt;
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 1e11985..7f5b189 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -137,7 +137,7 @@ static inline struct scsi_driver *scsi_cmd_to_driver(struct
scsi_cmnd *cmd)
if (!cmd->request->rq_disk)
return NULL;
- return *(struct scsi_driver **)cmd->request->rq_disk->private_data;
+ return cmd->request->rq_disk->private_data;
}
extern struct scsi_cmnd *scsi_get_command(struct scsi_device *, gfp_t);
_______________________________________________
devel mailing list
devel@open-fcoe.org
https://lists.open-fcoe.org/mailman/listinfo/devel
