回复: [PATCH v2] scsi: bsg: read io_uring command fields once

Hi Rahul,

Thanks for reporting this and for v2. The READ_ONCE() approach looks
good to me.

Reviewed-by: Yang Xiuwei <[email protected]>

Thanks,
Yang Xiuwei

主 题：[PATCH v2] scsi: bsg: read io_uring command fields once
日 期：2026年05月28日03:17
发件人：Rahul Chandelkar
收件人：rc,James E . J . Bottomley,Martin K . Petersen,Jens Axboe,FUJITA Tomonori
抄送人：linux-scsi,linux-block,io-uring,linux-kernel,Bart Van Assche,Caleb Sander Mateos,stable

scsi_bsg_uring_cmd() reads struct bsg_uring_cmd fields directly from the shared mmap'd io_uring SQE. On the inline execution path, io_uring may still point at userspace-visible SQE storage, so a concurrent userspace thread can change fields between validation and use. request_len is checked against the size of scmd->cmnd, then used again for scmd->cmd_len and copy_from_user(). If userspace changes request_len after the bounds check, the later copy can overflow the 32-byte scmd->cmnd buffer. Transfer fields are also read again by scsi_bsg_map_user_buffer(), leaving direction, address and length open to the same race. Use READ_ONCE() to load each bsg_uring_cmd field needed by scsi_bsg_uring_cmd() into a local variable, then use those locals for both validation and execution. Pass the stable transfer direction, address and length into scsi_bsg_map_user_buffer() so the helper no longer re-derives them from the SQE. This fixes the double-fetch without copying the whole io_uring command payload. Tested with KASAN on QEMU (virtio-scsi, 2 vCPUs). Without this fix, a two-thread race produces: BUG: KASAN: wild-memory-access in scsi_queue_rq+0x4a3/0x58a0 Write of size 96 at addr dead000000001000 by task poc/67 Call Trace: kasan_report+0xce/0x100 __asan_memset+0x23/0x50 scsi_queue_rq+0x4a3/0x58a0 scsi_bsg_uring_cmd+0x942/0x1570 io_uring_cmd+0x2f6/0x950 io_issue_sqe+0xe5/0x22d0 Link: https://lore.kernel.org/all/[email protected]/T/#u Fixes: 7b6d3255e7f8 ("scsi: bsg: add io_uring passthrough handler") Cc: [email protected] Signed-off-by: Rahul Chandelkar <[email protected]> --- Changes in v2: - Use READ_ONCE() for individual fields instead of memcpying the command payload. - Pass stable transfer parameters to scsi_bsg_map_user_buffer() so it does not re-read the SQE. - Do not carry the Reviewed-by tag from v1 because the implementation strategy changed. drivers/scsi/scsi_bsg.c | 54 ++++++++++++++++++++++++++--------------- 1 file changed, 35 insertions(+), 19 deletions(-) diff --git a/drivers/scsi/scsi_bsg.c b/drivers/scsi/scsi_bsg.c index e80dec53174e..ccbe3d98e4ff 100644 --- a/drivers/scsi/scsi_bsg.c +++ b/drivers/scsi/scsi_bsg.c @@ -76,12 +76,10 @@ static enum rq_end_io_ret scsi_bsg_uring_cmd_done(struct request *req, static int scsi_bsg_map_user_buffer(struct request *req, struct io_uring_cmd *ioucmd, - unsigned int issue_flags, gfp_t gfp_mask) + unsigned int issue_flags, gfp_t gfp_mask, + bool is_write, u64 buf_addr, + unsigned long buf_len) { - const struct bsg_uring_cmd *cmd = io_uring_sqe128_cmd(ioucmd->sqe, struct bsg_uring_cmd); - bool is_write = cmd->dout_xfer_len > 0; - u64 buf_addr = is_write ? cmd->dout_xferp : cmd->din_xferp; - unsigned long buf_len = is_write ? cmd->dout_xfer_len : cmd->din_xfer_len; struct iov_iter iter; int ret; @@ -104,26 +102,40 @@ static int scsi_bsg_uring_cmd(struct request_queue *q, struct io_uring_cmd *iouc unsigned int issue_flags, bool open_for_write) { struct scsi_bsg_uring_cmd_pdu *pdu = scsi_bsg_uring_cmd_pdu(ioucmd); - const struct bsg_uring_cmd *cmd = io_uring_sqe128_cmd(ioucmd->sqe, struct bsg_uring_cmd); + const struct bsg_uring_cmd *cmd = + io_uring_sqe128_cmd(ioucmd->sqe, struct bsg_uring_cmd); struct scsi_cmnd *scmd; struct request *req; blk_mq_req_flags_t blk_flags = 0; gfp_t gfp_mask = GFP_KERNEL; + u64 request = READ_ONCE(cmd->request); + u32 request_len = READ_ONCE(cmd->request_len); + u32 protocol = READ_ONCE(cmd->protocol); + u32 subprotocol = READ_ONCE(cmd->subprotocol); + u32 max_response_len = READ_ONCE(cmd->max_response_len); + u64 response = READ_ONCE(cmd->response); + u64 dout_xferp = READ_ONCE(cmd->dout_xferp); + u32 dout_xfer_len = READ_ONCE(cmd->dout_xfer_len); + u32 dout_iovec_count = READ_ONCE(cmd->dout_iovec_count); + u64 din_xferp = READ_ONCE(cmd->din_xferp); + u32 din_xfer_len = READ_ONCE(cmd->din_xfer_len); + u32 din_iovec_count = READ_ONCE(cmd->din_iovec_count); + u32 timeout_ms = READ_ONCE(cmd->timeout_ms); int ret; - if (cmd->protocol != BSG_PROTOCOL_SCSI || - cmd->subprotocol != BSG_SUB_PROTOCOL_SCSI_CMD) + if (protocol != BSG_PROTOCOL_SCSI || + subprotocol != BSG_SUB_PROTOCOL_SCSI_CMD) return -EINVAL; - if (!cmd->request || cmd->request_len == 0) + if (!request || request_len == 0) return -EINVAL; - if (cmd->dout_xfer_len && cmd->din_xfer_len) { + if (dout_xfer_len && din_xfer_len) { pr_warn_once("BIDI support in bsg has been removed.\n"); return -EOPNOTSUPP; } - if (cmd->dout_iovec_count > 0 || cmd->din_iovec_count > 0) + if (dout_iovec_count > 0 || din_iovec_count > 0) return -EOPNOTSUPP; if (issue_flags & IO_URING_F_NONBLOCK) { @@ -131,20 +143,20 @@ static int scsi_bsg_uring_cmd(struct request_queue *q, struct io_uring_cmd *iouc gfp_mask = GFP_NOWAIT; } - req = scsi_alloc_request(q, cmd->dout_xfer_len ? + req = scsi_alloc_request(q, dout_xfer_len ? REQ_OP_DRV_OUT : REQ_OP_DRV_IN, blk_flags); if (IS_ERR(req)) return PTR_ERR(req); scmd = blk_mq_rq_to_pdu(req); - if (cmd->request_len > sizeof(scmd->cmnd)) { + if (request_len > sizeof(scmd->cmnd)) { ret = -EINVAL; goto out_free_req; } - scmd->cmd_len = cmd->request_len; + scmd->cmd_len = request_len; scmd->allowed = SG_DEFAULT_RETRIES; - if (copy_from_user(scmd->cmnd, uptr64(cmd->request), cmd->request_len)) { + if (copy_from_user(scmd->cmnd, uptr64(request), request_len)) { ret = -EFAULT; goto out_free_req; } @@ -154,12 +166,18 @@ static int scsi_bsg_uring_cmd(struct request_queue *q, struct io_uring_cmd *iouc goto out_free_req; } - pdu->response_addr = cmd->response; - scmd->sense_len = cmd->max_response_len ? - min(cmd->max_response_len, SCSI_SENSE_BUFFERSIZE) : SCSI_SENSE_BUFFERSIZE; + pdu->response_addr = response; + scmd->sense_len = max_response_len ? + min(max_response_len, SCSI_SENSE_BUFFERSIZE) : SCSI_SENSE_BUFFERSIZE; - if (cmd->dout_xfer_len || cmd->din_xfer_len) { - ret = scsi_bsg_map_user_buffer(req, ioucmd, issue_flags, gfp_mask); + if (dout_xfer_len || din_xfer_len) { + bool is_write = dout_xfer_len > 0; + u64 buf_addr = is_write ? dout_xferp : din_xferp; + unsigned long buf_len = is_write ? dout_xfer_len : din_xfer_len; + + ret = scsi_bsg_map_user_buffer(req, ioucmd, issue_flags, + gfp_mask, is_write, buf_addr, + buf_len); if (ret) goto out_free_req; pdu->bio = req->bio; @@ -167,8 +185,8 @@ static int scsi_bsg_uring_cmd(struct request_queue *q, struct io_uring_cmd *iouc pdu->bio = NULL; } - req->timeout = cmd->timeout_ms ? - msecs_to_jiffies(cmd->timeout_ms) : BLK_DEFAULT_SG_TIMEOUT; + req->timeout = timeout_ms ? + msecs_to_jiffies(timeout_ms) : BLK_DEFAULT_SG_TIMEOUT; req->end_io = scsi_bsg_uring_cmd_done; req->end_io_data = ioucmd; -- 2.54.0 </[email protected]>

---