Hi Luiz,
On Sun, May 24, 2015, Luiz Augusto von Dentz wrote:
> On Sun, May 24, 2015 at 4:58 PM, Johan Hedberg <johan.hedberg@xxxxxxxxx> wrote:
> > Hi Luiz,
> >
> > On Sun, May 24, 2015, Luiz Augusto von Dentz wrote:
> >> +static void print_group_list(const char *label, uint8_t length,
> >> + const void *data, uint16_t size)
> >> +{
> >> + uint8_t count;
> >> +
> >> + if (length == 0)
> >> + return;
> >> +
> >> + count = size / length;
> >> +
> >> + print_field("%s: %u entr%s", label, count, count == 1 ? "y" : "ies");
> >> +
> >> + while (size >= length) {
> >> + print_handle_range("Handle range", data);
> >> + print_uuid("UUID", data + 4, length - 4);
> >> +
> >> + data += length;
> >> + size -= length;
> >> + }
> >> +
> >> + packet_hexdump(data, size);
> >> +}
> >
> > Looks good, but to avoid btmon crashes (e.g. when analyzing codenomicon
> > logs) might be good to add check for 'length >= 6' and jump to
> > packet_hexdump if you get something smaller than that.
>
> print_uuid already does that and we have a check for minimal size of 4
> for att_read_group_type_rsp,
I was aware of the checks in print_uuid() but not of the minimum size 4
check that has already happened before entering the print_group_list
function. That should indeed be enough protection for buffer overflows.
> what I think we really want to check field by field like is done in
> avctp.c for example, so we get exactly where the pdu is breaking, but
> to get to that we will have to fix much more than print_group_list
Sounds reasonable to me.
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
