diff mbox series

scsi_debug: Fix a recently introduced regression

Message ID 20190208212127.236791-1-bvanassche@acm.org (mailing list archive)
State Mainlined
Commit c208556ab3627b5f53144e9cbf739d6371d80543
Headers show
Series scsi_debug: Fix a recently introduced regression | expand

Commit Message

Bart Van Assche Feb. 8, 2019, 9:21 p.m. UTC
A recent commit removed an element from opcode_info_arr[] but did not
modify opcode_ind_arr[] nor was SDEB_I_XDWRITEREAD removed. Remove
SDEB_I_XDWRITEREAD and bring the two arrays again in sync. This patch
avoids that the following is reported:

BUG: KASAN: null-ptr-deref in scsi_debug_queuecommand+0x60f/0xc90 [scsi_debug]
Read of size 1 at addr 0000000000000001 by task iscsi-test-cu/683
CPU: 3 PID: 683 Comm: iscsi-test-cu Not tainted 5.0.0-rc5-dbg+ #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
Call Trace:
 dump_stack+0x86/0xca
 kasan_report.cold.3+0x5/0x3e
 __asan_load1+0x47/0x50
 scsi_debug_queuecommand+0x60f/0xc90 [scsi_debug]
 scsi_queue_rq+0xc17/0x12e0
 blk_mq_dispatch_rq_list+0x5fc/0xb10
 blk_mq_sched_dispatch_requests+0x2f7/0x300
 __blk_mq_run_hw_queue+0xd6/0x180
 __blk_mq_delay_run_hw_queue+0x25c/0x290
 blk_mq_run_hw_queue+0x119/0x1b0
 blk_mq_sched_insert_request+0x274/0x350
 blk_execute_rq_nowait+0x78/0x90
 blk_execute_rq+0xcc/0x140
 sg_io+0x30f/0x700
 scsi_cmd_ioctl+0x4d4/0x540
 scsi_cmd_blk_ioctl+0x7b/0x8b
 sd_ioctl+0xba/0x150
 blkdev_ioctl+0x6e1/0xea0
 block_ioctl+0x79/0x90
 do_vfs_ioctl+0x12b/0x9b0
 ksys_ioctl+0x41/0x80
 __x64_sys_ioctl+0x43/0x50
 do_syscall_64+0x71/0x210
 entry_SYSCALL_64_after_hwframe+0x49/0xbe

Cc: Christoph Hellwig <hch@lst.de>
Cc: Douglas Gilbert <dgilbert@interlog.com>
Fixes: ae3d56d81507 ("scsi: remove bidirectional command support")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/scsi_debug.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

Comments

Christoph Hellwig Feb. 12, 2019, 7:57 a.m. UTC | #1
On Fri, Feb 08, 2019 at 01:21:27PM -0800, Bart Van Assche wrote:
> A recent commit removed an element from opcode_info_arr[] but did not
> modify opcode_ind_arr[] nor was SDEB_I_XDWRITEREAD removed. Remove
> SDEB_I_XDWRITEREAD and bring the two arrays again in sync. This patch
> avoids that the following is reported:

This looks fine to me:

Reviewed-by: Christoph Hellwig <hch@lst.de>

But that whole table scheme looks way to fragile to me..
Douglas Gilbert Feb. 12, 2019, 2:36 p.m. UTC | #2
On 2019-02-12 2:57 a.m., Christoph Hellwig wrote:
> On Fri, Feb 08, 2019 at 01:21:27PM -0800, Bart Van Assche wrote:
>> A recent commit removed an element from opcode_info_arr[] but did not
>> modify opcode_ind_arr[] nor was SDEB_I_XDWRITEREAD removed. Remove
>> SDEB_I_XDWRITEREAD and bring the two arrays again in sync. This patch
>> avoids that the following is reported:
> 
> This looks fine to me:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> But that whole table scheme looks way to fragile to me..

Yes, as it was designed to help people add, not remove, functionality.
Also it makes the output of REPORT SUPPORTED OPERATION CODES totally
believable, unlike many real SCSI devices and target emulations.

Doug Gilbert
Martin K. Petersen Feb. 12, 2019, 4:08 p.m. UTC | #3
Bart,

> A recent commit removed an element from opcode_info_arr[] but did not
> modify opcode_ind_arr[] nor was SDEB_I_XDWRITEREAD removed. Remove
> SDEB_I_XDWRITEREAD and bring the two arrays again in sync. This patch
> avoids that the following is reported:

Applied to 5.1/scsi-queue, thanks!
diff mbox series

Patch

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index d4aede324e84..5800f0b06a8d 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -351,12 +351,11 @@  enum sdeb_opcode_index {
 	SDEB_I_ATA_PT = 22,		/* 12, 16 */
 	SDEB_I_SEND_DIAG = 23,
 	SDEB_I_UNMAP = 24,
-	SDEB_I_XDWRITEREAD = 25,	/* 10 only */
-	SDEB_I_WRITE_BUFFER = 26,
-	SDEB_I_WRITE_SAME = 27,		/* 10, 16 */
-	SDEB_I_SYNC_CACHE = 28,		/* 10, 16 */
-	SDEB_I_COMP_WRITE = 29,
-	SDEB_I_LAST_ELEMENT = 30,	/* keep this last (previous + 1) */
+	SDEB_I_WRITE_BUFFER = 25,
+	SDEB_I_WRITE_SAME = 26,		/* 10, 16 */
+	SDEB_I_SYNC_CACHE = 27,		/* 10, 16 */
+	SDEB_I_COMP_WRITE = 28,
+	SDEB_I_LAST_ELEMENT = 29,	/* keep this last (previous + 1) */
 };
 
 
@@ -377,7 +376,7 @@  static const unsigned char opcode_ind_arr[256] = {
 /* 0x40; 0x40->0x5f: 10 byte cdbs */
 	0, SDEB_I_WRITE_SAME, SDEB_I_UNMAP, 0, 0, 0, 0, 0,
 	0, 0, 0, 0, 0, SDEB_I_LOG_SENSE, 0, 0,
-	0, 0, 0, SDEB_I_XDWRITEREAD, 0, SDEB_I_MODE_SELECT, SDEB_I_RESERVE,
+	0, 0, 0, 0, 0, SDEB_I_MODE_SELECT, SDEB_I_RESERVE,
 	    SDEB_I_RELEASE,
 	0, 0, SDEB_I_MODE_SENSE, 0, 0, 0, 0, 0,
 /* 0x60; 0x60->0x7d are reserved, 0x7e is "extended cdb" */
@@ -614,7 +613,7 @@  static const struct opcode_info_t opcode_info_arr[SDEB_I_LAST_ELEMENT + 1] = {
 	    {16,  0xf8, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0, 0,
 	     0, 0xff, 0x3f, 0xc7} },		/* COMPARE AND WRITE */
 
-/* 30 */
+/* 29 */
 	{0xff, 0, 0, 0, NULL, NULL,		/* terminating element */
 	    {0,  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0} },
 };