[PATCH] Bluetooth: Fix SCO connection reference

[PATCH] Bluetooth: Fix SCO connection reference

[Claudio Takahasi]

This patch fixes decrementing SCO connection reference right after
stablishing the SCO connection with defer setup enabled. The dump below
shows a disconnection command with handle 0, the connection is still in
BT_CONNECT2 state and there isn't a handle associated with it.

< HCI Command: Accept Synchronous Connection (0x01|0x0029) plen 21
  bdaddr 78:47:1D:B3:72:6C
> HCI Event: Command Status (0x0f) plen 4
  Accept Synchronous Connection (0x01|0x0029) status 0x00 ncmd 1
< HCI Command: Disconnect (0x01|0x0006) plen 3
  handle 0 reason 0x13
  Reason: Remote User Terminated Connection
> HCI Event: Command Status (0x0f) plen 4
  Disconnect (0x01|0x0006) status 0x00 ncmd 1
> HCI Event: Synchronous Connect Complete (0x2c) plen 17
  status 0x00 handle 46 bdaddr 78:47:1D:B3:72:6C
  type eSCO
  Air mode: CVSD
< SCO data: handle 46 flags 0x00 dlen 48

Signed-off-by: Claudio Takahasi <claudio.takahasi@openbossa.org>
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
---
 net/bluetooth/hci_event.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 0a2b128..7a05759 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1860,7 +1860,6 @@ static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
 		} else {
 			conn->state = BT_CONNECT2;
 			hci_proto_connect_cfm(conn, 0);
-			hci_conn_put(conn);
 		}
 	} else {
 		/* Connection rejected */
-- 
1.7.11.7

[PATCH] Bluetooth: Add rejecting SCO

[Claudio Takahasi]

This patch sends Reject Synchronous Connection Request Command when
hci_conn_timeout is triggered, and the SCO connection is in BT_CONNECT2
state. It prevents inconsistency if the remote host doesn't implement
properly the timeout for the connection request, and it removes the
connection reference left when the socket is closed for incoming SCO
connections.

[ 2650.129080] sco_sock_release: sock ffff8801ca417400, sk ffff88020c408800
[ 2650.129092] sco_sock_clear_timer: sock ffff88020c408800 state 6
[ 2650.129101] __sco_sock_close: sk ffff88020c408800 state 6 socket
	ffff8801ca417400
[ 2650.129108] sco_chan_del: sk ffff88020c408800, conn ffff8801c650ea20,
	err 104
[ 2650.129114] hci_conn_put: hcon ffff88020c40a800 orig refcnt 1
[ 2650.129128] sco_sock_kill: sk ffff88020c408800 state 9
[ 2650.129135] sco_sock_destruct: sk ffff88020c408800
[ 2650.138468] hci_conn_timeout: hcon ffff88020c40a800 state BT_CONNECT2

Signed-off-by: Claudio Takahasi <claudio.takahasi@openbossa.org>
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
---
 net/bluetooth/hci_conn.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index b9f9016..aed48fe 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -117,6 +117,16 @@ static void hci_acl_create_connection_cancel(struct hci_conn *conn)
 	hci_send_cmd(conn->hdev, HCI_OP_CREATE_CONN_CANCEL, sizeof(cp), &cp);
 }
 
+static void hci_reject_sco(struct hci_conn *conn)
+{
+	struct hci_cp_reject_sync_conn_req cp;
+
+	cp.reason = HCI_ERROR_REMOTE_USER_TERM;
+	bacpy(&cp.bdaddr, &conn->dst);
+
+	hci_send_cmd(conn->hdev, HCI_OP_REJECT_SYNC_CONN_REQ, sizeof(cp), &cp);
+}
+
 void hci_disconnect(struct hci_conn *conn, __u8 reason)
 {
 	struct hci_cp_disconnect cp;
@@ -276,7 +286,8 @@ static void hci_conn_timeout(struct work_struct *work)
 				hci_acl_create_connection_cancel(conn);
 			else if (conn->type == LE_LINK)
 				hci_le_create_connection_cancel(conn);
-		}
+		} else if (conn->type == SCO_LINK || conn->type == ESCO_LINK)
+			hci_reject_sco(conn);
 		break;
 	case BT_CONFIG:
 	case BT_CONNECTED:
