* re: Bluetooth: Add secure flag for mgmt_pin_code_req
@ 2011-09-21 7:08 Dan Carpenter
2011-09-21 13:17 ` Rymarkiewicz Waldemar
2011-09-22 5:58 ` [PATCH] Bluetooth: Fix possible NULL pointer dereference Waldemar Rymarkiewicz
0 siblings, 2 replies; 7+ messages in thread
From: Dan Carpenter @ 2011-09-21 7:08 UTC (permalink / raw)
To: waldemar.rymarkiewicz; +Cc: linux-bluetooth
Hello Waldemar Rymarkiewicz,
This is a semi-automatic email about new static checker warnings.
Thu Apr 28 12:07:59 2011 +0200
a770bb5aea84: "Bluetooth: Add secure flag for mgmt_pin_code_req"
Leads to the following Smatch complaint:
net/bluetooth/hci_event.c +2189 hci_pin_code_request_evt()
error: we previously assumed 'conn' could be null (see line 2177)
net/bluetooth/hci_event.c
2176 conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &ev->bdaddr);
2177 if (conn && conn->state == BT_CONNECTED) {
^^^^
conn can be NULL.
2178 hci_conn_hold(conn);
2179 conn->disc_timeout = HCI_PAIRING_TIMEOUT;
2180 hci_conn_put(conn);
2181 }
2182
2183 if (!test_bit(HCI_PAIRABLE, &hdev->flags))
2184 hci_send_cmd(hdev, HCI_OP_PIN_CODE_NEG_REPLY,
2185 sizeof(ev->bdaddr), &ev->bdaddr);
2186 else if (test_bit(HCI_MGMT, &hdev->flags)) {
2187 u8 secure;
2188
2189 if (conn->pending_sec_level == BT_SECURITY_HIGH)
^^^^^^^^^^^^^^^^^^^^^^^
dereferenced unconditionally here.
2190 secure = 1;
2191 else
regards,
dan carpenter
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Bluetooth: Add secure flag for mgmt_pin_code_req
2011-09-21 7:08 Bluetooth: Add secure flag for mgmt_pin_code_req Dan Carpenter
@ 2011-09-21 13:17 ` Rymarkiewicz Waldemar
2011-09-22 5:58 ` [PATCH] Bluetooth: Fix possible NULL pointer dereference Waldemar Rymarkiewicz
1 sibling, 0 replies; 7+ messages in thread
From: Rymarkiewicz Waldemar @ 2011-09-21 13:17 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-bluetooth
> dereferenced unconditionally here.
>
> 2190 secure = 1;
> 2191 else
Obviously, will sent a patch in a while.
Thanks,
/Waldek
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] Bluetooth: Fix possible NULL pointer dereference
2011-09-21 7:08 Bluetooth: Add secure flag for mgmt_pin_code_req Dan Carpenter
2011-09-21 13:17 ` Rymarkiewicz Waldemar
@ 2011-09-22 5:58 ` Waldemar Rymarkiewicz
2011-09-22 9:51 ` Marcel Holtmann
1 sibling, 1 reply; 7+ messages in thread
From: Waldemar Rymarkiewicz @ 2011-09-22 5:58 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Johan Hedberg, padovan, Waldemar Rymarkiewicz
From: Waldemar Rymarkiewicz <waldemar.rymarkiewicz@gmail.com>
Checking conn->pending_sec_level if there is no connection leads to potential
null pointer dereference. Don't process pin_code_request_event at all if no
connection exists.
Signed-off-by: Waldemar Rymarkiewicz <waldemar.rymarkiewicz@gmail.com>
---
net/bluetooth/hci_event.c | 30 ++++++++++++++++--------------
1 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index a520787..41c2562 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2175,24 +2175,26 @@ static inline void hci_pin_code_request_evt(struct hci_dev *hdev, struct sk_buff
hci_dev_lock(hdev);
conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &ev->bdaddr);
- if (conn && conn->state == BT_CONNECTED) {
- hci_conn_hold(conn);
- conn->disc_timeout = HCI_PAIRING_TIMEOUT;
- hci_conn_put(conn);
- }
+ if (conn) {
+ if (conn->state == BT_CONNECTED) {
+ hci_conn_hold(conn);
+ conn->disc_timeout = HCI_PAIRING_TIMEOUT;
+ hci_conn_put(conn);
+ }
- if (!test_bit(HCI_PAIRABLE, &hdev->flags))
- hci_send_cmd(hdev, HCI_OP_PIN_CODE_NEG_REPLY,
+ if (!test_bit(HCI_PAIRABLE, &hdev->flags))
+ hci_send_cmd(hdev, HCI_OP_PIN_CODE_NEG_REPLY,
sizeof(ev->bdaddr), &ev->bdaddr);
- else if (test_bit(HCI_MGMT, &hdev->flags)) {
- u8 secure;
+ else if (test_bit(HCI_MGMT, &hdev->flags)) {
+ u8 secure;
- if (conn->pending_sec_level == BT_SECURITY_HIGH)
- secure = 1;
- else
- secure = 0;
+ if (conn->pending_sec_level == BT_SECURITY_HIGH)
+ secure = 1;
+ else
+ secure = 0;
- mgmt_pin_code_request(hdev->id, &ev->bdaddr, secure);
+ mgmt_pin_code_request(hdev->id, &ev->bdaddr, secure);
+ }
}
hci_dev_unlock(hdev);
--
1.7.6.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] Bluetooth: Fix possible NULL pointer dereference
2011-09-22 5:58 ` [PATCH] Bluetooth: Fix possible NULL pointer dereference Waldemar Rymarkiewicz
@ 2011-09-22 9:51 ` Marcel Holtmann
2011-09-22 10:28 ` Rymarkiewicz Waldemar
0 siblings, 1 reply; 7+ messages in thread
From: Marcel Holtmann @ 2011-09-22 9:51 UTC (permalink / raw)
To: Waldemar Rymarkiewicz
Cc: linux-bluetooth, Johan Hedberg, padovan, Waldemar Rymarkiewicz
Hi Waldemar,
> Checking conn->pending_sec_level if there is no connection leads to potential
> null pointer dereference. Don't process pin_code_request_event at all if no
> connection exists.
>
> Signed-off-by: Waldemar Rymarkiewicz <waldemar.rymarkiewicz@gmail.com>
> ---
> net/bluetooth/hci_event.c | 30 ++++++++++++++++--------------
> 1 files changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index a520787..41c2562 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -2175,24 +2175,26 @@ static inline void hci_pin_code_request_evt(struct hci_dev *hdev, struct sk_buff
> hci_dev_lock(hdev);
>
> conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &ev->bdaddr);
> - if (conn && conn->state == BT_CONNECTED) {
> - hci_conn_hold(conn);
> - conn->disc_timeout = HCI_PAIRING_TIMEOUT;
> - hci_conn_put(conn);
> - }
> + if (conn) {
what is from with this:
if (!conn)
goto unlock;
Regards
Marcel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Bluetooth: Fix possible NULL pointer dereference
2011-09-22 9:51 ` Marcel Holtmann
@ 2011-09-22 10:28 ` Rymarkiewicz Waldemar
2011-09-22 10:58 ` Marcel Holtmann
0 siblings, 1 reply; 7+ messages in thread
From: Rymarkiewicz Waldemar @ 2011-09-22 10:28 UTC (permalink / raw)
To: Marcel Holtmann
Cc: linux-bluetooth, Johan Hedberg, padovan, Waldemar Rymarkiewicz
Marcel,
>> conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK,&ev->bdaddr);
>> - if (conn&& conn->state == BT_CONNECTED) {
>> - hci_conn_hold(conn);
>> - conn->disc_timeout = HCI_PAIRING_TIMEOUT;
>> - hci_conn_put(conn);
>> - }
>> + if (conn) {
>
> what is from with this:
>
> if (!conn)
> goto unlock;
>
Well, the file is mixed in style, thus I chose the style as for nearby
functions. Anyway, if this form is preferable let me know and will update.
/Waldek
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Bluetooth: Fix possible NULL pointer dereference
2011-09-22 10:28 ` Rymarkiewicz Waldemar
@ 2011-09-22 10:58 ` Marcel Holtmann
0 siblings, 0 replies; 7+ messages in thread
From: Marcel Holtmann @ 2011-09-22 10:58 UTC (permalink / raw)
To: Rymarkiewicz Waldemar
Cc: linux-bluetooth, Johan Hedberg, padovan, Waldemar Rymarkiewicz
Hi Waldemar,
> >> conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK,&ev->bdaddr);
> >> - if (conn&& conn->state == BT_CONNECTED) {
> >> - hci_conn_hold(conn);
> >> - conn->disc_timeout = HCI_PAIRING_TIMEOUT;
> >> - hci_conn_put(conn);
> >> - }
> >> + if (conn) {
> >
> > what is from with this:
> >
> > if (!conn)
> > goto unlock;
> >
>
> Well, the file is mixed in style, thus I chose the style as for nearby
> functions. Anyway, if this form is preferable let me know and will update.
for simple code pieces, I don't really mind, but this function is
getting more and more complicated. So less indentation is preferred.
Regards
Marcel
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] Bluetooth : Fix possible NULL pointer dereference
@ 2017-06-17 15:45 Thomas Gagneret
0 siblings, 0 replies; 7+ messages in thread
From: Thomas Gagneret @ 2017-06-17 15:45 UTC (permalink / raw)
To: Marcel Holtmann, Gustavo Padovan, Johan Hedberg, linux-bluetooth
Cc: Thomas Gagneret
If we disconnect a device before completing the connection, connection
will no longer be available in connection list, thus conn will be NULL.
Signed-off-by: Thomas Gagneret <tgagneret@witekio.com>
---
net/bluetooth/hci_conn.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index dc59eae54717..901731c16136 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -723,20 +723,17 @@ static void create_le_conn_complete(struct hci_dev *hdev, u8 status, u16 opcode)
hci_dev_lock(hdev);
conn = hci_lookup_le_connect(hdev);
+ if (!conn)
+ goto done;
if (!status) {
hci_connect_le_scan_cleanup(conn);
- goto done;
+ } else {
+ BT_ERR("HCI request failed to create LE connection: status 0x%2.2x",
+ status);
+ hci_le_conn_failed(conn, status);
}
- BT_ERR("HCI request failed to create LE connection: status 0x%2.2x",
- status);
-
- if (!conn)
- goto done;
-
- hci_le_conn_failed(conn, status);
-
done:
hci_dev_unlock(hdev);
}
--
2.13.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-06-17 15:45 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-21 7:08 Bluetooth: Add secure flag for mgmt_pin_code_req Dan Carpenter
2011-09-21 13:17 ` Rymarkiewicz Waldemar
2011-09-22 5:58 ` [PATCH] Bluetooth: Fix possible NULL pointer dereference Waldemar Rymarkiewicz
2011-09-22 9:51 ` Marcel Holtmann
2011-09-22 10:28 ` Rymarkiewicz Waldemar
2011-09-22 10:58 ` Marcel Holtmann
2017-06-17 15:45 [PATCH] Bluetooth : " Thomas Gagneret
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.