Re: [PATCH] megaraid_sas: fix memory leak if SGL has zero length entries

2013-06-29 Thread Bjørn Mork
Bjørn Mork  writes:

> commit 98cb7e44 ([SCSI] megaraid_sas: Sanity check user
> supplied length before passing it to dma_alloc_coherent())
> introduced a memory leak.  Memory allocated for entries
> following zero length SGL entries will not be freed.
>
> Reference: http://bugs.debian.org/688198
> Cc: 
> Signed-off-by: Bjørn Mork 
> ---
>  drivers/scsi/megaraid/megaraid_sas_base.c |   10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c 
> b/drivers/scsi/megaraid/megaraid_sas_base.c
> index d2c5366..12b6be4 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> @@ -4854,10 +4854,12 @@ megasas_mgmt_fw_ioctl(struct megasas_instance 
> *instance,
>   sense, sense_handle);
>   }
>  
> - for (i = 0; i < ioc->sge_count && kbuff_arr[i]; i++) {
> - dma_free_coherent(&instance->pdev->dev,
> - kern_sge32[i].length,
> - kbuff_arr[i], kern_sge32[i].phys_addr);
> + for (i = 0; i < ioc->sge_count; i++) {
> + if (kbuff_arr[i])
> + dma_free_coherent(&instance->pdev->dev,
> +   kern_sge32[i].length,
> +   kbuff_arr[i],
> +   kern_sge32[i].phys_addr);
>   }
>  
>   megasas_return_cmd(instance, cmd);


This patch was acked by Adam Radford 4 Dec 2012:
http://permalink.gmane.org/gmane.linux.kernel.stable/36537
but it looks like it got lost somewhere after that.

Please let me know asap if it should be resent.  I'm otherwise going to
clean it out of my todo queue.


Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] scsi: qla2xxx: change some variables to hexadecimal string via %*phN format

2013-06-29 Thread Oleksandr Khoshaba
The patch modifies some variables using the format '%*phN' and prints them as a 
hexadecimal string with the separator ''.

Signed-off-by: Oleksandr Khoshaba 
---
 drivers/scsi/qla2xxx/qla_gs.c  |   12 ++--
 drivers/scsi/qla2xxx/qla_isr.c |9 ++---
 drivers/scsi/qla2xxx/qla_mr.c  |   10 ++
 3 files changed, 6 insertions(+), 25 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c
index 8b2f75a..4155e15 100644
--- a/drivers/scsi/qla2xxx/qla_gs.c
+++ b/drivers/scsi/qla2xxx/qla_gs.c
@@ -1394,16 +1394,8 @@ qla2x00_fdmi_rhba(scsi_qla_host_t *vha)
qla2x00_update_ms_fdmi_iocb(vha, size + 16);
 
ql_dbg(ql_dbg_disc, vha, 0x202e,
-   "RHBA identifier = "
-   "%02x%02x%02x%02x%02x%02x%02x%02x size=%d.\n",
-   ct_req->req.rhba.hba_identifier[0],
-   ct_req->req.rhba.hba_identifier[1],
-   ct_req->req.rhba.hba_identifier[2],
-   ct_req->req.rhba.hba_identifier[3],
-   ct_req->req.rhba.hba_identifier[4],
-   ct_req->req.rhba.hba_identifier[5],
-   ct_req->req.rhba.hba_identifier[6],
-   ct_req->req.rhba.hba_identifier[7], size);
+   "RHBA identifier = %8phN size=%d.\n",
+   ct_req->req.rhba.hba_identifier, size);
ql_dump_buffer(ql_dbg_disc + ql_dbg_buffer, vha, 0x2076,
entries, size);
 
diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index d2a4c75..b269682 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -2212,16 +2212,11 @@ check_scsi_status:
 out:
