Re: [PATCH v4] Bluetooth: bnep: reject short frames before parsing

[Cen Zhang]

Hi Luiz,

Thanks for taking another look.

> This still looks incorrect to me, why should we do * 2? If look at
> BNEP_FILTER_NET_TYPE_SET it just use the header to pull using
> get_unaligned, I know this is an existing problem but except if the
> BNEP_SETUP_CONN_REQ header length works on a multiple of 2 this makes
> no sense to me.

The reason I used `* 2` there is that I read that byte as the UUID size,
not as a total payload length.

For BNEP_SETUP_CONN_REQ, the byte after the control type is the UUID
Size field.  The setup request then carries two UUIDs of that size: the
destination service UUID and the source service UUID.  So the intended
skip was:

  UUID Size field itself
  destination service UUID, uuid_size bytes
  source service UUID, uuid_size bytes

That is different from BNEP_FILTER_NET_TYPE_SET, where the header has a
16-bit length field and the value from get_unaligned_be16() is already
the total filter-list payload length.

That said, I agree the patch makes this much less clear than it should.
The comment says "data (len * 2 bytes)", which makes it look like a
generic length field, and the raw `* 2` is too opaque.

I can send a v5 that names the byte `uuid_size` and uses
`uuid_size + uuid_size` with a comment explaining that the two parts are
the destination and source service UUIDs.If I have misunderstood anything
here, sorry for the confusion. I would be happy to help with any
further changes
or testing.

Thanks,
Zhang Cen