[PATCH v2 0/3] net/9p: safely abort fd RPCs on fatal signals

[PATCH v2 0/3] net/9p: safely abort fd RPCs on fatal signals

[Ze Tan]

Hi,

This series fixes a hung task reported by Syzkaller when a thread blocked
in a 9p fd RPC receives a fatal signal while another thread in the same
group is waiting in coredump_wait(). The interrupted RPC currently enters
the TFLUSH path and can wait indefinitely if the 9p server is no longer
responding, preventing the thread from exiting and the coredump from
completing.

Ze Tan (3):
  net/9p: handle replies racing with interrupted RPCs
  net/9p: prepare fd transports for asynchronous aborts
  net/9p: abort interrupted fd RPCs on fatal signals

 include/net/9p/client.h    |  3 +++
 include/net/9p/transport.h |  4 ++++
 net/9p/client.c            | 35 +++++++++++++++++++++++++++++++++++
 net/9p/trans_fd.c          | 10 +++++++++-
 4 files changed, 51 insertions(+), 1 deletion(-)


base-commit: 58717b2a1365d06c8c64b72aa948541b53fe31eb
-- 
2.43.0

[PATCH v2 1/3] net/9p: handle replies racing with interrupted RPCs

[Ze Tan]

A reply can arrive after io_wait_event_killable() reports an interruption
but before the client starts cancellation. In that case the request is
already complete and must not be flushed or returned as an error.

Recheck REQ_STATUS_RCVD before entering the flush path in both regular and
zero-copy RPCs.

Signed-off-by: Ze Tan <tanze@kylinos.cn>
---
 net/9p/client.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/net/9p/client.c b/net/9p/client.c
