An RFCOMM connect() issued while a BR/EDR link is being authenticated
makes lockdep report a circular dependency, and the reported cycle is a
real AB/BA between rfcomm_mutex and hdev->lock.

rfcomm_security_cfm() is called from the HCI event path, which already
holds hdev->lock:

  hci_rx_work()
    hci_event_packet()
      hci_cc_read_enc_key_size()   [hdev->lock]
        hci_encrypt_cfm()          [hci_cb_list_lock]
          rfcomm_security_cfm()    [rfcomm_mutex]

while an RFCOMM connect() from userspace takes the same two locks the
other way round:

  rfcomm_sock_connect()
    rfcomm_dlc_open()              [rfcomm_mutex]
      __rfcomm_dlc_open()
        rfcomm_session_create()
          kernel_connect()
            l2cap_sock_connect()
              l2cap_chan_connect() [hdev->lock]

  WARNING: possible circular locking dependency detected
  kworker/u131:1/1128 is trying to acquire lock:
  rfcomm_mutex, at: rfcomm_security_cfm+0x31/0x3e0 [rfcomm]
  but task is already holding lock:
  hci_cb_list_lock, at: hci_cc_read_enc_key_size+0x1d2/0xcc0
  Chain exists of:
    rfcomm_mutex --> &hdev->lock --> hci_cb_list_lock

hci_auth_complete_evt() and hci_encrypt_change_evt() reach the callback
the same way.

Both orders have to be seen in the same boot, which is why a BR/EDR
connection alone is not enough to show it: a session set up by the
remote side is created by rfcomm_accept_connection() in krfcommd, which
calls kernel_accept() and never takes hdev->lock under rfcomm_mutex.
Connecting a device that authenticates and encrypts the link and then
calling connect() on an RFCOMM socket towards any address - the connect
does not have to succeed, the order is recorded before the page timeout
- reports it every time.

The callback does not have to run in the HCI event context at all: it
only updates DLC flags and timers that krfcommd consumes in
rfcomm_process_dlcs(), and it already ends with rfcomm_schedule().  So
queue the confirmation instead of taking rfcomm_mutex from the HCI
event path, and let krfcommd apply it under rfcomm_mutex on its next
pass, ahead of session processing.

The queued entry pins both the connection and the controller, and
krfcommd takes hdev->lock while applying it, so the lookup and
hci_conn_check_secure() run in the same context as before.  A session
that was torn down and set up again while the confirmation was queued
runs over a different hci_conn and is skipped.  A confirmation that
cannot be allocated is dropped and the DLC closes on its auth timeout.

Fixes: 759c185d0bbd ("Bluetooth: RFCOMM: serialize security confirmation handling")
Reported-by: Pauli Virtanen <pav@iki.fi>
Closes: https://lore.kernel.org/linux-bluetooth/5e76a95e934e451e7006db28827c2d64af5a88be.camel@iki.fi/
Reported-by: syzbot+74071deb72339c215b2e@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/linux-bluetooth/6a92fadc.08e933ee.dbf97.008f.GAE@google.com/
Cc: stable@vger.kernel.org
Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
---

The commit this fixes is in v7.3-rc1 and is marked for stable, so this
probably wants the bluetooth fixes tree rather than -next.

v1: https://lore.kernel.org/linux-bluetooth/20260902235132.453044-1-mikhail.v.gavrilov@gmail.com/

v2:
 - free queued confirmations from rfcomm_init() and rfcomm_exit() after
   hci_unregister_cb(), instead of at the end of rfcomm_run(); the init
   error path stops the thread before unregistering the callback, so
   the old placement leaked there
 - pin the controller too and take hdev->lock while the confirmation is
   applied, so conn->sec_level is read in the same context as before
 - skip a session that runs over a different hci_conn than the one the
   confirmation was reported for
 - context-analysis annotations for security_cfm_list and
   __rfcomm_security_cfm(); not verified with clang, done by inspection
 - the reproducer below now uses spaces, gitlint tripped over the tabs

Tested on 7.3.0-rc1 with an MT7922 controller (btusb).  Without the
patch the steps above report the inversion on every run.  With v2
applied the reproducer leaves the validator armed and silent
(debug_locks: 1), and a 2.5 hour session with three BR/EDR headsets
(soundcore Liberty 5, FIIO UTWS17, JBL Tour Pro 3), HFP/SCO audio and
AVRCP produced no lockdep report.

