Skip to contentSkip to conversation

Bluetooth: enable context analysis

RFC PATCH Superseded Pauli Virtanen 3 participants 9 messages May 16 – Jun 1

Subsystems (2)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover Bluetooth: enable context analysis

Set up compiler context analysis that generate compiler warnings on problems that Clang -Wthread-safety can detect:

https://docs.kernel.org/dev-tools/context-analysis.html

Clang 22, and probably Clang 23 [1] will be required. Sparse locking analysis support was removed in commit 5b63d0ae94ccfd64dcbdb693d88eb3650eb3c64c, this is its successor.

This series enables the analysis and adds minimal annotations to silence some false positives.

Also, one patch to fix what looks like a legitimate locking issue in iso.c.

In future, it probably is a good idea to make more use of it and add __must_hold, __guarded_by etc annotations.

Kernel test robot appears to be checking for these, but not sure in what trees [2]

BlueZ testbot doesn't check these currently but it's possible to add
https://github.com/bluez/action-ci/pull/4

[1] https://lore.kernel.org/all/CANpmjNN4O=W70sAc5gaVkTAFdrGGOW+XBMyuehfz3_QMiT=uCw@mail.gmail.com/
[2] https://lore.kernel.org/all/[email protected]/

Pauli Virtanen (5):
Bluetooth: af_bluetooth: Add minimal context analysis annotations
Bluetooth: hci_core: Add minimal context analysis annotations
Bluetooth: ISO: lock same hdev as what is released
Bluetooth: L2CAP: Add minimal context analysis annotations
Bluetooth: enable context analysis

 drivers/bluetooth/Makefile   |  2 ++
 net/bluetooth/Makefile       |  2 ++
 net/bluetooth/af_bluetooth.c |  7 +++++--
 net/bluetooth/hci_core.c     |  3 +++
 net/bluetooth/iso.c          | 14 ++++++++------
 net/bluetooth/l2cap_sock.c   |  1 +
 6 files changed, 21 insertions(+), 8 deletions(-)
Signature · 1 line
2.54.0
#1 lore raw

RFC PATCH 1/5 Bluetooth: af_bluetooth: Add minimal context analysis annotations

Add minimal compiler context analysis annotations, required for compilation to pass.

---
 net/bluetooth/af_bluetooth.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
net/bluetooth/af_bluetooth.c +5 -2
index 9d68dd86023c..f42467159947 100644
@@ -212,6 +212,7 @@ bool bt_sock_linked(struct bt_sock_list *l, struct sock *s)
 EXPORT_SYMBOL(bt_sock_linked);
 
 void bt_accept_enqueue(struct sock *parent, struct sock *sk, bool bh)
