[PATCH v3 0/2] scsi: bsg: io_uring passthrough TOCTOU and mapping

[PATCH v3 0/2] scsi: bsg: io_uring passthrough TOCTOU and mapping

[Yang Xiuwei]

Hi,

Two patches for the BSG io_uring passthrough path.

1/2: fix TOCTOU on the shared SQE.

Builds on Rahul Chandelkar's earlier TOCTOU patches:
Link: https://lore.kernel.org/r/20260527105931.3950913-1-rc@rexion.ai
Link: https://lore.kernel.org/r/20260527191817.142769-1-rc@rexion.ai

2/2: IO_URING_F_NONBLOCK only needs BLK_MQ_REQ_NOWAIT for request
allocation. Map user buffers with GFP_KERNEL.

Changes in v3:
- Reword 1/2 commit message.
Link: https://lore.kernel.org/r/20260727014117.1742202-1-yangxiuwei@kylinos.cn

Changes in v2:
- Also READ_ONCE max_response_len (Caleb).
Link: https://lore.kernel.org/r/20260720032338.461681-1-yangxiuwei@kylinos.cn

Rahul Chandelkar (1):
  scsi: bsg: fix TOCTOU in io_uring passthrough command setup

Yang Xiuwei (1):
  scsi: bsg: map io_uring user buffers with GFP_KERNEL

 drivers/scsi/scsi_bsg.c | 51 +++++++++++++++++++++++------------------
 1 file changed, 29 insertions(+), 22 deletions(-)

-- 
2.25.1

[PATCH v3 1/2] scsi: bsg: fix TOCTOU in io_uring passthrough command setup

[Yang Xiuwei]

From: Rahul Chandelkar <rc@rexion.ai>

scsi_bsg_uring_cmd() reads bsg_uring_cmd from the shared mmap'd SQE.
Userspace can change a field after we check it and before we use it.
request_len is the sharp case: it can grow past sizeof(scmd->cmnd)
after the bound check and overflow scmd->cmnd in copy_from_user().

READ_ONCE() the SQE fields we check or use into locals before use.

Fixes: 7b6d3255e7f8 ("scsi: bsg: add io_uring passthrough handler")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/20260527105931.3950913-1-rc@rexion.ai
Signed-off-by: Rahul Chandelkar <rc@rexion.ai>
Co-developed-by: Yang Xiuwei <yangxiuwei@kylinos.cn>
Signed-off-by: Yang Xiuwei <yangxiuwei@kylinos.cn>
---
 drivers/scsi/scsi_bsg.c | 44 ++++++++++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 16 deletions(-)

diff --git a/drivers/scsi/scsi_bsg.c b/drivers/scsi/scsi_bsg.c
index e80dec53174e..7758c5f22a7e 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,21 +102,29 @@ 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);
+	u64 dout_xferp = READ_ONCE(cmd->dout_xferp);
+	u32 dout_xfer_len = READ_ONCE(cmd->dout_xfer_len);
+	u64 din_xferp = READ_ONCE(cmd->din_xferp);
+	u32 din_xfer_len = READ_ONCE(cmd->din_xfer_len);
+	u32 max_response_len = READ_ONCE(cmd->max_response_len);
 	int ret;
 
 	if (cmd->protocol != BSG_PROTOCOL_SCSI ||
 	    cmd->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;
 	}
@@ -131,20 +137,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;
 	}
@@ -155,11 +161,17 @@ static int scsi_bsg_uring_cmd(struct request_queue *q, struct io_uring_cmd *iouc
 	}
 
 	pdu->response_addr = cmd->response;
-	scmd->sense_len = cmd->max_response_len ?
-		min(cmd->max_response_len, SCSI_SENSE_BUFFERSIZE) : SCSI_SENSE_BUFFERSIZE;
+	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;
-- 
2.25.1

[PATCH v3 2/2] scsi: bsg: map io_uring user buffers with GFP_KERNEL

[Yang Xiuwei]

IO_URING_F_NONBLOCK only needs BLK_MQ_REQ_NOWAIT for request
allocation. Mapping the user buffer may sleep, so use GFP_KERNEL
instead of GFP_NOWAIT.

Signed-off-by: Yang Xiuwei <yangxiuwei@kylinos.cn>
---
 drivers/scsi/scsi_bsg.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/scsi_bsg.c b/drivers/scsi/scsi_bsg.c
