Skip to contentSkip to conversation

bluetooth: Don't check for SMP security too early

PATCH In review Chuck Ebbert 2 participants 2 messages Sep 4, 2015

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 bluetooth: Don't check for SMP security too early

Commit 25ba26539 ("Bluetooth: Fix NULL pointer dereference in smp_conn_security") added a check for NULL SMP, but it was checked too early. It is possible for this function to return success even when that is NULL. Move the check down to just before the variable gets used.

Fixes 25ba26539 ("Bluetooth: Fix NULL pointer dereference in smp_conn_security")

---

NOTE: UNTESTED, no signoff

net/bluetooth/smp.c +6 -6
index ad82324..0510a57 100644
@@ -2311,12 +2311,6 @@ int smp_conn_security(struct hci_conn *hcon, __u8 sec_level)
 	if (!conn)
 		return 1;
 
-	chan = conn->smp;
-	if (!chan) {
-		BT_ERR("SMP security requested but not available");
-		return 1;
-	}
-
 	if (!hci_dev_test_flag(hcon->hdev, HCI_LE_ENABLED))
 		return 1;
 
@@ -2330,6 +2324,12 @@ int smp_conn_security(struct hci_conn *hcon, __u8 sec_level)
 		if (smp_ltk_encrypt(conn, hcon->pending_sec_level))
 			return 0;
 
+	chan = conn->smp;
+	if (!chan) {
+		BT_ERR("SMP security requested but not available");
+		return 1;
+	}
+
 	l2cap_chan_lock(chan);
 
 	/* If SMP is already in progress ignore this request */
#1 lore raw

Hi,

Quoted 11 lines · Chuck Ebbert

Commit 25ba26539 ("Bluetooth: Fix NULL pointer dereference in smp_conn_security") added a check for NULL SMP, but it was checked too early. It is possible for this function to return success even when that is NULL. Move the check down to just before the variable gets used.

Fixes 25ba26539 ("Bluetooth: Fix NULL pointer dereference in smp_conn_security")

---

NOTE: UNTESTED, no signoff

Looks like the exact same fix I just sent myself :) If the fix works (which I think it should) you should of course get the credits since your patch made it to the list a bit before mine. You might want to check the commit message and my other email for a bit deeper analysis of the issue.

Johan