On Wed, Apr 03, 2019 at 04:08:34PM -0700, Cong Wang wrote:
> static void hci_le_ext_adv_report_evt(struct hci_dev *hdev, struct sk_buff
> *skb)
> {
> - u8 num_reports = skb->data[0];
> - void *ptr = &skb->data[1];
> + unsigned int len;
> + u8 num_reports;
> +
> + if (unlikely(!pskb_may_pull(skb, 1)))
> + return;
> + num_reports = skb->data[0];
> + len = 1;
>
> hci_dev_lock(hdev);
>
> while (num_reports--) {
> - struct hci_ev_le_ext_adv_report *ev = ptr;
> + struct hci_ev_le_ext_adv_report *ev;
> u8 legacy_evt_type;
> u16 evt_type;
> + u8 ev_len;
> +
> + if (unlikely(!pskb_may_pull(skb, len + sizeof(*ev))))
> + break;
> + ev = (void *)skb->data + len;
> + ev_len = ev->length + 1;
The "+ 1" is a bug. It was discussed in a different thread. Jaganath
says he has sent a patch to fix it already.
Probably it worked in testing because the num_reports was always 1, but
now when we add this condition with the "+ 1" bug at the start of the
loop the the the num_reports == 1 case will be broken as well.
Tomas and I should get Reported-by tags for parts of this patch.
> + if (unlikely(!pskb_may_pull(skb, len + sizeof(*ev) + ev_len)))
> + break;
> + ev = (void *)skb->data + len;
>
> evt_type = __le16_to_cpu(ev->evt_type);
> legacy_evt_type = ext_evt_type_to_legacy(evt_type);
> @@ -5601,7 +5645,7 @@ static void hci_le_ext_adv_report_evt(struct hci_dev
> *hdev, struct sk_buff *skb)
> ev->data, ev->length);
> }
>
> - ptr += sizeof(*ev) + ev->length + 1;
> + len += sizeof(*ev) + ev_len;
> }
>
> hci_dev_unlock(hdev);
regards,
dan carpenter
