From nobody Sat Apr 11 12:42:51 2026 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id DAE60ECAAD3 for ; Sun, 4 Sep 2022 11:29:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234016AbiIDL3s (ORCPT ); Sun, 4 Sep 2022 07:29:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36096 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229760AbiIDL3o (ORCPT ); Sun, 4 Sep 2022 07:29:44 -0400 Received: from nautica.notk.org (nautica.notk.org [91.121.71.147]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8473613F42 for ; Sun, 4 Sep 2022 04:29:41 -0700 (PDT) Received: by nautica.notk.org (Postfix, from userid 108) id 55280C01E; Sun, 4 Sep 2022 13:29:39 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=codewreck.org; s=2; t=1662290979; bh=TIX6/2ODGmdUhaznVTeEVDy0KsXHFZF3HnjlGjHKsVo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=UWhFuMRBuIfYXNukdKvIkD9MhWXYEi89Bcx0MsF4aspuNy6cA4fUkCg+eya24n14O AqZZ9IHDURV8OdOy5gzm8NORCP0R6g5l5otEBksmRIIgIyh4RAwe5VuRqREeugk1j+ Z4oi+Oz//DapUxYLpDIj1vETo4fdAhJjMfsiSkSlK/UDCoCDU9fWhKxiTkRSXqdJhA aobfBn6kSrxORYbS1vBsgPhGFF71p6JJE737jotEa1XxmxsJUgu8+QH7zSCy9p+rJN jRI78pEZC9lq5YjCX+d6kvoghzS0xX24NrjsStzLUEypPF0ovM1YWELdRR4RkZ5YyQ zPNgAnsSyZ/qg== Received: from odin.codewreck.org (localhost [127.0.0.1]) by nautica.notk.org (Postfix) with ESMTPS id 068A7C009; Sun, 4 Sep 2022 13:29:34 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=codewreck.org; s=2; t=1662290977; bh=TIX6/2ODGmdUhaznVTeEVDy0KsXHFZF3HnjlGjHKsVo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=c5LTk3SoAl/YF5WMb4+StbKrkLP5gvvpiLMAUkeLSrLtCBMZ9Eq1BchCJGs5IwNq0 IHVSVq6LnMw2Jj5gP2wW3Lvj4a+buHY0dzxPWzBBBL/zVqh0eUAnlua/BZ5xYqblyA idLIhBPEQHqvmZ95xbw7xiGflc15yHLL9fNqBhby0tb7sIDR63Y2UyhVbyMzzekRsQ nwi4/jUKHZ22UAFyUUHv7THwq4Kl3kyKwV0segi7cLa+q5SpcCHcX3FkfiFqusr5dk MMKEuyxpk5ERDzd7o09LCNMvVA8tgbx8R8qbyKB5ONfjjkexuJzLrHo31iSS9ml+i+ qnan7Hj0iK6zQ== Received: from localhost (odin.codewreck.org [local]) by odin.codewreck.org (OpenSMTPD) with ESMTPA id a2d2f7aa; Sun, 4 Sep 2022 11:29:32 +0000 (UTC) From: Dominique Martinet To: v9fs-developer@lists.sourceforge.net, Tetsuo Handa Cc: syzkaller-bugs@googlegroups.com, linux_oss@crudebyte.com, linux-kernel@vger.kernel.org, Dominique Martinet , syzbot Subject: [PATCH] net/9p: use a dedicated spinlock for trans_fd Date: Sun, 4 Sep 2022 20:29:28 +0900 Message-Id: <20220904112928.1308799-1-asmadeus@codewreck.org> X-Mailer: git-send-email 2.37.1 In-Reply-To: <2470e028-9b05-2013-7198-1fdad071d999@I-love.SAKURA.ne.jp> References: <2470e028-9b05-2013-7198-1fdad071d999@I-love.SAKURA.ne.jp> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" Shamelessly copying the explanation from Tetsuo Handa's suggested patch[1] (slightly reworded): syzbot is reporting inconsistent lock state in p9_req_put()[2], for p9_tag_remove() from p9_req_put() from IRQ context is using spin_lock_irqsave() on "struct p9_client"->lock but trans_fd (not from IRQ context) is using spin_lock(). Since the locks actually protect different things in client.c and in trans_fd.c, just replace trans_fd.c's lock by a new one specific to the transport instead of using spin_lock_irq* variants everywhere (client.c's protect the idr for tag allocations, while trans_fd.c's protects its own req list and request status field that acts as the transport's state machine) Link: https://lkml.kernel.org/r/2470e028-9b05-2013-7198-1fdad071d999@I-love= .SAKURA.ne.jp [1] Link: https://syzkaller.appspot.com/bug?extid=3D2f20b523930c32c160cc [2] Reported-by: syzbot Reported-by: Tetsuo Handa Signed-off-by: Dominique Martinet --- Tetsuo Handa-san, thank you very much! I'm sorry for not respecting your opinion but it's been a pleasure to have submissions from someone on JST :) Both this and your previous patch only impact trans_fd which I can't test super easily, so while I've sent the patch here I'll only queue it to -next hopefully next week after I've had time to setup a compatible server again... net/9p/trans_fd.c | 42 ++++++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c index ef5760971f1e..5b4807411281 100644 --- a/net/9p/trans_fd.c +++ b/net/9p/trans_fd.c @@ -91,6 +91,7 @@ struct p9_poll_wait { * @mux_list: list link for mux to manage multiple connections (?) * @client: reference to client instance for this connection * @err: error state + * @req_lock: lock protecting req_list and requests statuses * @req_list: accounting for requests which have been sent * @unsent_req_list: accounting for requests that haven't been sent * @rreq: read request @@ -114,6 +115,7 @@ struct p9_conn { struct list_head mux_list; struct p9_client *client; int err; + spinlock_t req_lock; struct list_head req_list; struct list_head unsent_req_list; struct p9_req_t *rreq; @@ -189,10 +191,10 @@ static void p9_conn_cancel(struct p9_conn *m, int err) =20 p9_debug(P9_DEBUG_ERROR, "mux %p err %d\n", m, err); =20 - spin_lock(&m->client->lock); + spin_lock(&m->req_lock); =20 if (m->err) { - spin_unlock(&m->client->lock); + spin_unlock(&m->req_lock); return; } =20 @@ -205,7 +207,7 @@ static void p9_conn_cancel(struct p9_conn *m, int err) list_move(&req->req_list, &cancel_list); } =20 - spin_unlock(&m->client->lock); + spin_unlock(&m->req_lock); =20 list_for_each_entry_safe(req, rtmp, &cancel_list, req_list) { p9_debug(P9_DEBUG_ERROR, "call back req %p\n", req); @@ -360,7 +362,7 @@ static void p9_read_work(struct work_struct *work) if ((m->rreq) && (m->rc.offset =3D=3D m->rc.capacity)) { p9_debug(P9_DEBUG_TRANS, "got new packet\n"); m->rreq->rc.size =3D m->rc.offset; - spin_lock(&m->client->lock); + spin_lock(&m->req_lock); if (m->rreq->status =3D=3D REQ_STATUS_SENT) { list_del(&m->rreq->req_list); p9_client_cb(m->client, m->rreq, REQ_STATUS_RCVD); @@ -369,14 +371,14 @@ static void p9_read_work(struct work_struct *work) p9_debug(P9_DEBUG_TRANS, "Ignore replies associated with a cancelled request\n"); } else { - spin_unlock(&m->client->lock); + spin_unlock(&m->req_lock); p9_debug(P9_DEBUG_ERROR, "Request tag %d errored out while we were reading the reply\n", m->rc.tag); err =3D -EIO; goto error; } - spin_unlock(&m->client->lock); + spin_unlock(&m->req_lock); m->rc.sdata =3D NULL; m->rc.offset =3D 0; m->rc.capacity =3D 0; @@ -454,10 +456,10 @@ static void p9_write_work(struct work_struct *work) } =20 if (!m->wsize) { - spin_lock(&m->client->lock); + spin_lock(&m->req_lock); if (list_empty(&m->unsent_req_list)) { clear_bit(Wworksched, &m->wsched); - spin_unlock(&m->client->lock); + spin_unlock(&m->req_lock); return; } =20 @@ -472,7 +474,7 @@ static void p9_write_work(struct work_struct *work) m->wpos =3D 0; p9_req_get(req); m->wreq =3D req; - spin_unlock(&m->client->lock); + spin_unlock(&m->req_lock); } =20 p9_debug(P9_DEBUG_TRANS, "mux %p pos %d size %d\n", @@ -670,10 +672,10 @@ static int p9_fd_request(struct p9_client *client, st= ruct p9_req_t *req) if (m->err < 0) return m->err; =20 - spin_lock(&client->lock); + spin_lock(&m->req_lock); req->status =3D REQ_STATUS_UNSENT; list_add_tail(&req->req_list, &m->unsent_req_list); - spin_unlock(&client->lock); + spin_unlock(&m->req_lock); =20 if (test_and_clear_bit(Wpending, &m->wsched)) n =3D EPOLLOUT; @@ -688,11 +690,13 @@ static int p9_fd_request(struct p9_client *client, st= ruct p9_req_t *req) =20 static int p9_fd_cancel(struct p9_client *client, struct p9_req_t *req) { + struct p9_trans_fd *ts =3D client->trans; + struct p9_conn *m =3D &ts->conn; int ret =3D 1; =20 p9_debug(P9_DEBUG_TRANS, "client %p req %p\n", client, req); =20 - spin_lock(&client->lock); + spin_lock(&m->req_lock); =20 if (req->status =3D=3D REQ_STATUS_UNSENT) { list_del(&req->req_list); @@ -700,21 +704,24 @@ static int p9_fd_cancel(struct p9_client *client, str= uct p9_req_t *req) p9_req_put(client, req); ret =3D 0; } - spin_unlock(&client->lock); + spin_unlock(&m->req_lock); =20 return ret; } =20 static int p9_fd_cancelled(struct p9_client *client, struct p9_req_t *req) { + struct p9_trans_fd *ts =3D client->trans; + struct p9_conn *m =3D &ts->conn; + p9_debug(P9_DEBUG_TRANS, "client %p req %p\n", client, req); =20 - spin_lock(&client->lock); + spin_lock(&m->req_lock); /* Ignore cancelled request if message has been received * before lock. */ if (req->status =3D=3D REQ_STATUS_RCVD) { - spin_unlock(&client->lock); + spin_unlock(&m->req_lock); return 0; } =20 @@ -723,7 +730,8 @@ static int p9_fd_cancelled(struct p9_client *client, st= ruct p9_req_t *req) */ list_del(&req->req_list); req->status =3D REQ_STATUS_FLSHD; - spin_unlock(&client->lock); + spin_unlock(&m->req_lock); + p9_req_put(client, req); =20 return 0; @@ -832,6 +840,7 @@ static int p9_fd_open(struct p9_client *client, int rfd= , int wfd) =20 client->trans =3D ts; client->status =3D Connected; + spin_lock_init(&ts->conn.req_lock); =20 return 0; =20 @@ -866,6 +875,7 @@ static int p9_socket_open(struct p9_client *client, str= uct socket *csocket) p->wr =3D p->rd =3D file; client->trans =3D p; client->status =3D Connected; + spin_lock_init(&p->conn.req_lock); =20 p->rd->f_flags |=3D O_NONBLOCK; =20 --=20 2.35.1