This patch series fixes a NULL pointer dereference in skb_dequeue()
during QCA devcoredump handling, and adds some safety checks to make the
parsing more robust.

The first patch fixes the logic bug where dump packets were mistakenly
passed to hci_recv_frame() and freed prematurely. This was caused by
handle_dump_pkt_qca() returning 0 even when the dump was successfully
handled. It also refactors dump packet detection into separate helpers
for ACL and event packets.

The second patch adds bounds checks and replaces direct pointer access
with skb_pull() and skb_pull_data() to avoid accessing invalid memory
on malformed packets.

Tested on WCN7851 (0489:e0f3) with devcoredump enabled. Crash no
longer occurs and dumps are processed correctly.
  
Changes in v4:
- Fix unused variable error in the first patch
- Refine commit messages

Changes in v3:
- Use skb_pull_data() for safe packet header access
- Split dump packet detection into separate ACL and event helpers

Changes in v2:
- Fixed typo in the title
- Re-flowed commit message line to fit 72 characters
- Added blank line before btusb_recv_acl_qca()

En-Wei Wu (2):
  Bluetooth: btusb: avoid NULL pointer dereference in skb_dequeue()
  Bluetooth: btusb: use skb_pull to avoid unsafe access in QCA dump
    handling

 drivers/bluetooth/btusb.c | 120 +++++++++++++++++++++++---------------
 1 file changed, 74 insertions(+), 46 deletions(-)

-- 
2.43.0

Hi En-Wei,

On Mon, Apr 21, 2025 at 9:00 AM En-Wei Wu <en-wei.wu@canonical.com> wrote:
>
> This patch series fixes a NULL pointer dereference in skb_dequeue()
> during QCA devcoredump handling, and adds some safety checks to make the
> parsing more robust.

While at it, please move this logic to qca specific file, there is no
reason for this logic to remain inside btusb.c

> The first patch fixes the logic bug where dump packets were mistakenly
> passed to hci_recv_frame() and freed prematurely. This was caused by
> handle_dump_pkt_qca() returning 0 even when the dump was successfully
> handled. It also refactors dump packet detection into separate helpers
> for ACL and event packets.
>
> The second patch adds bounds checks and replaces direct pointer access
> with skb_pull() and skb_pull_data() to avoid accessing invalid memory
> on malformed packets.
>
> Tested on WCN7851 (0489:e0f3) with devcoredump enabled. Crash no
> longer occurs and dumps are processed correctly.
>
> Changes in v4:
> - Fix unused variable error in the first patch
> - Refine commit messages
>
> Changes in v3:
> - Use skb_pull_data() for safe packet header access
> - Split dump packet detection into separate ACL and event helpers
>
> Changes in v2:
> - Fixed typo in the title
> - Re-flowed commit message line to fit 72 characters
> - Added blank line before btusb_recv_acl_qca()
>
> En-Wei Wu (2):
>   Bluetooth: btusb: avoid NULL pointer dereference in skb_dequeue()
>   Bluetooth: btusb: use skb_pull to avoid unsafe access in QCA dump
>     handling
>
>  drivers/bluetooth/btusb.c | 120 +++++++++++++++++++++++---------------
>  1 file changed, 74 insertions(+), 46 deletions(-)
>
> --
> 2.43.0
>


-- 
Luiz Augusto von Dentz

Hi Luiz,

> While at it, please move this logic to qca specific file, there is no
> reason for this logic to remain inside btusb.c
I'll work on the v5 soon. Here is what I plan for v5:

[PATCH v4 1/3]: Move the device-core-dump logic of QCA from btusb.c to btqca.c
[PATCH v4 2/3]: Fix the original NULL pointer dereference as in [PATCH v3 1/2]
[PATCH v4 3/3]: Use skb_pull for safer skb access as in [PATCH v3
2/2], and use skb_clone to avoid directly resuming skb->data and
skb->len

Many thanks,
En-Wei.

