From: Andrei Emeltchenko <[email protected]>
Changes:
v3: rebased unapplied patches
v2: added helper function for block calculation, added patches for
checking BREDR when going to Sniff mode and complete Read Local Version
HCI command.
v1: Changed HCI_<block,packet>_FLOW_CTL_MODE => HCI_FLOW_CTL_MODE_<block,packet>
RFCv1: Initial version
AMP initialization and block flow control code.
Andrei Emeltchenko (5):
Bluetooth: Split ctrl init to BREDR and AMP parts
Bluetooth: Initialize default flow control mode
Bluetooth: Check for flow control mode
Bluetooth: Clean up magic pointers
Bluetooth: Correct packet len calculation
include/net/bluetooth/hci.h | 11 ++++++-
net/bluetooth/hci_core.c | 66 ++++++++++++++++++++++++++++++++-----------
net/bluetooth/hci_event.c | 23 ++++++++++-----
3 files changed, 74 insertions(+), 26 deletions(-)
--
1.7.4.1
Hi Gustavo,
On Tue, Dec 20, 2011 at 01:02:57PM -0800, Marcel Holtmann wrote:
> > > Remove unneeded skb_pull and correct packet length calculation
> > > removing magic number. Move BT_DBG after len check otherwise
> > > it could possibly access wrong memory.
> > >
> > > Signed-off-by: Andrei Emeltchenko <[email protected]>
> > > ---
> > > net/bluetooth/hci_event.c | 9 ++++-----
> > > 1 files changed, 4 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > > index 919e3c0..47e1476 100644
> > > --- a/net/bluetooth/hci_event.c
> > > +++ b/net/bluetooth/hci_event.c
> > > @@ -2266,20 +2266,19 @@ static inline void hci_num_comp_pkts_evt(struct hci_dev *hdev, struct sk_buff *s
> > > struct hci_ev_num_comp_pkts *ev = (void *) skb->data;
> > > int i;
> > >
> > > - skb_pull(skb, sizeof(*ev));
> > > -
> > > - BT_DBG("%s num_hndl %d", hdev->name, ev->num_hndl);
> > > -
> > > if (hdev->flow_ctl_mode != HCI_FLOW_CTL_MODE_PACKET_BASED) {
> > > BT_ERR("Wrong event for mode %d", hdev->flow_ctl_mode);
> > > return;
> > > }
> > >
> > > - if (skb->len < ev->num_hndl * 4) {
> > > + if (skb->len < sizeof(*ev) || skb->len < sizeof(*ev) +
> > > + ev->num_hndl * sizeof(struct hci_comp_pkts_info)) {
> >
> > I think you can remove the first part of this check.
>
> you are accessing ev->num_handl, so you need to ensure that you have at
> least ev->num_handl size of data in your SKB. So this code is correct.
BTW: I resent the remaining patch from this series. Cannot add more to
Marcel's comment above.
Best regards
Andrei Emeltchenko
Hi Gustavo,
> > Remove unneeded skb_pull and correct packet length calculation
> > removing magic number. Move BT_DBG after len check otherwise
> > it could possibly access wrong memory.
> >
> > Signed-off-by: Andrei Emeltchenko <[email protected]>
> > ---
> > net/bluetooth/hci_event.c | 9 ++++-----
> > 1 files changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > index 919e3c0..47e1476 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -2266,20 +2266,19 @@ static inline void hci_num_comp_pkts_evt(struct hci_dev *hdev, struct sk_buff *s
> > struct hci_ev_num_comp_pkts *ev = (void *) skb->data;
> > int i;
> >
> > - skb_pull(skb, sizeof(*ev));
> > -
> > - BT_DBG("%s num_hndl %d", hdev->name, ev->num_hndl);
> > -
> > if (hdev->flow_ctl_mode != HCI_FLOW_CTL_MODE_PACKET_BASED) {
> > BT_ERR("Wrong event for mode %d", hdev->flow_ctl_mode);
> > return;
> > }
> >
> > - if (skb->len < ev->num_hndl * 4) {
> > + if (skb->len < sizeof(*ev) || skb->len < sizeof(*ev) +
> > + ev->num_hndl * sizeof(struct hci_comp_pkts_info)) {
>
> I think you can remove the first part of this check.
you are accessing ev->num_handl, so you need to ensure that you have at
least ev->num_handl size of data in your SKB. So this code is correct.
Regards
Marcel
Hi Andrei,
* Emeltchenko Andrei <[email protected]> [2011-12-19 16:31:30 +0200]:
> From: Andrei Emeltchenko <[email protected]>
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> Acked-by: Marcel Holtmann <[email protected]>
> ---
> include/net/bluetooth/hci.h | 7 ++++++-
> net/bluetooth/hci_event.c | 8 ++++----
> 2 files changed, 10 insertions(+), 5 deletions(-)
patches 1 to 4 applied, thanks.
Gustavo
Hi Andrei,
* Emeltchenko Andrei <[email protected]> [2011-12-19 16:31:31 +0200]:
> From: Andrei Emeltchenko <[email protected]>
>
> Remove unneeded skb_pull and correct packet length calculation
> removing magic number. Move BT_DBG after len check otherwise
> it could possibly access wrong memory.
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> ---
> net/bluetooth/hci_event.c | 9 ++++-----
> 1 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 919e3c0..47e1476 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -2266,20 +2266,19 @@ static inline void hci_num_comp_pkts_evt(struct hci_dev *hdev, struct sk_buff *s
> struct hci_ev_num_comp_pkts *ev = (void *) skb->data;
> int i;
>
> - skb_pull(skb, sizeof(*ev));
> -
> - BT_DBG("%s num_hndl %d", hdev->name, ev->num_hndl);
> -
> if (hdev->flow_ctl_mode != HCI_FLOW_CTL_MODE_PACKET_BASED) {
> BT_ERR("Wrong event for mode %d", hdev->flow_ctl_mode);
> return;
> }
>
> - if (skb->len < ev->num_hndl * 4) {
> + if (skb->len < sizeof(*ev) || skb->len < sizeof(*ev) +
> + ev->num_hndl * sizeof(struct hci_comp_pkts_info)) {
I think you can remove the first part of this check.
Gustavo
Hi Andrei,
> Remove unneeded skb_pull and correct packet length calculation
> removing magic number. Move BT_DBG after len check otherwise
> it could possibly access wrong memory.
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> ---
> net/bluetooth/hci_event.c | 9 ++++-----
> 1 files changed, 4 insertions(+), 5 deletions(-)
Acked-by: Marcel Holtmann <[email protected]>
Regards
Marcel
From: Andrei Emeltchenko <[email protected]>
Signed-off-by: Andrei Emeltchenko <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
net/bluetooth/hci_event.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index ceb7979..5138caf 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2271,6 +2271,11 @@ static inline void hci_num_comp_pkts_evt(struct hci_dev *hdev, struct sk_buff *s
BT_DBG("%s num_hndl %d", hdev->name, ev->num_hndl);
+ if (hdev->flow_ctl_mode != HCI_FLOW_CTL_MODE_PACKET_BASED) {
+ BT_ERR("Wrong event for mode %d", hdev->flow_ctl_mode);
+ return;
+ }
+
if (skb->len < ev->num_hndl * 4) {
BT_DBG("%s bad parameters", hdev->name);
return;
--
1.7.4.1
From: Andrei Emeltchenko <[email protected]>
Signed-off-by: Andrei Emeltchenko <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
include/net/bluetooth/hci.h | 4 ++++
net/bluetooth/hci_core.c | 4 ++++
2 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index c019b27..4ab1880 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -280,6 +280,10 @@ enum {
#define HCI_ERROR_LOCAL_HOST_TERM 0x16
#define HCI_ERROR_PAIRING_NOT_ALLOWED 0x18
+/* Flow control modes */
+#define HCI_FLOW_CTL_MODE_PACKET_BASED 0x00
+#define HCI_FLOW_CTL_MODE_BLOCK_BASED 0x01
+
/* ----- HCI Commands ---- */
#define HCI_OP_NOP 0x0000
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index bb089e3..884eb85 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -199,6 +199,8 @@ static void bredr_init(struct hci_dev *hdev)
__le16 param;
__u8 flt_type;
+ hdev->flow_ctl_mode = HCI_FLOW_CTL_MODE_PACKET_BASED;
+
/* Mandatory initialization */
/* Reset */
@@ -245,6 +247,8 @@ static void bredr_init(struct hci_dev *hdev)
static void amp_init(struct hci_dev *hdev)
{
+ hdev->flow_ctl_mode = HCI_FLOW_CTL_MODE_BLOCK_BASED;
+
/* Reset */
hci_send_cmd(hdev, HCI_OP_RESET, 0, NULL);
--
1.7.4.1
From: Andrei Emeltchenko <[email protected]>
Remove unneeded skb_pull and correct packet length calculation
removing magic number. Move BT_DBG after len check otherwise
it could possibly access wrong memory.
Signed-off-by: Andrei Emeltchenko <[email protected]>
---
net/bluetooth/hci_event.c | 9 ++++-----
1 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 919e3c0..47e1476 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2266,20 +2266,19 @@ static inline void hci_num_comp_pkts_evt(struct hci_dev *hdev, struct sk_buff *s
struct hci_ev_num_comp_pkts *ev = (void *) skb->data;
int i;
- skb_pull(skb, sizeof(*ev));
-
- BT_DBG("%s num_hndl %d", hdev->name, ev->num_hndl);
-
if (hdev->flow_ctl_mode != HCI_FLOW_CTL_MODE_PACKET_BASED) {
BT_ERR("Wrong event for mode %d", hdev->flow_ctl_mode);
return;
}
- if (skb->len < ev->num_hndl * 4) {
+ if (skb->len < sizeof(*ev) || skb->len < sizeof(*ev) +
+ ev->num_hndl * sizeof(struct hci_comp_pkts_info)) {
BT_DBG("%s bad parameters", hdev->name);
return;
}
+ BT_DBG("%s num_hndl %d", hdev->name, ev->num_hndl);
+
for (i = 0; i < ev->num_hndl; i++) {
struct hci_comp_pkts_info *info = &ev->handles[i];
struct hci_conn *conn;
--
1.7.4.1
From: Andrei Emeltchenko <[email protected]>
Signed-off-by: Andrei Emeltchenko <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
include/net/bluetooth/hci.h | 7 ++++++-
net/bluetooth/hci_event.c | 8 ++++----
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 4ab1880..5b2fed5 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -982,9 +982,14 @@ struct hci_ev_role_change {
} __packed;
#define HCI_EV_NUM_COMP_PKTS 0x13
+struct hci_comp_pkts_info {
+ __le16 handle;
+ __le16 count;
+} __packed;
+
struct hci_ev_num_comp_pkts {
__u8 num_hndl;
- /* variable length part */
+ struct hci_comp_pkts_info handles[0];
} __packed;
#define HCI_EV_MODE_CHANGE 0x14
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 5138caf..919e3c0 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2264,7 +2264,6 @@ static inline void hci_role_change_evt(struct hci_dev *hdev, struct sk_buff *skb
static inline void hci_num_comp_pkts_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
struct hci_ev_num_comp_pkts *ev = (void *) skb->data;
- __le16 *ptr;
int i;
skb_pull(skb, sizeof(*ev));
@@ -2281,12 +2280,13 @@ static inline void hci_num_comp_pkts_evt(struct hci_dev *hdev, struct sk_buff *s
return;
}
- for (i = 0, ptr = (__le16 *) skb->data; i < ev->num_hndl; i++) {
+ for (i = 0; i < ev->num_hndl; i++) {
+ struct hci_comp_pkts_info *info = &ev->handles[i];
struct hci_conn *conn;
__u16 handle, count;
- handle = get_unaligned_le16(ptr++);
- count = get_unaligned_le16(ptr++);
+ handle = __le16_to_cpu(info->handle);
+ count = __le16_to_cpu(info->count);
conn = hci_conn_hash_lookup_handle(hdev, handle);
if (!conn)
--
1.7.4.1
From: Andrei Emeltchenko <[email protected]>
Current controller initialization is moved tp bredr_init and new
function added amp_init to handle later AMP init sequence. Current
AMP init sequence include Reset and Read Local Version.
Signed-off-by: Andrei Emeltchenko <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>
---
net/bluetooth/hci_core.c | 64 ++++++++++++++++++++++++++++++++------------
net/bluetooth/hci_event.c | 3 ++
2 files changed, 49 insertions(+), 18 deletions(-)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index d6382db..bb089e3 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -193,33 +193,18 @@ static void hci_reset_req(struct hci_dev *hdev, unsigned long opt)
hci_send_cmd(hdev, HCI_OP_RESET, 0, NULL);
}
-static void hci_init_req(struct hci_dev *hdev, unsigned long opt)
+static void bredr_init(struct hci_dev *hdev)
{
struct hci_cp_delete_stored_link_key cp;
- struct sk_buff *skb;
__le16 param;
__u8 flt_type;
- BT_DBG("%s %ld", hdev->name, opt);
-
- /* Driver initialization */
-
- /* Special commands */
- while ((skb = skb_dequeue(&hdev->driver_init))) {
- bt_cb(skb)->pkt_type = HCI_COMMAND_PKT;
- skb->dev = (void *) hdev;
-
- skb_queue_tail(&hdev->cmd_q, skb);
- queue_work(hdev->workqueue, &hdev->cmd_work);
- }
- skb_queue_purge(&hdev->driver_init);
-
/* Mandatory initialization */
/* Reset */
if (!test_bit(HCI_QUIRK_NO_RESET, &hdev->quirks)) {
- set_bit(HCI_RESET, &hdev->flags);
- hci_send_cmd(hdev, HCI_OP_RESET, 0, NULL);
+ set_bit(HCI_RESET, &hdev->flags);
+ hci_send_cmd(hdev, HCI_OP_RESET, 0, NULL);
}
/* Read Local Supported Features */
@@ -258,6 +243,49 @@ static void hci_init_req(struct hci_dev *hdev, unsigned long opt)
hci_send_cmd(hdev, HCI_OP_DELETE_STORED_LINK_KEY, sizeof(cp), &cp);
}
+static void amp_init(struct hci_dev *hdev)
+{
+ /* Reset */
+ hci_send_cmd(hdev, HCI_OP_RESET, 0, NULL);
+
+ /* Read Local Version */
+ hci_send_cmd(hdev, HCI_OP_READ_LOCAL_VERSION, 0, NULL);
+}
+
+static void hci_init_req(struct hci_dev *hdev, unsigned long opt)
+{
+ struct sk_buff *skb;
+
+ BT_DBG("%s %ld", hdev->name, opt);
+
+ /* Driver initialization */
+
+ /* Special commands */
+ while ((skb = skb_dequeue(&hdev->driver_init))) {
+ bt_cb(skb)->pkt_type = HCI_COMMAND_PKT;
+ skb->dev = (void *) hdev;
+
+ skb_queue_tail(&hdev->cmd_q, skb);
+ queue_work(hdev->workqueue, &hdev->cmd_work);
+ }
+ skb_queue_purge(&hdev->driver_init);
+
+ switch (hdev->dev_type) {
+ case HCI_BREDR:
+ bredr_init(hdev);
+ break;
+
+ case HCI_AMP:
+ amp_init(hdev);
+ break;
+
+ default:
+ BT_ERR("Unknown device type %d", hdev->dev_type);
+ break;
+ }
+
+}
+
static void hci_le_init_req(struct hci_dev *hdev, unsigned long opt)
{
BT_DBG("%s", hdev->name);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 4275816..ceb7979 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -556,6 +556,9 @@ static void hci_set_le_support(struct hci_dev *hdev)
static void hci_setup(struct hci_dev *hdev)
{
+ if (hdev->dev_type != HCI_BREDR)
+ return;
+
hci_setup_event_mask(hdev);
if (hdev->hci_ver > BLUETOOTH_VER_1_1)
--
1.7.4.1