[PATCH 0/6] scsi: make SG_IO user copyback failure-safe

[PATCH 0/6] scsi: make SG_IO user copyback failure-safe

[Wang Yingjie]

Several SCSI passthrough completion paths touch userspace while their
completed request still owns a queue tag.  Sense or READ-bounce copyback
can fault on a mapping backed by the same one-tag device, leaving the
nested I/O unable to obtain the retained tag.  The synchronous and
io_uring BSG paths additionally discard data-copyback failures.

Snapshot request-backed completion state, free each request before
userspace access, and preserve the existing error precedence while
reporting copyback failures.  This covers block-device SG_IO, CDDA,
and both synchronous and io_uring BSG completion.

Same-device one-tag tests previously left SG_IO, CDDA, and BSG tasks in
D state in blk_rq_unmap_user() -> filemap_fault.  The fixed paths
completed without a stuck task or hung-task report.  BSG tests with an
unaligned PROT_NONE destination returned false success in 20/20 runs
before the error-propagation changes and -EFAULT in 20/20 runs after
them, while writable controls continued to succeed.

The series passed scripts/checkpatch.pl --strict, focused W=1 builds,
the full bzImage build used for the wider audit, and QEMU differential
tests on the affected paths.

Yingjie Wang (6):
  scsi: core: release SG_IO request before copying to userspace
  scsi: sr: release CDDA request before copying to userspace
  scsi: bsg: release synchronous request before user copyback
  scsi: bsg: report synchronous data copyback failures
  scsi: bsg: release io_uring request before user copyback
  scsi: bsg: report io_uring data copyback failures

 drivers/scsi/scsi_bsg.c   | 60 ++++++++++++++++++++++++++-------------
 drivers/scsi/scsi_ioctl.c | 45 ++++++++++++++++++-----------
 drivers/scsi/sr.c         |  3 ++
 3 files changed, 72 insertions(+), 36 deletions(-)


base-commit: 0fcb05b72344f40806cbe810de8872f1698bf053
-- 
2.43.0

[PATCH 1/6] scsi: core: release SG_IO request before copying to userspace

[Wang Yingjie]

From: Yingjie Wang <x-professor@outlook.com>

The block-device SG_IO path copies sense and READ bounce data to
userspace while the completed request still owns its queue tag.  Either
copy can fault.  If the fault reads from the same one-tag device, the
nested I/O cannot obtain a tag and the ioctl remains stuck in
uninterruptible sleep.

Snapshot all request-backed status, residual, and sense data, then free
the request before either userspace copy.  Preserve the existing error
precedence between sense copy and data copyback.

A same-device INQUIRY on a one-tag /dev/sr0 queue hung beyond 10 seconds
before this change, with sg_io -> blk_rq_unmap_user -> filemap_fault in
the stack.  The fixed case completed without a stuck task or hung-task
warning.

Fixes: 31151ba2cef1 ("fix mismerge in ll_rw_blk.c")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Yingjie Wang <x-professor@outlook.com>
---
 drivers/scsi/scsi_ioctl.c | 45 +++++++++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 16 deletions(-)

diff --git a/drivers/scsi/scsi_ioctl.c b/drivers/scsi/scsi_ioctl.c
index c14f81403a09..0d1f8eb22dfb 100644
--- a/drivers/scsi/scsi_ioctl.c
+++ b/drivers/scsi/scsi_ioctl.c
@@ -388,41 +388,38 @@ static int scsi_fill_sghdr_rq(struct scsi_device *sdev, struct request *rq,
 	return 0;
 }
 
