* [PATCH] Bluetooth: Prevent buffer overflow for large advertisement data @ 2018-04-19 11:03 Chriz Chow 2018-04-19 12:28 ` Szymon Janc 0 siblings, 1 reply; 4+ messages in thread From: Chriz Chow @ 2018-04-19 11:03 UTC (permalink / raw) To: marcel, linux-bluetooth; +Cc: Chriz Chow In the Bluetooth Core Specifications 5.0 it has no limitation of the length of Advertising or Scan Response Data. Therefore, there are some devices sending out advertising data longer than HCI_MAX_AD_LENGTH, causing the buffer last_adv_data overflows. It prevents the issue by checking the data length before copying. Signed-off-by: Chriz Chow <chriz.chow@aminocom.com> --- net/bluetooth/hci_event.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 139707c..1300bb0 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -1118,6 +1118,9 @@ static void store_pending_adv_report(struct hci_dev *hdev, bdaddr_t *bdaddr, { struct discovery_state *d = &hdev->discovery; + if (len > HCI_MAX_AD_LENGTH) + return; + bacpy(&d->last_adv_addr, bdaddr); d->last_adv_addr_type = bdaddr_type; d->last_adv_rssi = rssi; -- 2.7.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] Bluetooth: Prevent buffer overflow for large advertisement data 2018-04-19 11:03 [PATCH] Bluetooth: Prevent buffer overflow for large advertisement data Chriz Chow @ 2018-04-19 12:28 ` Szymon Janc 2018-04-20 7:46 ` [PATCH v2] " Chriz Chow 0 siblings, 1 reply; 4+ messages in thread From: Szymon Janc @ 2018-04-19 12:28 UTC (permalink / raw) To: Chriz Chow; +Cc: marcel, linux-bluetooth, Chriz Chow Hi Chriz, On Thursday, 19 April 2018 13:03:09 CEST Chriz Chow wrote: > In the Bluetooth Core Specifications 5.0 it has no limitation of > the length of Advertising or Scan Response Data. Therefore, > there are some devices sending out advertising data longer than > HCI_MAX_AD_LENGTH, causing the buffer last_adv_data overflows. > > It prevents the issue by checking the data length before copying. > > Signed-off-by: Chriz Chow <chriz.chow@aminocom.com> > --- > net/bluetooth/hci_event.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c > index 139707c..1300bb0 100644 > --- a/net/bluetooth/hci_event.c > +++ b/net/bluetooth/hci_event.c > @@ -1118,6 +1118,9 @@ static void store_pending_adv_report(struct hci_dev > *hdev, bdaddr_t *bdaddr, { > struct discovery_state *d = &hdev->discovery; > > + if (len > HCI_MAX_AD_LENGTH) > + return; > + > bacpy(&d->last_adv_addr, bdaddr); > d->last_adv_addr_type = bdaddr_type; > d->last_adv_rssi = rssi; This would indicate that controller is sending LE Advertising Report Event with illegal length value (valid range is 0x00-0x1f). If there are such broken controllers around we could check those but I think this should be done as soon as possible ie. in hci_le_adv_report_evt() function. Also, >31 bytes of advertising data is only for Extended Advertising which would require using LE Set Extended Scan commands and would result in LE Extended Advertising Reports being generated instead (none of those are yet supported in kernel). -- pozdrawiam Szymon Janc ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] Bluetooth: Prevent buffer overflow for large advertisement data 2018-04-19 12:28 ` Szymon Janc @ 2018-04-20 7:46 ` Chriz Chow 2018-04-23 17:58 ` Marcel Holtmann 0 siblings, 1 reply; 4+ messages in thread From: Chriz Chow @ 2018-04-20 7:46 UTC (permalink / raw) To: Marcel Holtmann, Johan Hedberg, linux-bluetooth, Szymon Janc; +Cc: Chriz Chow There are some controllers sending out advertising data with illegal length value which is longer than HCI_MAX_AD_LENGTH, causing the buffer last_adv_data overflows. To avoid these controllers from overflowing the buffer, we do not process the advertisement data if its length is incorrect. Signed-off-by: Chriz Chow <chriz.chow@aminocom.com> --- net/bluetooth/hci_event.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 139707c..dc4cfd5 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -4942,10 +4942,14 @@ static void hci_le_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb) struct hci_ev_le_advertising_info *ev = ptr; s8 rssi; - rssi = ev->data[ev->length]; - process_adv_report(hdev, ev->evt_type, &ev->bdaddr, - ev->bdaddr_type, NULL, 0, rssi, - ev->data, ev->length); + if (ev->length <= HCI_MAX_AD_LENGTH) { + rssi = ev->data[ev->length]; + process_adv_report(hdev, ev->evt_type, &ev->bdaddr, + ev->bdaddr_type, NULL, 0, rssi, + ev->data, ev->length); + } else { + BT_ERR("Dropped invalid adv data"); + } ptr += sizeof(*ev) + ev->length + 1; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] Bluetooth: Prevent buffer overflow for large advertisement data 2018-04-20 7:46 ` [PATCH v2] " Chriz Chow @ 2018-04-23 17:58 ` Marcel Holtmann 0 siblings, 0 replies; 4+ messages in thread From: Marcel Holtmann @ 2018-04-23 17:58 UTC (permalink / raw) To: Chriz Chow; +Cc: Johan Hedberg, BlueZ development, Szymon Janc, Chriz Chow Hi Chriz, > There are some controllers sending out advertising data with illegal > length value which is longer than HCI_MAX_AD_LENGTH, causing the > buffer last_adv_data overflows. To avoid these controllers from > overflowing the buffer, we do not process the advertisement data > if its length is incorrect. > > Signed-off-by: Chriz Chow <chriz.chow@aminocom.com> > --- > net/bluetooth/hci_event.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) patch has been applied to bluetooth-next tree. Regards Marcel ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-04-23 17:58 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-19 11:03 [PATCH] Bluetooth: Prevent buffer overflow for large advertisement data Chriz Chow 2018-04-19 12:28 ` Szymon Janc 2018-04-20 7:46 ` [PATCH v2] " Chriz Chow 2018-04-23 17:58 ` Marcel Holtmann
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.