Re: [PATCH BlueZ] Add support for parsing the remote name during LE Scan

[Johan Hedberg <johan.hedberg@gmail.com>]

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