* [PATCH 1/5] scsi_debug: fix buffer overrun when DIF/DIX is enabled and virtual_gb > 0
2013-09-18 12:27 [PATCH 0/5] scsi_debug: several fixes related to data integrity support Akinobu Mita
@ 2013-09-18 12:27 ` Akinobu Mita
2013-09-20 22:13 ` Martin K. Petersen
2013-09-18 12:27 ` [PATCH 2/5] scsi_debug: factor out copying PI from dif_storep to prot_sglist Akinobu Mita
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Akinobu Mita @ 2013-09-18 12:27 UTC (permalink / raw)
To: linux-scsi
Cc: Akinobu Mita, James E.J. Bottomley, Douglas Gilbert,
Martin K. Petersen
If the module parameter virtual_gb is greater than 0, the READ command
may request the blocks which exceed actual ramdisk storage (fake_storep).
prot_verify_read() should treat those blocks as wrap around the end of
fake_storep. But it actually causes fake_storep and dif_storep buffer
overruns.
This fixes these buffer overruns. In order to simplify the fix,
this also introduces fake_store() and dif_store() which return
corresponding wrap around addresses.
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: Douglas Gilbert <dgilbert@interlog.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: linux-scsi@vger.kernel.org
---
drivers/scsi/scsi_debug.c | 48 +++++++++++++++++++++++++++++------------------
1 file changed, 30 insertions(+), 18 deletions(-)
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 01c0ffa..f640b6b 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -293,6 +293,20 @@ static unsigned char ctrl_m_pg[] = {0xa, 10, 2, 0, 0, 0, 0, 0,
static unsigned char iec_m_pg[] = {0x1c, 0xa, 0x08, 0, 0, 0, 0, 0,
0, 0, 0x0, 0x0};
+static void *fake_store(unsigned long long lba)
+{
+ lba = do_div(lba, sdebug_store_sectors);
+
+ return fake_storep + lba * scsi_debug_sector_size;
+}
+
+static struct sd_dif_tuple *dif_store(sector_t sector)
+{
+ sector = do_div(sector, sdebug_store_sectors);
+
+ return dif_storep + sector;
+}
+
static int sdebug_add_adapter(void);
static void sdebug_remove_adapter(void);
@@ -1782,24 +1796,19 @@ static int prot_verify_read(struct scsi_cmnd *SCpnt, sector_t start_sec,
struct scatterlist *psgl;
struct sd_dif_tuple *sdt;
sector_t sector;
- sector_t tmp_sec = start_sec;
void *paddr;
+ const void *dif_store_end = dif_storep + sdebug_store_sectors;
- start_sec = do_div(tmp_sec, sdebug_store_sectors);
-
- sdt = dif_storep + start_sec;
-
- for (i = 0 ; i < sectors ; i++) {
+ for (i = 0; i < sectors; i++) {
int ret;
- if (sdt[i].app_tag == 0xffff)
- continue;
-
sector = start_sec + i;
+ sdt = dif_store(sector);
- ret = dif_verify(&sdt[i],
- fake_storep + sector * scsi_debug_sector_size,
- sector, ei_lba);
+ if (sdt->app_tag == 0xffff)
+ continue;
+
+ ret = dif_verify(sdt, fake_store(sector), sector, ei_lba);
if (ret) {
dif_errors++;
return ret;
@@ -1814,16 +1823,19 @@ static int prot_verify_read(struct scsi_cmnd *SCpnt, sector_t start_sec,
scsi_for_each_prot_sg(SCpnt, psgl, scsi_prot_sg_count(SCpnt), i) {
int len = min(psgl->length, resid);
+ void *start = dif_store(sector);
+ int rest = 0;
+
+ if (dif_store_end < start + len)
+ rest = start + len - dif_store_end;
paddr = kmap_atomic(sg_page(psgl)) + psgl->offset;
- memcpy(paddr, dif_storep + sector, len);
+ memcpy(paddr, start, len - rest);
+
+ if (rest)
+ memcpy(paddr + len - rest, dif_storep, rest);
sector += len / sizeof(*dif_storep);
- if (sector >= sdebug_store_sectors) {
- /* Force wrap */
- tmp_sec = sector;
- sector = do_div(tmp_sec, sdebug_store_sectors);
- }
resid -= len;
kunmap_atomic(paddr);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 1/5] scsi_debug: fix buffer overrun when DIF/DIX is enabled and virtual_gb > 0
2013-09-18 12:27 ` [PATCH 1/5] scsi_debug: fix buffer overrun when DIF/DIX is enabled and virtual_gb > 0 Akinobu Mita
@ 2013-09-20 22:13 ` Martin K. Petersen
0 siblings, 0 replies; 11+ messages in thread
From: Martin K. Petersen @ 2013-09-20 22:13 UTC (permalink / raw)
To: Akinobu Mita
Cc: linux-scsi, James E.J. Bottomley, Douglas Gilbert,
Martin K. Petersen
>>>>> "Akinobu" == Akinobu Mita <akinobu.mita@gmail.com> writes:
Akinobu> If the module parameter virtual_gb is greater than 0, the READ
Akinobu> command may request the blocks which exceed actual ramdisk
Akinobu> storage (fake_storep). prot_verify_read() should treat those
Akinobu> blocks as wrap around the end of fake_storep. But it actually
Akinobu> causes fake_storep and dif_storep buffer overruns.
Akinobu> This fixes these buffer overruns. In order to simplify the
Akinobu> fix, this also introduces fake_store() and dif_store() which
Akinobu> return corresponding wrap around addresses.
Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/5] scsi_debug: factor out copying PI from dif_storep to prot_sglist
2013-09-18 12:27 [PATCH 0/5] scsi_debug: several fixes related to data integrity support Akinobu Mita
2013-09-18 12:27 ` [PATCH 1/5] scsi_debug: fix buffer overrun when DIF/DIX is enabled and virtual_gb > 0 Akinobu Mita
@ 2013-09-18 12:27 ` Akinobu Mita
2013-09-20 22:20 ` Martin K. Petersen
2013-09-18 12:27 ` [PATCH 3/5] scsi_debug: avoid partial copying PI from prot_sglist to dif_storep Akinobu Mita
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Akinobu Mita @ 2013-09-18 12:27 UTC (permalink / raw)
To: linux-scsi
Cc: Akinobu Mita, James E.J. Bottomley, Douglas Gilbert,
Martin K. Petersen
If data integrity support is enabled, prot_verify_read() is called in
response to READ commands and it verifies protection info from dif_storep
by comparing against fake_storep, and copies protection info to
prot_sglist.
This factors out the portion of copying protection info into a separate
function. It will also be reused in the next change after supporting
the opposite direction (copying prot_sglist to dif_storep).
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: Douglas Gilbert <dgilbert@interlog.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: linux-scsi@vger.kernel.org
---
drivers/scsi/scsi_debug.c | 52 ++++++++++++++++++++++++++---------------------
1 file changed, 29 insertions(+), 23 deletions(-)
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index f640b6b..99e74d7 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1789,37 +1789,16 @@ static int dif_verify(struct sd_dif_tuple *sdt, const void *data,
return 0;
}
-static int prot_verify_read(struct scsi_cmnd *SCpnt, sector_t start_sec,
- unsigned int sectors, u32 ei_lba)
+static void dif_copy_prot(struct scsi_cmnd *SCpnt, sector_t sector,
+ unsigned int sectors)
{
unsigned int i, resid;
struct scatterlist *psgl;
- struct sd_dif_tuple *sdt;
- sector_t sector;
void *paddr;
const void *dif_store_end = dif_storep + sdebug_store_sectors;
- for (i = 0; i < sectors; i++) {
- int ret;
-
- sector = start_sec + i;
- sdt = dif_store(sector);
-
- if (sdt->app_tag == 0xffff)
- continue;
-
- ret = dif_verify(sdt, fake_store(sector), sector, ei_lba);
- if (ret) {
- dif_errors++;
- return ret;
- }
-
- ei_lba++;
- }
-
/* 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);
@@ -1839,7 +1818,34 @@ static int prot_verify_read(struct scsi_cmnd *SCpnt, sector_t start_sec,
resid -= len;
kunmap_atomic(paddr);
}
+}
+
+static int prot_verify_read(struct scsi_cmnd *SCpnt, sector_t start_sec,
+ unsigned int sectors, u32 ei_lba)
+{
+ unsigned int i;
+ struct sd_dif_tuple *sdt;
+ sector_t sector;
+
+ for (i = 0; i < sectors; i++) {
+ int ret;
+
+ sector = start_sec + i;
+ sdt = dif_store(sector);
+
+ if (sdt->app_tag == 0xffff)
+ continue;
+
+ ret = dif_verify(sdt, fake_store(sector), sector, ei_lba);
+ if (ret) {
+ dif_errors++;
+ return ret;
+ }
+
+ ei_lba++;
+ }
+ dif_copy_prot(SCpnt, start_sec, sectors);
dix_reads++;
return 0;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 2/5] scsi_debug: factor out copying PI from dif_storep to prot_sglist
2013-09-18 12:27 ` [PATCH 2/5] scsi_debug: factor out copying PI from dif_storep to prot_sglist Akinobu Mita
@ 2013-09-20 22:20 ` Martin K. Petersen
0 siblings, 0 replies; 11+ messages in thread
From: Martin K. Petersen @ 2013-09-20 22:20 UTC (permalink / raw)
To: Akinobu Mita
Cc: linux-scsi, James E.J. Bottomley, Douglas Gilbert,
Martin K. Petersen
>>>>> "Akinobu" == Akinobu Mita <akinobu.mita@gmail.com> writes:
Akinobu> If data integrity support is enabled, prot_verify_read() is
Akinobu> called in response to READ commands and it verifies protection
Akinobu> info from dif_storep by comparing against fake_storep, and
Akinobu> copies protection info to prot_sglist.
Akinobu> This factors out the portion of copying protection info into a
Akinobu> separate function. It will also be reused in the next change
Akinobu> after supporting the opposite direction (copying prot_sglist to
Akinobu> dif_storep).
Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/5] scsi_debug: avoid partial copying PI from prot_sglist to dif_storep
2013-09-18 12:27 [PATCH 0/5] scsi_debug: several fixes related to data integrity support Akinobu Mita
2013-09-18 12:27 ` [PATCH 1/5] scsi_debug: fix buffer overrun when DIF/DIX is enabled and virtual_gb > 0 Akinobu Mita
2013-09-18 12:27 ` [PATCH 2/5] scsi_debug: factor out copying PI from dif_storep to prot_sglist Akinobu Mita
@ 2013-09-18 12:27 ` Akinobu Mita
2013-09-20 22:23 ` Martin K. Petersen
2013-09-18 12:27 ` [PATCH 4/5] scsi_debug: fix invalid value check for guard module parameter Akinobu Mita
2013-09-18 12:27 ` [PATCH 5/5] scsi_debug: fix sparse warnings related to data integrity field Akinobu Mita
4 siblings, 1 reply; 11+ messages in thread
From: Akinobu Mita @ 2013-09-18 12:27 UTC (permalink / raw)
To: linux-scsi
Cc: Akinobu Mita, James E.J. Bottomley, Douglas Gilbert,
Martin K. Petersen
If data integrity support is enabled, prot_verify_write() is called in
response to WRITE commands and it verifies protection info from
prot_sglist by comparing against data sglist, and copies protection info
to dif_storep.
When multiple blocks are transfered by a WRITE command, it verifies and
copies these blocks one by one. So if it fails to verify protection
info in the middle of blocks, the actual data transfer to fake_storep
isn't proceeded at all although protection info for some blocks are
already copied to dif_storep. Therefore, it breaks the data integrity
between fake_storep and dif_storep.
This fixes it by ensuring that copying protection info to dif_storep is
done after all blocks are successfully verified. Reusing dif_copy_prot()
with supporting the opposite direction simplifies this fix.
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: Douglas Gilbert <dgilbert@interlog.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: linux-scsi@vger.kernel.org
---
drivers/scsi/scsi_debug.c | 40 +++++++++++++++++-----------------------
1 file changed, 17 insertions(+), 23 deletions(-)
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 99e74d7..43369e9 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1790,7 +1790,7 @@ static int dif_verify(struct sd_dif_tuple *sdt, const void *data,
}
static void dif_copy_prot(struct scsi_cmnd *SCpnt, sector_t sector,
- unsigned int sectors)
+ unsigned int sectors, bool read)
{
unsigned int i, resid;
struct scatterlist *psgl;
@@ -1809,10 +1809,18 @@ static void dif_copy_prot(struct scsi_cmnd *SCpnt, sector_t sector,
rest = start + len - dif_store_end;
paddr = kmap_atomic(sg_page(psgl)) + psgl->offset;
- memcpy(paddr, start, len - rest);
- if (rest)
- memcpy(paddr + len - rest, dif_storep, rest);
+ if (read)
+ memcpy(paddr, start, len - rest);
+ else
+ memcpy(start, paddr, len - rest);
+
+ if (rest) {
+ if (read)
+ memcpy(paddr + len - rest, dif_storep, rest);
+ else
+ memcpy(dif_storep, paddr + len - rest, rest);
+ }
sector += len / sizeof(*dif_storep);
resid -= len;
@@ -1845,7 +1853,7 @@ static int prot_verify_read(struct scsi_cmnd *SCpnt, sector_t start_sec,
ei_lba++;
}
- dif_copy_prot(SCpnt, start_sec, sectors);
+ dif_copy_prot(SCpnt, start_sec, sectors, true);
dix_reads++;
return 0;
@@ -1928,15 +1936,12 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, sector_t start_sec,
{
int i, j, ret;
struct sd_dif_tuple *sdt;
- struct scatterlist *dsgl = scsi_sglist(SCpnt);
+ struct scatterlist *dsgl;
struct scatterlist *psgl = scsi_prot_sglist(SCpnt);
void *daddr, *paddr;
- sector_t tmp_sec = start_sec;
- sector_t sector;
+ sector_t sector = start_sec;
int ppage_offset;
- sector = do_div(tmp_sec, sdebug_store_sectors);
-
BUG_ON(scsi_sg_count(SCpnt) == 0);
BUG_ON(scsi_prot_sg_count(SCpnt) == 0);
@@ -1964,25 +1969,13 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, sector_t start_sec,
sdt = paddr + ppage_offset;
- ret = dif_verify(sdt, daddr + j, start_sec, ei_lba);
+ ret = dif_verify(sdt, daddr + j, sector, ei_lba);
if (ret) {
dump_sector(daddr + j, scsi_debug_sector_size);
goto out;
}
- /* Would be great to copy this in bigger
- * chunks. However, for the sake of
- * correctness we need to verify each sector
- * before writing it to "stable" storage
- */
- memcpy(dif_storep + sector, sdt, sizeof(*sdt));
-
sector++;
-
- if (sector == sdebug_store_sectors)
- sector = 0; /* Force wrap */
-
- start_sec++;
ei_lba++;
ppage_offset += sizeof(struct sd_dif_tuple);
}
@@ -1991,6 +1984,7 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, sector_t start_sec,
kunmap_atomic(daddr);
}
+ dif_copy_prot(SCpnt, start_sec, sectors, false);
dix_writes++;
return 0;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 3/5] scsi_debug: avoid partial copying PI from prot_sglist to dif_storep
2013-09-18 12:27 ` [PATCH 3/5] scsi_debug: avoid partial copying PI from prot_sglist to dif_storep Akinobu Mita
@ 2013-09-20 22:23 ` Martin K. Petersen
0 siblings, 0 replies; 11+ messages in thread
From: Martin K. Petersen @ 2013-09-20 22:23 UTC (permalink / raw)
To: Akinobu Mita
Cc: linux-scsi, James E.J. Bottomley, Douglas Gilbert,
Martin K. Petersen
>>>>> "Akinobu" == Akinobu Mita <akinobu.mita@gmail.com> writes:
Akinobu> If data integrity support is enabled, prot_verify_write() is
Akinobu> called in response to WRITE commands and it verifies protection
Akinobu> info from prot_sglist by comparing against data sglist, and
Akinobu> copies protection info to dif_storep.
Akinobu> When multiple blocks are transfered by a WRITE command, it
Akinobu> verifies and copies these blocks one by one. So if it fails to
Akinobu> verify protection info in the middle of blocks, the actual data
Akinobu> transfer to fake_storep isn't proceeded at all although
Akinobu> protection info for some blocks are already copied to
Akinobu> dif_storep. Therefore, it breaks the data integrity between
Akinobu> fake_storep and dif_storep.
Akinobu> This fixes it by ensuring that copying protection info to
Akinobu> dif_storep is done after all blocks are successfully verified.
Akinobu> Reusing dif_copy_prot() with supporting the opposite direction
Akinobu> simplifies this fix.
Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/5] scsi_debug: fix invalid value check for guard module parameter
2013-09-18 12:27 [PATCH 0/5] scsi_debug: several fixes related to data integrity support Akinobu Mita
` (2 preceding siblings ...)
2013-09-18 12:27 ` [PATCH 3/5] scsi_debug: avoid partial copying PI from prot_sglist to dif_storep Akinobu Mita
@ 2013-09-18 12:27 ` Akinobu Mita
2013-09-20 22:23 ` Martin K. Petersen
2013-09-18 12:27 ` [PATCH 5/5] scsi_debug: fix sparse warnings related to data integrity field Akinobu Mita
4 siblings, 1 reply; 11+ messages in thread
From: Akinobu Mita @ 2013-09-18 12:27 UTC (permalink / raw)
To: linux-scsi
Cc: Akinobu Mita, Joe Perches, James E.J. Bottomley, Douglas Gilbert,
Martin K. Petersen
In the module initialization, invalid value for guard module parameter
is detected by the following check:
if (scsi_debug_guard > 1) {
printk(KERN_ERR "scsi_debug_init: guard must be 0 or 1\n");
return -EINVAL;
}
But this check isn't enough, because the type of scsi_debug_guard is
'int' and scsi_debug_guard could be a negative value.
This fixes it by changing the type of scsi_debug_guard to 'unsigned int'
instead of adding extra check for a negative value.
Reported-by: Joe Perches <joe@perches.com>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Joe Perches <joe@perches.com>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: Douglas Gilbert <dgilbert@interlog.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: linux-scsi@vger.kernel.org
---
drivers/scsi/scsi_debug.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 43369e9..a21322d 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -169,7 +169,7 @@ static int scsi_debug_dix = DEF_DIX;
static int scsi_debug_dsense = DEF_D_SENSE;
static int scsi_debug_every_nth = DEF_EVERY_NTH;
static int scsi_debug_fake_rw = DEF_FAKE_RW;
-static int scsi_debug_guard = DEF_GUARD;
+static unsigned int scsi_debug_guard = DEF_GUARD;
static int scsi_debug_lowest_aligned = DEF_LOWEST_ALIGNED;
static int scsi_debug_max_luns = DEF_MAX_LUNS;
static int scsi_debug_max_queue = SCSI_DEBUG_CANQUEUE;
@@ -2754,7 +2754,7 @@ module_param_named(dix, scsi_debug_dix, int, S_IRUGO);
module_param_named(dsense, scsi_debug_dsense, int, S_IRUGO | S_IWUSR);
module_param_named(every_nth, scsi_debug_every_nth, int, S_IRUGO | S_IWUSR);
module_param_named(fake_rw, scsi_debug_fake_rw, int, S_IRUGO | S_IWUSR);
-module_param_named(guard, scsi_debug_guard, int, S_IRUGO);
+module_param_named(guard, scsi_debug_guard, uint, S_IRUGO);
module_param_named(lbpu, scsi_debug_lbpu, int, S_IRUGO);
module_param_named(lbpws, scsi_debug_lbpws, int, S_IRUGO);
module_param_named(lbpws10, scsi_debug_lbpws10, int, S_IRUGO);
@@ -3184,7 +3184,7 @@ DRIVER_ATTR(dif, S_IRUGO, sdebug_dif_show, NULL);
static ssize_t sdebug_guard_show(struct device_driver *ddp, char *buf)
{
- return scnprintf(buf, PAGE_SIZE, "%d\n", scsi_debug_guard);
+ return scnprintf(buf, PAGE_SIZE, "%u\n", scsi_debug_guard);
}
DRIVER_ATTR(guard, S_IRUGO, sdebug_guard_show, NULL);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 4/5] scsi_debug: fix invalid value check for guard module parameter
2013-09-18 12:27 ` [PATCH 4/5] scsi_debug: fix invalid value check for guard module parameter Akinobu Mita
@ 2013-09-20 22:23 ` Martin K. Petersen
0 siblings, 0 replies; 11+ messages in thread
From: Martin K. Petersen @ 2013-09-20 22:23 UTC (permalink / raw)
To: Akinobu Mita
Cc: linux-scsi, Joe Perches, James E.J. Bottomley, Douglas Gilbert,
Martin K. Petersen
>>>>> "Akinobu" == Akinobu Mita <akinobu.mita@gmail.com> writes:
Akinobu> In the module initialization, invalid value for guard module
Akinobu> parameter is detected by the following check:
Akinobu> if (scsi_debug_guard > 1) {
Akinobu> printk(KERN_ERR "scsi_debug_init: guard must be
Akinobu> 0 or 1\n"); return -EINVAL;
Akinobu> }
Akinobu> But this check isn't enough, because the type of
Akinobu> scsi_debug_guard is 'int' and scsi_debug_guard could be a
Akinobu> negative value.
Akinobu> This fixes it by changing the type of scsi_debug_guard to
Akinobu> 'unsigned int' instead of adding extra check for a negative
Akinobu> value.
Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 5/5] scsi_debug: fix sparse warnings related to data integrity field
2013-09-18 12:27 [PATCH 0/5] scsi_debug: several fixes related to data integrity support Akinobu Mita
` (3 preceding siblings ...)
2013-09-18 12:27 ` [PATCH 4/5] scsi_debug: fix invalid value check for guard module parameter Akinobu Mita
@ 2013-09-18 12:27 ` Akinobu Mita
2013-09-20 22:24 ` Martin K. Petersen
4 siblings, 1 reply; 11+ messages in thread
From: Akinobu Mita @ 2013-09-18 12:27 UTC (permalink / raw)
To: linux-scsi
Cc: Akinobu Mita, Fengguang Wu, James E.J. Bottomley, Douglas Gilbert,
Martin K. Petersen
Each member in data integrity field tuple is big-endian. But the
endianness of the values being compared with these members are not
annotated. So this fixes these sparse warnings.
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Fengguang Wu <fengguang.wu@intel.com>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: Douglas Gilbert <dgilbert@interlog.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: linux-scsi@vger.kernel.org
---
drivers/scsi/scsi_debug.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index a21322d..80b8b10 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1745,25 +1745,22 @@ static int do_device_access(struct scsi_cmnd *scmd,
return ret;
}
-static u16 dif_compute_csum(const void *buf, int len)
+static __be16 dif_compute_csum(const void *buf, int len)
{
- u16 csum;
+ __be16 csum;
- switch (scsi_debug_guard) {
- case 1:
- csum = ip_compute_csum(buf, len);
- break;
- case 0:
+ if (scsi_debug_guard)
+ csum = (__force __be16)ip_compute_csum(buf, len);
+ else
csum = cpu_to_be16(crc_t10dif(buf, len));
- break;
- }
+
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);
+ __be16 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",
@@ -1841,7 +1838,7 @@ static int prot_verify_read(struct scsi_cmnd *SCpnt, sector_t start_sec,
sector = start_sec + i;
sdt = dif_store(sector);
- if (sdt->app_tag == 0xffff)
+ if (sdt->app_tag == cpu_to_be16(0xffff))
continue;
ret = dif_verify(sdt, fake_store(sector), sector, ei_lba);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 11+ messages in thread