if (logit)
ql_dbg(ql_dbg_io, fcport->vha, 0x3022,
-   "FCP command status: 0x%x-0x%x (0x%x) "
-   "nexus=%ld:%d:%d portid=%02x%02x%02x oxid=0x%x "
-   "cdb=%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x len=0x%x "
-   "rsp_info=0x%x resid=0x%x fw_resid=0x%x.\n",
+   "FCP command status: 0x%x-0x%x (0x%x) nexus=%ld:%d:%d 
portid=%02x%02x%02x oxid=0x%x cdb=%10phN len=0x%x rsp_info=0x%x resid=0x%x 
fw_resid=0x%x.\n",
comp_status, scsi_status, res, vha->host_no,
cp->device->id, cp->device->lun, fcport->d_id.b.domain,
fcport->d_id.b.area, fcport->d_id.b.al_pa, ox_id,
-   cp->cmnd[0], cp->cmnd[1], cp->cmnd[2], cp->cmnd[3],
-   cp->cmnd[4], cp->cmnd[5], cp->cmnd[6], cp->cmnd[7],
-   cp->cmnd[8], cp->cmnd[9], scsi_bufflen(cp), rsp_info_len,
+   cp->cmnd, scsi_bufflen(cp), rsp_info_len,
resid_len, fw_resid_len);
 
if (!res)
diff --git a/drivers/scsi/qla2xxx/qla_mr.c b/drivers/scsi/qla2xxx/qla_mr.c
index a6df558..8c3ffcc 100644
--- a/drivers/scsi/qla2xxx/qla_mr.c
+++ b/drivers/scsi/qla2xxx/qla_mr.c
@@ -2517,16 +2517,10 @@ check_scsi_status:
 
if (logit)
ql_dbg(ql_dbg_io, fcport->vha, 0x3058,
-   "FCP command status: 0x%x-0x%x (0x%x) "
-   "nexus=%ld:%d:%d tgt_id: 0x%x lscsi_status: 0x%x"
-   "cdb=%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x len=0x%x "
-   "rsp_info=0x%x resid=0x%x fw_resid=0x%x "
-   "sense_len=0x%x, par_sense_len=0x%x, rsp_info_len=0x%x\n",
+   "FCP command status: 0x%x-0x%x (0x%x) nexus=%ld:%d:%d 
tgt_id: 0x%x lscsi_status: 0x%x cdb=%10phN len=0x%x rsp_info=0x%x resid=0x%x 
fw_resid=0x%x sense_len=0x%x, par_sense_len=0x%x, rsp_info_len=0x%x\n",
comp_status, scsi_status, res, vha->host_no,
cp->device->id, cp->device->lun, fcport->tgt_id,
-   lscsi_status, cp->cmnd[0], cp->cmnd[1], cp->cmnd[2],
-   cp->cmnd[3], cp->cmnd[4], cp->cmnd[5], cp->cmnd[6],
-   cp->cmnd[7], cp->cmnd[8], cp->cmnd[9], scsi_bufflen(cp),
+   lscsi_status, cp->cmnd, scsi_bufflen(cp),
rsp_info_len, resid_len, fw_resid_len, sense_len,
par_sense_len, rsp_info_len);
 
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 3/6] scsi_debug: fix NULL pointer dereference with parameters dif=0 dix=1

2013-06-29 Thread Akinobu Mita
The protection info dif_storep is allocated only when parameter dif is
not zero.  But it will be accessed when reading or writing to the storage
installed with parameter dix is not zero.

So kernel crashes if scsi_debug module is loaded with parameters dix=1 and
dif=0.

This fixes it by making dif_storep available if parameter dix is not zero
instead of checking if parameter dif is not zero.

Signed-off-by: Akinobu Mita 
Acked-by: Douglas Gilbert 
Acked-by: "Martin K. Petersen" 
Cc: "James E.J. Bottomley" 
Cc: Douglas Gilbert 
Cc: "Martin K. Petersen" 
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi_debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index bcf73e4..e83e661 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -3372,7 +3372,7 @@ static int __init scsi_debug_init(void)
if (scsi_debug_num_parts > 0)
sdebug_build_parts(fake_storep, sz);
 
