* [PATCH] Bluetooth: stop proccessing malicious adv data
@ 2021-11-01 7:12 Pavel Skripkin
2021-11-16 5:02 ` Pavel Skripkin
2021-11-16 13:55 ` Marcel Holtmann
0 siblings, 2 replies; 3+ messages in thread
From: Pavel Skripkin @ 2021-11-01 7:12 UTC (permalink / raw)
To: marcel, johan.hedberg, luiz.dentz, davem, kuba
Cc: linux-bluetooth, netdev, linux-kernel, Pavel Skripkin,
syzbot+e3fcb9c4f3c2a931dc40
Syzbot reported slab-out-of-bounds read in hci_le_adv_report_evt(). The
problem was in missing validaion check.
We should check if data is not malicious and we can read next data block.
If we won't check ptr validness, code can read a way beyond skb->end and
it can cause problems, of course.
Fixes: e95beb414168 ("Bluetooth: hci_le_adv_report_evt code refactoring")
Reported-and-tested-by: syzbot+e3fcb9c4f3c2a931dc40@syzkaller.appspotmail.com
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
net/bluetooth/hci_event.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 0bca035bf2dc..50d1d62c15ec 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -5780,7 +5780,8 @@ 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;
- if (ev->length <= HCI_MAX_AD_LENGTH) {
+ if (ev->length <= HCI_MAX_AD_LENGTH &&
+ ev->data + ev->length <= skb_tail_pointer(skb)) {
rssi = ev->data[ev->length];
process_adv_report(hdev, ev->evt_type, &ev->bdaddr,
ev->bdaddr_type, NULL, 0, rssi,
@@ -5790,6 +5791,11 @@ static void hci_le_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb)
}
ptr += sizeof(*ev) + ev->length + 1;
+
+ if (ptr > (void *) skb_tail_pointer(skb) - sizeof(*ev)) {
+ bt_dev_err(hdev, "Malicious advertising data. Stopping processing");
+ break;
+ }
}
hci_dev_unlock(hdev);
--
2.33.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] Bluetooth: stop proccessing malicious adv data
2021-11-01 7:12 [PATCH] Bluetooth: stop proccessing malicious adv data Pavel Skripkin
@ 2021-11-16 5:02 ` Pavel Skripkin
2021-11-16 13:55 ` Marcel Holtmann
1 sibling, 0 replies; 3+ messages in thread
From: Pavel Skripkin @ 2021-11-16 5:02 UTC (permalink / raw)
To: marcel, johan.hedberg, luiz.dentz, davem, kuba
Cc: linux-bluetooth, netdev, linux-kernel, syzbot+e3fcb9c4f3c2a931dc40
On 11/1/21 10:12, Pavel Skripkin wrote:
> Syzbot reported slab-out-of-bounds read in hci_le_adv_report_evt(). The
> problem was in missing validaion check.
>
> We should check if data is not malicious and we can read next data block.
> If we won't check ptr validness, code can read a way beyond skb->end and
> it can cause problems, of course.
>
> Fixes: e95beb414168 ("Bluetooth: hci_le_adv_report_evt code refactoring")
> Reported-and-tested-by: syzbot+e3fcb9c4f3c2a931dc40@syzkaller.appspotmail.com
> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
> ---
Hi, Bluetooth maintainers!
friendly ping :)
If anything is wrong with this one, please, let me know
With regards,
Pavel Skripkin
> net/bluetooth/hci_event.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 0bca035bf2dc..50d1d62c15ec 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -5780,7 +5780,8 @@ 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;
>
> - if (ev->length <= HCI_MAX_AD_LENGTH) {
> + if (ev->length <= HCI_MAX_AD_LENGTH &&
> + ev->data + ev->length <= skb_tail_pointer(skb)) {
> rssi = ev->data[ev->length];
> process_adv_report(hdev, ev->evt_type, &ev->bdaddr,
> ev->bdaddr_type, NULL, 0, rssi,
> @@ -5790,6 +5791,11 @@ static void hci_le_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb)
> }
>
> ptr += sizeof(*ev) + ev->length + 1;
> +
> + if (ptr > (void *) skb_tail_pointer(skb) - sizeof(*ev)) {
> + bt_dev_err(hdev, "Malicious advertising data. Stopping processing");
> + break;
> + }
> }
>
> hci_dev_unlock(hdev);
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Bluetooth: stop proccessing malicious adv data
2021-11-01 7:12 [PATCH] Bluetooth: stop proccessing malicious adv data Pavel Skripkin
2021-11-16 5:02 ` Pavel Skripkin
@ 2021-11-16 13:55 ` Marcel Holtmann
1 sibling, 0 replies; 3+ messages in thread
From: Marcel Holtmann @ 2021-11-16 13:55 UTC (permalink / raw)
To: Pavel Skripkin
Cc: Johan Hedberg, Luiz Augusto von Dentz, David S. Miller,
Jakub Kicinski, linux-bluetooth, netdev, linux-kernel,
syzbot+e3fcb9c4f3c2a931dc40
Hi Pavel,
> Syzbot reported slab-out-of-bounds read in hci_le_adv_report_evt(). The
> problem was in missing validaion check.
>
> We should check if data is not malicious and we can read next data block.
> If we won't check ptr validness, code can read a way beyond skb->end and
> it can cause problems, of course.
>
> Fixes: e95beb414168 ("Bluetooth: hci_le_adv_report_evt code refactoring")
> Reported-and-tested-by: syzbot+e3fcb9c4f3c2a931dc40@syzkaller.appspotmail.com
> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
> ---
> net/bluetooth/hci_event.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-11-16 13:55 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-01 7:12 [PATCH] Bluetooth: stop proccessing malicious adv data Pavel Skripkin
2021-11-16 5:02 ` Pavel Skripkin
2021-11-16 13:55 ` 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.