2012-06-08 03:55:13

by Paulo Alcantara

[permalink] [raw]
Subject: [PATCH] Bluetooth: Report correct error codes on disconnect

When sending LE start encryption command to the target device that hasn't
any long-term key, its controller will send a LE change event with status
0x06 (PIN or Key missing), as it is asynchronous event just sent after
the LE command status, the LE disconnect complete event was overwriting
this error code with its error code, thus user space was getting the
unexpected error code.

This patch fixes the exposing of error codes sent from LE change events
to user space on disconnect ones by setting them early in disc_reason so
that hci_disconn_complete_evt() function can decide which error code to
use by checking disc_reason at the lower layer. If the disc_reason is 0,
then hci_disconn_complete_evt() needs to use the event status instead.

Signed-off-by: Paulo Alcantara <[email protected]>
---
include/net/bluetooth/hci_core.h | 6 ++++++
net/bluetooth/hci_event.c | 13 +++++++++++--
net/bluetooth/l2cap_core.c | 10 ++++++++++
3 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 20fd573..5d98ae0 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -358,6 +358,7 @@ extern rwlock_t hci_cb_list_lock;
extern int l2cap_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr);
extern int l2cap_connect_cfm(struct hci_conn *hcon, u8 status);
extern int l2cap_disconn_ind(struct hci_conn *hcon);
+extern void l2cap_set_disconn_reason(struct hci_conn *hcon, u8 reason);
extern int l2cap_disconn_cfm(struct hci_conn *hcon, u8 reason);
extern int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt);
extern int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb,
@@ -781,6 +782,11 @@ static inline int hci_proto_disconn_ind(struct hci_conn *conn)
return l2cap_disconn_ind(conn);
}

+static inline void hci_set_disconn_reason(struct hci_conn *hconn, __u8 reason)
+{
+ l2cap_set_disconn_reason(hconn, reason);
+}
+
static inline void hci_proto_disconn_cfm(struct hci_conn *conn, __u8 reason)
{
switch (conn->type) {
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 47656be..2c7e4a7 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1915,9 +1915,17 @@ static void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
}

if (ev->status == 0) {
+ __u8 reason;
+
if (conn->type == ACL_LINK && conn->flush_key)
hci_remove_link_key(hdev, &conn->dst);
- hci_proto_disconn_cfm(conn, ev->reason);
+
+ reason = hci_proto_disconn_ind(conn);
+ if (reason)
+ hci_proto_disconn_cfm(conn, reason);
+ else
+ hci_proto_disconn_cfm(conn, ev->reason);
+
hci_conn_del(conn);
}

@@ -2054,7 +2062,8 @@ static void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *skb)
clear_bit(HCI_CONN_ENCRYPT_PEND, &conn->flags);

if (ev->status && conn->state == BT_CONNECTED) {
- hci_acl_disconn(conn, HCI_ERROR_AUTH_FAILURE);
+ hci_set_disconn_reason(conn, ev->status);
+ hci_acl_disconn(conn, ev->status);
hci_conn_put(conn);
goto unlock;
}
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index f9bffe3..f2d16e0 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -5335,6 +5335,16 @@ int l2cap_disconn_ind(struct hci_conn *hcon)
return conn->disc_reason;
}

+void l2cap_set_disconn_reason(struct hci_conn *hcon, u8 reason)
+{
+ struct l2cap_conn *conn = hcon->l2cap_data;
+
+ BT_DBG("hcon %p reason %d", hcon, reason);
+
+ if (conn)
+ conn->disc_reason = reason;
+}
+
int l2cap_disconn_cfm(struct hci_conn *hcon, u8 reason)
{
BT_DBG("hcon %p reason %d", hcon, reason);
--
1.7.10.2



2012-06-12 06:59:07

by Paulo Alcantara

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Report correct error codes on disconnect

From: Paulo Alcantara <[email protected]>
Date: Fri, 8 Jun 2012 00:55:13 -0300

> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 47656be..2c7e4a7 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -1915,9 +1915,17 @@ static void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
> }
>
> if (ev->status == 0) {
> + __u8 reason;
> +
> if (conn->type == ACL_LINK && conn->flush_key)
> hci_remove_link_key(hdev, &conn->dst);
> - hci_proto_disconn_cfm(conn, ev->reason);
> +
> + reason = hci_proto_disconn_ind(conn);
> + if (reason)
> + hci_proto_disconn_cfm(conn, reason);
> + else
> + hci_proto_disconn_cfm(conn, ev->reason);
> +

This is check is wrong since disc_reason is _always_ set to
HCI_ERROR_REMOTE_USER_TERM by default and it can be set to
HCI_ERROR_AUTH_FAILURE as well in l2cap_connect_req(), but it is _never_ set to
0 in anywhere else. I still do not know the reason why is always set to
HCI_ERROR_REMOTE_USER_TERM by default, but I've got two "solutions" for handling
this specific issue so far.

Primarily, it's worth to note that what I'm doing here is to avoid overwriting
error codes returned by LE encryption change event and exposing wrong error ones
to user space. BlueZ should handle specific error codes (e.g.: 0x06 "PIN or key
missing") that are returned by LE encrpytion change event, and not enabling auto
connection when there is "PIN or key missing" error code set in this event, for
example.

Well, the "solutions" that I've got are:

1) We can set the disconnect reason to 0 by default and set it accordingly.

2) Or change this check in hci_disconn_complete_evt() function to:

/* If reason was previously set in hci_encrypt_change_evt(), then we need to use
* the reason set in disc_reason instead of the event one.
*/
if (reason != HCI_ERROR_REMOTE_USER_TERM && reason != HCI_ERROR_AUTH_FAILURE)
hci_proto_disconn_cfm(conn, reason);
else
hci_proto_disconn_cfm(conn, ev->reason);

(Although, the 2nd solution looks better and probably correct to me, IMO)

Do you guys have any other solution or comment that I could take into account,
or even any improvement in one of my solutions given above ?

Paulo