* 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; 6+ 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] 6+ 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; 6+ messages in thread From: Rymarkiewicz Waldemar @ 2011-09-21 13:17 UTC (permalink / raw) To: Dan Carpenter; +Cc: linux-bluetooth@vger.kernel.org > dereferenced unconditionally here. > > 2190 secure = 1; > 2191 else Obviously, will sent a patch in a while. Thanks, /Waldek ^ permalink raw reply [flat|nested] 6+ 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; 6+ 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] 6+ 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; 6+ 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] 6+ 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; 6+ messages in thread From: Rymarkiewicz Waldemar @ 2011-09-22 10:28 UTC (permalink / raw) To: Marcel Holtmann Cc: linux-bluetooth@vger.kernel.org, Johan Hedberg, padovan@profusion.mobi, 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] 6+ 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; 6+ messages in thread From: Marcel Holtmann @ 2011-09-22 10:58 UTC (permalink / raw) To: Rymarkiewicz Waldemar Cc: linux-bluetooth@vger.kernel.org, Johan Hedberg, padovan@profusion.mobi, 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] 6+ messages in thread
end of thread, other threads:[~2011-09-22 10:58 UTC | newest] Thread overview: 6+ 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
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).