On Thu, Apr 4, 2019 at 1:35 AM Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
>
> 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.
It could be, at least it is not a bug introduced by my patch therefore
I have no obligation to fix it?
I am happy to rebase if that fix is merged before mine, like for any other
changes, if this is what you are trying to say.
>
> Tomas and I should get Reported-by tags for parts of this patch.
Sure, sorry for missing it, as I got reports from syzbot directly and
before you reported it. Will add it in v3.
Thanks.