index 7758c5f22a7e..ddb33ae68483 100644
--- a/drivers/scsi/scsi_bsg.c
+++ b/drivers/scsi/scsi_bsg.c
@@ -76,9 +76,8 @@ 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,
-				    bool is_write, u64 buf_addr,
-				    unsigned long buf_len)
+				    unsigned int issue_flags, bool is_write,
+				    u64 buf_addr, unsigned long buf_len)
 {
 	struct iov_iter iter;
 	int ret;
@@ -89,10 +88,10 @@ static int scsi_bsg_map_user_buffer(struct request *req,
 						&iter, ioucmd, issue_flags);
 		if (ret < 0)
 			return ret;
-		ret = blk_rq_map_user_iov(req->q, req, NULL, &iter, gfp_mask);
+		ret = blk_rq_map_user_iov(req->q, req, NULL, &iter, GFP_KERNEL);
 	} else {
 		ret = blk_rq_map_user(req->q, req, NULL, uptr64(buf_addr),
-				      buf_len, gfp_mask);
+				      buf_len, GFP_KERNEL);
 	}
 
 	return ret;
@@ -107,7 +106,6 @@ static int scsi_bsg_uring_cmd(struct request_queue *q, struct io_uring_cmd *iouc
 	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);
 	u64 dout_xferp = READ_ONCE(cmd->dout_xferp);
@@ -132,10 +130,8 @@ static int scsi_bsg_uring_cmd(struct request_queue *q, struct io_uring_cmd *iouc
 	if (cmd->dout_iovec_count > 0 || cmd->din_iovec_count > 0)
 		return -EOPNOTSUPP;
 
-	if (issue_flags & IO_URING_F_NONBLOCK) {
+	if (issue_flags & IO_URING_F_NONBLOCK)
 		blk_flags = BLK_MQ_REQ_NOWAIT;
-		gfp_mask = GFP_NOWAIT;
-	}
 
 	req = scsi_alloc_request(q, dout_xfer_len ?
 				 REQ_OP_DRV_OUT : REQ_OP_DRV_IN, blk_flags);
@@ -170,8 +166,7 @@ static int scsi_bsg_uring_cmd(struct request_queue *q, struct io_uring_cmd *iouc
 		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);
+					       is_write, buf_addr, buf_len);
 		if (ret)
 			goto out_free_req;
 		pdu->bio = req->bio;
-- 
2.25.1

Re: [PATCH v3 1/2] scsi: bsg: fix TOCTOU in io_uring passthrough command setup

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The completion callback writes out-of-bounds to user space by failing to cap `sense_len` with `max_response_len`.
--

--- Patch [1]: [PATCH v3 1/2] scsi: bsg: fix TOCTOU in io_uring passthrough command setup ---
commit 2cc4e90253ec0a7b16f7c596b055fcc02296f149
Author: Rahul Chandelkar <rc@rexion.ai>

scsi: bsg: fix TOCTOU in io_uring passthrough command setup

This commit addresses a TOCTOU vulnerability by reading bsg_uring_cmd
fields from the shared mmap'd SQE into local variables once before use.
This ensures bounds checks cannot be bypassed if userspace changes the
values concurrently.

Link: https://lore.kernel.org/r/20260527105931.3950913-1-rc@rexion.ai

> diff --git a/drivers/scsi/scsi_bsg.c b/drivers/scsi/scsi_bsg.c
> index e80dec53174ea..7758c5f22a7e9 100644
> --- a/drivers/scsi/scsi_bsg.c
> +++ b/drivers/scsi/scsi_bsg.c

[ ... ]

> @@ -155,11 +161,17 @@ static int scsi_bsg_uring_cmd(struct request_queue *q, struct io_uring_cmd *iouc
>  	}
>  
>  	pdu->response_addr = cmd->response;
> -	scmd->sense_len = cmd->max_response_len ?
> -		min(cmd->max_response_len, SCSI_SENSE_BUFFERSIZE) : SCSI_SENSE_BUFFERSIZE;
> +	scmd->sense_len = max_response_len ?
> +		min(max_response_len, SCSI_SENSE_BUFFERSIZE) : SCSI_SENSE_BUFFERSIZE;

[Severity: High]
This isn't a bug introduced by this patch, but the completion logic in
scsi_bsg_uring_task_cb() contains a buffer overflow vulnerability affecting
user space.

When a command completes with a check condition, the SCSI core in
scsi_io_completion_nz_result() overwrites scmd->sense_len with the actual
length of the sense data.

Later in scsi_bsg_uring_task_cb(), the code calculates the write length:

	sense_len_wr = min_t(u8, scmd->sense_len,
			     SCSI_SENSE_BUFFERSIZE);
	...
	if (copy_to_user(uptr64(pdu->response_addr), scmd->sense_buffer,
			 sense_len_wr))

Since max_response_len was only stored in scmd->sense_len (which was
overwritten) and not saved in the PDU, the kernel uses the unbounded sense
data length.

Can this copy unbounded sense data to user space, overflowing the provided
buffer if the sense data exceeds the original max_response_len?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817013959.165425-1-yangxiuwei@kylinos.cn?part=1

