Re: [PATCH 2/4] Bluetooth: coredump: Fix btmon heap-buffer-overflow

[Luiz Augusto von Dentz]

Hi Zijun,

On Sun, Sep 13, 2026 at 11:43 PM Zijun Hu <zijun.hu@xxxxxxxxxxxxxxxx> wrote:
>
> The collected coredump data can exceed btmon's maximum packet size of
> 1490 bytes, But hci_devcd_dump() forwards the entire coredump data as an
> HCI_DIAG_PKT without checking its size, causing below issue in btmon.
>
> Why ?
>
> For a monitor packet, its payload length @len in header may exceed
> BTSNOOP_MAX_PACKET_SIZE, btmon uses it to access payload in @buf.
> so cause heap-buffer-overflow.
>
> Kernel:
> include/net/bluetooth/hci_mon.h
> struct hci_mon_hdr {
>         __le16  opcode;
>         __le16  index;
>         __le16  len;
> } __packed;
>
> BlueZ:
> src/shared/btsnoop.h
> monitor/control.c
> struct control_data {
>         uint16_t channel;
>         int fd;
>         unsigned char buf[BTSNOOP_MAX_PACKET_SIZE];
>         uint16_t offset;
> };
>
> Issue:
> ERROR: AddressSanitizer: heap-buffer-overflow on address 0x51b00000065c at pc 0x7f50b987a029 bp 0x7ffde88dc8d0 sp 0x7ffde88dc088
> READ of size 17916 at 0x51b00000065c thread T0
>     #0 0x7f50b987a028 in write ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1096
>     #1 0x5b4f3443ae3a in btsnoop_write ../src/shared/btsnoop.c:289
>     #2 0x5b4f3429cbf0 in data_callback ../monitor/control.c:969
>     #3 0x5b4f3444fa9d in mainloop_run ../src/shared/mainloop.c:104
>     #4 0x5b4f34451da6 in mainloop_run_with_signal ../src/shared/mainloop-notify.c:196
>     #5 0x5b4f342953fc in main ../monitor/main.c:303
>     #6 0x7f50b8c2a1c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
>     #7 0x7f50b8c2a28a in __libc_start_main_impl ../csu/libc-start.c:360
>     #8 0x5b4f34295ed4 in _start (/usr/bin/btmon+0x29fed4) (BuildId: b41caafb24db693946c283eaea48112186863d2d)

This probably needs to be fixed in btmon if it attempts to read past
the buffer size. Also, since it now supports vendor packets that may
not be limited by HCI packet size, perhaps we should allocate based on
the channel MTU which probably needs per-vendor configuration to allow
collecting its vendor packets.

> Fix by forwarding the collected dump data as an HCI_DIAG_PKT only when
> its size <= 1490.
>
> Fixes: b257e02ecc46 ("HCI: coredump: Log devcd dumps into the monitor")
> Signed-off-by: Zijun Hu <zijun.hu@xxxxxxxxxxxxxxxx>
> ---
>  net/bluetooth/coredump.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/net/bluetooth/coredump.c b/net/bluetooth/coredump.c
> index 71fc8dab4004..61b45ba58a65 100644
> --- a/net/bluetooth/coredump.c
> +++ b/net/bluetooth/coredump.c
> @@ -250,36 +250,43 @@ static void hci_devcd_handle_pkt_pattern(struct hci_dev *hdev,
>         }
>
>         pattern = skb_pull_data(skb, sizeof(*pattern));
>
>         if (!hci_devcd_memset(hdev, pattern->pattern, pattern->len))
>                 bt_dev_dbg(hdev, "Failed to set pattern");
>  }
>
> +/* Align with BlueZ's BTSNOOP_MAX_PACKET_SIZE. */
> +#define HCI_DEVCD_DIAG_MAX_SIZE (1486 + 4)
> +
>  static void hci_devcd_dump(struct hci_dev *hdev)
>  {
>         struct sk_buff *skb;
>         u32 size;
>
>         bt_dev_dbg(hdev, "state %s", hci_devcd_state_name(hdev->dump.state));
>
>         size = hdev->dump.tail - hdev->dump.head;
>
> -       /* Send a copy to monitor as a diagnostic packet */
> -       skb = bt_skb_alloc(size, GFP_ATOMIC);
> -       if (skb) {
> -               skb_put_data(skb, hdev->dump.head, size);
> -               hci_recv_diag(hdev, skb);
> +       if (size <= HCI_DEVCD_DIAG_MAX_SIZE) {
> +               /* Send a copy to monitor as a diagnostic packet */
> +               skb = bt_skb_alloc(size, GFP_ATOMIC);
> +               if (skb) {
> +                       skb_put_data(skb, hdev->dump.head, size);
> +                       hci_recv_diag(hdev, skb);
> +               }
>         }
>
>         /* Emit a devcoredump with the available data */
>         dev_coredumpv(&hdev->dev, hdev->dump.head, size, GFP_KERNEL);
>  }
>
> +#undef HCI_DEVCD_DIAG_MAX_SIZE
> +
>  static void hci_devcd_handle_pkt_complete(struct hci_dev *hdev,
>                                           struct sk_buff *skb)
>  {
>         u32 dump_size;
>
>         if (hdev->dump.state != HCI_DEVCOREDUMP_ACTIVE) {
>                 DBG_UNEXPECTED_STATE();
>                 return;
>
> --
> 2.34.1
>


-- 
Luiz Augusto von Dentz