2015-09-04 08:56:26

by Chuck Ebbert

[permalink] [raw]
Subject: [PATCH - untested] 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

diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index ad82324..0510a57 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -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 */


2015-09-04 09:30:03

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH - untested] bluetooth: Don't check for SMP security too early

Hi,

On Fri, Sep 04, 2015, Chuck Ebbert wrote:
> 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