index ef64546c6d52..b9860ccb224b 100644
--- a/net/9p/client.c
+++ b/net/9p/client.c
@@ -612,6 +612,11 @@ p9_client_rpc(struct p9_client *c, int8_t type, const char *fmt, ...)
 		err = req->t_err;
 	}
 	if (err == -ERESTARTSYS && c->status == Connected) {
+		if (READ_ONCE(req->status) == REQ_STATUS_RCVD) {
+			err = 0;
+			goto recalc_sigpending;
+		}
+
 		p9_debug(P9_DEBUG_MUX, "flushing\n");
 		sigpending = 1;
 		clear_thread_flag(TIF_SIGPENDING);
@@ -697,6 +702,11 @@ static struct p9_req_t *p9_client_zc_rpc(struct p9_client *c, int8_t type,
 		err = req->t_err;
 	}
 	if (err == -ERESTARTSYS && c->status == Connected) {
+		if (READ_ONCE(req->status) == REQ_STATUS_RCVD) {
+			err = 0;
+			goto recalc_sigpending;
+		}
+
 		p9_debug(P9_DEBUG_MUX, "flushing\n");
 		sigpending = 1;
 		clear_thread_flag(TIF_SIGPENDING);
-- 
2.43.0

[PATCH v2 2/3] net/9p: prepare fd transports for asynchronous aborts

[Ze Tan]

The fd transports keep their request reference while a sent request is on
the receive list. That reference is released when a reply is consumed or
when p9_conn_cancel() tears the connection down, so the caller can stop
waiting without freeing the request or reusing its tag.

Add an explicit transport capability and teach the fd reply path to
consume replies for requests whose caller has aborted. Leave the capability
disabled for transports with different request or DMA lifetime rules.

Signed-off-by: Ze Tan <tanze@kylinos.cn>
---
 include/net/9p/client.h    |  3 +++
 include/net/9p/transport.h |  4 ++++
 net/9p/trans_fd.c          | 10 +++++++++-
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/include/net/9p/client.h b/include/net/9p/client.h
index 55c6cb54bd25..700dcb37c1dc 100644
--- a/include/net/9p/client.h
+++ b/include/net/9p/client.h
@@ -58,6 +58,8 @@ enum p9_trans_status {
  * @REQ_STATUS_UNSENT: request waiting to be sent
  * @REQ_STATUS_SENT: request sent to server
  * @REQ_STATUS_RCVD: response received from server
+ * @REQ_STATUS_ABORTED: caller stopped waiting, but the request keeps its tag
+ *                      reserved until a reply arrives or the transport closes
  * @REQ_STATUS_FLSHD: request has been flushed
  * @REQ_STATUS_ERROR: request encountered an error on the client side
  */
@@ -67,6 +69,7 @@ enum p9_req_status_t {
 	REQ_STATUS_UNSENT,
 	REQ_STATUS_SENT,
 	REQ_STATUS_RCVD,
+	REQ_STATUS_ABORTED,
 	REQ_STATUS_FLSHD,
 	REQ_STATUS_ERROR,
 };
diff --git a/include/net/9p/transport.h b/include/net/9p/transport.h
index a912bbaa862f..93349fe33dff 100644
--- a/include/net/9p/transport.h
+++ b/include/net/9p/transport.h
@@ -34,6 +34,9 @@
  * @supports_vmalloc: set if this transport can work with vmalloc'd buffers
  *                    (non-physically contiguous memory). Transports requiring
  *                    DMA should leave this as false.
+ * @supports_async_abort: set if a sent request remains referenced by the
+ *                        transport until its reply is consumed or the
+ *                        transport is closed
  * @create: member function to create a new connection on this transport
  * @close: member function to discard a connection on this transport
  * @request: member function to issue a request to the transport
@@ -55,6 +58,7 @@ struct p9_trans_module {
 	bool pooled_rbuffers;
 	bool def;		/* this transport should be default */
 	bool supports_vmalloc;	/* can work with vmalloc'd buffers */
+	bool supports_async_abort; /* keeps sent requests alive after caller exits */
 	struct module *owner;
 	int (*create)(struct p9_client *client,
 		      struct fs_context *fc);
diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
index eb685b52aeb2..d5cff8c2c88c 100644
--- a/net/9p/trans_fd.c
+++ b/net/9p/trans_fd.c
@@ -293,7 +293,9 @@ static void p9_read_work(struct work_struct *work)
 			 m, m->rc.size, m->rc.tag);
 
 		m->rreq = p9_tag_lookup(m->client, m->rc.tag);
-		if (!m->rreq || (m->rreq->status != REQ_STATUS_SENT)) {
+		if (!m->rreq ||
+		    (m->rreq->status != REQ_STATUS_SENT &&
+		     m->rreq->status != REQ_STATUS_ABORTED)) {
 			p9_debug(P9_DEBUG_ERROR, "Unexpected packet tag %d\n",
 				 m->rc.tag);
 			err = -EIO;
@@ -332,6 +334,9 @@ static void p9_read_work(struct work_struct *work)
 		if (m->rreq->status == REQ_STATUS_SENT) {
 			list_del(&m->rreq->req_list);
 			p9_client_cb(m->client, m->rreq, REQ_STATUS_RCVD);
+		} else if (m->rreq->status == REQ_STATUS_ABORTED) {
+			list_del(&m->rreq->req_list);
+			p9_client_cb(m->client, m->rreq, REQ_STATUS_ABORTED);
 		} else if (m->rreq->status == REQ_STATUS_FLSHD) {
 			/* Ignore replies associated with a cancelled request. */
 			p9_debug(P9_DEBUG_TRANS,
@@ -996,6 +1001,7 @@ static struct p9_trans_module p9_tcp_trans = {
 	.pooled_rbuffers = false,
 	.def = false,
 	.supports_vmalloc = true,
+	.supports_async_abort = true,
 	.create = p9_fd_create_tcp,
 	.close = p9_fd_close,
 	.request = p9_fd_request,
@@ -1011,6 +1017,7 @@ static struct p9_trans_module p9_unix_trans = {
 	.maxsize = MAX_SOCK_BUF,
 	.def = false,
 	.supports_vmalloc = true,
+	.supports_async_abort = true,
 	.create = p9_fd_create_unix,
 	.close = p9_fd_close,
 	.request = p9_fd_request,
@@ -1026,6 +1033,7 @@ static struct p9_trans_module p9_fd_trans = {
 	.maxsize = MAX_SOCK_BUF,
 	.def = false,
 	.supports_vmalloc = true,
+	.supports_async_abort = true,
 	.create = p9_fd_create,
 	.close = p9_fd_close,
 	.request = p9_fd_request,
-- 
2.43.0

[PATCH v2 3/3] net/9p: abort interrupted fd RPCs on fatal signals

[Ze Tan]

Syzkaller reported a hung task while a thread group was dumping core. One
thread was blocked in p9_client_rpc() on a 9p fd mount while another
thread entered coredump_wait(). The coredump path sent a fatal signal to
the blocked thread and waited for it to exit, but the 9p client sent TFLUSH
and waited for an unresponsive server to acknowledge it.

  INFO: task syz.1.4497:22259 blocked for more than 143 seconds.
  Call trace:
   __switch_to
   __schedule
   schedule
   schedule_timeout
   __wait_for_common
   wait_for_completion_state
   vfs_coredump
   get_signal
   do_notify_resume

Commit 6b4f48728faa ("net/9p: fix infinite loop in p9_client_rpc on fatal
signal") stopped retrying an interrupted TFLUSH when a fatal signal is
pending. The original non-flush RPC can still enter the TFLUSH path first,
which needlessly depends on the server while the task is trying to exit.

For transports that explicitly retain sent requests across an
asynchronous abort, cancel unsent requests as usual and mark sent requests
aborted without issuing TFLUSH. The fd receive or teardown path keeps the
request and tag alive until it releases the transport reference.

Do not enable this path for zero-copy RPCs or transports without the
required lifetime guarantee.

Fixes: 91b8534fa8f5 ("9p: make rpc code common and rework flush code")
Signed-off-by: Ze Tan <tanze@kylinos.cn>
---
 net/9p/client.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/net/9p/client.c b/net/9p/client.c
index b9860ccb224b..47353cf0750d 100644
--- a/net/9p/client.c
+++ b/net/9p/client.c
@@ -538,6 +538,22 @@ static struct p9_req_t *p9_client_prepare_req(struct p9_client *c,
 	return ERR_PTR(err);
 }
 
+static void p9_client_abort(struct p9_client *c, struct p9_req_t *req)
+{
+	/*
+	 * A fatal signal cannot wait for TFLUSH, but a sent request must keep
+	 * its tag until a late reply arrives or the transport is torn down.
+	 */
+	if (!c->trans_mod->supports_async_abort ||
+	    READ_ONCE(req->status) >= REQ_STATUS_RCVD)
+		return;
+
+	if (!c->trans_mod->cancel(c, req))
+		return;
+
+	cmpxchg(&req->status, REQ_STATUS_SENT, REQ_STATUS_ABORTED);
+}
+
 /**
  * p9_client_rpc - issue a request and wait for a response
  * @c: client session
@@ -617,6 +633,15 @@ p9_client_rpc(struct p9_client *c, int8_t type, const char *fmt, ...)
 			goto recalc_sigpending;
 		}
 
+		if (fatal_signal_pending(current) &&
+		    c->trans_mod->supports_async_abort) {
+			p9_debug(P9_DEBUG_MUX, "fatal signal: skip flush\n");
+			p9_client_abort(c, req);
+			if (READ_ONCE(req->status) == REQ_STATUS_RCVD)
+				err = 0;
+			goto recalc_sigpending;
+		}
+
 		p9_debug(P9_DEBUG_MUX, "flushing\n");
 		sigpending = 1;
 		clear_thread_flag(TIF_SIGPENDING);
-- 
2.43.0