[lvc-project] [PATCH 2/2] scsi: add basic CDB check to bsg_sg_io()

[Dmitry Antipov]

Local fuzzing of 6.12.113 has reported the following crash:

[ T6325] KASAN: null-ptr-deref in range [0x0000000000000010-0x0000000000000017]
[ T6325] CPU: 1 UID: 0 PID: 6325 Comm: syz.0.17 Tainted: G W 6.12.113 #2
...
[  123.544088][ T6325] RIP: 0010:ufs_bsg_request+0x244/0x11f0
...
[  123.550658][ T6325] Call Trace:
[  123.550918][ T6325]  <TASK>
[  123.551154][ T6325]  ? __pfx_ufs_bsg_request+0x10/0x10
[  123.551599][ T6325]  bsg_queue_rq+0x4e2/0x780
[  123.551991][ T6325]  ? __pfx_bsg_queue_rq+0x10/0x10
[  123.552526][ T6325]  ? lockdep_unlock+0x5c/0xd0
[  123.553098][ T6325]  blk_mq_dispatch_rq_list+0x499/0x1990
[  123.553819][ T6325]  ? do_raw_spin_lock+0x12b/0x2f0
[  123.554458][ T6325]  ? __pfx_blk_mq_dispatch_rq_list+0x10/0x10
[  123.555332][ T6325]  ? do_raw_spin_unlock+0xf5/0x200
[  123.556247][ T6325]  __blk_mq_sched_dispatch_requests+0x355/0x1580
[  123.557644][ T6325]  ? __lock_acquire+0x761/0x2e30
[  123.559226][ T6325]  ? do_raw_spin_lock+0x12b/0x2f0
[  123.562128][ T6325]  ? __pfx___blk_mq_sched_dispatch_requests+0x10/0x10
[  123.565938][ T6325]  ? lock_acquire+0x115/0x350
[  123.568870][ T6325]  ? lock_acquire+0x115/0x350
[  123.571884][ T6325]  blk_mq_sched_dispatch_requests+0xd7/0x180
[  123.574304][ T6325]  ? blk_mq_run_hw_queue+0x317/0x4e0
[  123.575617][ T6325]  blk_mq_run_hw_queue+0x340/0x4e0
[  123.576586][ T6325]  blk_execute_rq+0x1e0/0x4c0
[  123.577254][ T6325]  ? import_ubuf+0xfb/0x1d0
[  123.577847][ T6325]  ? blk_rq_map_user+0x11b/0x1a0
[  123.578440][ T6325]  ? __pfx_blk_execute_rq+0x10/0x10
[  123.579013][ T6325]  ? _copy_from_user+0x94/0xb0
[  123.579509][ T6325]  bsg_transport_sg_io_fn+0x5ff/0xef0
[  123.580093][ T6325]  bsg_ioctl+0x7ec/0xa10
[  123.580603][ T6325]  ? __pfx_bsg_transport_sg_io_fn+0x10/0x10
[  123.581248][ T6325]  ? __pfx_bsg_ioctl+0x10/0x10
[  123.581695][ T6325]  ? __fget_files+0x2a/0x420
[  123.582057][ T6325]  ? bpf_lsm_file_ioctl+0x9/0x20
[  123.582437][ T6325]  ? __pfx_bsg_ioctl+0x10/0x10
[  123.582789][ T6325]  __se_sys_ioctl+0xfc/0x170
[  123.583123][ T6325]  do_syscall_64+0x166/0x520
[  123.583470][ T6325]  ? trace_irq_disable+0x3b/0x140
[  123.583904][ T6325]  ? entry_SYSCALL_64_after_hwframe+0x77/0x7f
[  123.584412][ T6325]  entry_SYSCALL_64_after_hwframe+0x77/0x7f

This is caused by ioctl(..., SG_IO, {struct sg_io_v4 hdr = {
.subprotocol = BSG_SUB_PROTOCOL_SCSI_TRANSPORT, .request = NULL,
.request_len = 0 }}). So if 'hdr' is not a task management
function, perform basic 'request' and 'request_len' check
assuming that there should be a valid fixed length CDB at least.

Signed-off-by: Dmitry Antipov <dmantipov at yandex.ru>
---
 block/bsg-lib.c         | 2 --
 block/bsg.c             | 9 +++++++++
 include/linux/bsg-lib.h | 2 ++
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/block/bsg-lib.c b/block/bsg-lib.c
index 536cee4430a7..e7ae60f73870 100644
--- a/block/bsg-lib.c
+++ b/block/bsg-lib.c
@@ -16,8 +16,6 @@
 #include <scsi/scsi_cmnd.h>
 #include <scsi/sg.h>
 
-#define uptr64(val) ((void __user *)(uintptr_t)(val))
-
 struct bsg_set {
 	struct blk_mq_tag_set	tag_set;
 	struct bsg_device	*bd;
diff --git a/block/bsg.c b/block/bsg.c
index 82aaf3cee582..80f367ae7d24 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -11,6 +11,7 @@
 #include <linux/percpu.h>
 #include <linux/idr.h>
 #include <linux/bsg.h>
+#include <linux/bsg-lib.h>
 #include <linux/slab.h>
 #include <linux/io_uring/cmd.h>
 
@@ -66,6 +67,14 @@ static int bsg_sg_io(struct bsg_device *bd, bool open_for_write,
 		return -EFAULT;
 	if (hdr.guard != 'Q')
 		return -EINVAL;
+
+	if (hdr.subprotocol != BSG_SUB_PROTOCOL_SCSI_TMF) {
+		if (hdr.request_len < SCSI_MIN_CDB_SIZE)
+			return -EINVAL;
+		if (!access_ok(uptr64(hdr.request), hdr.request_len))
+			return -EFAULT;
+	}
+
 	ret = bd->sg_io_fn(bd->queue, &hdr, open_for_write,
 			   bsg_timeout(bd, &hdr));
 	if (!ret && copy_to_user(uarg, &hdr, sizeof(hdr)))
diff --git a/include/linux/bsg-lib.h b/include/linux/bsg-lib.h
index a697246feeac..fac7c89516ca 100644
--- a/include/linux/bsg-lib.h
+++ b/include/linux/bsg-lib.h
@@ -17,6 +17,8 @@ struct device;
 struct scatterlist;
 struct request_queue;
 
+#define uptr64(val) ((void __user *)(uintptr_t)(val))
+
 typedef int (bsg_job_fn) (struct bsg_job *);
 typedef enum blk_eh_timer_return (bsg_timeout_fn)(struct request *);
 
-- 
2.56.0