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 --- 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 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 --- 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 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 --- 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