Hi Vinicius,
On Fri, Oct 14, 2011, Johan Hedberg wrote:
> Hi Vinicius,
>
> On Thu, Oct 13, 2011, Vinicius Costa Gomes wrote:
> > + while (len < HCI_MAX_EIR_LENGTH - 1) {
> > + uint8_t field_len = eir_data[0];
> > +
> > + /* Check for the end of EIR */
> > + if (field_len == 0)
> > + break;
>
> I suppose there should also be a check for:
>
> if (len + field_len > HCI_MAX_EIR_LENGTH)
> goto failed;
>
> Otherwise you're gonna access past the end of the eir_data buffer when
> you do the memcpy later.
>
> > +
> > + switch (eir_data[1]) {
> > + case EIR_NAME_SHORT:
> > + case EIR_NAME_COMPLETE:
> > + if (field_len > HCI_MAX_NAME_LENGTH)
> > + goto failed;
>
> If you add the if-statement I suggested earlier you can remove this one
> (since it becomes redundant).
>
> > +
> > + memcpy(name, &eir_data[2], field_len - 1);
> > + return;
> > + }
> > +
> > + len += field_len + 1;
> > + eir_data += field_len + 1;
> > + }
> > +
> > +failed:
> > + sprintf(name, "(unknown)");
> > + return;
> > +}
>
> Please remove the unnecessary return statement here.
I had a slight confusion with my local patch handling and your patch
went upstream by mistake. So, I ended up fixing these issues by myself.
There were actually several more issues which I spotted and fixed in the
same go:
- read_flags() had missing checks too
- The LE EIR data length is variable and max 0x1f (31) bytes, i.e. not
HCI_MAX_EIR_LENGTH
- Because of the above limit the name is max 29 bytes
Please check upstream (github) and verify that I didn't make any mistake
(since I was only able to do only limited testing here).
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