-static int scsi_complete_sghdr_rq(struct request *rq, struct sg_io_hdr *hdr,
-		struct bio *bio)
+static int scsi_complete_sghdr_rq(struct sg_io_hdr *hdr, int result,
+				  unsigned int resid_len,
+				  const u8 *sense_buffer,
+				  unsigned int sense_len)
 {
-	struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(rq);
-	int r, ret = 0;
+	int ret = 0;
 
 	/*
 	 * fill in all the output members
 	 */
-	hdr->status = scmd->result & 0xff;
-	hdr->masked_status = sg_status_byte(scmd->result);
+	hdr->status = result & 0xff;
+	hdr->masked_status = sg_status_byte(result);
 	hdr->msg_status = COMMAND_COMPLETE;
-	hdr->host_status = host_byte(scmd->result);
+	hdr->host_status = host_byte(result);
 	hdr->driver_status = 0;
 	if (scsi_status_is_check_condition(hdr->status))
 		hdr->driver_status = DRIVER_SENSE;
 	hdr->info = 0;
 	if (hdr->masked_status || hdr->host_status || hdr->driver_status)
 		hdr->info |= SG_INFO_CHECK;
-	hdr->resid = scmd->resid_len;
+	hdr->resid = resid_len;
 	hdr->sb_len_wr = 0;
 
-	if (scmd->sense_len && hdr->sbp) {
-		int len = min((unsigned int) hdr->mx_sb_len, scmd->sense_len);
+	if (sense_len && hdr->sbp) {
+		int len = min_t(unsigned int, hdr->mx_sb_len, sense_len);
 
-		if (!copy_to_user(hdr->sbp, scmd->sense_buffer, len))
+		if (!copy_to_user(hdr->sbp, sense_buffer, len))
 			hdr->sb_len_wr = len;
 		else
 			ret = -EFAULT;
 	}
 
-	r = blk_rq_unmap_user(bio);
-	if (!ret)
-		ret = r;
-
 	return ret;
 }
 
@@ -436,6 +433,9 @@ static int sg_io(struct scsi_device *sdev, struct sg_io_hdr *hdr,
 	struct request *rq;
 	struct scsi_cmnd *scmd;
 	struct bio *bio;
+	u8 sense_buffer[SCSI_SENSE_BUFFERSIZE];
+	unsigned int sense_len, resid_len;
+	int result, unmap_ret;
 
 	if (hdr->interface_id != 'S')
 		return -EINVAL;
@@ -487,7 +487,20 @@ static int sg_io(struct scsi_device *sdev, struct sg_io_hdr *hdr,
 
 	hdr->duration = jiffies_to_msecs(jiffies - start_time);
 
-	ret = scsi_complete_sghdr_rq(rq, hdr, bio);
+	result = scmd->result;
+	resid_len = scmd->resid_len;
+	sense_len = min_t(unsigned int, scmd->sense_len,
+			  sizeof(sense_buffer));
+	if (sense_len)
+		memcpy(sense_buffer, scmd->sense_buffer, sense_len);
+
+	blk_mq_free_request(rq);
+	ret = scsi_complete_sghdr_rq(hdr, result, resid_len, sense_buffer,
+				     sense_len);
+	unmap_ret = blk_rq_unmap_user(bio);
+	if (!ret)
+		ret = unmap_ret;
+	return ret;
 
 out_put_request:
 	blk_mq_free_request(rq);
-- 
2.43.0

[PATCH 2/6] scsi: sr: release CDDA request before copying to userspace

[Wang Yingjie]

From: Yingjie Wang <x-professor@outlook.com>

sr_read_cdda_bpc() calls blk_rq_unmap_user() while its completed request
still owns the only queue tag.  For an unaligned READ buffer, unmap
copies bounce data to userspace and can fault on a nonresident mapping
of the same CD-ROM.  The nested read then waits for the tag held by the
outer request.

Free the request after saving all request-backed state and before
copying the retained bio back to userspace.

The standalone CDROMREADAUDIO reproducer previously remained in D state
in sr_read_cdda_bpc -> blk_rq_unmap_user -> filemap_fault and blocked
the disk-events worker in blk_mq_get_tag.  With this change it returned
immediately, left no stuck process, and produced no hung-task report.

Fixes: 31151ba2cef1 ("fix mismerge in ll_rw_blk.c")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Yingjie Wang <x-professor@outlook.com>
---
 drivers/scsi/sr.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index c36c54ecd354..de43d6e56479 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -968,8 +968,11 @@ static int sr_read_cdda_bpc(struct cdrom_device_info *cdi, void __user *ubuf,
 		ret = -EIO;
 	}
 
+	blk_mq_free_request(rq);
 	if (blk_rq_unmap_user(bio))
 		ret = -EFAULT;
+	return ret;
+
 out_put_request:
 	blk_mq_free_request(rq);
 	return ret;
-- 
2.43.0

[PATCH 3/6] scsi: bsg: release synchronous request before user copyback

[Wang Yingjie]

From: Yingjie Wang <x-professor@outlook.com>

scsi_bsg_sg_io_fn() retains a completed request while copying both sense
data and READ bounce data to userspace.  A fault on a mapping of the
same one-tag SCSI device starts nested I/O that cannot acquire the
retained tag.

Snapshot the request-backed completion fields and sense data, save the
transfer direction, and free the request before either userspace copy.
The retained bio can then be unmapped safely.

A BSG INQUIRY into a nonresident mapping of the same /dev/sr0 queue
previously remained in D state in blk_rq_unmap_user -> filemap_fault.
The fixed trigger returned in 0.06 seconds and produced no hung-task
report.

Fixes: 10e8855b9451 ("bsg: add SG_IO to SG v4")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Yingjie Wang <x-professor@outlook.com>
---
 drivers/scsi/scsi_bsg.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/scsi_bsg.c b/drivers/scsi/scsi_bsg.c
index 5eec248a77a6..4937bc494f79 100644
--- a/drivers/scsi/scsi_bsg.c
+++ b/drivers/scsi/scsi_bsg.c
@@ -199,6 +199,10 @@ static int scsi_bsg_sg_io_fn(struct request_queue *q, struct sg_io_v4 *hdr,
 	struct scsi_cmnd *scmd;
 	struct request *rq;
 	struct bio *bio;
+	u8 sense_buffer[SCSI_SENSE_BUFFERSIZE];
+	u32 scsi_result, resid_len;
+	u8 sense_len;
+	bool is_read;
 	int ret;
 
 	if (hdr->protocol != BSG_PROTOCOL_SCSI  ||
@@ -243,37 +247,46 @@ static int scsi_bsg_sg_io_fn(struct request_queue *q, struct sg_io_v4 *hdr,
 
 	bio = rq->bio;
 	blk_execute_rq(rq, !(hdr->flags & BSG_FLAG_Q_AT_TAIL));
+	scsi_result = scmd->result;
+	resid_len = scmd->resid_len;
+	sense_len = min_t(unsigned int, scmd->sense_len,
+			  sizeof(sense_buffer));
+	if (sense_len)
+		memcpy(sense_buffer, scmd->sense_buffer, sense_len);
+	is_read = rq_data_dir(rq) == READ;
+	blk_mq_free_request(rq);
 
 	/*
 	 * fill in all the output members
 	 */
-	hdr->device_status = scmd->result & 0xff;
-	hdr->transport_status = host_byte(scmd->result);
+	hdr->device_status = scsi_result & 0xff;
+	hdr->transport_status = host_byte(scsi_result);
 	hdr->driver_status = 0;
-	if (scsi_status_is_check_condition(scmd->result))
+	if (scsi_status_is_check_condition(scsi_result))
 		hdr->driver_status = DRIVER_SENSE;
 	hdr->info = 0;
 	if (hdr->device_status || hdr->transport_status || hdr->driver_status)
 		hdr->info |= SG_INFO_CHECK;
 	hdr->response_len = 0;
 
-	if (scmd->sense_len && hdr->response) {
+	if (sense_len && hdr->response) {
 		int len = min_t(unsigned int, hdr->max_response_len,
-				scmd->sense_len);
+				sense_len);
 
-		if (copy_to_user(uptr64(hdr->response), scmd->sense_buffer,
+		if (copy_to_user(uptr64(hdr->response), sense_buffer,
 				 len))
 			ret = -EFAULT;
 		else
 			hdr->response_len = len;
 	}
 
-	if (rq_data_dir(rq) == READ)
-		hdr->din_resid = scmd->resid_len;
+	if (is_read)
+		hdr->din_resid = resid_len;
 	else
-		hdr->dout_resid = scmd->resid_len;
+		hdr->dout_resid = resid_len;
 
 	blk_rq_unmap_user(bio);
+	return ret;
 
 out_put_request:
 	blk_mq_free_request(rq);
-- 
2.43.0

[PATCH 4/6] scsi: bsg: report synchronous data copyback failures

[Wang Yingjie]

From: Yingjie Wang <x-professor@outlook.com>

scsi_bsg_sg_io_fn() ignores the result of blk_rq_unmap_user().  A
successful command can therefore return success even though copying READ
bounce data to userspace failed.

Preserve an earlier sense-buffer copy error and otherwise return the
data unmap error.

A virtio-scsi INQUIRY with an unaligned PROT_NONE destination returned
false success in 20/20 runs before this change and -EFAULT in 20/20 runs
afterward.  Writable controls continued to succeed.

Fixes: 10e8855b9451 ("bsg: add SG_IO to SG v4")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Yingjie Wang <x-professor@outlook.com>
---
 drivers/scsi/scsi_bsg.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_bsg.c b/drivers/scsi/scsi_bsg.c
index 4937bc494f79..f95dc5da7480 100644
--- a/drivers/scsi/scsi_bsg.c
+++ b/drivers/scsi/scsi_bsg.c
@@ -203,7 +203,7 @@ static int scsi_bsg_sg_io_fn(struct request_queue *q, struct sg_io_v4 *hdr,
 	u32 scsi_result, resid_len;
 	u8 sense_len;
 	bool is_read;
-	int ret;
+	int ret, unmap_ret;
 
 	if (hdr->protocol != BSG_PROTOCOL_SCSI  ||
 	    hdr->subprotocol != BSG_SUB_PROTOCOL_SCSI_CMD)
@@ -285,7 +285,9 @@ static int scsi_bsg_sg_io_fn(struct request_queue *q, struct sg_io_v4 *hdr,
 	else
 		hdr->dout_resid = resid_len;
 
-	blk_rq_unmap_user(bio);
+	unmap_ret = blk_rq_unmap_user(bio);
+	if (!ret)
+		ret = unmap_ret;
 	return ret;
 
 out_put_request:
-- 
2.43.0

[PATCH 5/6] scsi: bsg: release io_uring request before user copyback

[Wang Yingjie]

From: Yingjie Wang <x-professor@outlook.com>

The BSG io_uring end-I/O callback returns RQ_END_IO_NONE, so task work
still owns the completed request and its tag.  Task work then copies
READ bounce data and optional sense data to userspace.  A fault on the
same one-tag device deadlocks nested I/O behind the retained request.

Snapshot the result, residual, and sense bytes into local storage, then
free the request before either userspace copy.

A current-mainline BSG INQUIRY into a nonresident mapping of the same
/dev/sr0 queue previously stuck in scsi_bsg_uring_task_cb ->
blk_rq_unmap_user -> filemap_fault.  The fixed kernel completed 20/20
repetitions and left no blocked I/O or hung-task report.

Fixes: 7b6d3255e7f8 ("scsi: bsg: add io_uring passthrough handler")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Yingjie Wang <x-professor@outlook.com>
---
 drivers/scsi/scsi_bsg.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/scsi_bsg.c b/drivers/scsi/scsi_bsg.c
index f95dc5da7480..341142dda71d 100644
--- a/drivers/scsi/scsi_bsg.c
+++ b/drivers/scsi/scsi_bsg.c
@@ -35,32 +35,37 @@ static void scsi_bsg_uring_task_cb(struct io_tw_req tw_req, io_tw_token_t tw)
 	struct scsi_bsg_uring_cmd_pdu *pdu = scsi_bsg_uring_cmd_pdu(ioucmd);
 	struct request *rq = pdu->req;
 	struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(rq);
+	u8 sense_buffer[SCSI_SENSE_BUFFERSIZE];
+	u32 scsi_result = scmd->result;
+	u32 resid_len = scmd->resid_len;
 	u64 res2;
 	int ret = 0;
 	u8 driver_status = 0;
 	u8 sense_len_wr = 0;
 
-	if (pdu->bio)
-		blk_rq_unmap_user(pdu->bio);
-
-	if (scsi_status_is_check_condition(scmd->result)) {
+	if (scsi_status_is_check_condition(scsi_result)) {
 		driver_status = DRIVER_SENSE;
 		if (pdu->response_addr)
 			sense_len_wr = min_t(unsigned int, pdu->max_response_len,
 					     scmd->sense_len);
 	}
+	if (sense_len_wr)
+		memcpy(sense_buffer, scmd->sense_buffer, sense_len_wr);
+
+	blk_mq_free_request(rq);
+	if (pdu->bio)
+		blk_rq_unmap_user(pdu->bio);
 
 	if (sense_len_wr) {
-		if (copy_to_user(uptr64(pdu->response_addr), scmd->sense_buffer,
+		if (copy_to_user(uptr64(pdu->response_addr), sense_buffer,
 				 sense_len_wr))
 			ret = -EFAULT;
 	}
 
-	res2 = bsg_scsi_res2_build(status_byte(scmd->result), driver_status,
-				  host_byte(scmd->result), sense_len_wr,
-				  scmd->resid_len);
+	res2 = bsg_scsi_res2_build(status_byte(scsi_result),
+				   driver_status, host_byte(scsi_result),
+				   sense_len_wr, resid_len);
 
-	blk_mq_free_request(rq);
 	io_uring_cmd_done32(ioucmd, ret, res2,
 			    IO_URING_CMD_TASK_WORK_ISSUE_FLAGS);
 }
-- 
2.43.0

[PATCH 6/6] scsi: bsg: report io_uring data copyback failures

[Wang Yingjie]

From: Yingjie Wang <x-professor@outlook.com>

scsi_bsg_uring_task_cb() discards the return from blk_rq_unmap_user().
A successful data-in command can therefore post CQE 0 even when bounce
data was not delivered to userspace.

Use the unmap result as the CQE status.  A later sense-buffer copy
failure remains -EFAULT as before.

A raw BSG INQUIRY into an unaligned anonymous PROT_NONE mapping posted
false success in 20/20 runs before this change and -EFAULT in 20/20 runs
afterward.  Writable controls continued to succeed.

Fixes: 7b6d3255e7f8 ("scsi: bsg: add io_uring passthrough handler")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Yingjie Wang <x-professor@outlook.com>
---
 drivers/scsi/scsi_bsg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_bsg.c b/drivers/scsi/scsi_bsg.c
index 341142dda71d..f776c4a74fb3 100644
--- a/drivers/scsi/scsi_bsg.c
+++ b/drivers/scsi/scsi_bsg.c
@@ -54,7 +54,7 @@ static void scsi_bsg_uring_task_cb(struct io_tw_req tw_req, io_tw_token_t tw)
 
 	blk_mq_free_request(rq);
 	if (pdu->bio)
-		blk_rq_unmap_user(pdu->bio);
+		ret = blk_rq_unmap_user(pdu->bio);
 
 	if (sense_len_wr) {
 		if (copy_to_user(uptr64(pdu->response_addr), sense_buffer,
-- 
2.43.0

Re: [PATCH 0/6] scsi: make SG_IO user copyback failure-safe

[Bart Van Assche]

On 10/18/26 8:41 AM, Wang Yingjie wrote:
> Several SCSI passthrough completion paths touch userspace while their
> completed request still owns a queue tag.  Sense or READ-bounce copyback
> can fault on a mapping backed by the same one-tag device, leaving the
> nested I/O unable to obtain the retained tag.  The synchronous and
> io_uring BSG paths additionally discard data-copyback failures.
How common are one-tag devices? Has it been considered to disable SG_IO
for one-tag devices instead of complicating the SG_IO implementation?

Thanks,

Bart.

Re: [PATCH 0/6] scsi: make SG_IO user copyback failure-safe

[Wang Yingjie]

Hi Bart,

> How common are one-tag devices?

I don't have deployment figures, but usb-storage sets .can_queue = 1 in
usb_stor_host_template. The QEMU reproducer used an IDE CD-ROM through
ata_piix, whose host template also has .can_queue = 1.

> Has it been considered to disable SG_IO
> for one-tag devices instead of complicating the SG_IO implementation?

My concern with disabling SG_IO is that it would also reject working
passthrough commands on those devices. For example, smartctl uses SG_IO
on block devices for SCSI/SAT commands. The lifetime changes preserve that
interface by saving the request-backed completion data, freeing the
completed request, and then doing the potentially faulting user copies.
That does require extra storage for the completion results and sense data.

Separately, patches 4/6 and 6/6 fix lost data-copyback errors and do not
depend on the queue depth. I can split those out if that helps review.

Thanks,
Yingjie