From 1a4f69ef15ec29b213e2b086b2502644e8ef76ee Mon Sep 17 00:00:00 2001 From: Dominique Martinet <asmadeus@codewreck.org> Date: Mon, 5 Dec 2022 21:39:01 +0900 Subject: 9p/client: fix data race on req->status KCSAN reported a race between writing req->status in p9_client_cb and accessing it in p9_client_rpc's wait_event. Accesses to req itself is protected by the data barrier (writing req fields, write barrier, writing status // reading status, read barrier, reading other req fields), but status accesses themselves apparently also must be annotated properly with WRITE_ONCE/READ_ONCE when we access it without locks. Follows: - error paths writing status in various threads all can notify p9_client_rpc, so these all also need WRITE_ONCE - there's a similar read loop in trans_virtio for zc case that also needs READ_ONCE - other reads in trans_fd should be protected by the trans_fd lock and lists state machine, as corresponding writers all are within trans_fd and should be under the same lock. If KCSAN complains on them we likely will have something else to fix as well, so it's better to leave them unmarked and look again if required. Link: https://lkml.kernel.org/r/20221205124756.426350-1-asmadeus@codewreck.org Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org> Suggested-by: Marco Elver <elver@google.com> Acked-by: Marco Elver <elver@google.com> Reviewed-by: Christian Schoenebeck <linux_oss@crudebyte.com> Signed-off-by: Dominique Martinet <asmadeus@codewreck.org> --- net/9p/trans_xen.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'net/9p/trans_xen.c') diff --git a/net/9p/trans_xen.c b/net/9p/trans_xen.c index de2d2ca8819a..9630b1275557 100644 --- a/net/9p/trans_xen.c +++ b/net/9p/trans_xen.c @@ -157,7 +157,7 @@ again: &masked_prod, masked_cons, XEN_9PFS_RING_SIZE(ring)); - p9_req->status = REQ_STATUS_SENT; + WRITE_ONCE(p9_req->status, REQ_STATUS_SENT); virt_wmb(); /* write ring before updating pointer */ prod += size; ring->intf->out_prod = prod; @@ -212,7 +212,7 @@ static void p9_xen_response(struct work_struct *work) dev_warn(&priv->dev->dev, "requested packet size too big: %d for tag %d with capacity %zd\n", h.size, h.tag, req->rc.capacity); - req->status = REQ_STATUS_ERROR; + WRITE_ONCE(req->status, REQ_STATUS_ERROR); goto recv_error; } -- cgit v1.2.3