-- 
1.7.11.7

Re: [PATCH] Bluetooth: Add rejecting SCO

[Anderson Lizardo]

Hi Claudio,

On Thu, Apr 11, 2013 at 10:35 AM, Claudio Takahasi
<claudio.takahasi@openbossa.org> wrote:
> @@ -276,7 +286,8 @@ static void hci_conn_timeout(struct work_struct *work)
>                                 hci_acl_create_connection_cancel(conn);
>                         else if (conn->type == LE_LINK)
>                                 hci_le_create_connection_cancel(conn);
> -               }
> +               } else if (conn->type == SCO_LINK || conn->type == ESCO_LINK)
> +                       hci_reject_sco(conn);

IIRC for kernel you always use curly braces if one of the if()
branches have it (even if one of the branches has a single line), i.e.
use:

if (...) {
...
} else {
...
}

instead of:

if (...) {
...
} else
...

Best Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

[PATCH v1] Bluetooth: Add rejecting SCO

[Claudio Takahasi]

This patch sends Reject Synchronous Connection Request Command when
hci_conn_timeout is triggered, and the SCO connection is in BT_CONNECT2
state. It prevents inconsistency if the remote host doesn't implement
properly the timeout for the connection request, and it removes the
connection reference left when the socket is closed for incoming SCO
connections.

[ 2650.129080] sco_sock_release: sock ffff8801ca417400, sk ffff88020c408800
[ 2650.129092] sco_sock_clear_timer: sock ffff88020c408800 state 6
[ 2650.129101] __sco_sock_close: sk ffff88020c408800 state 6 socket
	ffff8801ca417400
[ 2650.129108] sco_chan_del: sk ffff88020c408800, conn ffff8801c650ea20,
	err 104
[ 2650.129114] hci_conn_put: hcon ffff88020c40a800 orig refcnt 1
[ 2650.129128] sco_sock_kill: sk ffff88020c408800 state 9
[ 2650.129135] sco_sock_destruct: sk ffff88020c408800
[ 2650.138468] hci_conn_timeout: hcon ffff88020c40a800 state BT_CONNECT2

Signed-off-by: Claudio Takahasi <claudio.takahasi@openbossa.org>
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
---
 net/bluetooth/hci_conn.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 30d7dfc..b1a02ce 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -117,6 +117,16 @@ static void hci_acl_create_connection_cancel(struct hci_conn *conn)
 	hci_send_cmd(conn->hdev, HCI_OP_CREATE_CONN_CANCEL, sizeof(cp), &cp);
 }
 
+static void hci_reject_sco(struct hci_conn *conn)
+{
+	struct hci_cp_reject_sync_conn_req cp;
+
+	cp.reason = HCI_ERROR_REMOTE_USER_TERM;
+	bacpy(&cp.bdaddr, &conn->dst);
+
+	hci_send_cmd(conn->hdev, HCI_OP_REJECT_SYNC_CONN_REQ, sizeof(cp), &cp);
+}
+
 void hci_disconnect(struct hci_conn *conn, __u8 reason)
 {
 	struct hci_cp_disconnect cp;
@@ -276,6 +286,8 @@ static void hci_conn_timeout(struct work_struct *work)
 				hci_acl_create_connection_cancel(conn);
 			else if (conn->type == LE_LINK)
 				hci_le_create_connection_cancel(conn);
+		} else if (conn->type == SCO_LINK || conn->type == ESCO_LINK) {
+			hci_reject_sco(conn);
 		}
 		break;
 	case BT_CONFIG:
-- 
1.7.11.7

Re: [PATCH v1] Bluetooth: Add rejecting SCO

[Gustavo Padovan]

Hi Claudio,

* Claudio Takahasi <claudio.takahasi@openbossa.org> [2013-04-11 13:54:56 -0300]:

