From 518d9df87105a078984c90c75cf6e7f67e3c928c Mon Sep 17 00:00:00 2001 From: Akinobu Mita Date: Sat, 29 Jun 2013 17:59:14 +0900 Subject: [SCSI] scsi_debug: fix invalid address passed to kunmap_atomic() 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" Signed-off-by: James Bottomley --- drivers/scsi/scsi_debug.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'drivers/scsi/scsi_debug.c') diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index 0a537a0515ca..d51bddde5b1f 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); } -- cgit v1.2.3 From fc3fc352b6e7a6452596f8a13cedeaa6bcfe9689 Mon Sep 17 00:00:00 2001 From: Akinobu Mita Date: Sat, 29 Jun 2013 17:59:15 +0900 Subject: [SCSI] scsi_debug: fix incorrectly nested kmap_atomic() 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" Signed-off-by: James Bottomley --- drivers/scsi/scsi_debug.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'drivers/scsi/scsi_debug.c') diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index d51bddde5b1f..bcf73e44d2fb 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; } -- cgit v1.2.3 From 7cb69d0397233546d82191e524235fdb9d1d91aa Mon Sep 17 00:00:00 2001 From: Akinobu Mita Date: Sat, 29 Jun 2013 17:59:16 +0900 Subject: [SCSI] scsi_debug: fix NULL pointer dereference with parameters dif=0 dix=1 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" Signed-off-by: James Bottomley --- drivers/scsi/scsi_debug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/scsi/scsi_debug.c') diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index bcf73e44d2fb..e83e661b0257 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); -- cgit v1.2.3 From e9926b4376544d5a2dc4d310d4d0006c634b1a93 Mon Sep 17 00:00:00 2001 From: Akinobu Mita Date: Sat, 29 Jun 2013 17:59:17 +0900 Subject: [SCSI] scsi_debug: invalidate protection info for unmapped region 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" Signed-off-by: James Bottomley --- drivers/scsi/scsi_debug.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'drivers/scsi/scsi_debug.c') diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index e83e661b0257..83efec2919b1 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); } -- cgit v1.2.3 From e18d8bea33077d259cc826c6d3fa76d36af61876 Mon Sep 17 00:00:00 2001 From: Akinobu Mita Date: Sat, 29 Jun 2013 17:59:18 +0900 Subject: [SCSI] scsi_debug: simplify offset calculation for dif_storep 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" Signed-off-by: James Bottomley --- drivers/scsi/scsi_debug.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) (limited to 'drivers/scsi/scsi_debug.c') diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index 83efec2919b1..fc8b3aa083d4 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++; -- cgit v1.2.3 From beb40ea42bd65511bc275a7d58d3753835906be5 Mon Sep 17 00:00:00 2001 From: Akinobu Mita Date: Sat, 29 Jun 2013 17:59:19 +0900 Subject: [SCSI] scsi_debug: reduce duplication between prot_verify_read and prot_verify_write In order to reduce code duplication between prot_verify_read() and prot_verify_write(), this moves common code into the new functions. [jejb: fix unitialised variable warning] Signed-off-by: Akinobu Mita Acked-by: Douglas Gilbert Acked-by: "Martin K. Petersen" Signed-off-by: James Bottomley --- drivers/scsi/scsi_debug.c | 137 ++++++++++++++++++---------------------------- 1 file changed, 52 insertions(+), 85 deletions(-) (limited to 'drivers/scsi/scsi_debug.c') diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index fc8b3aa083d4..3f096a6681ac 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -1710,6 +1710,50 @@ 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; + } + 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 & 0xffffffff)) { + 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 +1769,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 == 0xffff) 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)); - dif_errors++; - return 0x01; - } - - if (scsi_debug_dif == SD_DIF_TYPE1_PROTECTION && - be32_to_cpu(sdt[i].ref_tag) != (sector & 0xffffffff)) { - 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); + ret = dif_verify(&sdt[i], + fake_storep + sector * scsi_debug_sector_size, + sector, ei_lba); + if (ret) { dif_errors++; - return 0x03; + return ret; } ei_lba++; @@ -1880,7 +1890,6 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, sector_t start_sec, sector_t tmp_sec = start_sec; sector_t sector; int ppage_offset; - unsigned short csum; sector = do_div(tmp_sec, sdebug_store_sectors); @@ -1911,50 +1920,8 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, sector_t start_sec, sdt = paddr + ppage_offset; - switch (scsi_debug_guard) { - case 1: - csum = ip_compute_csum(daddr + j, - scsi_debug_sector_size); - break; - case 0: - csum = cpu_to_be16(crc_t10dif(daddr + j, - scsi_debug_sector_size)); - break; - default: - BUG(); - ret = 0; - goto out; - } - - if (sdt->guard_tag != csum) { - printk(KERN_ERR - "%s: GUARD check failed on sector %lu " \ - "rcvd 0x%04x, calculated 0x%04x\n", - __func__, (unsigned long)sector, - be16_to_cpu(sdt->guard_tag), - be16_to_cpu(csum)); - ret = 0x01; - dump_sector(daddr + j, scsi_debug_sector_size); - goto out; - } - - if (scsi_debug_dif == SD_DIF_TYPE1_PROTECTION && - be32_to_cpu(sdt->ref_tag) - != (start_sec & 0xffffffff)) { - printk(KERN_ERR - "%s: REF check failed on sector %lu\n", - __func__, (unsigned long)sector); - ret = 0x03; - dump_sector(daddr + j, scsi_debug_sector_size); - goto out; - } - - if (scsi_debug_dif == SD_DIF_TYPE2_PROTECTION && - be32_to_cpu(sdt->ref_tag) != ei_lba) { - printk(KERN_ERR - "%s: REF check failed on sector %lu\n", - __func__, (unsigned long)sector); - ret = 0x03; + ret = dif_verify(sdt, daddr + j, start_sec, ei_lba); + if (ret) { dump_sector(daddr + j, scsi_debug_sector_size); goto out; } -- cgit v1.2.3