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

[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>
---
 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        |   10 +++++-----
 5 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 2fb268f..4e750df 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -195,6 +195,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 507fcec..86f74f6 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2062,6 +2062,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;
@@ -2072,6 +2078,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 0f2b1be..67053f8 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4884,6 +4884,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 82b6368..7e3386f 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -596,12 +596,12 @@ 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 {
-			err = -EINVAL;
+			if (!l2cap_chan_check_security(chan))
+				bt_sk(sk)->suspended = true;
+			else
+				sk->sk_state_change(sk);
 		}
 		break;
 
-- 
1.7.10

[PATCH 2/2] Bluetooth: report the right security level in getsockopt

[Gustavo Padovan]

During a security level elevation we need to keep track of the current
security level of a connection until the new one is not confirmed.

Signed-off-by: Gustavo Padovan <gustavo@padovan.org>
---
 include/net/bluetooth/l2cap.h |    1 +
 net/bluetooth/l2cap_sock.c    |    6 +++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 92c0423..ff34be7 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -446,6 +446,7 @@ struct l2cap_chan {
 	__le16		sport;
 
 	__u8		sec_level;
+	__u8		current_sl;
 
 	__u8		ident;
 
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 7e3386f..8f59fa6 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -379,7 +379,10 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname, ch
 		}
 
 		memset(&sec, 0, sizeof(sec));
-		sec.level = chan->sec_level;
+		if (bt_sk(sk)->suspended)
+			sec.level = chan->current_sl;
+		else
+			sec.level = chan->sec_level;
 
 		if (sk->sk_state == BT_CONNECTED)
 			sec.key_size = chan->conn->hcon->enc_key_size;
@@ -577,6 +580,7 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, ch
 			break;
 		}
 
+		chan->current_sl = chan->sec_level;
 		chan->sec_level = sec.level;
 
 		if (!chan->conn)
-- 
1.7.10

Re: [PATCH 2/2] Bluetooth: report the right security level in getsockopt

[Marcel Holtmann]

Hi Gustavo,

> During a security level elevation we need to keep track of the current
> security level of a connection until the new one is not confirmed.
> 
> Signed-off-by: Gustavo Padovan <gustavo@padovan.org>
> ---
>  include/net/bluetooth/l2cap.h |    1 +
>  net/bluetooth/l2cap_sock.c    |    6 +++++-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 92c0423..ff34be7 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -446,6 +446,7 @@ struct l2cap_chan {
>  	__le16		sport;
>  
>  	__u8		sec_level;
> +	__u8		current_sl;

this name is not good. Call it active_sec_level or cur_sec_level.

>  
>  	__u8		ident;
>  
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index 7e3386f..8f59fa6 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -379,7 +379,10 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname, ch
>  		}
>  
>  		memset(&sec, 0, sizeof(sec));
> -		sec.level = chan->sec_level;
> +		if (bt_sk(sk)->suspended)
> +			sec.level = chan->current_sl;
> +		else
> +			sec.level = chan->sec_level;

I think getsockopt should just return the actual current active security
level.

And why are we not accessing this via chan->conn->hcon->sec_level and
its pending_sec_level variables.

>  
>  		if (sk->sk_state == BT_CONNECTED)
>  			sec.key_size = chan->conn->hcon->enc_key_size;
> @@ -577,6 +580,7 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, ch
>  			break;
>  		}
>  
> +		chan->current_sl = chan->sec_level;
>  		chan->sec_level = sec.level;
>  
>  		if (!chan->conn)

Regards

Marcel

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

[Marcel Holtmann]

Hi Gustavo,

> 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>
> ---
>  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        |   10 +++++-----
>  5 files changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> index 2fb268f..4e750df 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -195,6 +195,7 @@ struct bt_sock {
>  	struct list_head accept_q;
>  	struct sock *parent;
>  	u32 defer_setup;
> +	bool  suspended;

you have double spaces here. Please pay attention.

>  };
>  
>  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 507fcec..86f74f6 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -2062,6 +2062,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;
> @@ -2072,6 +2078,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 0f2b1be..67053f8 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -4884,6 +4884,11 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
>  
>  		if (!status && (chan->state == BT_CONNECTED ||
>  						chan->state == BT_CONFIG)) {

You do know that this one will be showing up in Dave's coding style
complaint. You might wanna fix this with a pre-patch first.

> +			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 82b6368..7e3386f 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -596,12 +596,12 @@ 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 {
> -			err = -EINVAL;
> +			if (!l2cap_chan_check_security(chan))
> +				bt_sk(sk)->suspended = true;
> +			else
> +				sk->sk_state_change(sk);
>  		}
>  		break;

Why is this change correct? You are changing this from applying to only
incoming connections (CONNECT2) to also outgoing connections (CONNECT).

Regards

Marcel

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

[Gustavo Padovan]

Hi Marcel,

* Marcel Holtmann <marcel@holtmann.org> [2012-05-06 09:07:26 -0700]:

> Hi Gustavo,
> 
> > 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>
> > ---
> >  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        |   10 +++++-----
> >  5 files changed, 19 insertions(+), 6 deletions(-)
> > 
> > diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> > index 2fb268f..4e750df 100644
> > --- a/include/net/bluetooth/bluetooth.h
> > +++ b/include/net/bluetooth/bluetooth.h
> > @@ -195,6 +195,7 @@ struct bt_sock {
> >  	struct list_head accept_q;
> >  	struct sock *parent;
> >  	u32 defer_setup;
> > +	bool  suspended;
> 
> you have double spaces here. Please pay attention.

My bad, I let this pass.

> 
> >  };
> >  
> >  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 507fcec..86f74f6 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -2062,6 +2062,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;
> > @@ -2072,6 +2078,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 0f2b1be..67053f8 100644
> > --- a/net/bluetooth/l2cap_core.c
> > +++ b/net/bluetooth/l2cap_core.c
> > @@ -4884,6 +4884,11 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
> >  
> >  		if (!status && (chan->state == BT_CONNECTED ||
> >  						chan->state == BT_CONFIG)) {
> 
> You do know that this one will be showing up in Dave's coding style
> complaint. You might wanna fix this with a pre-patch first.

Code style fixes are not suitable for 3.4-rc6, I don't we have a problem here.

> 
> > +			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 82b6368..7e3386f 100644
> > --- a/net/bluetooth/l2cap_sock.c
> > +++ b/net/bluetooth/l2cap_sock.c
> > @@ -596,12 +596,12 @@ 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 {
> > -			err = -EINVAL;
> > +			if (!l2cap_chan_check_security(chan))
> > +				bt_sk(sk)->suspended = true;
> > +			else
> > +				sk->sk_state_change(sk);
> >  		}
> >  		break;
> 
> Why is this change correct? You are changing this from applying to only
> incoming connections (CONNECT2) to also outgoing connections (CONNECT).

It wrong, what I want here actually is:

 ((sk->sk_state == BT_CONNECT2 && bt_sk(sk)->defer_setup) ||
  sk->sk_state == BT_CONNECTED)

 We need to allow connected links to increase the security level too.

	Gustavo

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

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

[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@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        |   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