On 2019-01-23 5:56 p.m., Bart Van Assche wrote:
On Fri, 2019-01-11 at 13:01 -0500, Douglas Gilbert wrote:
On 2019-01-10 6:22 p.m., Bart Van Assche wrote:
Hi Doug,
Have you ever tried to run the libiscsi conformance tests against
the scsi_debug driver? I tried the following:
modprobe scsi_debug delay=0 max_luns=3
dev=$(for f in /sys/bus/pseudo/drivers/scsi_debug/adapter*/host*/target*/[0-9]*/block/*; do echo $f; break; done)
dev=/dev/$(basename $dev)
libiscsi/test-tool/iscsi-test-cu --dataloss --allow-sanitize "$dev"
That test triggers the following output:
BUG: unable to handle kernel paging request at ffffa8d741235e00
PGD 13b141067 P4D 13b141067 PUD 13b146067 PMD 6fc5a067 PTE 0
Oops: 0002 [#1] SMP PTI
CPU: 3 PID: 4967 Comm: iscsi-test-cu Not tainted 4.18.0-13-generic #14-Ubuntu
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
RIP: 0010:memcpy_erms+0x6/0x10
Since memory corruption errors have been found elsewhere in
lk 5.0-rc1 and a fix looks like it is pending, I will leave this
one alone as I can't replicate it.
Hi Doug,
I can replicate this crash easily. I also noticed that this crash only occurs if
the scsi_debug driver is loaded with fake_rw=0. It does not occur with fake_rw=1.
It seems like the following code in resp_write_same() assumes that fake_storep != NULL?
/* if ndob then zero 1 logical block, else fetch 1 logical block */
if (ndob) {
memset(fake_storep + lba_off, 0, sdebug_sector_size);
ret = 0;
} else
ret = fetch_to_dev_buffer(scp, fake_storep + lba_off,
sdebug_sector_size);
It is table driven. It shouldn't call that function if FF_MEDIA_IO is part of
that command's flag and fake_storep is NULL. Both WS10 and WS16 have that flag.
But there is a problem if virtual_gb > 0 .
Could you try the attached patch, it should wrap cleanly in the virtual_gb > 0
case.
Doug Gilbert
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 661512bec3ac..b190277d945c 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -735,7 +735,7 @@ static inline bool scsi_debug_lbp(void)
(sdebug_lbpu || sdebug_lbpws || sdebug_lbpws10);
}
-static void *fake_store(unsigned long long lba)
+static void *lba2fake_store(unsigned long long lba)
{
lba = do_div(lba, sdebug_store_sectors);
@@ -2514,8 +2514,8 @@ static int do_device_access(struct scsi_cmnd *scmd, u32 sg_skip, u64 lba,
return ret;
}
-/* If fake_store(lba,num) compares equal to arr(num), then copy top half of
- * arr into fake_store(lba,num) and return true. If comparison fails then
+/* If lba2fake_store(lba,num) compares equal to arr(num), then copy top half of
+ * arr into lba2fake_store(lba,num) and return true. If comparison fails then
* return false. */
static bool comp_write_worker(u64 lba, u32 num, const u8 *arr)
{
@@ -2643,7 +2643,7 @@ static int prot_verify_read(struct scsi_cmnd *SCpnt, sector_t start_sec,
if (sdt->app_tag == cpu_to_be16(0xffff))
continue;
- ret = dif_verify(sdt, fake_store(sector), sector, ei_lba);
+ ret = dif_verify(sdt, lba2fake_store(sector), sector, ei_lba);
if (ret) {
dif_errors++;
return ret;
@@ -3261,10 +3261,12 @@ static int resp_write_scat(struct scsi_cmnd *scp,
static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num,
u32 ei_lba, bool unmap, bool ndob)
{
+ int ret;
unsigned long iflags;
unsigned long long i;
- int ret;
- u64 lba_off;
+ u32 lb_size = sdebug_sector_size;
+ u64 block, lbaa;
+ u8 *fs1p;
ret = check_device_access_params(scp, lba, num);
if (ret)
@@ -3276,31 +3278,30 @@ static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num,
unmap_region(lba, num);
goto out;
}
-
- lba_off = lba * sdebug_sector_size;
+ lbaa = lba;
+ block = do_div(lbaa, sdebug_store_sectors);
/* if ndob then zero 1 logical block, else fetch 1 logical block */
+ fs1p = fake_storep + (block * lb_size);
if (ndob) {
- memset(fake_storep + lba_off, 0, sdebug_sector_size);
+ memset(fs1p, 0, lb_size);
ret = 0;
} else
- ret = fetch_to_dev_buffer(scp, fake_storep + lba_off,
- sdebug_sector_size);
+ ret = fetch_to_dev_buffer(scp, fs1p, lb_size);
if (-1 == ret) {
write_unlock_irqrestore(&atomic_rw, iflags);
return DID_ERROR << 16;
- } else if (sdebug_verbose && !ndob && (ret < sdebug_sector_size))
+ } else if (sdebug_verbose && !ndob && (ret < lb_size))
sdev_printk(KERN_INFO, scp->device,
"%s: %s: lb size=%u, IO sent=%d bytes\n",
- my_name, "write same",
- sdebug_sector_size, ret);
+ my_name, "write same", lb_size, ret);
/* Copy first sector to remaining blocks */
- for (i = 1 ; i < num ; i++)
- memcpy(fake_storep + ((lba + i) * sdebug_sector_size),
- fake_storep + lba_off,
- sdebug_sector_size);
-
+ for (i = 1 ; i < num ; i++) {
+ lbaa = lba + i;
+ block = do_div(lbaa, sdebug_store_sectors);
+ memcpy(fake_storep + (block * lb_size), fs1p, lb_size);
+ }
if (scsi_debug_lbp())
map_region(lba, num);
out:
