| Message ID | 20211101071212.15355-1-paskripkin@gmail.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Bluetooth: stop proccessing malicious adv data | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Single patches do not need cover letters |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/cc_maintainers | success | CCed 7 of 7 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Fixes tag looks correct |
| netdev/checkpatch | warning | CHECK: No space is necessary after a cast |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | No static functions without inline keyword in header files |
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); >
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
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);
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(-)