linux-kernel - [PATCH] 9p/trans_fd: mark concurrent read and writes to p9

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20250308-p9_conn_err_benign_data_race-v1-1-729e57d5832b@iencinas.com>
Date: Sat, 08 Mar 2025 18:47:38 +0100
From: Ignacio Encinas <ignacio@...cinas.com>
To: linux-kernel-mentees@...ts.linux.dev, skhan@...uxfoundation.org, 
 Eric Van Hensbergen <ericvh@...nel.org>, 
 Latchesar Ionkov <lucho@...kov.net>, 
 Dominique Martinet <asmadeus@...ewreck.org>, 
 Christian Schoenebeck <linux_oss@...debyte.com>
Cc: Sishuai Gong <sishuai.system@...il.com>, Marco Elver <elver@...gle.com>, 
 v9fs@...ts.linux.dev, linux-kernel@...r.kernel.org, 
 syzbot+d69a7cc8c683c2cb7506@...kaller.appspotmail.com, 
 syzbot+483d6c9b9231ea7e1851@...kaller.appspotmail.com, 
 Ignacio Encinas <ignacio@...cinas.com>
Subject: [PATCH] 9p/trans_fd: mark concurrent read and writes to
 p9_conn->err

Writes for the error value of a connection are spinlock-protected inside
p9_conn_cancel, but lockless reads are present elsewhere to avoid
performing unnecessary work after an error has been met.

Mark the write and lockless reads to make KCSAN happy. Mark the write as
exclusive following the recommendation in "Lock-Protected Writes with
Lockless Reads" in tools/memory-model/Documentation/access-marking.txt
while we are at it.

Reported-by: syzbot+d69a7cc8c683c2cb7506@...kaller.appspotmail.com
Reported-by: syzbot+483d6c9b9231ea7e1851@...kaller.appspotmail.com
Signed-off-by: Ignacio Encinas <ignacio@...cinas.com>
---
Hello! I noticed these syzbot reports that seem to repeat periodically
and figured I should send a patch. 

The read-paths look very similar to the one changed here [1]. Perhaps it
would make sense to make them the same?

[1] https://lore.kernel.org/all/ZTZtHdqifXlWG8nN@codewreck.org/
---
 net/9p/trans_fd.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
index 196060dc6138af10e99ad04a76ee36a11f770c65..5458e6530084cabeb01d13e9b9a4b1b8f338e494 100644
--- a/net/9p/trans_fd.c
+++ b/net/9p/trans_fd.c
@@ -196,7 +196,8 @@ static void p9_conn_cancel(struct p9_conn *m, int err)
 		return;
 	}
 
-	m->err = err;
+	WRITE_ONCE(m->err, err);
+	ASSERT_EXCLUSIVE_WRITER(m->err);
 
 	list_for_each_entry_safe(req, rtmp, &m->req_list, req_list) {
 		list_move(&req->req_list, &cancel_list);
@@ -283,7 +284,7 @@ static void p9_read_work(struct work_struct *work)
 
 	m = container_of(work, struct p9_conn, rq);
 
-	if (m->err < 0)
+	if (READ_ONCE(m->err) < 0)
 		return;
 
 	p9_debug(P9_DEBUG_TRANS, "start mux %p pos %zd\n", m, m->rc.offset);
@@ -450,7 +451,7 @@ static void p9_write_work(struct work_struct *work)
 
 	m = container_of(work, struct p9_conn, wq);
 
-	if (m->err < 0) {
+	if (READ_ONCE(m->err) < 0) {
 		clear_bit(Wworksched, &m->wsched);
 		return;
 	}
@@ -622,7 +623,7 @@ static void p9_poll_mux(struct p9_conn *m)
 	__poll_t n;
 	int err = -ECONNRESET;
 
-	if (m->err < 0)
+	if (READ_ONCE(m->err) < 0)
 		return;
 
 	n = p9_fd_poll(m->client, NULL, &err);
@@ -673,7 +674,7 @@ static int p9_fd_request(struct p9_client *client, struct p9_req_t *req)
 
 	spin_lock(&m->req_lock);
 
-	if (m->err < 0) {
+	if (READ_ONCE(m->err) < 0) {
 		spin_unlock(&m->req_lock);
 		return m->err;
 	}

---
base-commit: 2a520073e74fbb956b5564818fc5529dcc7e9f0e
change-id: 20250308-p9_conn_err_benign_data_race-2758fe8bbed0

Best regards,
-- 
Ignacio Encinas <ignacio@...cinas.com>