The connect() side used for testing, so that it does not depend on which
end sets up the HFP session:

  #include <stdint.h>
  #include <string.h>
  #include <unistd.h>
  #include <sys/socket.h>

  #define BTPROTO_RFCOMM 3

  struct sockaddr_rc {
      unsigned short  rc_family;
      uint8_t         rc_bdaddr[6];   /* little endian */
      uint8_t         rc_channel;
  };

  int main(void)
  {
      struct sockaddr_rc addr = { .rc_family = AF_BLUETOOTH,
                                  .rc_channel = 1 };
      int fd = socket(AF_BLUETOOTH, SOCK_STREAM, BTPROTO_RFCOMM);

      memcpy(addr.rc_bdaddr, "\x55\x44\x33\x22\x11\x00", 6);
      connect(fd, (struct sockaddr *)&addr, sizeof(addr));
      close(fd);
      return 0;
  }

 net/bluetooth/rfcomm/core.c | 178 ++++++++++++++++++++++++++++--------
 1 file changed, 140 insertions(+), 38 deletions(-)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index f7463f092283..246c811dfca1 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -49,6 +49,18 @@ static DEFINE_MUTEX(rfcomm_mutex);
 
 static LIST_HEAD(session_list);
 
+/* Security confirmations handed over from the HCI event handler to krfcommd */
+struct rfcomm_sec_cfm {
+	struct list_head	list;
+	struct hci_dev		*hdev;
+	struct hci_conn		*conn;
+	u8			status;
+	u8			encrypt;
+};
+
+static DEFINE_SPINLOCK(security_cfm_lock);
+static __guarded_by(&security_cfm_lock) LIST_HEAD(security_cfm_list);
+
 static int rfcomm_send_frame(struct rfcomm_session *s, u8 *data, int len);
 static int rfcomm_send_sabm(struct rfcomm_session *s, u8 dlci);
 static int rfcomm_send_disc(struct rfcomm_session *s, u8 dlci);
@@ -2122,6 +2134,117 @@ static void rfcomm_process_sessions(void)
 	rfcomm_unlock();
 }
 