> This patch sends Reject Synchronous Connection Request Command when
> hci_conn_timeout is triggered, and the SCO connection is in BT_CONNECT2
> state. It prevents inconsistency if the remote host doesn't implement
> properly the timeout for the connection request, and it removes the
> connection reference left when the socket is closed for incoming SCO
> connections.
> 
> [ 2650.129080] sco_sock_release: sock ffff8801ca417400, sk ffff88020c408800
> [ 2650.129092] sco_sock_clear_timer: sock ffff88020c408800 state 6
> [ 2650.129101] __sco_sock_close: sk ffff88020c408800 state 6 socket
> 	ffff8801ca417400
> [ 2650.129108] sco_chan_del: sk ffff88020c408800, conn ffff8801c650ea20,
> 	err 104
> [ 2650.129114] hci_conn_put: hcon ffff88020c40a800 orig refcnt 1
> [ 2650.129128] sco_sock_kill: sk ffff88020c408800 state 9
> [ 2650.129135] sco_sock_destruct: sk ffff88020c408800
> [ 2650.138468] hci_conn_timeout: hcon ffff88020c40a800 state BT_CONNECT2
> 
> Signed-off-by: Claudio Takahasi <claudio.takahasi@openbossa.org>
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
> ---
>  net/bluetooth/hci_conn.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)

Patch has been applied to bluetooth-next with an improved commit subject.
Thanks.

	Gustavo

[PATCH] Bluetooth: Remove unneeded parameter

[Claudio Takahasi]

This patch removes the status parameter of the l2cap_conn_add function.
The parameter 'status' is always 0.

Signed-off-by: Claudio Takahasi <claudio.takahasi@openbossa.org>
---
 net/bluetooth/l2cap_core.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 7c7e932..0104c02 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1502,12 +1502,12 @@ static void security_timeout(struct work_struct *work)
 	}
 }
 
-static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon, u8 status)
+static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon)
 {
 	struct l2cap_conn *conn = hcon->l2cap_data;
 	struct hci_chan *hchan;
 
-	if (conn || status)
+	if (conn)
 		return conn;
 
 	hchan = hci_chan_create(hcon);
@@ -1695,7 +1695,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
 		goto done;
 	}
 
-	conn = l2cap_conn_add(hcon, 0);
+	conn = l2cap_conn_add(hcon);
 	if (!conn) {
 		hci_conn_put(hcon);
 		err = -ENOMEM;
@@ -6313,7 +6313,7 @@ void l2cap_connect_cfm(struct hci_conn *hcon, u8 status)
 	BT_DBG("hcon %p bdaddr %pMR status %d", hcon, &hcon->dst, status);
 
 	if (!status) {
-		conn = l2cap_conn_add(hcon, status);
+		conn = l2cap_conn_add(hcon);
 		if (conn)
 			l2cap_conn_ready(conn);
 	} else {
@@ -6482,7 +6482,7 @@ int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
 		goto drop;
 
 	if (!conn)
-		conn = l2cap_conn_add(hcon, 0);
+		conn = l2cap_conn_add(hcon);
 
 	if (!conn)
 		goto drop;
-- 
1.7.11.7

[PATCH v1] Bluetooth: Remove unneeded parameter

[Claudio Takahasi]

This patch removes the status parameter of the l2cap_conn_add function.
The parameter 'status' is always 0.

Signed-off-by: Claudio Takahasi <claudio.takahasi@openbossa.org>
---
 net/bluetooth/l2cap_core.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 7cdb93c..e09b89b 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1502,12 +1502,12 @@ static void security_timeout(struct work_struct *work)
 	}
 }
 
-static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon, u8 status)
+static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon)
 {
 	struct l2cap_conn *conn = hcon->l2cap_data;
 	struct hci_chan *hchan;
 
-	if (conn || status)
+	if (conn)
 		return conn;
 
 	hchan = hci_chan_create(hcon);
@@ -1695,7 +1695,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
 		goto done;
 	}
 