+	__context_unsafe(/* conditional locking */)
 {
 	const struct cred *old_cred;
 	struct pid *old_pid;
@@ -819,7 +820,8 @@ EXPORT_SYMBOL(bt_sock_wait_ready);
 
 #ifdef CONFIG_PROC_FS
 static void *bt_seq_start(struct seq_file *seq, loff_t *pos)
-	__acquires(seq->private->l->lock)
+	__acquires_shared(&((struct bt_sock_list *)
+			    pde_data(file_inode(seq->file)))->lock)
 {
 	struct bt_sock_list *l = pde_data(file_inode(seq->file));
 
@@ -835,7 +837,8 @@ static void *bt_seq_next(struct seq_file *seq, void *v, loff_t *pos)
 }
 
 static void bt_seq_stop(struct seq_file *seq, void *v)
-	__releases(seq->private->l->lock)
+	__releases_shared(&((struct bt_sock_list *)
+			    pde_data(file_inode(seq->file)))->lock)
 {
 	struct bt_sock_list *l = pde_data(file_inode(seq->file));
 
Signature · 1 line
2.54.0
#2 lore raw

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=1095802

---Test result---

Test Summary:

CheckPatch                    PASS      3.56 seconds
GitLint                       FAIL      1.79 seconds
SubjectPrefix                 PASS      0.59 seconds
BuildKernel                   PASS      26.34 seconds
CheckAllWarning               PASS      29.76 seconds
CheckSparse                   PASS      30.21 seconds
BuildKernel32                 PASS      28.37 seconds
TestRunnerSetup               PASS      552.61 seconds
TestRunner_l2cap-tester       PASS      379.41 seconds
TestRunner_iso-tester         PASS      591.09 seconds
TestRunner_bnep-tester        PASS      18.71 seconds
TestRunner_mgmt-tester        PASS      2023.25 seconds
TestRunner_rfcomm-tester      PASS      63.64 seconds
TestRunner_sco-tester         PASS      141.94 seconds
TestRunner_ioctl-tester       PASS      133.61 seconds
TestRunner_mesh-tester        PASS      60.34 seconds
TestRunner_smp-tester         PASS      18.18 seconds
TestRunner_userchan-tester    PASS      19.47 seconds
TestRunner_6lowpan-tester     PASS      51.14 seconds
IncrementalBuild              PASS      32.11 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[RFC,3/5] Bluetooth: ISO: lock same hdev as what is released

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
11: B1 Line exceeds max length (87>80): "net/bluetooth/iso.c:1095:2: warning: releasing mutex 'iso_conn_get_hdev(sk->conn).lock'"

https://github.com/bluez/bluetooth-next/pull/202

---
Regards,
Linux Bluetooth

#3 lore raw

RFC PATCH 2/5 Bluetooth: hci_core: Add minimal context analysis annotations

In reply to Pauli Virtanen

Add minimal compiler context analysis annotations, required for compilation to pass.

compiler-context-analysis.h doesn't have tools to deal with the conditional SRCU locking on return value used here, so just disable the analysis in places.

---
 net/bluetooth/hci_core.c | 3 +++
 1 file changed, 3 insertions(+)
net/bluetooth/hci_core.c +3 -0
index c46c1236ebfa..fa87cb0bcf7b 100644
@@ -65,6 +65,7 @@ static DEFINE_IDA(hci_index_ida);
 /* Get HCI device by index.
  * Device is held on return. */
 static struct hci_dev *__hci_dev_get(int index, int *srcu_index)
+	__context_unsafe(/* conditional locking */)
 {
 	struct hci_dev *hdev = NULL, *d;
 
@@ -92,11 +93,13 @@ struct hci_dev *hci_dev_get(int index)
 }
 
 static struct hci_dev *hci_dev_get_srcu(int index, int *srcu_index)
+	__context_unsafe(/* conditional locking vs return */)
 {
 	return __hci_dev_get(index, srcu_index);
 }
 
 static void hci_dev_put_srcu(struct hci_dev *hdev, int srcu_index)
+	__context_unsafe(/* conditional locking vs return */)
 {
 	srcu_read_unlock(&hdev->srcu, srcu_index);
 	hci_dev_put(hdev);
Signature · 1 line
2.54.0
#4 lore raw

RFC PATCH 3/5 Bluetooth: ISO: lock same hdev as what is released

In reply to Pauli Virtanen

bis may be deleted concurrently after release_sock(sk).

To avoid this, lock the hdev obtained in iso_conn_get_hdev that we have refcount for. Also, obtain refcount on the hci_conn to avoid comparing freed pointer in the race check below.

Addresses valid context analysis warning

net/bluetooth/iso.c:1095:2: warning: releasing mutex 'iso_conn_get_hdev(sk->conn).lock' that was not held [-Wthread-safety-analysis]

Fixes d3413703d5f8b ("Bluetooth: ISO: Add support to bind to trigger PAST")
---
 net/bluetooth/iso.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)
net/bluetooth/iso.c +8 -6
index 7cb2864fe872..34991fc7e0a7 100644
@@ -1027,13 +1027,16 @@ static int iso_sock_rebind_bis(struct sock *sk, struct sockaddr_iso *sa,
 	return err;
 }
 
-static struct hci_dev *iso_conn_get_hdev(struct iso_conn *conn)
+static struct hci_dev *iso_conn_get_hdev(struct iso_conn *conn,
+					 struct hci_conn **hcon)
 {
 	struct hci_dev *hdev = NULL;
 
 	iso_conn_lock(conn);
-	if (conn->hcon)
+	if (conn->hcon) {
 		hdev = hci_dev_hold(conn->hcon->hdev);
+		*hcon = hci_conn_get(conn->hcon);
+	}
 	iso_conn_unlock(conn);
 
 	return hdev;
@@ -1065,18 +1068,16 @@ static int iso_sock_rebind_bc(struct sock *sk, struct sockaddr_iso *sa,
 	if (!bdaddr_type_is_le(sa->iso_bc->bc_bdaddr_type))
 		return -EINVAL;
 
-	hdev = iso_conn_get_hdev(iso_pi(sk)->conn);
+	hdev = iso_conn_get_hdev(iso_pi(sk)->conn, &bis);
 	if (!hdev)
 		return -EINVAL;
 
-	bis = iso_pi(sk)->conn->hcon;
-
 	/* Release the socket before lookups since that requires hci_dev_lock
 	 * which shall not be acquired while holding sock_lock for proper
 	 * ordering.
 	 */
 	release_sock(sk);
-	hci_dev_lock(bis->hdev);
+	hci_dev_lock(hdev);
 	lock_sock(sk);
 
 	if (!iso_pi(sk)->conn || iso_pi(sk)->conn->hcon != bis) {
@@ -1093,6 +1094,7 @@ static int iso_sock_rebind_bc(struct sock *sk, struct sockaddr_iso *sa,
 
 unlock:
 	hci_dev_unlock(hdev);
+	hci_conn_put(bis);
 	hci_dev_put(hdev);
 
 	return err;
Signature · 1 line
2.54.0
#5 lore raw

RFC PATCH 4/5 Bluetooth: L2CAP: Add minimal context analysis annotations

In reply to Pauli Virtanen

Context analysis has what looks like false positive vs. taking conn->lock in l2cap_sock_shutdown().

chan->conn may be deleted concurrently and become NULL while retaking chan->lock, but since chan shall not be reused, chan->conn cannot be replaced by a different l2cap_conn.

---
 net/bluetooth/l2cap_sock.c | 1 +
 1 file changed, 1 insertion(+)
net/bluetooth/l2cap_sock.c +1 -0
index cf590a67d364..bbf883098b7f 100644
@@ -1336,6 +1336,7 @@ static int __l2cap_wait_ack(struct sock *sk, struct l2cap_chan *chan)
 }
 
 static int l2cap_sock_shutdown(struct socket *sock, int how)
+	__context_unsafe(/* complex chan->conn locking */)
 {
 	struct sock *sk = sock->sk;
 	struct l2cap_chan *chan;
Signature · 1 line
2.54.0
#6 lore raw

RFC PATCH 5/5 Bluetooth: enable context analysis

In reply to Pauli Virtanen

Enable compiler context analysis for Bluetooth subsystem and drivers.

---
 drivers/bluetooth/Makefile | 2 ++
 net/bluetooth/Makefile     | 2 ++
 2 files changed, 4 insertions(+)
drivers/bluetooth/Makefile +2 -0
index bafc26250b63..e6b1c1180d1d 100644
@@ -50,3 +50,5 @@ hci_uart-$(CONFIG_BT_HCIUART_AG6XX)	+= hci_ag6xx.o
 hci_uart-$(CONFIG_BT_HCIUART_MRVL)	+= hci_mrvl.o
 hci_uart-$(CONFIG_BT_HCIUART_AML)	+= hci_aml.o
 hci_uart-objs				:= $(hci_uart-y)
+
+CONTEXT_ANALYSIS := y
net/bluetooth/Makefile +2 -0
index 41049b280887..ff466ea97436 100644
@@ -25,3 +25,5 @@ bluetooth-$(CONFIG_BT_MSFTEXT) += msft.o
 bluetooth-$(CONFIG_BT_AOSPEXT) += aosp.o
 bluetooth-$(CONFIG_BT_DEBUGFS) += hci_debugfs.o
 bluetooth-$(CONFIG_BT_SELFTEST) += selftest.o
+
+CONTEXT_ANALYSIS := y
Signature · 1 line
2.54.0
#7 lore raw
In reply to Pauli Virtanen

Hi Pauli,

Quoted 16 lines · Pauli Virtanen

Set up compiler context analysis that generate compiler warnings on problems that Clang -Wthread-safety can detect:

https://docs.kernel.org/dev-tools/context-analysis.html

Clang 22, and probably Clang 23 [1] will be required. Sparse locking analysis support was removed in commit 5b63d0ae94ccfd64dcbdb693d88eb3650eb3c64c, this is its successor.

This series enables the analysis and adds minimal annotations to silence some false positives.

Also, one patch to fix what looks like a legitimate locking issue in iso.c.

Quoted 8 lines

In future, it probably is a good idea to make more use of it and add __must_hold, __guarded_by etc annotations.

Kernel test robot appears to be checking for these, but not sure in what trees [2]

BlueZ testbot doesn't check these currently but it's possible to add
https://github.com/bluez/action-ci/pull/4

I would like to merge the above changes to action-ci but apparently there are some conflicts. Can you fix that? It seems fine as a starting point so we might as well merge it if there is nothing blocking it. Or did you submit it as an RFC because something is still pending?

Quoted 22 lines

[1] https://lore.kernel.org/all/CANpmjNN4O=W70sAc5gaVkTAFdrGGOW+XBMyuehfz3_QMiT=uCw@mail.gmail.com/
[2] https://lore.kernel.org/all/[email protected]/

Pauli Virtanen (5):
Bluetooth: af_bluetooth: Add minimal context analysis annotations
Bluetooth: hci_core: Add minimal context analysis annotations
Bluetooth: ISO: lock same hdev as what is released
Bluetooth: L2CAP: Add minimal context analysis annotations
Bluetooth: enable context analysis

 drivers/bluetooth/Makefile   |  2 ++
 net/bluetooth/Makefile       |  2 ++
 net/bluetooth/af_bluetooth.c |  7 +++++--
 net/bluetooth/hci_core.c     |  3 +++
 net/bluetooth/iso.c          | 14 ++++++++------
 net/bluetooth/l2cap_sock.c   |  1 +
 6 files changed, 21 insertions(+), 8 deletions(-)

--
2.54.0

Signature · 1 line
Luiz Augusto von Dentz
#8 lore raw

Hi Luiz,

1. kesäkuuta 2026 19.42.39 UTC Luiz Augusto von Dentz <[email protected]> kirjoitti:

Quoted 25 lines

Hi Pauli,

Quoted 22 lines · Pauli Virtanen

Set up compiler context analysis that generate compiler warnings on problems that Clang -Wthread-safety can detect:

https://docs.kernel.org/dev-tools/context-analysis.html

Clang 22, and probably Clang 23 [1] will be required. Sparse locking analysis support was removed in commit 5b63d0ae94ccfd64dcbdb693d88eb3650eb3c64c, this is its successor.

This series enables the analysis and adds minimal annotations to silence some false positives.

Also, one patch to fix what looks like a legitimate locking issue in iso.c.

In future, it probably is a good idea to make more use of it and add __must_hold, __guarded_by etc annotations.

Kernel test robot appears to be checking for these, but not sure in what trees [2]

Quoted 8 lines
Quoted 2 lines

BlueZ testbot doesn't check these currently but it's possible to add
https://github.com/bluez/action-ci/pull/4

I would like to merge the above changes to action-ci but apparently there are some conflicts. Can you fix that? It seems fine as a starting point so we might as well merge it if there is nothing blocking it. Or did you submit it as an RFC because something is still pending?

I can fix the conflicts in the action-ci, should have time for that next week.

This series is also RFC mostly since we don't have this in CI.

Quoted 25 lines
Quoted 23 lines

[1] https://lore.kernel.org/all/CANpmjNN4O=W70sAc5gaVkTAFdrGGOW+XBMyuehfz3_QMiT=uCw@mail.gmail.com/
[2] https://lore.kernel.org/all/[email protected]/

Pauli Virtanen (5):
Bluetooth: af_bluetooth: Add minimal context analysis annotations
Bluetooth: hci_core: Add minimal context analysis annotations
Bluetooth: ISO: lock same hdev as what is released
Bluetooth: L2CAP: Add minimal context analysis annotations
Bluetooth: enable context analysis

 drivers/bluetooth/Makefile   |  2 ++
 net/bluetooth/Makefile       |  2 ++
 net/bluetooth/af_bluetooth.c |  7 +++++--
 net/bluetooth/hci_core.c     |  3 +++
 net/bluetooth/iso.c          | 14 ++++++++------
 net/bluetooth/l2cap_sock.c   |  1 +
 6 files changed, 21 insertions(+), 8 deletions(-)

--
2.54.0