-   if (scsi_debug_dif) {
+   if (scsi_debug_dix) {
int dif_size;
 
dif_size = sdebug_store_sectors * sizeof(struct sd_dif_tuple);
-- 
1.8.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 4/6] scsi_debug: invalidate protection info for unmapped region

2013-06-29 Thread Akinobu Mita
When UNMAP command is issued with the data integrity support enabled,
the protection info for the unmapped region is remain unchanged.
So READ command for the region later on causes data integrity failure.

This fixes it by invalidating protection info for the unmapped region
by filling with 0xff pattern.

Signed-off-by: Akinobu Mita 
Acked-by: Douglas Gilbert 
Acked-by: "Martin K. Petersen" 
Cc: "James E.J. Bottomley" 
Cc: Douglas Gilbert 
Cc: "Martin K. Petersen" 
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi_debug.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index e83e661..83efec2 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -2064,6 +2064,11 @@ static void unmap_region(sector_t lba, unsigned int len)
   scsi_debug_sector_size *
   scsi_debug_unmap_granularity);
}
+   if (dif_storep) {
+   memset(dif_storep + lba, 0xff,
+  sizeof(*dif_storep) *
+  scsi_debug_unmap_granularity);
+   }
}
lba = map_index_to_lba(index + 1);
}
-- 
1.8.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 2/6] scsi_debug: fix incorrectly nested kmap_atomic()

2013-06-29 Thread Akinobu Mita
In the function prot_verify_write(), kmap_atomic()/kunmap_atomic() for
data page and kmap_atomic()/kunmap_atomic() for protection information
page are not nested each other.

It worked perfectly before commit 3e4d3af501cccdc8a8cca41bdbe57d54ad7e7e73
("mm: stack based kmap_atomic()").  Because the kmap_atomic slot KM_IRQ0
was used for data page and the slot KM_IRQ1 was used for protection page.

But KM_types are gone and kmap_atomic() is using stack based implementation.
So two different kmap_atomic() usages must be strictly nested now.

This change ensures kmap_atomic() usage is strictly nested.

Signed-off-by: Akinobu Mita 
Acked-by: Douglas Gilbert 
Acked-by: "Martin K. Petersen" 
Cc: "James E.J. Bottomley" 
Cc: Douglas Gilbert 
Cc: "Martin K. Petersen" 
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi_debug.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index d51bddd..bcf73e4 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1891,12 +1891,12 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
BUG_ON(scsi_sg_count(SCpnt) == 0);
BUG_ON(scsi_prot_sg_count(SCpnt) == 0);
 
-   paddr = kmap_atomic(sg_page(psgl)) + psgl->offset;
ppage_offset = 0;
 
/* For each data page */
scsi_for_each_sg(SCpnt, dsgl, scsi_sg_count(SCpnt), i) {
daddr = kmap_atomic(sg_page(dsgl)) + dsgl->offset;
+   paddr = kmap_atomic(sg_page(psgl)) + psgl->offset;
 
/* For each sector-sized chunk in data page */
for (j = 0; j < dsgl->length; j += scsi_debug_sector_size) {
@@ -1980,19 +1980,18 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
ppage_offset += sizeof(struct sd_dif_tuple);
}
 
+   kunmap_atomic(paddr);
kunmap_atomic(daddr);
}
 
-   kunmap_atomic(paddr);
-
dix_writes++;
 
return 0;
 
 out:
dif_errors++;
-   kunmap_atomic(daddr);
kunmap_atomic(paddr);
+   kunmap_atomic(daddr);
return ret;
 }
 
-- 
1.8.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 0/6] scsi_debug: bug fixes and cleanups for data integrity support

2013-06-29 Thread Akinobu Mita
Hi James,

Please consider to apply these patches to your tree.  The only difference
between v3 and v4 is adding Acked-by from Douglas and Martin.

This patch set includes bug fixes which I hit when I was tried testing
the data integrity support in scsi_debug on x86_32.

And it also includes cleanups which helps increasing readability and
further bug fixing in data integrity support.

