Re: [PATCH] Bluetooth: btnxpuart: Keep FW dump header in coredump chunks

[Ali Ahmet Memis <ali@iusegentoo.com>]

Hi Luiz,

On Tue, Aug 18, 2026 at 12:59 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Yeah, and it concatenates the header states into a dump skb,
> interleaving data with headers. The worst part is that if the analyzer
> finds an issue with the header it can consider the entire trace
> malformed. This means both the kernel and userspace try to parse the
> same thing but may interpret what each chunk means differently.

The kernel uses these two fields only for framing. It does not interpret
them as part of the dump format or validate their contents. It also does
not compare buf_len with the actual payload length.

This cannot be moved to userspace because the driver has no other way to
know where a dump starts and ends. Without seq_num and buf_len, it
cannot know when to call hci_devcd_init() and hci_devcd_complete().

I agree there is some overlap here, and I think this is also why a short
chunk should not be dropped silently.

> Well, it does change, it doesn't append anything if the chunk is less
> than a header and that won't even show up for the analyzer to analyze.

You're right. That statement was too broad. A chunk shorter than the
header is dropped, so the analyzer never sees it.

> Now, regarding this, I wonder if the driver receives a chunk smaller
> than the header size, whether we should either append it and leave
> interpretation to userspace, or 2 drop the entire dump because we
> can't guarantee its consistency if we suspect data corruption when the
> firmware is not supposed to generate chunks smaller than the header
> size.

I would go with the second option.

Appending it does not really leave the interpretation to userspace. Each
header tells the analyzer how many bytes belong to that chunk. If we add
a fragment without a header, all following headers are shifted and the
rest of the dump becomes difficult to parse.

Aborting seems like a better fit. The firmware should not generate a
chunk smaller than the header, so seeing one means something has already
gone wrong. We can no longer rely on the rest of the dump being
consistent.

hci_devcd_abort() keeps the data collected so far and marks the dump
with HCI_DEVCOREDUMP_ABORT in the State line. Userspace can then see
that the dump is truncated. hci_qca and hci_vhci already use it on
their error paths.

> Yeah, the decision of considering a chunk valid or not seems to be
> split between kernel and userspace. The kernel evaluates if the chunk
> is too short but won't check any data consistency after that;
> userspace will evaluate the headers but won't get a chance to evaluate
> if any small chunk was dropped in the process.

I think aborting closes that gap. The kernel decides that a chunk too
short to contain a header is invalid, and userspace can see that the
dump was aborted instead of silently missing bytes.

Would you prefer this as a separate patch on top? This patch only
restores the header in the appended chunk, so its Fixes tag can stay
focused on that regression. Handling short chunks with an abort would
be a separate behavior change.

Thanks,
Ali