[PATCH] net/9p: publish fd cancellation error before status

[PATCH] net/9p: publish fd cancellation error before status

[usualtree]

p9_conn_cancel() publishes REQ_STATUS_ERROR under req_lock, but only
sets req->t_err after dropping the lock. A waiter in p9_client_rpc()
can observe the error status and read the initial zero t_err before
the transport stores the actual error. The zero value skips the
transport error path and leads to parsing an empty response instead
of returning the transport error.

Store t_err while moving both pending and unsent requests to the
cancellation list. Add a write barrier before publishing
REQ_STATUS_ERROR so that the error store is visible before the status.

Add a read barrier after p9_client_rpc() confirms REQ_STATUS_ERROR and
before either the debug log or the error path reads t_err. Keep the
existing post-wait barrier for normal response ordering. The new
barrier covers a signal-interrupted wait where the second status read
is the first to observe ERROR.

Keep callbacks outside req_lock and preserve the existing list cleanup
behavior.

The issue was reproduced in QEMU using an fd transport mount and a
temporary delay after the early status publication. The waiter
observed t_err == 0 before this change and -ECONNRESET after it. The
delay and observation instrumentation are not part of this patch.

Kimi assisted with the reproducer, fix, and validation, based on a
request to find and locally verify a small 9P upstream fix.

Reported-by: Rahul Priolkar <rahul.priolkar@asu.edu>
Closes: https://lore.kernel.org/lkml/20260911092132.3328659-1-rahul.priolkar@asu.edu/
Fixes: 11c10956515b ("9p/fd: fix issue of list_del corruption in p9_fd_cancel()")
Signed-off-by: usualtree <1340399194@qq.com>
---
 net/9p/client.c   |  5 +++++
 net/9p/trans_fd.c | 17 +++++++++++++----
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/net/9p/client.c b/net/9p/client.c
index e63e8c2a85..d68827af8c 100644
--- a/net/9p/client.c
+++ b/net/9p/client.c
@@ -611,6 +611,11 @@ p9_client_rpc(struct p9_client *c, int8_t type, const char *fmt, ...)
 	}
 
 	if (READ_ONCE(req->status) == REQ_STATUS_ERROR) {
+		/*
+		 * Pair with the smp_wmb() in p9_conn_cancel(): once the error
+		 * status is visible, t_err must be visible too.
+		 */
+		smp_rmb();
 		p9_debug(P9_DEBUG_ERROR, "req_status error %d\n", req->t_err);
 		err = req->t_err;
 	}
diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
index 159d57e41e..5bfeaacbb3 100644
--- a/net/9p/trans_fd.c
+++ b/net/9p/trans_fd.c
@@ -172,20 +172,29 @@ static void p9_conn_cancel(struct p9_conn *m, int err)
 
 	list_for_each_entry_safe(req, rtmp, &m->req_list, req_list) {
 		list_move(&req->req_list, &cancel_list);
-		WRITE_ONCE(req->status, REQ_STATUS_ERROR);
+		if (!req->t_err)
+			req->t_err = err;
 	}
 	list_for_each_entry_safe(req, rtmp, &m->unsent_req_list, req_list) {
 		list_move(&req->req_list, &cancel_list);
-		WRITE_ONCE(req->status, REQ_STATUS_ERROR);
+		if (!req->t_err)
+			req->t_err = err;
 	}
 
+	/*
+	 * The error code must be visible before the error status. Pair with
+	 * the read barriers in p9_client_rpc(), including the barrier after
+	 * the error-status check for waits interrupted by a signal.
+	 */
+	smp_wmb();
+	list_for_each_entry(req, &cancel_list, req_list)
+		WRITE_ONCE(req->status, REQ_STATUS_ERROR);
+
 	spin_unlock(&m->req_lock);
 
 	list_for_each_entry_safe(req, rtmp, &cancel_list, req_list) {
 		p9_debug(P9_DEBUG_ERROR, "call back req %p\n", req);
 		list_del(&req->req_list);
-		if (!req->t_err)
-			req->t_err = err;
 		p9_client_cb(m->client, req, REQ_STATUS_ERROR);
 	}
 }
-- 
2.52.0.windows.1

Re: [PATCH] net/9p: publish fd cancellation error before status

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] net/9p: Signed-off-by trailer uses handle instead of real name
--

commit a32079abdb296b813661b18de6c966dc330ce262
Author: usualtree <1340399194@qq.com>

> net/9p: publish fd cancellation error before status
>
> p9_conn_cancel() publishes REQ_STATUS_ERROR under req_lock, but only
> sets req->t_err after dropping the lock. A waiter in p9_client_rpc()
> can observe the error status and read the initial zero t_err before
> the transport stores the actual error. The zero value skips the
> transport error path and leads to parsing an empty response instead
> of returning the transport error.
>
> Store t_err while moving both pending and unsent requests to the
> cancellation list. Add a write barrier before publishing
> REQ_STATUS_ERROR so that the error store is visible before the status.
>
> Add a read barrier after p9_client_rpc() confirms REQ_STATUS_ERROR and
> before either the debug log or the error path reads t_err. Keep the
> existing post-wait barrier for normal response ordering. The new
> barrier covers a signal-interrupted wait where the second status read
> is the first to observe ERROR.
>
> Keep callbacks outside req_lock and preserve the existing list cleanup
> behavior.
>
> The issue was reproduced in QEMU using an fd transport mount and a
> temporary delay after the early status publication. The waiter
> observed t_err == 0 before this change and -ECONNRESET after it. The
> delay and observation instrumentation are not part of this patch.
>
> Kimi assisted with the reproducer, fix, and validation, based on a
> request to find and locally verify a small 9P upstream fix.
>
> Reported-by: Rahul Priolkar <rahul.priolkar@asu.edu>
> Closes: https://lore.kernel.org/lkml/20260911092132.3328659-1-rahul.priolkar@asu.edu/
> Fixes: 11c10956515b ("9p/fd: fix issue of list_del corruption in p9_fd_cancel()")
> Signed-off-by: usualtree <1340399194@qq.com>

[Severity: High]
The Author header and Signed-off-by trailer use an account handle
("usualtree") rather than a real name, which does not meet the Developer's
Certificate of Origin requirements in
Documentation/process/submitting-patches.rst.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_10059585FDBE07269A8A28C34F0D2574C00A@qq.com?part=1