* Changes from v3
- Add Acked-by lines

* Changes from v2
- Add new bug fix patch for UNMAP command support
- Change the way to fix for the patch "fix invalid address passed to
  kunmap_atomic()"
- Reduce more lines of code for the patch "reduce duplication between
  prot_verify_read and prot_verify_writ"

* Changes from v1
- Split the patch "fix data integrity support on highmem machine" into
  two separate patches.
- Add new cleanup patch "reduce duplication between prot_verify_read and
  prot_verify_write".

Cc: "James E.J. Bottomley" 
Cc: Douglas Gilbert 
Cc: "Martin K. Petersen" 
Cc: linux-scsi@vger.kernel.org

Akinobu Mita (6):
  scsi_debug: fix invalid address passed to kunmap_atomic()
  scsi_debug: fix incorrectly nested kmap_atomic()
  scsi_debug: fix NULL pointer dereference with parameters dif=0 dix=1
  scsi_debug: invalidate protection info for unmapped region
  scsi_debug: simplify offset calculation for dif_storep
  scsi_debug: reduce duplication between prot_verify_read and
prot_verify_write

 drivers/scsi/scsi_debug.c | 176 +++---
 1 file changed, 72 insertions(+), 104 deletions(-)

-- 
1.8.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 5/6] scsi_debug: simplify offset calculation for dif_storep

2013-06-29 Thread Akinobu Mita
dif_storep is declared as pointer to unsigned char type.  But it is
actually used to store vmalloced array of struct sd_dif_tuple.

This changes the type of dif_storep to the pointer to struct sd_dif_tuple.
It simplifies offset calculation for dif_storep and enables to remove
hardcoded size of struct sd_dif_tuple.