-	conn = l2cap_conn_add(hcon, 0);
+	conn = l2cap_conn_add(hcon);
 	if (!conn) {
 		hci_conn_drop(hcon);
 		err = -ENOMEM;
@@ -6313,7 +6313,7 @@ void l2cap_connect_cfm(struct hci_conn *hcon, u8 status)
 	BT_DBG("hcon %p bdaddr %pMR status %d", hcon, &hcon->dst, status);
 
 	if (!status) {
-		conn = l2cap_conn_add(hcon, status);
+		conn = l2cap_conn_add(hcon);
 		if (conn)
 			l2cap_conn_ready(conn);
 	} else {
@@ -6482,7 +6482,7 @@ int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
 		goto drop;
 
 	if (!conn)
-		conn = l2cap_conn_add(hcon, 0);
+		conn = l2cap_conn_add(hcon);
 
 	if (!conn)
 		goto drop;
-- 
1.7.11.7

Re: [PATCH v1] Bluetooth: Remove unneeded parameter

[Gustavo Padovan]

Hi Claudio,

* Claudio Takahasi <claudio.takahasi@openbossa.org> [2013-04-11 13:55:50 -0300]:

> This patch removes the status parameter of the l2cap_conn_add function.
> The parameter 'status' is always 0.
> 
> Signed-off-by: Claudio Takahasi <claudio.takahasi@openbossa.org>
> ---
>  net/bluetooth/l2cap_core.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)

Patch has been applied to bluetooth-next. Thanks.

	Gustavo

[PATCH] Bluetooth: Use GFP_KERNEL in sco_conn_add

[Claudio Takahasi]

This patch changes the memory allocation flags in the sco_conn_add
function, replacing the type to GFP_KERNEL. This function is executed
in process context and it is not called inside an atomic section.

Signed-off-by: Claudio Takahasi <claudio.takahasi@openbossa.org>
---
 net/bluetooth/sco.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index d919d11..ca393b4 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -83,7 +83,7 @@ static struct sco_conn *sco_conn_add(struct hci_conn *hcon)
 	if (conn)
 		return conn;
 
-	conn = kzalloc(sizeof(struct sco_conn), GFP_ATOMIC);
+	conn = kzalloc(sizeof(struct sco_conn), GFP_KERNEL);
 	if (!conn)
 		return NULL;
 
-- 
1.7.11.7

Re: [PATCH] Bluetooth: Use GFP_KERNEL in sco_conn_add

[Gustavo Padovan]

Hi Claudio,

* Claudio Takahasi <claudio.takahasi@openbossa.org> [2013-04-11 11:35:45 -0300]:

> This patch changes the memory allocation flags in the sco_conn_add
> function, replacing the type to GFP_KERNEL. This function is executed
> in process context and it is not called inside an atomic section.
> 
> Signed-off-by: Claudio Takahasi <claudio.takahasi@openbossa.org>
> ---
>  net/bluetooth/sco.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Patch has been applied to bluetooth-next. Thanks.

	Gustavo

[PATCH] Bluetooth: Minor coding style fix

[Claudio Takahasi]

This patch removes unneeded initialization and empty line.

Signed-off-by: Claudio Takahasi <claudio.takahasi@openbossa.org>
---
 net/bluetooth/sco.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index ca393b4..af7750e 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -481,8 +481,7 @@ static int sco_sock_connect(struct socket *sock, struct sockaddr *addr, int alen
 {
 	struct sockaddr_sco *sa = (struct sockaddr_sco *) addr;
 	struct sock *sk = sock->sk;
-	int err = 0;
-
+	int err;
 
 	BT_DBG("sk %p", sk);
 
-- 
1.7.11.7

Re: [PATCH] Bluetooth: Minor coding style fix

[Gustavo Padovan]

Hi Claudio,

* Claudio Takahasi <claudio.takahasi@openbossa.org> [2013-04-11 11:35:46 -0300]:

> This patch removes unneeded initialization and empty line.
> 
> Signed-off-by: Claudio Takahasi <claudio.takahasi@openbossa.org>
> ---
>  net/bluetooth/sco.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Patch has been applied to bluetooth-next. Thanks.

	Gustavo

[PATCH v1] Bluetooth: Fix SCO connection reference

[Claudio Takahasi]

This patch fixes decrementing SCO connection reference right after
stablishing the SCO connection with defer setup enabled. The dump below
shows a disconnection command with handle 0, the connection is still in
BT_CONNECT2 state and there isn't a handle associated with it.

< HCI Command: Accept Synchronous Connection (0x01|0x0029) plen 21
  bdaddr 78:47:1D:B3:72:6C
> HCI Event: Command Status (0x0f) plen 4
  Accept Synchronous Connection (0x01|0x0029) status 0x00 ncmd 1
< HCI Command: Disconnect (0x01|0x0006) plen 3
  handle 0 reason 0x13
  Reason: Remote User Terminated Connection
> HCI Event: Command Status (0x0f) plen 4
  Disconnect (0x01|0x0006) status 0x00 ncmd 1
> HCI Event: Synchronous Connect Complete (0x2c) plen 17
  status 0x00 handle 46 bdaddr 78:47:1D:B3:72:6C
  type eSCO
  Air mode: CVSD
< SCO data: handle 46 flags 0x00 dlen 48

Signed-off-by: Claudio Takahasi <claudio.takahasi@openbossa.org>
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
---
 net/bluetooth/hci_event.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 2cf28b1..f6ea3c7 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1860,7 +1860,6 @@ static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
 		} else {
 			conn->state = BT_CONNECT2;
 			hci_proto_connect_cfm(conn, 0);
-			hci_conn_drop(conn);
 		}
 	} else {
 		/* Connection rejected */
-- 
1.7.11.7

Re: [PATCH] Bluetooth: Fix SCO connection reference

[Gustavo Padovan]

Hi Claudio,

* Claudio Takahasi <claudio.takahasi@openbossa.org> [2013-04-11 11:35:42 -0300]:

> This patch fixes decrementing SCO connection reference right after
> stablishing the SCO connection with defer setup enabled. The dump below
> shows a disconnection command with handle 0, the connection is still in
> BT_CONNECT2 state and there isn't a handle associated with it.
> 
> < HCI Command: Accept Synchronous Connection (0x01|0x0029) plen 21
>   bdaddr 78:47:1D:B3:72:6C
> > HCI Event: Command Status (0x0f) plen 4
>   Accept Synchronous Connection (0x01|0x0029) status 0x00 ncmd 1
> < HCI Command: Disconnect (0x01|0x0006) plen 3
>   handle 0 reason 0x13
>   Reason: Remote User Terminated Connection
> > HCI Event: Command Status (0x0f) plen 4
>   Disconnect (0x01|0x0006) status 0x00 ncmd 1
> > HCI Event: Synchronous Connect Complete (0x2c) plen 17
>   status 0x00 handle 46 bdaddr 78:47:1D:B3:72:6C
>   type eSCO
>   Air mode: CVSD
> < SCO data: handle 46 flags 0x00 dlen 48
> 
> Signed-off-by: Claudio Takahasi <claudio.takahasi@openbossa.org>
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
> ---
>  net/bluetooth/hci_event.c | 1 -
>  1 file changed, 1 deletion(-)

Patch has been applied to bluetooth-next. Thanks.

	Gustavo

Re: [PATCH] Bluetooth: Fix SCO connection reference

[Gustavo Padovan]

* Gustavo Padovan <gustavo@padovan.org> [2013-04-11 16:36:54 -0300]:

> Hi Claudio,
> 
> * Claudio Takahasi <claudio.takahasi@openbossa.org> [2013-04-11 11:35:42 -0300]:
> 
> > This patch fixes decrementing SCO connection reference right after
> > stablishing the SCO connection with defer setup enabled. The dump below
> > shows a disconnection command with handle 0, the connection is still in
> > BT_CONNECT2 state and there isn't a handle associated with it.
> > 
> > < HCI Command: Accept Synchronous Connection (0x01|0x0029) plen 21
> >   bdaddr 78:47:1D:B3:72:6C
> > > HCI Event: Command Status (0x0f) plen 4
> >   Accept Synchronous Connection (0x01|0x0029) status 0x00 ncmd 1
> > < HCI Command: Disconnect (0x01|0x0006) plen 3
> >   handle 0 reason 0x13
> >   Reason: Remote User Terminated Connection
> > > HCI Event: Command Status (0x0f) plen 4
> >   Disconnect (0x01|0x0006) status 0x00 ncmd 1
> > > HCI Event: Synchronous Connect Complete (0x2c) plen 17
> >   status 0x00 handle 46 bdaddr 78:47:1D:B3:72:6C
> >   type eSCO
> >   Air mode: CVSD
> > < SCO data: handle 46 flags 0x00 dlen 48
> > 
> > Signed-off-by: Claudio Takahasi <claudio.takahasi@openbossa.org>
> > Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
> > ---
> >  net/bluetooth/hci_event.c | 1 -
> >  1 file changed, 1 deletion(-)
> 
> Patch has been applied to bluetooth-next. Thanks.

Actually I applied v1 of this patch.

	Gustavo