[PATCH 1/4] nvme: release passthrough resources before user copyback

[PATCH 1/4] nvme: release passthrough resources before user copyback

[王 颖杰]

blk_rq_unmap_user() may fault while copying READ bounce data to
userspace.  nvme_submit_user_cmd() currently performs that copy while
the completed passthrough request still owns a tag.  If the destination
faults on the same namespace and no other tag is available, the nested
read cannot make progress.

Save the request-backed result, free the request, and finish
command-effect serialization before unmapping the bio.  The saved bio
remains valid independently of the request and can then be copied back
without retaining scarce queue resources or passthrough locks.

With an NVMe I/O queue depth of two, a same-namespace file-backed
destination hung beyond 60 seconds before this change.  It completed
immediately in 20/20 runs afterward, and subsequent normal I/O remained
live.

Fixes: d29ec8241c10 ("nvme: submit internal commands through the block layer")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Yingjie Wang <x-professor@outlook.com>
---
 drivers/nvme/host/ioctl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
index 6539d475009..e45572bc87d 100644
--- a/drivers/nvme/host/ioctl.c
+++ b/drivers/nvme/host/ioctl.c
@@ -221,12 +221,12 @@ static int nvme_submit_user_cmd(struct request_queue *q,
 	ret = nvme_execute_rq(req, false);
 	if (result)
 		*result = le64_to_cpu(nvme_req(req)->result.u64);
-	if (bio)
-		blk_rq_unmap_user(bio);
 	blk_mq_free_request(req);
 
 	if (effects)
 		nvme_passthru_end(ctrl, ns, effects, cmd, ret);
+	if (bio)
+		blk_rq_unmap_user(bio);
 	return ret;
 
 out_free_req:
-- 
2.43.0

Re: [PATCH 1/4] nvme: release passthrough resources before user copyback

[hch]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 2/4] nvme: report synchronous passthrough copyback failures

[王 颖杰]

nvme_submit_user_cmd() discards the return from blk_rq_unmap_user().
For a successful unaligned READ, a failed bounce-buffer copy can
therefore leave the destination unchanged while the ioctl still reports
success.

Keep the real controller status for nvme_passthru_end(), then return the
copyback error only when the command itself succeeded.

A one-block READ into an unaligned PROT_NONE destination returned false
success in 20/20 runs before this change.  It returned -EFAULT in 20/20
runs afterward, while writable controls continued to succeed.

Fixes: d29ec8241c10 ("nvme: submit internal commands through the block layer")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Yingjie Wang <x-professor@outlook.com>
---
 drivers/nvme/host/ioctl.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
index e45572bc87d..9b830641a5b 100644
--- a/drivers/nvme/host/ioctl.c
+++ b/drivers/nvme/host/ioctl.c
@@ -200,7 +200,7 @@ static int nvme_submit_user_cmd(struct request_queue *q,
 	struct request *req;
 	struct bio *bio;
 	u32 effects;
-	int ret;
+	int ret, unmap_ret = 0;
 
 	req = nvme_alloc_user_request(q, cmd, 0, 0);
 	if (IS_ERR(req))
@@ -226,7 +226,9 @@ static int nvme_submit_user_cmd(struct request_queue *q,
 	if (effects)
 		nvme_passthru_end(ctrl, ns, effects, cmd, ret);
 	if (bio)
-		blk_rq_unmap_user(bio);
+		unmap_ret = blk_rq_unmap_user(bio);
+	if (!ret)
+		ret = unmap_ret;
 	return ret;
 
 out_free_req:
-- 
2.43.0

Re: [PATCH 2/4] nvme: report synchronous passthrough copyback failures

[hch]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 3/4] nvme: report io_uring task-work copyback failures

[王 颖杰]

nvme_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 copied to the requested userspace destination.

Preserve an earlier NVMe or block error and otherwise report the unmap
error in the CQE.

A real NVMe READ into an unaligned 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: 456cba386e94 ("nvme: wire-up uring-cmd support for io-passthru on char-device.")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Yingjie Wang <x-professor@outlook.com>
---
 drivers/nvme/host/ioctl.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
index 9b830641a5b..06c3f3232ba 100644
--- a/drivers/nvme/host/ioctl.c
+++ b/drivers/nvme/host/ioctl.c
@@ -439,9 +439,12 @@ static void nvme_uring_task_cb(struct io_tw_req tw_req, io_tw_token_t tw)
 {
 	struct io_uring_cmd *ioucmd = io_uring_cmd_from_tw(tw_req);
 	struct nvme_uring_cmd_pdu *pdu = nvme_uring_cmd_pdu(ioucmd);
+	int ret = 0;
 
 	if (pdu->bio)
-		blk_rq_unmap_user(pdu->bio);
+		ret = blk_rq_unmap_user(pdu->bio);
+	if (!pdu->status)
+		pdu->status = ret;
 	io_uring_cmd_done32(ioucmd, pdu->status, pdu->result,
 			    IO_URING_CMD_TASK_WORK_ISSUE_FLAGS);
 }
-- 
2.43.0

Re: [PATCH 3/4] nvme: report io_uring task-work copyback failures

[hch]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 4/4] nvme: harden local IOPOLL user copyback

[王 颖杰]

The local NVMe IOPOLL completion path performs blk_rq_unmap_user()
inline, ignores a possible copyback failure, and relies on the block
layer to free the request only after the callback returns.

Free the completed request explicitly before touching userspace, return
RQ_END_IO_NONE to transfer that ownership, and report an unmap error
when no earlier command error exists.  This keeps the inline path
consistent with task-work completion without adding task-work overhead.

The completion PDU is stored in io_uring_cmd rather than the request,
so its saved bio, status, and result remain valid after
blk_mq_free_request().

With a real NVMe poll queue and the task-work path already fixed,
1,280/1,280 commands into an unaligned PROT_NONE destination still
posted success before this change.  All 1,280 posted -EFAULT afterward,
while writable controls remained successful.

Fixes: f7bc22ca0d55 ("nvme/io_uring: optimize IOPOLL completions for local ring context")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Yingjie Wang <x-professor@outlook.com>
---
 drivers/nvme/host/ioctl.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
index 06c3f3232ba..e4ea30e9129 100644
--- a/drivers/nvme/host/ioctl.c
+++ b/drivers/nvme/host/ioctl.c
@@ -474,12 +474,18 @@ static enum rq_end_io_ret nvme_uring_cmd_end_io(struct request *req,
 	 */
 	if (blk_rq_is_poll(req) && iob &&
 	    iob->poll_ctx == io_uring_cmd_ctx_handle(ioucmd)) {
+		int ret = 0;
+
+		blk_mq_free_request(req);
 		if (pdu->bio)
-			blk_rq_unmap_user(pdu->bio);
+			ret = blk_rq_unmap_user(pdu->bio);
+		if (!pdu->status)
+			pdu->status = ret;
 		io_uring_cmd_done32(ioucmd, pdu->status, pdu->result, 0);
-	} else {
-		io_uring_cmd_do_in_task_lazy(ioucmd, nvme_uring_task_cb);
+		return RQ_END_IO_NONE;
 	}
+
+	io_uring_cmd_do_in_task_lazy(ioucmd, nvme_uring_task_cb);
 	return RQ_END_IO_FREE;
 }
 
-- 
2.43.0

Re: [PATCH 4/4] nvme: harden local IOPOLL user copyback

[hch]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>