[PATCH v2 2/3] Bluetooth: L2CAP: add locking annotations for l2cap_chan_lock/unlock

Pauli Virtanen <[email protected]>
Newsgroups dev.linux.lists.llvm,org.kernel.vger.linux-bluetooth,org.kernel.vger.linux-kernel
Message-ID <a0e6db05b52994c3c27d854e2d35ab0256b93769.1784018377.git.pav@iki.fi>
Add minimal context analysis annotations to l2cap_chan_lock/unlock() and
callers required for no warnings.

Signed-off-by: Pauli Virtanen <[email protected]>
---

Notes:
    v2:
    - split code changes and adding annotations to separate patches
    
    Possibly the l2cap_ops::alloc_skb callback should have generally
    __must_hold(&chan->lock) and not just in l2cap_sock.c
    
    This would imply l2cap_chan_send() should hold the lock, but trying to
    add the annotations reveals l2cap_chan_send is called without chan->lock
    from some places:
    
    net/bluetooth/smp.c:589:2: warning: calling function 'l2cap_chan_send' requires holding mutex 'conn->smp->lock' exclusively [-Wthread-safety-analysis]
      589 |         l2cap_chan_send(chan, &msg, 1 + len, NULL);
          |         ^
    1 warning generated.
    net/bluetooth/6lowpan.c:455:8: warning: calling function 'l2cap_chan_send' requires holding mutex 'chan->lock' exclusively [-Wthread-safety-analysis]
      455 |         err = l2cap_chan_send(chan, &msg, skb->len, NULL);
          |               ^
    
    Don't know now if the context in these places allows l2cap_chan_lock()

 include/net/bluetooth/l2cap.h | 2 ++
 net/bluetooth/l2cap_core.c    | 1 +
 net/bluetooth/l2cap_sock.c    | 1 +
 3 files changed, 4 insertions(+)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index ef6ce1c20a4f..53a68fc32f10 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -825,11 +825,13 @@ struct l2cap_chan *l2cap_chan_hold_unless_zero(struct l2cap_chan *c);
 void l2cap_chan_put(struct l2cap_chan *c);
 
 static inline void l2cap_chan_lock(struct l2cap_chan *chan)
+	__acquires(&chan->lock)
 {
 	mutex_lock_nested(&chan->lock, atomic_read(&chan->nesting));
 }
 
 static inline void l2cap_chan_unlock(struct l2cap_chan *chan)
+	__releases(&chan->lock)
 {
 	mutex_unlock(&chan->lock);
 }
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 322a1895c1bd..5a76c348712d 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4078,6 +4078,7 @@ static struct l2cap_chan *l2cap_new_connection(struct l2cap_conn *conn,
 
 static void l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd,
 			  u8 *data, u8 rsp_code)
+	__context_unsafe(/* conditional locking */)
 {
 	struct l2cap_conn_req *req = (struct l2cap_conn_req *) data;
 	struct l2cap_conn_rsp rsp;
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 735167f73f31..4afc5b370b97 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1740,6 +1740,7 @@ static void l2cap_sock_state_change_cb(struct l2cap_chan *chan, int state,
 static struct sk_buff *l2cap_sock_alloc_skb_cb(struct l2cap_chan *chan,
 					       unsigned long hdr_len,
 					       unsigned long len, int nb)
+	__must_hold(&chan->lock)
 {
 	struct sock *sk = chan->data;
 	struct sk_buff *skb;
-- 
2.55.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.