| Message ID | 20211224120216.35896-1-yuehaibing@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | [-next] scsi: storvsc: Fix unsigned comparison to zero | expand |
From: YueHaibing <yuehaibing@huawei.com> Sent: Friday, December 24, 2021 4:02 AM > > The unsigned variable sg_count is being assigned a return value > from the call to scsi_dma_map() that can return -ENOMEM. > > Fixes: 743b237c3a7b ("scsi: storvsc: Add Isolation VM support for storvsc driver") > Signed-off-by: YueHaibing <yuehaibing@huawei.com> Good catch! But I wonder if a simpler fix is to just remove the "unsigned" from the declaration of sg_count. Then we don't need to add the sg_cnt local variable. A little less complexity is almost always better. :-) Michael > --- > drivers/scsi/storvsc_drv.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c > index ae293600d799..072c752a8c36 100644 > --- a/drivers/scsi/storvsc_drv.c > +++ b/drivers/scsi/storvsc_drv.c > @@ -1837,7 +1837,7 @@ static int storvsc_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scmnd) > unsigned int hvpg_count = HVPFN_UP(offset_in_hvpg + length); > struct scatterlist *sg; > unsigned long hvpfn, hvpfns_to_add; > - int j, i = 0; > + int sg_cnt, j, i = 0; > > if (hvpg_count > MAX_PAGE_BUFFER_COUNT) { > > @@ -1851,11 +1851,11 @@ static int storvsc_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scmnd) > payload->range.len = length; > payload->range.offset = offset_in_hvpg; > > - sg_count = scsi_dma_map(scmnd); > - if (sg_count < 0) > + sg_cnt = scsi_dma_map(scmnd); > + if (sg_cnt < 0) > return SCSI_MLQUEUE_DEVICE_BUSY; > > - for_each_sg(sgl, sg, sg_count, j) { > + for_each_sg(sgl, sg, sg_cnt, j) { > /* > * Init values for the current sgl entry. hvpfns_to_add > * is in units of Hyper-V size pages. Handling the > -- > 2.17.1
On 2021/12/27 3:49, Michael Kelley (LINUX) wrote: > From: YueHaibing <yuehaibing@huawei.com> Sent: Friday, December 24, 2021 4:02 AM >> >> The unsigned variable sg_count is being assigned a return value >> from the call to scsi_dma_map() that can return -ENOMEM. >> >> Fixes: 743b237c3a7b ("scsi: storvsc: Add Isolation VM support for storvsc driver") >> Signed-off-by: YueHaibing <yuehaibing@huawei.com> > > Good catch! But I wonder if a simpler fix is to just remove the "unsigned" > from the declaration of sg_count. Then we don't need to add the > sg_cnt local variable. A little less complexity is almost always better. :-) Ok， will respine > > Michael > >> --- >> drivers/scsi/storvsc_drv.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c >> index ae293600d799..072c752a8c36 100644 >> --- a/drivers/scsi/storvsc_drv.c >> +++ b/drivers/scsi/storvsc_drv.c >> @@ -1837,7 +1837,7 @@ static int storvsc_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scmnd) >> unsigned int hvpg_count = HVPFN_UP(offset_in_hvpg + length); >> struct scatterlist *sg; >> unsigned long hvpfn, hvpfns_to_add; >> - int j, i = 0; >> + int sg_cnt, j, i = 0; >> >> if (hvpg_count > MAX_PAGE_BUFFER_COUNT) { >> >> @@ -1851,11 +1851,11 @@ static int storvsc_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scmnd) >> payload->range.len = length; >> payload->range.offset = offset_in_hvpg; >> >> - sg_count = scsi_dma_map(scmnd); >> - if (sg_count < 0) >> + sg_cnt = scsi_dma_map(scmnd); >> + if (sg_cnt < 0) >> return SCSI_MLQUEUE_DEVICE_BUSY; >> >> - for_each_sg(sgl, sg, sg_count, j) { >> + for_each_sg(sgl, sg, sg_cnt, j) { >> /* >> * Init values for the current sgl entry. hvpfns_to_add >> * is in units of Hyper-V size pages. Handling the >> -- >> 2.17.1 > > . >
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index ae293600d799..072c752a8c36 100644 --- a/drivers/scsi/storvsc_drv.c +++ b/drivers/scsi/storvsc_drv.c @@ -1837,7 +1837,7 @@ static int storvsc_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scmnd) unsigned int hvpg_count = HVPFN_UP(offset_in_hvpg + length); struct scatterlist *sg; unsigned long hvpfn, hvpfns_to_add; - int j, i = 0; + int sg_cnt, j, i = 0; if (hvpg_count > MAX_PAGE_BUFFER_COUNT) { @@ -1851,11 +1851,11 @@ static int storvsc_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scmnd) payload->range.len = length; payload->range.offset = offset_in_hvpg; - sg_count = scsi_dma_map(scmnd); - if (sg_count < 0) + sg_cnt = scsi_dma_map(scmnd); + if (sg_cnt < 0) return SCSI_MLQUEUE_DEVICE_BUSY; - for_each_sg(sgl, sg, sg_count, j) { + for_each_sg(sgl, sg, sg_cnt, j) { /* * Init values for the current sgl entry. hvpfns_to_add * is in units of Hyper-V size pages. Handling the
The unsigned variable sg_count is being assigned a return value from the call to scsi_dma_map() that can return -ENOMEM. Fixes: 743b237c3a7b ("scsi: storvsc: Add Isolation VM support for storvsc driver") Signed-off-by: YueHaibing <yuehaibing@huawei.com> --- drivers/scsi/storvsc_drv.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)