On Tue, 22 Apr 2025 at 03:17, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi En-Wei,
>
> On Mon, Apr 21, 2025 at 9:00 AM En-Wei Wu <en-wei.wu@canonical.com> wrote:
> >
> > This patch series fixes a NULL pointer dereference in skb_dequeue()
> > during QCA devcoredump handling, and adds some safety checks to make the
> > parsing more robust.
>
> While at it, please move this logic to qca specific file, there is no
> reason for this logic to remain inside btusb.c
>
> > The first patch fixes the logic bug where dump packets were mistakenly
> > passed to hci_recv_frame() and freed prematurely. This was caused by
> > handle_dump_pkt_qca() returning 0 even when the dump was successfully
> > handled. It also refactors dump packet detection into separate helpers
> > for ACL and event packets.
> >
> > The second patch adds bounds checks and replaces direct pointer access
> > with skb_pull() and skb_pull_data() to avoid accessing invalid memory
> > on malformed packets.
> >
> > Tested on WCN7851 (0489:e0f3) with devcoredump enabled. Crash no
> > longer occurs and dumps are processed correctly.
> >
> > Changes in v4:
> > - Fix unused variable error in the first patch
> > - Refine commit messages
> >
> > Changes in v3:
> > - Use skb_pull_data() for safe packet header access
> > - Split dump packet detection into separate ACL and event helpers
> >
> > Changes in v2:
> > - Fixed typo in the title
> > - Re-flowed commit message line to fit 72 characters
> > - Added blank line before btusb_recv_acl_qca()
> >
> > En-Wei Wu (2):
> >   Bluetooth: btusb: avoid NULL pointer dereference in skb_dequeue()
> >   Bluetooth: btusb: use skb_pull to avoid unsafe access in QCA dump
> >     handling
> >
> >  drivers/bluetooth/btusb.c | 120 +++++++++++++++++++++++---------------
> >  1 file changed, 74 insertions(+), 46 deletions(-)
> >
> > --
> > 2.43.0
> >
>
>
> --
> Luiz Augusto von Dentz

> [PATCH v4 1/3]: Move the device-core-dump logic of QCA from btusb.c to btqca.c
> [PATCH v4 2/3]: Fix the original NULL pointer dereference as in [PATCH v3 1/2]
> [PATCH v4 3/3]: Use skb_pull for safer skb access as in [PATCH v3
> 2/2], and use skb_clone to avoid directly resuming skb->data and skb->len
Should be [PATCH v5 */3].

On Thu, 24 Apr 2025 at 09:46, En-Wei WU <en-wei.wu@canonical.com> wrote:
>
> Hi Luiz,
>
> > While at it, please move this logic to qca specific file, there is no
> > reason for this logic to remain inside btusb.c
> I'll work on the v5 soon. Here is what I plan for v5:
>
> [PATCH v4 1/3]: Move the device-core-dump logic of QCA from btusb.c to btqca.c
> [PATCH v4 2/3]: Fix the original NULL pointer dereference as in [PATCH v3 1/2]
> [PATCH v4 3/3]: Use skb_pull for safer skb access as in [PATCH v3
> 2/2], and use skb_clone to avoid directly resuming skb->data and
> skb->len
>
> Many thanks,
> En-Wei.
>
> On Tue, 22 Apr 2025 at 03:17, Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > Hi En-Wei,
> >
> > On Mon, Apr 21, 2025 at 9:00 AM En-Wei Wu <en-wei.wu@canonical.com> wrote:
> > >
> > > This patch series fixes a NULL pointer dereference in skb_dequeue()
> > > during QCA devcoredump handling, and adds some safety checks to make the
> > > parsing more robust.
> >
> > While at it, please move this logic to qca specific file, there is no
> > reason for this logic to remain inside btusb.c
> >
> > > The first patch fixes the logic bug where dump packets were mistakenly
> > > passed to hci_recv_frame() and freed prematurely. This was caused by
> > > handle_dump_pkt_qca() returning 0 even when the dump was successfully
> > > handled. It also refactors dump packet detection into separate helpers
> > > for ACL and event packets.
> > >
> > > The second patch adds bounds checks and replaces direct pointer access
> > > with skb_pull() and skb_pull_data() to avoid accessing invalid memory
> > > on malformed packets.
> > >
> > > Tested on WCN7851 (0489:e0f3) with devcoredump enabled. Crash no
> > > longer occurs and dumps are processed correctly.
> > >
> > > Changes in v4:
> > > - Fix unused variable error in the first patch
> > > - Refine commit messages
> > >
> > > Changes in v3:
> > > - Use skb_pull_data() for safe packet header access
> > > - Split dump packet detection into separate ACL and event helpers
> > >
> > > Changes in v2:
> > > - Fixed typo in the title
> > > - Re-flowed commit message line to fit 72 characters
> > > - Added blank line before btusb_recv_acl_qca()
> > >
> > > En-Wei Wu (2):
> > >   Bluetooth: btusb: avoid NULL pointer dereference in skb_dequeue()
> > >   Bluetooth: btusb: use skb_pull to avoid unsafe access in QCA dump
> > >     handling
> > >
> > >  drivers/bluetooth/btusb.c | 120 +++++++++++++++++++++++---------------
> > >  1 file changed, 74 insertions(+), 46 deletions(-)
> > >
> > > --
> > > 2.43.0
> > >
> >
> >
> > --
> > Luiz Augusto von Dentz