From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AB43C3D16E8; Mon, 20 Apr 2026 13:18:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776691086; cv=none; b=U8W3u+6is5DZgj98Ze9rzqksZaJSGQ2Us8OXGKHIH7mhz9P8IlymlUTD4VAcmGHzBaEUNlVuZBQYhiQdbQ5Jdwhj2atQNbhgfLRzyrRfBaCNcvq8xXW6v5bhW16DKUn9J6obPkZNYHCYq7l53d+SdxtJhpt3J+CXSzdSY1Ivwew= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776691086; c=relaxed/simple; bh=E9cH7PuQ2b5BSKlFNo0n1LqxRO0ZWNDze2FOxEKZVDo=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=P6m62F8DbbiYUixObV/ftKNMTp8OKtr9XuN9DFhQP5F/55dHdgbYyKWOy8LXBp6AuewDp4M3wR3/QAIZMu/FlM+IhrQ8WZjeOOYQsVTbOHaAqfQhY2jrNJbmUBObC8YcVx1PxW3czUZEu0J50zvN20kDvhsMoTjA3f9a16zXya4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=eLMiVTtQ; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="eLMiVTtQ" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0FABDC2BCB6; Mon, 20 Apr 2026 13:18:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1776691086; bh=E9cH7PuQ2b5BSKlFNo0n1LqxRO0ZWNDze2FOxEKZVDo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=eLMiVTtQmJ11L/b0fSZshHTKo8HXHVMZTmA9zNQTsRay4MzrmFmyuMw8NzpnDl9ug 2m2av9Q75PTrLSmsg5lXfpAqt6NIYKiF6QHPApJShfmaxXKkoZIge88HBkKDE5e/LZ eiYHpbDCZ0ZyluMeQfim0X4cYunTPyfGYpbOOaGxcfvNLbniFI/UZhoN7iObDC6ZoR CblrPPDPHhcGtDkxIpq42mYIhODDVvKP7oN3aNABwU7rkfOD+xp+MctPZ9wyX15HTo HXOakphyD582Bk3tx4rAuIqSnarmr15FyV6UjXr0iU9iOlO6CmFH9w7Tk/+Wy3su2r XP5p+kwP0VtFw== From: Sasha Levin To: patches@lists.linux.dev, stable@vger.kernel.org Cc: Cen Zhang , Luiz Augusto von Dentz , Sasha Levin , marcel@holtmann.org, johan.hedberg@gmail.com, davem@davemloft.net, kuba@kernel.org, linux-bluetooth@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH AUTOSEL 6.18] Bluetooth: hci_sync: annotate data-races around hdev->req_status Date: Mon, 20 Apr 2026 09:09:11 -0400 Message-ID: <20260420131539.986432-85-sashal@kernel.org> X-Mailer: git-send-email 2.53.0 In-Reply-To: <20260420131539.986432-1-sashal@kernel.org> References: <20260420131539.986432-1-sashal@kernel.org> Precedence: bulk X-Mailing-List: stable@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-stable: review X-Patchwork-Hint: Ignore X-stable-base: Linux 6.18.23 Content-Transfer-Encoding: 8bit From: Cen Zhang [ Upstream commit b6807cfc195ef99e1ac37b2e1e60df40295daa8c ] __hci_cmd_sync_sk() sets hdev->req_status under hdev->req_lock: hdev->req_status = HCI_REQ_PEND; However, several other functions read or write hdev->req_status without holding any lock: - hci_send_cmd_sync() reads req_status in hci_cmd_work (workqueue) - hci_cmd_sync_complete() reads/writes from HCI event completion - hci_cmd_sync_cancel() / hci_cmd_sync_cancel_sync() read/write - hci_abort_conn() reads in connection abort path Since __hci_cmd_sync_sk() runs on hdev->req_workqueue while hci_send_cmd_sync() runs on hdev->workqueue, these are different workqueues that can execute concurrently on different CPUs. The plain C accesses constitute a data race. Add READ_ONCE()/WRITE_ONCE() annotations on all concurrent accesses to hdev->req_status to prevent potential compiler optimizations that could affect correctness (e.g., load fusing in the wait_event condition or store reordering). Signed-off-by: Cen Zhang Signed-off-by: Luiz Augusto von Dentz Signed-off-by: Sasha Levin --- LLM Generated explanations, may be completely bogus: Error: Failed to generate final synthesis net/bluetooth/hci_conn.c | 2 +- net/bluetooth/hci_core.c | 2 +- net/bluetooth/hci_sync.c | 20 ++++++++++---------- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index 24b71ec8897ff..71a24be2a6d67 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -2967,7 +2967,7 @@ int hci_abort_conn(struct hci_conn *conn, u8 reason) * hci_connect_le serializes the connection attempts so only one * connection can be in BT_CONNECT at time. */ - if (conn->state == BT_CONNECT && hdev->req_status == HCI_REQ_PEND) { + if (conn->state == BT_CONNECT && READ_ONCE(hdev->req_status) == HCI_REQ_PEND) { switch (hci_skb_event(hdev->sent_cmd)) { case HCI_EV_CONN_COMPLETE: case HCI_EV_LE_CONN_COMPLETE: diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 8ccec73dce45c..0f86b81b39730 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -4125,7 +4125,7 @@ static int hci_send_cmd_sync(struct hci_dev *hdev, struct sk_buff *skb) kfree_skb(skb); } - if (hdev->req_status == HCI_REQ_PEND && + if (READ_ONCE(hdev->req_status) == HCI_REQ_PEND && !hci_dev_test_and_set_flag(hdev, HCI_CMD_PENDING)) { kfree_skb(hdev->req_skb); hdev->req_skb = skb_clone(hdev->sent_cmd, GFP_KERNEL); diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index 9a7bd4a4b14c4..f498ab28f1aa0 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -25,11 +25,11 @@ static void hci_cmd_sync_complete(struct hci_dev *hdev, u8 result, u16 opcode, { bt_dev_dbg(hdev, "result 0x%2.2x", result); - if (hdev->req_status != HCI_REQ_PEND) + if (READ_ONCE(hdev->req_status) != HCI_REQ_PEND) return; hdev->req_result = result; - hdev->req_status = HCI_REQ_DONE; + WRITE_ONCE(hdev->req_status, HCI_REQ_DONE); /* Free the request command so it is not used as response */ kfree_skb(hdev->req_skb); @@ -167,20 +167,20 @@ struct sk_buff *__hci_cmd_sync_sk(struct hci_dev *hdev, u16 opcode, u32 plen, hci_cmd_sync_add(&req, opcode, plen, param, event, sk); - hdev->req_status = HCI_REQ_PEND; + WRITE_ONCE(hdev->req_status, HCI_REQ_PEND); err = hci_req_sync_run(&req); if (err < 0) return ERR_PTR(err); err = wait_event_interruptible_timeout(hdev->req_wait_q, - hdev->req_status != HCI_REQ_PEND, + READ_ONCE(hdev->req_status) != HCI_REQ_PEND, timeout); if (err == -ERESTARTSYS) return ERR_PTR(-EINTR); - switch (hdev->req_status) { + switch (READ_ONCE(hdev->req_status)) { case HCI_REQ_DONE: err = -bt_to_errno(hdev->req_result); break; @@ -194,7 +194,7 @@ struct sk_buff *__hci_cmd_sync_sk(struct hci_dev *hdev, u16 opcode, u32 plen, break; } - hdev->req_status = 0; + WRITE_ONCE(hdev->req_status, 0); hdev->req_result = 0; skb = hdev->req_rsp; hdev->req_rsp = NULL; @@ -665,9 +665,9 @@ void hci_cmd_sync_cancel(struct hci_dev *hdev, int err) { bt_dev_dbg(hdev, "err 0x%2.2x", err); - if (hdev->req_status == HCI_REQ_PEND) { + if (READ_ONCE(hdev->req_status) == HCI_REQ_PEND) { hdev->req_result = err; - hdev->req_status = HCI_REQ_CANCELED; + WRITE_ONCE(hdev->req_status, HCI_REQ_CANCELED); queue_work(hdev->workqueue, &hdev->cmd_sync_cancel_work); } @@ -683,12 +683,12 @@ void hci_cmd_sync_cancel_sync(struct hci_dev *hdev, int err) { bt_dev_dbg(hdev, "err 0x%2.2x", err); - if (hdev->req_status == HCI_REQ_PEND) { + if (READ_ONCE(hdev->req_status) == HCI_REQ_PEND) { /* req_result is __u32 so error must be positive to be properly * propagated. */ hdev->req_result = err < 0 ? -err : err; - hdev->req_status = HCI_REQ_CANCELED; + WRITE_ONCE(hdev->req_status, HCI_REQ_CANCELED); wake_up_interruptible(&hdev->req_wait_q); } -- 2.53.0