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

[Luiz Augusto von Dentz]

Hi Cen,

On Thu, May 28, 2026 at 9:30 AM Cen Zhang <rollkingzzc@xxxxxxxxx> wrote:
>
> 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

This is what you get if you don't know where to look at:

struct bnep_setup_conn_req {
    uint8_t  type;
    uint8_t  ctrl;
    uint8_t  uuid_size;
    uint8_t  service[0];
} __attribute__((packed));
https://github.com/bluez/bluez/blob/master/lib/bluetooth/bnep.h#L79

It is in fact a tuple of src + dst service as parsed by btmon:

https://github.com/bluez/bluez/blob/master/monitor/bnep.c#L104

> 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.

Probably just reference the above, but the fact that you couldn't
mention any of the BlueZ references really bothers me. Although the
result was correct, the lack of references makes me believe I'm just
arguing with an AI prompt which doesn't have BlueZ userspace loaded as
context.


-- 
Luiz Augusto von Dentz