2012-07-17 16:21:34

by Malovany, Ram

[permalink] [raw]
Subject: [PATCH v3] Bluetooth: Device Scan and connection collision fix

From: Ram Malovany <[email protected]>

During search of devices, HCI Remote Name Request Command is sent for
every device which name was not included in inquiry result. But the
same command is also sent during incoming connection establishing
procedure. Function hci_check_pending_name() was fixed in order to
handle this situation which led to a kernel crash when initiating
an incoming connection from a device that was not found in the
inquiry while doing a search. There is no need to continue resolving
the next name if we get the request from the incoming connection
procedure as it will be done upon receiving another remote name
request complete event

Signed-off-by: Ram Malovany <[email protected]>
---
net/bluetooth/hci_event.c | 23 +++++++++++++++--------
1 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 41ff978..7a4f9e8 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1365,7 +1365,7 @@ static bool hci_resolve_next_name(struct hci_dev *hdev)
return false;

e = hci_inquiry_cache_lookup_resolve(hdev, BDADDR_ANY, NAME_NEEDED);
- if (hci_resolve_name(hdev, e) == 0) {
+ if (e && hci_resolve_name(hdev, e) == 0) {
e->name_state = NAME_PENDING;
return true;
}
@@ -1379,6 +1379,9 @@ static void hci_check_pending_name(struct hci_dev *hdev, struct hci_conn *conn,
struct discovery_state *discov = &hdev->discovery;
struct inquiry_entry *e;

+ BT_DBG("name %p, name_len %d, discov->state %d",
+ name, name_len, discov->state);
+
if (conn && !test_and_set_bit(HCI_CONN_MGMT_CONNECTED, &conn->flags))
mgmt_device_connected(hdev, bdaddr, ACL_LINK, 0x00, 0, name,
name_len, conn->dev_class);
@@ -1393,13 +1396,17 @@ static void hci_check_pending_name(struct hci_dev *hdev, struct hci_conn *conn,
return;

e = hci_inquiry_cache_lookup_resolve(hdev, bdaddr, NAME_PENDING);
- if (e) {
+
+ if (!e)
+ return;
+
+ list_del(&e->list);
+ if (name) {
e->name_state = NAME_KNOWN;
- list_del(&e->list);
- if (name)
- mgmt_remote_name(hdev, bdaddr, ACL_LINK, 0x00,
- e->data.rssi, name, name_len);
- }
+ mgmt_remote_name(hdev, bdaddr, ACL_LINK, 0x00,
+ e->data.rssi, name, name_len);
+ } else
+ e->name_state = NAME_NOT_KNOWN;

if (hci_resolve_next_name(hdev))
return;
@@ -1996,7 +2003,7 @@ static void hci_remote_name_evt(struct hci_dev *hdev, struct sk_buff *skb)
struct hci_ev_remote_name *ev = (void *) skb->data;
struct hci_conn *conn;

- BT_DBG("%s", hdev->name);
+ BT_DBG("%s, status 0x%2.2x", hdev->name, ev->status);

hci_conn_check_pending(hdev);

--
1.7.4.1



2012-07-18 07:18:14

by Malovany, Ram

[permalink] [raw]
Subject: RE: [PATCH v3] Bluetooth: Device Scan and connection collision fix

HI Andrei,

> -----Original Message-----
> From: Andrei Emeltchenko [mailto:[email protected]]
> Sent: Wednesday, July 18, 2012 10:14 AM
> To: Malovany, Ram; [email protected]
> Subject: Re: [PATCH v3] Bluetooth: Device Scan and connection collision fix
>
> On Wed, Jul 18, 2012 at 10:11:50AM +0300, Andrei Emeltchenko wrote:
> > Hi Ram,
> >
> > On Tue, Jul 17, 2012 at 07:21:34PM +0300, [email protected] wrote:
> > > From: Ram Malovany <[email protected]>
> > >
> > > During search of devices, HCI Remote Name Request Command is sent for
> > > every device which name was not included in inquiry result. But the
> > > same command is also sent during incoming connection establishing
> > > procedure. Function hci_check_pending_name() was fixed in order to
> > > handle this situation which led to a kernel crash when initiating
> > > an incoming connection from a device that was not found in the
> > > inquiry while doing a search. There is no need to continue resolving
> > > the next name if we get the request from the incoming connection
> > > procedure as it will be done upon receiving another remote name
> > > request complete event
>
> You can also change subject
> from:
> [PATCH v3] Bluetooth: Device Scan and connection collision fix
> to:
> [PATCH v3] Bluetooth: Fix Device Scan and connection collision
>
> Best regards
> Andrei Emeltchenko

Thanks , will send a new update.

Regards,

Ram


2012-07-18 07:13:39

by Andrei Emeltchenko

[permalink] [raw]
Subject: Re: [PATCH v3] Bluetooth: Device Scan and connection collision fix

On Wed, Jul 18, 2012 at 10:11:50AM +0300, Andrei Emeltchenko wrote:
> Hi Ram,
>
> On Tue, Jul 17, 2012 at 07:21:34PM +0300, [email protected] wrote:
> > From: Ram Malovany <[email protected]>
> >
> > During search of devices, HCI Remote Name Request Command is sent for
> > every device which name was not included in inquiry result. But the
> > same command is also sent during incoming connection establishing
> > procedure. Function hci_check_pending_name() was fixed in order to
> > handle this situation which led to a kernel crash when initiating
> > an incoming connection from a device that was not found in the
> > inquiry while doing a search. There is no need to continue resolving
> > the next name if we get the request from the incoming connection
> > procedure as it will be done upon receiving another remote name
> > request complete event

You can also change subject
from:
[PATCH v3] Bluetooth: Device Scan and connection collision fix
to:
[PATCH v3] Bluetooth: Fix Device Scan and connection collision

Best regards
Andrei Emeltchenko


2012-07-18 07:11:50

by Andrei Emeltchenko

[permalink] [raw]
Subject: Re: [PATCH v3] Bluetooth: Device Scan and connection collision fix

Hi Ram,

On Tue, Jul 17, 2012 at 07:21:34PM +0300, [email protected] wrote:
> From: Ram Malovany <[email protected]>
>
> During search of devices, HCI Remote Name Request Command is sent for
> every device which name was not included in inquiry result. But the
> same command is also sent during incoming connection establishing
> procedure. Function hci_check_pending_name() was fixed in order to
> handle this situation which led to a kernel crash when initiating
> an incoming connection from a device that was not found in the
> inquiry while doing a search. There is no need to continue resolving
> the next name if we get the request from the incoming connection
> procedure as it will be done upon receiving another remote name
> request complete event
>
> Signed-off-by: Ram Malovany <[email protected]>
> ---
> net/bluetooth/hci_event.c | 23 +++++++++++++++--------
> 1 files changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 41ff978..7a4f9e8 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -1365,7 +1365,7 @@ static bool hci_resolve_next_name(struct hci_dev *hdev)
> return false;
>
> e = hci_inquiry_cache_lookup_resolve(hdev, BDADDR_ANY, NAME_NEEDED);
> - if (hci_resolve_name(hdev, e) == 0) {
> + if (e && hci_resolve_name(hdev, e) == 0) {
> e->name_state = NAME_PENDING;
> return true;
> }
> @@ -1379,6 +1379,9 @@ static void hci_check_pending_name(struct hci_dev *hdev, struct hci_conn *conn,
> struct discovery_state *discov = &hdev->discovery;
> struct inquiry_entry *e;
>
> + BT_DBG("name %p, name_len %d, discov->state %d",
> + name, name_len, discov->state);
> +
> if (conn && !test_and_set_bit(HCI_CONN_MGMT_CONNECTED, &conn->flags))
> mgmt_device_connected(hdev, bdaddr, ACL_LINK, 0x00, 0, name,
> name_len, conn->dev_class);
> @@ -1393,13 +1396,17 @@ static void hci_check_pending_name(struct hci_dev *hdev, struct hci_conn *conn,
> return;
>
> e = hci_inquiry_cache_lookup_resolve(hdev, bdaddr, NAME_PENDING);
> - if (e) {
> +

no space here

> + if (!e)
> + return;
> +
> + list_del(&e->list);
> + if (name) {
> e->name_state = NAME_KNOWN;
> - list_del(&e->list);
> - if (name)
> - mgmt_remote_name(hdev, bdaddr, ACL_LINK, 0x00,
> - e->data.rssi, name, name_len);
> - }
> + mgmt_remote_name(hdev, bdaddr, ACL_LINK, 0x00,
> + e->data.rssi, name, name_len);
> + } else

and braces here (this is coding style)

Otherwise

Reviewed-by: Andrei Emeltchenko <[email protected]>

> + e->name_state = NAME_NOT_KNOWN;
>
> if (hci_resolve_next_name(hdev))
> return;
> @@ -1996,7 +2003,7 @@ static void hci_remote_name_evt(struct hci_dev *hdev, struct sk_buff *skb)
> struct hci_ev_remote_name *ev = (void *) skb->data;
> struct hci_conn *conn;
>
> - BT_DBG("%s", hdev->name);
> + BT_DBG("%s, status 0x%2.2x", hdev->name, ev->status);
>
> hci_conn_check_pending(hdev);
>
> --
> 1.7.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html