Re: pull request: bluetooth 2012-05-04

Re: pull request: bluetooth 2012-05-04

[Gustavo Padovan]

[-- Attachment #1: Type: text/plain, Size: 3881 bytes --]

Hi Dave,

I couldn't get this pull by John this week, he's been unresponsive.
We would love to see this code in the 3.4 kernel (not sure if this would be
possible if Linus do not release the -rc7, there are important fixes 
in the pull request, such as a fix to a regression that was breaking bluetooth
keyboards.

Please let me know if you have any problems with this! I checked this code for
coding style issues too. Thanks.

	Gustavo

* Gustavo Padovan <gustavo-THi1TnShQwVAfugRpC6u6w@public.gmane.org> [2012-05-08 04:07:54 -0300]:

> Hi John,
> 
> * Gustavo Padovan <gustavo-THi1TnShQwVAfugRpC6u6w@public.gmane.org> [2012-05-04 21:12:43 -0300]:
> 
> > Hi John,
> > 
> > A few more patch to 3.4. There is 3 fixes from Mat Martineau, a fix for
> > a incoming MTU check that was breaking ERTM, still on ERTM there is a
> > patch that fixes concurrency with one of our locks. His third patch is
> > related to lock fixes too.
> > Besides that Johan fixed a wrong bit set in the Inquiry code, Vishal
> > Agarwal added a EIR fix and finally we have support for one more usb id.
> > 
> > Please pull or let me know of any problems!
> 
> I saw that you didn't pull this yet, so I decided to re-do this pull request.
> We have more 3 important fixes to go in. They fix a regression that was
> preventing Bluetooth keyboard to work. I'll re-do the whole pull request
> message, so you can copy and paste it easily:
> 
> A few more patch to 3.4. There is 3 fixes from Mat Martineau, a fix for a
> incoming MTU check that was breaking ERTM, still on ERTM there is a patch that
> fixes concurrency with one of our locks. His third patch is related to lock
> fixes too.
> Besides that Johan fixed a wrong bit set in the Inquiry code, Vishal Agarwal
> added a EIR fix and finally we have support for one more usb id.  Finally
> Johan and I fixed a regression that was preventing bluetooth keyboard to work.
> 
> Please pull or let me know of any problems!
> 
> 	Gustavo
> 
> ---
> 
> The following changes since commit 985140369be1e886754d8ac0375dd64e4f727311:
> 
>   Add Foxconn / Hon Hai IDs for btusb module (2012-04-24 11:38:41 -0300)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth.git for-upstream
> 
> for you to fetch changes up to fbcb6a061652b38257a1d8bcf6ebd70fc1771549:
> 
>   Bluetooth: mgmt: Fix device_connected sending order (2012-05-07 20:56:30 +0300)
> 
> ----------------------------------------------------------------
> Gustavo Padovan (2):
>       Bluetooth: notify userspace of security level change
>       Bluetooth: report the right security level in getsockopt
> 
> Johan Hedberg (2):
>       Bluetooth: Fix Inquiry with RSSI event mask
>       Bluetooth: mgmt: Fix device_connected sending order
> 
> Mat Martineau (3):
>       Bluetooth: Fix a redundant and problematic incoming MTU check
>       Bluetooth: Restore locking semantics when looking up L2CAP channels
>       Bluetooth: Lock the L2CAP channel when sending
> 
> Michael Gruetzner (1):
>       bluetooth: Add support for Foxconn/Hon Hai AR5BBU22 0489:E03C
> 
> Vishal Agarwal (1):
>       Bluetooth: Fix EIR data generation for mgmt_device_found
> 
>  drivers/bluetooth/ath3k.c         |    6 ++++++
>  drivers/bluetooth/btusb.c         |    3 +++
>  include/net/bluetooth/bluetooth.h |    3 +--
>  include/net/bluetooth/hci_core.h  |   17 +++++++++++++++++
>  net/bluetooth/af_bluetooth.c      |    2 +-
>  net/bluetooth/hci_core.c          |    8 ++++++++
>  net/bluetooth/hci_event.c         |   17 +++++++++++++----
>  net/bluetooth/l2cap_core.c        |   35 ++++++++---------------------------
>  net/bluetooth/l2cap_sock.c        |   39 +++++++++++++++++++++++++--------------
>  9 files changed, 82 insertions(+), 48 deletions(-)


[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: pull request: bluetooth 2012-05-04

[John W. Linville]

On Fri, May 11, 2012 at 05:16:20PM -0300, Gustavo Padovan wrote:
> Hi Dave,
> 
> I couldn't get this pull by John this week, he's been unresponsive.
> We would love to see this code in the 3.4 kernel (not sure if this would be
> possible if Linus do not release the -rc7, there are important fixes 
> in the pull request, such as a fix to a regression that was breaking bluetooth
> keyboards.
> 
> Please let me know if you have any problems with this! I checked this code for
> coding style issues too. Thanks.

Well, I do apologize for my lack of responsiveness.  My employer saw
fit to have me (figuratively) locked in a room with no email access all
week, with me getting home rather late in the evenings and returning
early each morning.  I would have liked to have stayed awake for
hours each night catching-up, but I just didn't have the strength. :-(

With that said, I'm not at all sure that this batch of fixes is
appropriate for this late in the release cycle.  Normally at this
point I would expect to see "regression cause by commit 1234" or
"this common action results in this crash", all fixed by one-liners
wherever possible.  This batch is more like "causes some problems",
or "needs to be different" -- neither of which sound urgent enough
to be worth requesting delays in Linus's release schedule.

I am still quite exhausted from the past week, so it is possible that I
am overlooking the importance of some of these commits.  But for right
now, I am not convinced that there needs to be a rush on this request.
Dave can do as he sees fit, of course.

John

> 	Gustavo
> 
> * Gustavo Padovan <gustavo-THi1TnShQwVAfugRpC6u6w@public.gmane.org> [2012-05-08 04:07:54 -0300]:
> 
> > Hi John,
> > 
> > * Gustavo Padovan <gustavo-THi1TnShQwVAfugRpC6u6w@public.gmane.org> [2012-05-04 21:12:43 -0300]:
> > 
> > > Hi John,
> > > 
> > > A few more patch to 3.4. There is 3 fixes from Mat Martineau, a fix for
> > > a incoming MTU check that was breaking ERTM, still on ERTM there is a
> > > patch that fixes concurrency with one of our locks. His third patch is
> > > related to lock fixes too.
> > > Besides that Johan fixed a wrong bit set in the Inquiry code, Vishal
> > > Agarwal added a EIR fix and finally we have support for one more usb id.
> > > 
> > > Please pull or let me know of any problems!
> > 
> > I saw that you didn't pull this yet, so I decided to re-do this pull request.
> > We have more 3 important fixes to go in. They fix a regression that was
> > preventing Bluetooth keyboard to work. I'll re-do the whole pull request
> > message, so you can copy and paste it easily:
> > 
> > A few more patch to 3.4. There is 3 fixes from Mat Martineau, a fix for a
> > incoming MTU check that was breaking ERTM, still on ERTM there is a patch that
> > fixes concurrency with one of our locks. His third patch is related to lock
> > fixes too.
> > Besides that Johan fixed a wrong bit set in the Inquiry code, Vishal Agarwal
> > added a EIR fix and finally we have support for one more usb id.  Finally
> > Johan and I fixed a regression that was preventing bluetooth keyboard to work.
> > 
> > Please pull or let me know of any problems!
> > 
> > 	Gustavo
> > 
> > ---
> > 
> > The following changes since commit 985140369be1e886754d8ac0375dd64e4f727311:
> > 
> >   Add Foxconn / Hon Hai IDs for btusb module (2012-04-24 11:38:41 -0300)
> > 
> > are available in the git repository at:
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth.git for-upstream
> > 
> > for you to fetch changes up to fbcb6a061652b38257a1d8bcf6ebd70fc1771549:
> > 
> >   Bluetooth: mgmt: Fix device_connected sending order (2012-05-07 20:56:30 +0300)
> > 
> > ----------------------------------------------------------------
> > Gustavo Padovan (2):
> >       Bluetooth: notify userspace of security level change
> >       Bluetooth: report the right security level in getsockopt
> > 
> > Johan Hedberg (2):
> >       Bluetooth: Fix Inquiry with RSSI event mask
> >       Bluetooth: mgmt: Fix device_connected sending order
> > 
> > Mat Martineau (3):
> >       Bluetooth: Fix a redundant and problematic incoming MTU check
> >       Bluetooth: Restore locking semantics when looking up L2CAP channels
> >       Bluetooth: Lock the L2CAP channel when sending
> > 
> > Michael Gruetzner (1):
> >       bluetooth: Add support for Foxconn/Hon Hai AR5BBU22 0489:E03C
> > 
> > Vishal Agarwal (1):
> >       Bluetooth: Fix EIR data generation for mgmt_device_found
> > 
> >  drivers/bluetooth/ath3k.c         |    6 ++++++
> >  drivers/bluetooth/btusb.c         |    3 +++
> >  include/net/bluetooth/bluetooth.h |    3 +--
> >  include/net/bluetooth/hci_core.h  |   17 +++++++++++++++++
> >  net/bluetooth/af_bluetooth.c      |    2 +-
> >  net/bluetooth/hci_core.c          |    8 ++++++++
> >  net/bluetooth/hci_event.c         |   17 +++++++++++++----
> >  net/bluetooth/l2cap_core.c        |   35 ++++++++---------------------------
> >  net/bluetooth/l2cap_sock.c        |   39 +++++++++++++++++++++++++--------------
> >  9 files changed, 82 insertions(+), 48 deletions(-)
> 



-- 
John W. Linville		Someday the world will need a hero, and you
linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org			might be all we have.  Be ready.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: pull request: bluetooth 2012-05-04

[Gustavo Padovan]

Hi John,

* John W. Linville <linville@tuxdriver.com> [2012-05-12 14:22:27 -0400]:

> On Fri, May 11, 2012 at 05:16:20PM -0300, Gustavo Padovan wrote:
> > Hi Dave,
> > 
> > I couldn't get this pull by John this week, he's been unresponsive.
> > We would love to see this code in the 3.4 kernel (not sure if this would be
> > possible if Linus do not release the -rc7, there are important fixes 
> > in the pull request, such as a fix to a regression that was breaking bluetooth
> > keyboards.
> > 
> > Please let me know if you have any problems with this! I checked this code for
> > coding style issues too. Thanks.
> 
> Well, I do apologize for my lack of responsiveness.  My employer saw
> fit to have me (figuratively) locked in a room with no email access all
> week, with me getting home rather late in the evenings and returning
> early each morning.  I would have liked to have stayed awake for
> hours each night catching-up, but I just didn't have the strength. :-(
> 
> With that said, I'm not at all sure that this batch of fixes is
> appropriate for this late in the release cycle.  Normally at this
> point I would expect to see "regression cause by commit 1234" or
> "this common action results in this crash", all fixed by one-liners
> wherever possible.  This batch is more like "causes some problems",
> or "needs to be different" -- neither of which sound urgent enough
> to be worth requesting delays in Linus's release schedule.


In my point of view there are two commits there that are really necessary:

Gustavo Padovan (2):
      Bluetooth: report the right security level in getsockopt

Johan Hedberg (2):
      Bluetooth: mgmt: Fix device_connected sending order

They fix a userspace breakage caused by:

Author: Marcel Holtmann <marcel@holtmann.org>
Date:   Mon Feb 20 21:24:37 2012 +0100

    Bluetooth: Always enable management interface
    
    The management interface API has reached stable version 1.0 and thus
    it can now be always enabled. All future changes will be made backwards
    compatible.
    
    Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
    Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>


This cause no crash, but it make bluetooth keyboards stop to work with Linux.
This a serious breakage IMO. I really would like to see at least these two
patches in. The other fixes can wait.
 
	Gustavo

---

The following changes since commit 985140369be1e886754d8ac0375dd64e4f727311:

  Add Foxconn / Hon Hai IDs for btusb module (2012-04-24 11:38:41 -0300)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth for-upstream

for you to fetch changes up to 3ca67a07a20880c78bfce52017956c1472db69ff:

  Bluetooth: mgmt: Fix device_connected sending order (2012-05-12 16:02:06 -0300)

----------------------------------------------------------------
Gustavo Padovan (1):
      Bluetooth: notify userspace of security level change

Johan Hedberg (1):
      Bluetooth: mgmt: Fix device_connected sending order

 include/net/bluetooth/bluetooth.h |    1 +
 net/bluetooth/af_bluetooth.c      |    2 +-
 net/bluetooth/hci_core.c          |    8 ++++++++
 net/bluetooth/hci_event.c         |   11 +++++++++--
 net/bluetooth/l2cap_core.c        |    5 +++++
 net/bluetooth/l2cap_sock.c        |   15 ++++++++++-----
 6 files changed, 34 insertions(+), 8 deletions(-)

[PATCH 1/2] Bluetooth: notify userspace of security level change

[Gustavo Padovan]

When the userspace request a security level change it needs to be notified
of when the change is complete.
This patch make the socket non writable while the security request is
ongoing. If it succeeds POLL_OUT is emitted, otherwise the channel is
disconnected.

Signed-off-by: Gustavo Padovan <gustavo@padovan.org>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/bluetooth.h |    1 +
 net/bluetooth/af_bluetooth.c      |    2 +-
 net/bluetooth/hci_event.c         |    7 +++++++
 net/bluetooth/l2cap_core.c        |    5 +++++
 net/bluetooth/l2cap_sock.c        |   15 ++++++++++-----
 5 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 262ebd1..a65910b 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -191,6 +191,7 @@ struct bt_sock {
 	struct list_head accept_q;
 	struct sock *parent;
 	u32 defer_setup;
+	bool suspended;
 };
 
 struct bt_sock_list {
diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
index 72eb187..6fb68a9 100644
--- a/net/bluetooth/af_bluetooth.c
+++ b/net/bluetooth/af_bluetooth.c
@@ -450,7 +450,7 @@ unsigned int bt_sock_poll(struct file *file, struct socket *sock, poll_table *wa
 			sk->sk_state == BT_CONFIG)
 		return mask;
 
-	if (sock_writeable(sk))
+	if (!bt_sk(sk)->suspended && sock_writeable(sk))
 		mask |= POLLOUT | POLLWRNORM | POLLWRBAND;
 	else
 		set_bit(SOCK_ASYNC_NOSPACE, &sk->sk_socket->flags);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 7f87a70..ff38cc6 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2040,6 +2040,12 @@ static inline void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *
 
 		clear_bit(HCI_CONN_ENCRYPT_PEND, &conn->flags);
 
+		if (ev->status && conn->state == BT_CONNECTED) {
+			hci_acl_disconn(conn, 0x13);
+			hci_conn_put(conn);
+			goto unlock;
+		}
+
 		if (conn->state == BT_CONFIG) {
 			if (!ev->status)
 				conn->state = BT_CONNECTED;
@@ -2050,6 +2056,7 @@ static inline void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *
 			hci_encrypt_cfm(conn, ev->status, ev->encrypt);
 	}
 
+unlock:
 	hci_dev_unlock(hdev);
 }
 
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 38d934a..c073533 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4590,6 +4590,11 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
 
 		if (!status && (chan->state == BT_CONNECTED ||
 						chan->state == BT_CONFIG)) {
+			struct sock *sk = chan->sk;
+
+			bt_sk(sk)->suspended = false;
+			sk->sk_state_change(sk);
+
 			l2cap_check_encryption(chan, encrypt);
 			l2cap_chan_unlock(chan);
 			continue;
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 29122ed..4bdacf9 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -592,11 +592,16 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, ch
 			sk->sk_state = BT_CONFIG;
 			chan->state = BT_CONFIG;
 
-		/* or for ACL link, under defer_setup time */
-		} else if (sk->sk_state == BT_CONNECT2 &&
-					bt_sk(sk)->defer_setup) {
-			err = l2cap_chan_check_security(chan);
-		} else {
+		/* or for ACL link */
+		} else if ((sk->sk_state == BT_CONNECT2 &&
+			   bt_sk(sk)->defer_setup) ||
+			   sk->sk_state == BT_CONNECTED) {
+			if (!l2cap_chan_check_security(chan))
+				bt_sk(sk)->suspended = true;
+			else
+				sk->sk_state_change(sk);
+		}
+		else {
 			err = -EINVAL;
 		}
 		break;
-- 
1.7.10.1

[PATCH 2/2] Bluetooth: mgmt: Fix device_connected sending order

[Gustavo Padovan]

From: Johan Hedberg <johan.hedberg@intel.com>

The mgmt_ev_device_connected signal must be sent before any event
indications happen for sockets associated with the connection. Otherwise
e.g. device authorization for the sockets will fail with ENOTCONN as
user space things that there is no baseband link.

This patch fixes the issue by ensuring that the device_connected event
if sent (if it hasn't been so already) as soon as the first ACL data
packet arrives from the remote device.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/hci_core.c  |    8 ++++++++
 net/bluetooth/hci_event.c |    4 ++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index a7607e4..b5bab83 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2785,6 +2785,14 @@ static inline void hci_acldata_packet(struct hci_dev *hdev, struct sk_buff *skb)
 	if (conn) {
 		hci_conn_enter_active_mode(conn, BT_POWER_FORCE_ACTIVE_OFF);
 
+		hci_dev_lock(hdev);
+		if (test_bit(HCI_MGMT, &hdev->dev_flags) &&
+		    !test_and_set_bit(HCI_CONN_MGMT_CONNECTED, &conn->flags))
+			mgmt_device_connected(hdev, &conn->dst, conn->type,
+					      conn->dst_type, 0, NULL, 0,
+					      conn->dev_class);
+		hci_dev_unlock(hdev);
+
 		/* Send to upper protocol */
 		l2cap_recv_acldata(conn, skb, flags);
 		return;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index ff38cc6..7d66b8b 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2110,7 +2110,7 @@ static inline void hci_remote_features_evt(struct hci_dev *hdev, struct sk_buff
 		goto unlock;
 	}
 
-	if (!ev->status) {
+	if (!ev->status && !test_bit(HCI_CONN_MGMT_CONNECTED, &conn->flags)) {
 		struct hci_cp_remote_name_req cp;
 		memset(&cp, 0, sizeof(cp));
 		bacpy(&cp.bdaddr, &conn->dst);
@@ -2879,7 +2879,7 @@ static inline void hci_remote_ext_features_evt(struct hci_dev *hdev, struct sk_b
 	if (conn->state != BT_CONFIG)
 		goto unlock;
 
-	if (!ev->status) {
+	if (!ev->status && !test_bit(HCI_CONN_MGMT_CONNECTED, &conn->flags)) {
 		struct hci_cp_remote_name_req cp;
 		memset(&cp, 0, sizeof(cp));
 		bacpy(&cp.bdaddr, &conn->dst);
-- 
1.7.10.1

[PATCH 1/2] Bluetooth: notify userspace of security level change

[Gustavo Padovan]

It fixes L2CAP socket based security level elevation during a
connection. The HID profile needs this (for keyboards) and it is the only
way to achieve the security level elevation when using the management
interface to talk to the kernel (hence the management enabling patch
being the one that exposes this issue).

It enables the userspace a security level change when the socket is
already connected and create a way to notify the socket the result of the
request. At the moment of the request the socket is made non writable, if
the request fails the connections closes, otherwise the socket is made
writable again, POLL_OUT is emmited.

Signed-off-by: Gustavo Padovan <gustavo-THi1TnShQwVAfugRpC6u6w@public.gmane.org>
Acked-by: Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
Signed-off-by: Johan Hedberg <johan.hedberg-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 include/net/bluetooth/bluetooth.h |    1 +
 net/bluetooth/af_bluetooth.c      |    2 +-
 net/bluetooth/hci_event.c         |    7 +++++++
 net/bluetooth/l2cap_core.c        |    5 +++++
 net/bluetooth/l2cap_sock.c        |   12 ++++++++----
 5 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 262ebd1..a65910b 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -191,6 +191,7 @@ struct bt_sock {
 	struct list_head accept_q;
 	struct sock *parent;
 	u32 defer_setup;
+	bool suspended;
 };
 
 struct bt_sock_list {
diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
index 72eb187..6fb68a9 100644
--- a/net/bluetooth/af_bluetooth.c
+++ b/net/bluetooth/af_bluetooth.c
@@ -450,7 +450,7 @@ unsigned int bt_sock_poll(struct file *file, struct socket *sock, poll_table *wa
 			sk->sk_state == BT_CONFIG)
 		return mask;
 
-	if (sock_writeable(sk))
+	if (!bt_sk(sk)->suspended && sock_writeable(sk))
 		mask |= POLLOUT | POLLWRNORM | POLLWRBAND;
 	else
 		set_bit(SOCK_ASYNC_NOSPACE, &sk->sk_socket->flags);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 7f87a70..ff38cc6 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2040,6 +2040,12 @@ static inline void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *
 
 		clear_bit(HCI_CONN_ENCRYPT_PEND, &conn->flags);
 
+		if (ev->status && conn->state == BT_CONNECTED) {
+			hci_acl_disconn(conn, 0x13);
+			hci_conn_put(conn);
+			goto unlock;
+		}
+
 		if (conn->state == BT_CONFIG) {
 			if (!ev->status)
 				conn->state = BT_CONNECTED;
@@ -2050,6 +2056,7 @@ static inline void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *
 			hci_encrypt_cfm(conn, ev->status, ev->encrypt);
 	}
 
+unlock:
 	hci_dev_unlock(hdev);
 }
 
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 38d934a..c073533 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4590,6 +4590,11 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
 
 		if (!status && (chan->state == BT_CONNECTED ||
 						chan->state == BT_CONFIG)) {
+			struct sock *sk = chan->sk;
+
+			bt_sk(sk)->suspended = false;
+			sk->sk_state_change(sk);
+
 			l2cap_check_encryption(chan, encrypt);
 			l2cap_chan_unlock(chan);
 			continue;
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 29122ed..04e7c17 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -592,10 +592,14 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, ch
 			sk->sk_state = BT_CONFIG;
 			chan->state = BT_CONFIG;
 
-		/* or for ACL link, under defer_setup time */
-		} else if (sk->sk_state == BT_CONNECT2 &&
-					bt_sk(sk)->defer_setup) {
-			err = l2cap_chan_check_security(chan);
+		/* or for ACL link */
+		} else if ((sk->sk_state == BT_CONNECT2 &&
+			   bt_sk(sk)->defer_setup) ||
+			   sk->sk_state == BT_CONNECTED) {
+			if (!l2cap_chan_check_security(chan))
+				bt_sk(sk)->suspended = true;
+			else
+				sk->sk_state_change(sk);
 		} else {
 			err = -EINVAL;
 		}
-- 
1.7.10.1

Re: [PATCH 1/2] Bluetooth: notify userspace of security level change

[valdis.kletnieks]

[-- Attachment #1: Type: text/plain, Size: 571 bytes --]

On Sun, 13 May 2012 03:20:07 -0300, Gustavo Padovan said:

> It enables the userspace a security level change when the socket is
> already connected and create a way to notify the socket the result of the
> request. At the moment of the request the socket is made non writable, if
> the request fails the connections closes, otherwise the socket is made
> writable again, POLL_OUT is emmited.

What happens to in-flight data already written but not yet consumed?  I'm
smelling a possible race condition with data sent under the old level but
read under the new level....

[-- Attachment #2: Type: application/pgp-signature, Size: 865 bytes --]

Re: [PATCH 1/2] Bluetooth: notify userspace of security level change

[Gustavo Padovan]

Hi,

* Gustavo Padovan <gustavo@padovan.org> [2012-05-12 16:11:49 -0300]:

> When the userspace request a security level change it needs to be notified
> of when the change is complete.
> This patch make the socket non writable while the security request is
> ongoing. If it succeeds POLL_OUT is emitted, otherwise the channel is
> disconnected.

I just sent a second version of this patches that includes comments Johan made
in the last e-mail for better understanding of what the problem really is.

	Gustavo

Re: pull request: bluetooth 2012-05-04

[Johan Hedberg]

Hi,

On Sat, May 12, 2012, Gustavo Padovan wrote:
> In my point of view there are two commits there that are really necessary:
> 
> Gustavo Padovan (2):
>       Bluetooth: report the right security level in getsockopt
> 
> Johan Hedberg (2):
>       Bluetooth: mgmt: Fix device_connected sending order
> 
> They fix a userspace breakage caused by:
> 
> Author: Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
> Date:   Mon Feb 20 21:24:37 2012 +0100
> 
>     Bluetooth: Always enable management interface
>     
>     The management interface API has reached stable version 1.0 and thus
>     it can now be always enabled. All future changes will be made backwards
>     compatible.
>     
>     Signed-off-by: Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
>     Signed-off-by: Johan Hedberg <johan.hedberg-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> 
> This cause no crash, but it make bluetooth keyboards stop to work with Linux.
> This a serious breakage IMO. I really would like to see at least these two
> patches in. The other fixes can wait.

I'd just like to add some further background since I don't think it's
that obvious how these are regression fixes:

The device_connected fix should be quite self-explanatory, but it's
actually a wider issue than just for keyboards. All profiles that do
incoming connection authorization (e.g. headsets) will break without it
with specific hardware. The reason it wasn't caught earlier is that it
only occurs with specific Bluetooth adapters.

As for the security level patch, this fixes L2CAP socket based security
level elevation during a connection. The HID profile needs this (for
keyboards) and it is the only way to achieve the security level
elevation when using the management interface to talk to the kernel
(hence the management enabling patch being the one that exposes this
issue).

Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html