2012-06-11 06:19:44

by Vishal Agarwal

[permalink] [raw]
Subject: [PATCH] Bluetooth: Fix sending HCI_Disconnect only after connection

HCI_Disconnet should only be sent after connection is established.
If connection is not yet established and hci_disconnect is called
then disconnection complete will be received with a handle which
does not exist and hence this event will be ignored.
But as mgmt.c will not receive this event, its variable for pending
command is not cleared.This will result in future Disconnect commands
for that BD Address to be blocked with error busy.

Signed-off-by: Vishal Agarwal <[email protected]>
---
net/bluetooth/mgmt.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 958f764..3a857bf 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1598,7 +1598,7 @@ static int disconnect(struct sock *sk, struct hci_dev *hdev, void *data,
else
conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &cp->addr.bdaddr);

- if (!conn) {
+ if (!conn || conn->state == BT_OPEN || conn->state == BT_CLOSED) {
err = cmd_status(sk, hdev->id, MGMT_OP_DISCONNECT,
MGMT_STATUS_NOT_CONNECTED);
goto failed;
--
1.7.0.4



2012-06-12 06:04:47

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Fix sending HCI_Disconnect only after connection

Hi Vishal,

* vishal agarwal <[email protected]> [2012-06-12 11:20:44 +0530]:

> Hi Gustavo,
>
> On 6/12/12, Gustavo Padovan <[email protected]> wrote:
> > Hi Vishal,
> >
> > * Vishal Agarwal <[email protected]> [2012-06-11 11:49:44
> > +0530]:
> >
> >> HCI_Disconnet should only be sent after connection is established.
> >
> > minor: it's HCI_Disconnect here.
> Thanks for mentioning. I will correct it in the next patch.
> >
> >> If connection is not yet established and hci_disconnect is called
> >> then disconnection complete will be received with a handle which
> >> does not exist and hence this event will be ignored.
> >> But as mgmt.c will not receive this event, its variable for pending
> >> command is not cleared.This will result in future Disconnect commands
> >> for that BD Address to be blocked with error busy.
> >>
> >> Signed-off-by: Vishal Agarwal <[email protected]>
> >> ---
> >> net/bluetooth/mgmt.c | 2 +-
> >> 1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> >> index 958f764..3a857bf 100644
> >> --- a/net/bluetooth/mgmt.c
> >> +++ b/net/bluetooth/mgmt.c
> >> @@ -1598,7 +1598,7 @@ static int disconnect(struct sock *sk, struct
> >> hci_dev *hdev, void *data,
> >> else
> >> conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &cp->addr.bdaddr);
> >>
> >> - if (!conn) {
> >> + if (!conn || conn->state == BT_OPEN || conn->state == BT_CLOSED) {
> >
> > You might also want to check for BT_BOUND here, as this is also a pre
> > connect() state.
> >
> I did not see any place where BT_BOUND is used for conn->state. Should I still
> check for BT_BOUND state?

You are totally right, I just get confused. Check for BT_OPEN and BT_CLOSED is
enough.

Gustavo

2012-06-12 05:50:44

by vishal agarwal

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Fix sending HCI_Disconnect only after connection

Hi Gustavo,

On 6/12/12, Gustavo Padovan <[email protected]> wrote:
> Hi Vishal,
>
> * Vishal Agarwal <[email protected]> [2012-06-11 11:49:44
> +0530]:
>
>> HCI_Disconnet should only be sent after connection is established.
>
> minor: it's HCI_Disconnect here.
Thanks for mentioning. I will correct it in the next patch.
>
>> If connection is not yet established and hci_disconnect is called
>> then disconnection complete will be received with a handle which
>> does not exist and hence this event will be ignored.
>> But as mgmt.c will not receive this event, its variable for pending
>> command is not cleared.This will result in future Disconnect commands
>> for that BD Address to be blocked with error busy.
>>
>> Signed-off-by: Vishal Agarwal <[email protected]>
>> ---
>> net/bluetooth/mgmt.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
>> index 958f764..3a857bf 100644
>> --- a/net/bluetooth/mgmt.c
>> +++ b/net/bluetooth/mgmt.c
>> @@ -1598,7 +1598,7 @@ static int disconnect(struct sock *sk, struct
>> hci_dev *hdev, void *data,
>> else
>> conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &cp->addr.bdaddr);
>>
>> - if (!conn) {
>> + if (!conn || conn->state == BT_OPEN || conn->state == BT_CLOSED) {
>
> You might also want to check for BT_BOUND here, as this is also a pre
> connect() state.
>
I did not see any place where BT_BOUND is used for conn->state. Should I still
check for BT_BOUND state?

> Gustavo
> --
> 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
>
Thanks
Vishal

2012-06-12 03:05:22

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Fix sending HCI_Disconnect only after connection

Hi Vishal,

* Vishal Agarwal <[email protected]> [2012-06-11 11:49:44 +0530]:

> HCI_Disconnet should only be sent after connection is established.

minor: it's HCI_Disconnect here.

> If connection is not yet established and hci_disconnect is called
> then disconnection complete will be received with a handle which
> does not exist and hence this event will be ignored.
> But as mgmt.c will not receive this event, its variable for pending
> command is not cleared.This will result in future Disconnect commands
> for that BD Address to be blocked with error busy.
>
> Signed-off-by: Vishal Agarwal <[email protected]>
> ---
> net/bluetooth/mgmt.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 958f764..3a857bf 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -1598,7 +1598,7 @@ static int disconnect(struct sock *sk, struct hci_dev *hdev, void *data,
> else
> conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &cp->addr.bdaddr);
>
> - if (!conn) {
> + if (!conn || conn->state == BT_OPEN || conn->state == BT_CLOSED) {

You might also want to check for BT_BOUND here, as this is also a pre
connect() state.

Gustavo