* [scsi 1/2] scsi_debug: schedule_resp fix input variable check
@ 2015-02-23 11:13 Tomas Winkler
2015-02-23 11:13 ` [scsi 2/2] scsi_debug: fix REPORT LUNS Well Known LU Tomas Winkler
2015-02-23 14:49 ` [scsi 1/2] scsi_debug: schedule_resp fix input variable check Douglas Gilbert
0 siblings, 2 replies; 6+ messages in thread
From: Tomas Winkler @ 2015-02-23 11:13 UTC (permalink / raw)
To: James E.J. Bottomley"; +Cc: linux-scsi, Tomas Winkler
The function should never be called with cmnd NULL so
put a fat WARN there.
Fix also smatch wraning:
schedule_resp() warn: variable dereferenced before check 'cmnd'
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
drivers/scsi/scsi_debug.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index ccbe1282e975..f032aac75997 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -3947,11 +3947,18 @@ schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,
struct sdebug_queued_cmd *sqcp = NULL;
struct scsi_device *sdp = cmnd->device;
- if (NULL == cmnd || NULL == devip) {
- pr_warn("called with NULL cmnd or devip pointer\n");
+ /* this should never happend */
+ if (WARN_ON(!cmnd))
+ return SCSI_MLQUEUE_HOST_BUSY;
+
+ if (NULL == devip) {
+ pr_warn("called devip == NULL\n");
/* no particularly good error to report back */
return SCSI_MLQUEUE_HOST_BUSY;
}
+
+ sdp = cmnd->device;
+
if ((scsi_result) && (SCSI_DEBUG_OPT_NOISE & scsi_debug_opts))
sdev_printk(KERN_INFO, sdp, "%s: non-zero result=0x%x\n",
__func__, scsi_result);
--
1.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [scsi 2/2] scsi_debug: fix REPORT LUNS Well Known LU
2015-02-23 11:13 [scsi 1/2] scsi_debug: schedule_resp fix input variable check Tomas Winkler
@ 2015-02-23 11:13 ` Tomas Winkler
2015-02-23 18:28 ` Douglas Gilbert
2015-02-23 14:49 ` [scsi 1/2] scsi_debug: schedule_resp fix input variable check Douglas Gilbert
1 sibling, 1 reply; 6+ messages in thread
From: Tomas Winkler @ 2015-02-23 11:13 UTC (permalink / raw)
To: James E.J. Bottomley"; +Cc: linux-scsi, Tomas Winkler
The use case to report 'REPORT LUNS WLUN' described
in scsi_debug documentation didn't work because:
scsi_scan_host_selected() checks for:
lun < shost->max_lun
To fix this we set:
max_lun = SCSI_W_LUN_REPORT_LUNS + 1;
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
drivers/scsi/scsi_debug.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index f032aac75997..2c49ddd8d18b 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -700,7 +700,7 @@ static void sdebug_max_tgts_luns(void)
else
hpnt->max_id = scsi_debug_num_tgts;
/* scsi_debug_max_luns; */
- hpnt->max_lun = SCSI_W_LUN_REPORT_LUNS;
+ hpnt->max_lun = SCSI_W_LUN_REPORT_LUNS + 1;
}
spin_unlock(&sdebug_host_list_lock);
}
@@ -5344,7 +5344,8 @@ static int sdebug_driver_probe(struct device * dev)
hpnt->max_id = scsi_debug_num_tgts + 1;
else
hpnt->max_id = scsi_debug_num_tgts;
- hpnt->max_lun = SCSI_W_LUN_REPORT_LUNS; /* = scsi_debug_max_luns; */
+ /* = scsi_debug_max_luns; */
+ hpnt->max_lun = SCSI_W_LUN_REPORT_LUNS + 1;
host_prot = 0;
--
1.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [scsi 1/2] scsi_debug: schedule_resp fix input variable check
2015-02-23 11:13 [scsi 1/2] scsi_debug: schedule_resp fix input variable check Tomas Winkler
2015-02-23 11:13 ` [scsi 2/2] scsi_debug: fix REPORT LUNS Well Known LU Tomas Winkler
@ 2015-02-23 14:49 ` Douglas Gilbert
2015-02-23 15:14 ` Winkler, Tomas
1 sibling, 1 reply; 6+ messages in thread
From: Douglas Gilbert @ 2015-02-23 14:49 UTC (permalink / raw)
To: Tomas Winkler, James E.J. Bottomley"; +Cc: linux-scsi
On 15-02-23 06:13 AM, Tomas Winkler wrote:
> The function should never be called with cmnd NULL so
> put a fat WARN there.
> Fix also smatch wraning:
> schedule_resp() warn: variable dereferenced before check 'cmnd'
>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> ---
> drivers/scsi/scsi_debug.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index ccbe1282e975..f032aac75997 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -3947,11 +3947,18 @@ schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip,
> struct sdebug_queued_cmd *sqcp = NULL;
> struct scsi_device *sdp = cmnd->device;
This patch seems incorrect because it still dereferences
cmnd (in the above line) before it checks it for NULL.
> - if (NULL == cmnd || NULL == devip) {
> - pr_warn("called with NULL cmnd or devip pointer\n");
> + /* this should never happend */
happen?
The scsi_debug driver was written by Eric Youngdale to test
the original Linux SCSI subsystem (or a subsequent rewrite
of same). Remnants of its "trust nobody" style remain and
may have been of use to more recent tinkerers.
> + if (WARN_ON(!cmnd))
> + return SCSI_MLQUEUE_HOST_BUSY;
> +
> + if (NULL == devip) {
if (unlikely(NULL == devip)) {
> + pr_warn("called devip == NULL\n");
> /* no particularly good error to report back */
> return SCSI_MLQUEUE_HOST_BUSY;
> }
> +
> + sdp = cmnd->device;
> +
> if ((scsi_result) && (SCSI_DEBUG_OPT_NOISE & scsi_debug_opts))
> sdev_printk(KERN_INFO, sdp, "%s: non-zero result=0x%x\n",
> __func__, scsi_result);
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [scsi 1/2] scsi_debug: schedule_resp fix input variable check
2015-02-23 14:49 ` [scsi 1/2] scsi_debug: schedule_resp fix input variable check Douglas Gilbert
@ 2015-02-23 15:14 ` Winkler, Tomas
0 siblings, 0 replies; 6+ messages in thread
From: Winkler, Tomas @ 2015-02-23 15:14 UTC (permalink / raw)
To: dgilbert, James E.J. Bottomley"; +Cc: linux-scsi
> > @@ -3947,11 +3947,18 @@ schedule_resp(struct scsi_cmnd *cmnd, struct
> sdebug_dev_info *devip,
> > struct sdebug_queued_cmd *sqcp = NULL;
> > struct scsi_device *sdp = cmnd->device;
>
> This patch seems incorrect because it still dereferences
> cmnd (in the above line) before it checks it for NULL.
>
Opps, haven't sent the lastest version.
> > - if (NULL == cmnd || NULL == devip) {
> > - pr_warn("called with NULL cmnd or devip pointer\n");
> > + /* this should never happend */
>
> happen?
Thanks
>
> The scsi_debug driver was written by Eric Youngdale to test
> the original Linux SCSI subsystem (or a subsequent rewrite
> of same). Remnants of its "trust nobody" style remain and
> may have been of use to more recent tinkerers.
The check is preserved with bigger warning, there is just no single call for this function w/ cmnd == NULL.
>
> > + if (WARN_ON(!cmnd))
> > + return SCSI_MLQUEUE_HOST_BUSY;
> > +
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [scsi 2/2] scsi_debug: fix REPORT LUNS Well Known LU
2015-02-23 11:13 ` [scsi 2/2] scsi_debug: fix REPORT LUNS Well Known LU Tomas Winkler
@ 2015-02-23 18:28 ` Douglas Gilbert
2015-05-12 7:56 ` Winkler, Tomas
0 siblings, 1 reply; 6+ messages in thread
From: Douglas Gilbert @ 2015-02-23 18:28 UTC (permalink / raw)
To: Tomas Winkler, James E.J. Bottomley"; +Cc: linux-scsi
On 15-02-23 06:13 AM, Tomas Winkler wrote:
> The use case to report 'REPORT LUNS WLUN' described
> in scsi_debug documentation didn't work because:
> scsi_scan_host_selected() checks for:
> lun < shost->max_lun
>
> To fix this we set:
> max_lun = SCSI_W_LUN_REPORT_LUNS + 1;
>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Acked-by: Douglas Gilbert <dgilbert@interlog.com>
> ---
> drivers/scsi/scsi_debug.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index f032aac75997..2c49ddd8d18b 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -700,7 +700,7 @@ static void sdebug_max_tgts_luns(void)
> else
> hpnt->max_id = scsi_debug_num_tgts;
> /* scsi_debug_max_luns; */
> - hpnt->max_lun = SCSI_W_LUN_REPORT_LUNS;
> + hpnt->max_lun = SCSI_W_LUN_REPORT_LUNS + 1;
> }
> spin_unlock(&sdebug_host_list_lock);
> }
> @@ -5344,7 +5344,8 @@ static int sdebug_driver_probe(struct device * dev)
> hpnt->max_id = scsi_debug_num_tgts + 1;
> else
> hpnt->max_id = scsi_debug_num_tgts;
> - hpnt->max_lun = SCSI_W_LUN_REPORT_LUNS; /* = scsi_debug_max_luns; */
> + /* = scsi_debug_max_luns; */
> + hpnt->max_lun = SCSI_W_LUN_REPORT_LUNS + 1;
>
> host_prot = 0;
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [scsi 2/2] scsi_debug: fix REPORT LUNS Well Known LU
2015-02-23 18:28 ` Douglas Gilbert
@ 2015-05-12 7:56 ` Winkler, Tomas
0 siblings, 0 replies; 6+ messages in thread
From: Winkler, Tomas @ 2015-05-12 7:56 UTC (permalink / raw)
To: dgilbert, James E.J. Bottomley"
Cc: linux-scsi, linux-kernel, Christoph Hellwig
> On 15-02-23 06:13 AM, Tomas Winkler wrote:
> > The use case to report 'REPORT LUNS WLUN' described
> > in scsi_debug documentation didn't work because:
> > scsi_scan_host_selected() checks for:
> > lun < shost->max_lun
> >
> > To fix this we set:
> > max_lun = SCSI_W_LUN_REPORT_LUNS + 1;
> >
> > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
>
> Acked-by: Douglas Gilbert <dgilbert@interlog.com>
>
Is anyone picking up this changes ?
What is the review process on the scsi list
Thanks
Tomas
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-05-12 7:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-23 11:13 [scsi 1/2] scsi_debug: schedule_resp fix input variable check Tomas Winkler
2015-02-23 11:13 ` [scsi 2/2] scsi_debug: fix REPORT LUNS Well Known LU Tomas Winkler
2015-02-23 18:28 ` Douglas Gilbert
2015-05-12 7:56 ` Winkler, Tomas
2015-02-23 14:49 ` [scsi 1/2] scsi_debug: schedule_resp fix input variable check Douglas Gilbert
2015-02-23 15:14 ` Winkler, Tomas
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.