Re: [PATCH v3 2/2] scsi: bsg: map io_uring user buffers with GFP_KERNEL

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Unconditionally using GFP_KERNEL for memory allocation when IO_URING_F_NONBLOCK is set allows the io_uring submitter thread to sleep, violating non-blocking semantics.

Pre-existing issues:
- [High] Allocating memory with GFP_KERNEL while holding a block request tag can trigger a tag exhaustion deadlock in the block layer.
--

--- Patch [2]: [PATCH v3 2/2] scsi: bsg: map io_uring user buffers with GFP_KERNEL ---
commit 4107274feb8462924a88001746916a4f70f777fe
Author: Yang Xiuwei <yangxiuwei@kylinos.cn>

scsi: bsg: map io_uring user buffers with GFP_KERNEL

IO_URING_F_NONBLOCK only needs BLK_MQ_REQ_NOWAIT for request
allocation. Mapping the user buffer may sleep, so use GFP_KERNEL
instead of GFP_NOWAIT.

> diff --git a/drivers/scsi/scsi_bsg.c b/drivers/scsi/scsi_bsg.c
> index 7758c5f22a7e9..ddb33ae684837 100644
> --- a/drivers/scsi/scsi_bsg.c
> +++ b/drivers/scsi/scsi_bsg.c
> @@ -89,10 +88,10 @@ static int scsi_bsg_map_user_buffer(struct request *req,
>  						&iter, ioucmd, issue_flags);
>  		if (ret < 0)
>  			return ret;
> -		ret = blk_rq_map_user_iov(req->q, req, NULL, &iter, gfp_mask);
> +		ret = blk_rq_map_user_iov(req->q, req, NULL, &iter, GFP_KERNEL);
>  	} else {
>  		ret = blk_rq_map_user(req->q, req, NULL, uptr64(buf_addr),
> -				      buf_len, gfp_mask);
> +				      buf_len, GFP_KERNEL);
>  	}

[Severity: High]
This is a pre-existing issue, but does this allocation trigger a tag
exhaustion deadlock?

In scsi_bsg_uring_cmd(), scsi_alloc_request() has already been called, which
consumes a hardware tag from the block device's limited pool.

Calling blk_rq_map_user_iov() with GFP_KERNEL immediately after allows the
allocation to sleep in memory reclaim. If concurrent bsg io_uring commands
allocate all available hardware tags and then enter direct reclaim, and the
reclaim path attempts to write dirty pages back to the same block device, it
will require a hardware tag.

Since all tags would be held by threads waiting for memory, the reclaim
writeback cannot proceed. Shouldn't allocations performed while holding a
block tag use GFP_NOIO or GFP_NOWAIT to prevent block IO recursion?

[ ... ]

> @@ -132,10 +130,8 @@ static int scsi_bsg_uring_cmd(struct request_queue *q, struct io_uring_cmd *iouc
>  	if (cmd->dout_iovec_count > 0 || cmd->din_iovec_count > 0)
>  		return -EOPNOTSUPP;
>  
> -	if (issue_flags & IO_URING_F_NONBLOCK) {
> +	if (issue_flags & IO_URING_F_NONBLOCK)
>  		blk_flags = BLK_MQ_REQ_NOWAIT;
> -		gfp_mask = GFP_NOWAIT;
> -	}
>  
>  	req = scsi_alloc_request(q, dout_xfer_len ?
>  				 REQ_OP_DRV_OUT : REQ_OP_DRV_IN, blk_flags);

[Severity: High]
Does removing the gfp_mask downgrade here violate io_uring non-blocking
semantics?

When IO_URING_F_NONBLOCK is set, the io_uring core expects the issue handler
to be strictly non-blocking. If it cannot allocate memory without sleeping, it
should return -EAGAIN so the request can be deferred to an asynchronous io-wq
worker thread.

By removing GFP_NOWAIT and unconditionally using GFP_KERNEL for mapping the
buffer, won't this allow the submission thread (such as the SQPOLL thread)
to block in direct reclaim, stalling other independent operations on the
same ring?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817013959.165425-1-yangxiuwei@kylinos.cn?part=2

Re: [PATCH v3 0/2] scsi: bsg: io_uring passthrough TOCTOU and mapping

[Yang Xiuwei]

Hi,

sashiko noted a sense buffer overflow on the io_uring path:
Link: https://lore.kernel.org/linux-scsi/20260817015247.B9CF61F000E9@smtp.kernel.org/

Agreed. Completion copies scmd->sense_len without capping to
max_response_len. Setting sense_len at submit does not help; the
midlayer overwrites it with the real sense length.

I will add a fix as 3/3 in v4 (store max_response_len in the PDU and
cap the copy like the ioctl path).

Thanks,
Yang Xiuwei