scsi: bsg: report data copyback failures
Subsystems (2)
v2 cover scsi: bsg: report data copyback failures
The synchronous and io_uring SCSI BSG completion paths discard the return value from blk_rq_unmap_user(). For data-in commands that use a bounce buffer, unmapping copies the completed data back to userspace. A failure during that copyback is then lost, so SG_IO can return success and io_uring can post a CQE with res == 0 despite the copyback failure.
Propagate the unmap result while preserving existing sense-copy error precedence. The two fixes are independent and can be applied separately.
This is v2 of the two error-propagation patches from the six-patch series posted at:
https://lore.kernel.org/r/[email protected]
Changes in v2:
- Split these two error-propagation fixes from the request-release changes following Bart Van Assche's review.
- Rebase onto 734bef548a96 (Merge tag 'net-next-7.4').
- Recheck the synchronous Fixes tag. The SG_IO v4 completion helper already discarded blk_rq_unmap_user() errors when 10e8855b9451 introduced the ioctl path.
- Rerun the focused build and the synchronous and io_uring QEMU tests on the new base and final trees.
Validation on x86_64 used QEMU 8.2.2 and a virtio-scsi disk. On the base tree, ioctl() returned 0 and the io_uring CQE had res == 0 in 20/20 runs despite the copyback failure. With this series applied, ioctl() returned -1 with errno EFAULT in 20/20 runs, and the io_uring CQE had res == -EFAULT in 20/20 runs. Writable controls succeeded in 20/20 runs in both trees, and the guest logs contained no BUG, WARNING, KASAN, UBSAN, or panic diagnostics.
The base and final trees also completed:
make O=<build-dir> -j24 W=1 CONFIG_WERROR= bzImage
The command used CONFIG_WERROR= because the saved configuration enables WERROR and the full W=1 build reports pre-existing warnings in unrelated files. No warning was emitted for drivers/scsi/scsi_bsg.c.
An AI coding assistant was used to inspect the reviewed series, verify the history and call paths, and prepare this reroll. The code and test results are provided for human review.
Yingjie Wang (2):
scsi: bsg: report synchronous data copyback failures
scsi: bsg: report io_uring data copyback failures
drivers/scsi/scsi_bsg.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
base-commit: 734bef548a96e8774d6eef5dfa2026c6edc4e80b
Signature · 1 line
2.43.0
PATCH v2 1/2 scsi: bsg: report synchronous data copyback failures
scsi_bsg_sg_io_fn() ignores the result of blk_rq_unmap_user(). For a data-in command that uses a bounce buffer, unmapping copies the completed data to userspace. If that copy fails, SG_IO can report success despite the copyback failure.
Capture the unmap status. Preserve an earlier sense-buffer copy error and otherwise return the data copyback error. This matches the error precedence used by scsi_complete_sghdr_rq().
On x86_64, a virtio-scsi INQUIRY with an unaligned PROT_NONE destination made ioctl() return 0 in 20/20 runs before this change and return -1 with errno EFAULT in 20/20 runs afterward. Writable controls succeeded in 20/20 runs on both kernels.
--- drivers/scsi/scsi_bsg.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/scsi/scsi_bsg.c +4 -2
@@ -199,7 +199,7 @@ 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; -int ret; +int ret, unmap_ret;if (hdr->protocol !=BSG_PROTOCOL_SCSI || hdr->subprotocol !=BSG_SUB_PROTOCOL_SCSI_CMD )
@@ -273,7 +273,9 @@ static int scsi_bsg_sg_io_fn(struct request_queue *q, struct sg_io_v4 *hdr,else hdr->dout_resid = scmd->resid_len; -blk_rq_unmap_user (bio); + unmap_ret =blk_rq_unmap_user (bio); +if (!ret) + ret = unmap_ret; out_put_request:blk_mq_free_request (rq);
Signature · 1 line
2.43.0
PATCH v2 2/2 scsi: bsg: report io_uring data copyback failures
In reply to Yingjie Wangscsi_bsg_uring_task_cb() discards the result of blk_rq_unmap_user(). For a data-in command that uses a bounce buffer, the task work can therefore post a CQE with res == 0 despite a copyback failure.
Use the unmap result as the CQE status. A later sense-buffer copy failure continues to take precedence.
On x86_64, a raw BSG INQUIRY into an unaligned anonymous PROT_NONE mapping posted a CQE with res == 0 in 20/20 runs before this change and res == -EFAULT in 20/20 runs afterward. Writable controls succeeded in 20/20 runs on both kernels.
--- drivers/scsi/scsi_bsg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/scsi/scsi_bsg.c +1 -1
@@ -41,7 +41,7 @@ static void scsi_bsg_uring_task_cb(struct io_tw_req tw_req, io_tw_token_t tw)u8 sense_len_wr =0 ;if (pdu->bio) -blk_rq_unmap_user (pdu->bio); + ret =blk_rq_unmap_user (pdu->bio);if (scsi_status_is_check_condition (scmd->result)) { driver_status =DRIVER_SENSE ;
Signature · 1 line
2.43.0
Hi Yingjie,
Quoted 2 lines · Yingjie Wang
Propagate the unmap result while preserving existing sense-copy error precedence. The two fixes are independent and can be applied separately.
For the series:
Yingjie,
Quoted 6 lines
The synchronous and
io_uringSCSI BSG completion paths discard the return value fromblk_rq_unmap_user(). For data-in commands that use a bounce buffer, unmapping copies the completed data back to userspace. A failure during that copyback is then lost, soSG_IOcan return success andio_uringcan post a CQE with res == 0 despite the copyback failure.
Applied to 7.4/scsi-staging, thanks!
Signature · 1 line
Martin K. Petersen