+static void rfcomm_sec_cfm_free(struct rfcomm_sec_cfm *cfm)
+{
+	hci_conn_put(cfm->conn);
+	hci_dev_put(cfm->hdev);
+	kfree(cfm);
+}
+
+static struct hci_conn *rfcomm_session_hcon(struct rfcomm_session *s)
+{
+	struct l2cap_conn *conn = l2cap_pi(s->sock->sk)->chan->conn;
+
+	return conn ? conn->hcon : NULL;
+}
+
+static void __rfcomm_security_cfm(struct rfcomm_sec_cfm *cfm)
+	__must_hold(&rfcomm_mutex)
+{
+	struct rfcomm_session *s;
+	struct rfcomm_dlc *d, *n;
+
+	s = rfcomm_session_get(&cfm->hdev->bdaddr, &cfm->conn->dst);
+	if (!s)
+		return;
+
+	/* The confirmation belongs to the link it was reported for.  A
+	 * session that was torn down and set up again in the meantime runs
+	 * over a different connection and must not be judged by it.
+	 */
+	if (rfcomm_session_hcon(s) != cfm->conn)
+		return;
+
+	list_for_each_entry_safe(d, n, &s->dlcs, list) {
+		if (test_and_clear_bit(RFCOMM_SEC_PENDING, &d->flags)) {
+			rfcomm_dlc_clear_timer(d);
+			if (cfm->status || cfm->encrypt == 0x00) {
+				set_bit(RFCOMM_ENC_DROP, &d->flags);
+				continue;
+			}
+		}
+
+		if (d->state == BT_CONNECTED && !cfm->status &&
+		    cfm->encrypt == 0x00) {
+			if (d->sec_level == BT_SECURITY_MEDIUM) {
+				set_bit(RFCOMM_SEC_PENDING, &d->flags);
+				rfcomm_dlc_set_timer(d, RFCOMM_AUTH_TIMEOUT);
+				continue;
+			} else if (d->sec_level == BT_SECURITY_HIGH ||
+				   d->sec_level == BT_SECURITY_FIPS) {
+				set_bit(RFCOMM_ENC_DROP, &d->flags);
+				continue;
+			}
+		}
+
+		if (!test_and_clear_bit(RFCOMM_AUTH_PENDING, &d->flags))
+			continue;
+
+		if (!cfm->status && hci_conn_check_secure(cfm->conn,
+							  d->sec_level))
+			set_bit(RFCOMM_AUTH_ACCEPT, &d->flags);
+		else
+			set_bit(RFCOMM_AUTH_REJECT, &d->flags);
+	}
+}
+
+static void rfcomm_process_security_cfm(void)
+{
+	struct rfcomm_sec_cfm *cfm, *n;
+	LIST_HEAD(cfm_list);
+
+	spin_lock(&security_cfm_lock);
+	list_splice_init(&security_cfm_list, &cfm_list);
+	spin_unlock(&security_cfm_lock);
+
+	if (list_empty(&cfm_list))
+		return;
+
+	rfcomm_lock();
+
+	list_for_each_entry_safe(cfm, n, &cfm_list, list) {
+		/* Restores the context the callback used to run in, so that
+		 * hci_conn_check_secure() sees a stable sec_level.
+		 */
+		hci_dev_lock(cfm->hdev);
+		__rfcomm_security_cfm(cfm);
+		hci_dev_unlock(cfm->hdev);
+
+		list_del(&cfm->list);
+		rfcomm_sec_cfm_free(cfm);
+	}
+
+	rfcomm_unlock();
+}
+
+/* Drops confirmations that krfcommd will not get to any more.  Called once
+ * the HCI callback is unregistered and the thread is gone.
+ */
+static void rfcomm_flush_security_cfm(void)
+{
+	struct rfcomm_sec_cfm *cfm, *n;
+	LIST_HEAD(cfm_list);
+
+	spin_lock(&security_cfm_lock);
+	list_splice_init(&security_cfm_list, &cfm_list);
+	spin_unlock(&security_cfm_lock);
+
+	list_for_each_entry_safe(cfm, n, &cfm_list, list) {
+		list_del(&cfm->list);
+		rfcomm_sec_cfm_free(cfm);
+	}
+}
+
 static int rfcomm_add_listener(bdaddr_t *ba)
 {
 	struct sockaddr_l2 addr;
@@ -2201,6 +2324,7 @@ static int rfcomm_run(void *unused)
 	while (!kthread_should_stop()) {
 
 		/* Process stuff */
+		rfcomm_process_security_cfm();
 		rfcomm_process_sessions();
 
 		wait_woken(&wait, TASK_INTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT);
@@ -2214,50 +2338,25 @@ static int rfcomm_run(void *unused)
 
 static void rfcomm_security_cfm(struct hci_conn *conn, u8 status, u8 encrypt)
 {
-	struct rfcomm_session *s;
-	struct rfcomm_dlc *d, *n;
+	struct rfcomm_sec_cfm *cfm;
 
 	BT_DBG("conn %p status 0x%02x encrypt 0x%02x", conn, status, encrypt);
 
-	rfcomm_lock();
-
-	s = rfcomm_session_get(&conn->hdev->bdaddr, &conn->dst);
-	if (!s) {
-		rfcomm_unlock();
+	cfm = kmalloc_obj(*cfm);
+	if (!cfm)
 		return;
-	}
-
-	list_for_each_entry_safe(d, n, &s->dlcs, list) {
-		if (test_and_clear_bit(RFCOMM_SEC_PENDING, &d->flags)) {
-			rfcomm_dlc_clear_timer(d);
-			if (status || encrypt == 0x00) {
-				set_bit(RFCOMM_ENC_DROP, &d->flags);
-				continue;
-			}
-		}
-
-		if (d->state == BT_CONNECTED && !status && encrypt == 0x00) {
-			if (d->sec_level == BT_SECURITY_MEDIUM) {
-				set_bit(RFCOMM_SEC_PENDING, &d->flags);
-				rfcomm_dlc_set_timer(d, RFCOMM_AUTH_TIMEOUT);
-				continue;
-			} else if (d->sec_level == BT_SECURITY_HIGH ||
-				   d->sec_level == BT_SECURITY_FIPS) {
-				set_bit(RFCOMM_ENC_DROP, &d->flags);
-				continue;
-			}
-		}
 
-		if (!test_and_clear_bit(RFCOMM_AUTH_PENDING, &d->flags))
-			continue;
-
-		if (!status && hci_conn_check_secure(conn, d->sec_level))
-			set_bit(RFCOMM_AUTH_ACCEPT, &d->flags);
-		else
-			set_bit(RFCOMM_AUTH_REJECT, &d->flags);
-	}
+	/* hci_conn drops its own reference on hdev once it is deleted, so
+	 * both objects are pinned until krfcommd is done with them.
+	 */
+	cfm->hdev = hci_dev_hold(conn->hdev);
+	cfm->conn = hci_conn_get(conn);
+	cfm->status = status;
+	cfm->encrypt = encrypt;
 
-	rfcomm_unlock();
+	spin_lock(&security_cfm_lock);
+	list_add_tail(&cfm->list, &security_cfm_list);
+	spin_unlock(&security_cfm_lock);
 
 	rfcomm_schedule();
 }
@@ -2333,6 +2432,7 @@ static int __init rfcomm_init(void)
 
 unregister:
 	hci_unregister_cb(&rfcomm_cb);
+	rfcomm_flush_security_cfm();
 
 	return err;
 }
@@ -2345,6 +2445,8 @@ static void __exit rfcomm_exit(void)
 
 	kthread_stop(rfcomm_thread);
 
+	rfcomm_flush_security_cfm();
+
 	rfcomm_cleanup_ttys();
 
 	rfcomm_cleanup_sockets();
-- 
2.55.0