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!
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 78521bdb4a42770c4c2d48317c00ec0d7f451a08:
Bluetooth: Lock the L2CAP channel when sending (2012-05-04 20:56:29 -0300)
----------------------------------------------------------------
Johan Hedberg (1):
Bluetooth: Fix Inquiry with RSSI event mask
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 | 2 --
include/net/bluetooth/hci_core.h | 17 +++++++++++++++++
net/bluetooth/hci_event.c | 6 ++++--
net/bluetooth/l2cap_core.c | 30 +++---------------------------
net/bluetooth/l2cap_sock.c | 19 +++++++++++--------
7 files changed, 44 insertions(+), 39 deletions(-)
Hi,
* Gustavo Padovan <[email protected]> [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
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 <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
Signed-off-by: Johan Hedberg <[email protected]>
---
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
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 <[email protected]>
> 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 <[email protected]>
> Signed-off-by: Johan Hedberg <[email protected]>
>
>
> 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
From: Johan Hedberg <[email protected]>
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 <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
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
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 <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
Signed-off-by: Johan Hedberg <[email protected]>
---
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
Hi John,
* John W. Linville <[email protected]> [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 <[email protected]>
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 <[email protected]>
Signed-off-by: Johan Hedberg <[email protected]>
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(-)
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 <[email protected]> [2012-05-08 04:07:54 -0300]:
>
> > Hi John,
> >
> > * Gustavo Padovan <[email protected]> [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
[email protected] might be all we have. Be ready.
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 <[email protected]> [2012-05-08 04:07:54 -0300]:
> Hi John,
>
> * Gustavo Padovan <[email protected]> [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(-)
Hi John,
* Gustavo Padovan <[email protected]> [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(-)
Hi Marcel,
* Marcel Holtmann <[email protected]> [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 <[email protected]>
> > ---
> > 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
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 <[email protected]>
> ---
> 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