Signed-off-by: Akinobu Mita 
Acked-by: Douglas Gilbert 
Acked-by: "Martin K. Petersen" 
Cc: "James E.J. Bottomley" 
Cc: Douglas Gilbert 
Cc: "Martin K. Petersen" 
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi_debug.c | 18 +++---
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 83efec2..fc8b3aa 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -258,7 +258,7 @@ struct sdebug_queued_cmd {
 static struct sdebug_queued_cmd queued_arr[SCSI_DEBUG_CANQUEUE];
 
 static unsigned char * fake_storep;/* ramdisk storage */
-static unsigned char *dif_storep;  /* protection info */
+static struct sd_dif_tuple *dif_storep;/* protection info */
 static void *map_storep;   /* provisioning map */
 
 static unsigned long map_size;
@@ -277,11 +277,6 @@ static char sdebug_proc_name[] = "scsi_debug";
 
 static struct bus_type pseudo_lld_bus;
 
-static inline sector_t dif_offset(sector_t sector)
-{
-   return sector << 3;
-}
-
 static struct device_driver sdebug_driverfs_driver = {
.name   = sdebug_proc_name,
.bus= &pseudo_lld_bus,
@@ -1727,7 +1722,7 @@ static int prot_verify_read(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
 
start_sec = do_div(tmp_sec, sdebug_store_sectors);
 
-   sdt = (struct sd_dif_tuple *)(dif_storep + dif_offset(start_sec));
+   sdt = dif_storep + start_sec;
 
for (i = 0 ; i < sectors ; i++) {
u16 csum;
@@ -1782,16 +1777,17 @@ static int prot_verify_read(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
ei_lba++;
}
 
-   resid = sectors * 8; /* Bytes of protection data to copy into sgl */
+   /* Bytes of protection data to copy into sgl */
+   resid = sectors * sizeof(*dif_storep);
sector = start_sec;
 
scsi_for_each_prot_sg(SCpnt, psgl, scsi_prot_sg_count(SCpnt), i) {
int len = min(psgl->length, resid);
 
paddr = kmap_atomic(sg_page(psgl)) + psgl->offset;
-   memcpy(paddr, dif_storep + dif_offset(sector), len);
+   memcpy(paddr, dif_storep + sector, len);
 
-   sector += len >> 3;
+   sector += len / sizeof(*dif_storep);
if (sector >= sdebug_store_sectors) {
/* Force wrap */
tmp_sec = sector;
@@ -1968,7 +1964,7 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
 * correctness we need to verify each sector
 * before writing it to "stable" storage
 */
-   memcpy(dif_storep + dif_offset(sector), sdt, 8);
+   memcpy(dif_storep + sector, sdt, sizeof(*sdt));
 
sector++;
 
-- 
1.8.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 6/6] scsi_debug: reduce duplication between prot_verify_read and prot_verify_write

2013-06-29 Thread Akinobu Mita
In order to reduce code duplication between prot_verify_read() and
prot_verify_write(), this moves common code into the new functions.

Signed-off-by: Akinobu Mita 
Acked-by: Douglas Gilbert 
Acked-by: "Martin K. Petersen" 
Cc: "James E.J. Bottomley" 
Cc: Douglas Gilbert 
Cc: "Martin K. Petersen" 
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi_debug.c | 139 ++
 1 file changed, 54 insertions(+), 85 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index fc8b3aa..9790d56 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1710,6 +1710,52 @@ static int do_device_access(struct scsi_cmnd *scmd,
return ret;
 }
 
+static u16 dif_compute_csum(const void *buf, int len)
+{
+   u16 csum;
+
+   switch (scsi_debug_guard) {
+   case 1:
+   csum = ip_compute_csum(buf, len);
+   break;
+   case 0:
+   csum = cpu_to_be16(crc_t10dif(buf, len));
+   break;
+   default:
+   BUG();
+   }
+   return csum;
+}
+
+static int dif_verify(struct sd_dif_tuple *sdt, const void *data,
+ sector_t sector, u32 ei_lba)
+{
+   u16 csum = dif_compute_csum(data, scsi_debug_sector_size);
+
+   if (sdt->guard_tag != csum) {
+   pr_err("%s: GUARD check failed on sector %lu rcvd 0x%04x, data 
0x%04x\n",
+   __func__,
+   (unsigned long)sector,
+   be16_to_cpu(sdt->guard_tag),
+   be16_to_cpu(csum));
+   return 0x01;
+   }
+   if (scsi_debug_dif == SD_DIF_TYPE1_PROTECTION &&
+   be32_to_cpu(sdt->ref_tag) != (sector & 0x)) {
+   pr_err("%s: REF check failed on sector %lu\n",
+   __func__, (unsigned long)sector);
+   return 0x03;
+   }
+   if (scsi_debug_dif == SD_DIF_TYPE2_PROTECTION &&
+   be32_to_cpu(sdt->ref_tag) != ei_lba) {
+   pr_err("%s: REF check failed on sector %lu\n",
+   __func__, (unsigned long)sector);
+   dif_errors++;
+   return 0x03;
+   }
+   return 0;
+}
+
 static int prot_verify_read(struct scsi_cmnd *SCpnt, sector_t start_sec,
unsigned int sectors, u32 ei_lba)
 {
@@ -1725,53 +1771,19 @@ static int prot_verify_read(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
sdt = dif_storep + start_sec;
 
for (i = 0 ; i < sectors ; i++) {
-   u16 csum;
+   int ret;
 
if (sdt[i].app_tag == 0x)
continue;
 
sector = start_sec + i;
 
-   switch (scsi_debug_guard) {
-   case 1:
-   csum = ip_compute_csum(fake_storep +
-  sector * scsi_debug_sector_size,
-  scsi_debug_sector_size);
-   break;
-   case 0:
-   csum = crc_t10dif(fake_storep +
- sector * scsi_debug_sector_size,
- scsi_debug_sector_size);
-   csum = cpu_to_be16(csum);
-   break;
-   default:
-   BUG();
-   }
-
-   if (sdt[i].guard_tag != csum) {
-   printk(KERN_ERR "%s: GUARD check failed on sector %lu" \
-  " rcvd 0x%04x, data 0x%04x\n", __func__,
-  (unsigned long)sector,
-  be16_to_cpu(sdt[i].guard_tag),
-  be16_to_cpu(csum));
+   ret = dif_verify(&sdt[i],
+fake_storep + sector * scsi_debug_sector_size,
+sector, ei_lba);
+   if (ret) {
dif_errors++;
-   return 0x01;
-   }
-
-   if (scsi_debug_dif == SD_DIF_TYPE1_PROTECTION &&
-   be32_to_cpu(sdt[i].ref_tag) != (sector & 0x)) {
-   printk(KERN_ERR "%s: REF check failed on sector %lu\n",
-  __func__, (unsigned long)sector);
-   dif_errors++;
-   return 0x03;
-   }
-
-   if (scsi_debug_dif == SD_DIF_TYPE2_PROTECTION &&
-   be32_to_cpu(sdt[i].ref_tag) != ei_lba) {
-   printk(KERN_ERR "%s: REF check failed on sector %lu\n",
-  __func__, (unsigned long)sector);
-   dif_errors++;
-   return 0x03;
+   return ret;
}
 
ei_lba++;
@@ -1880,7 +1892,6 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t star

[PATCH v4 1/6] scsi_debug: fix invalid address passed to kunmap_atomic()

2013-06-29 Thread Akinobu Mita
In the function prot_verify_write(), the kmap address 'daddr' is
incremented in the loop for each data page.  Finally 'daddr' reaches
the next page boundary in the end of the loop, and the invalid address
is passed to kunmap_atomic().

Fix the issue by not incrementing 'daddr' in the loop and offsetting it
by the loop counter on demand.

Signed-off-by: Akinobu Mita 
Acked-by: Douglas Gilbert 
Acked-by: "Martin K. Petersen" 
Cc: "James E.J. Bottomley" 
Cc: Douglas Gilbert 
Cc: "Martin K. Petersen" 
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/scsi_debug.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 0a537a0..d51bddd 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1899,7 +1899,7 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
daddr = kmap_atomic(sg_page(dsgl)) + dsgl->offset;
 
/* For each sector-sized chunk in data page */
-   for (j = 0 ; j < dsgl->length ; j += scsi_debug_sector_size) {
+   for (j = 0; j < dsgl->length; j += scsi_debug_sector_size) {
 
/* If we're at the end of the current
 * protection page advance to the next one
@@ -1917,11 +1917,11 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
 
switch (scsi_debug_guard) {
case 1:
-   csum = ip_compute_csum(daddr,
+   csum = ip_compute_csum(daddr + j,
   scsi_debug_sector_size);
break;
case 0:
-   csum = cpu_to_be16(crc_t10dif(daddr,
+   csum = cpu_to_be16(crc_t10dif(daddr + j,
  scsi_debug_sector_size));
break;
default:
@@ -1938,7 +1938,7 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
   be16_to_cpu(sdt->guard_tag),
   be16_to_cpu(csum));
ret = 0x01;
-   dump_sector(daddr, scsi_debug_sector_size);
+   dump_sector(daddr + j, scsi_debug_sector_size);
goto out;
}
 
@@ -1949,7 +1949,7 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
   "%s: REF check failed on sector %lu\n",
   __func__, (unsigned long)sector);
ret = 0x03;
-   dump_sector(daddr, scsi_debug_sector_size);
+   dump_sector(daddr + j, scsi_debug_sector_size);
goto out;
}
 
@@ -1959,7 +1959,7 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
   "%s: REF check failed on sector %lu\n",
   __func__, (unsigned long)sector);
ret = 0x03;
-   dump_sector(daddr, scsi_debug_sector_size);
+   dump_sector(daddr + j, scsi_debug_sector_size);
goto out;
}
 
@@ -1977,7 +1977,6 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, 
sector_t start_sec,
 
start_sec++;
ei_lba++;
-   daddr += scsi_debug_sector_size;
ppage_offset += sizeof(struct sd_dif_tuple);
}
 
-- 
1.8.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html