linux-kernel - Re: [PATCH] Bluetooth: hci_event: validate HCI event packet Parameter Total Length

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CABBYNZJtp0wqL_SJEk0wVo4DuadrBirmJ5VOe_TyE_RN8jOJNA@mail.gmail.com>
Date: Thu, 23 Oct 2025 17:05:06 -0400
From: Luiz Augusto von Dentz <luiz.dentz@...il.com>
To: Raphael Pinsonneault-Thibeault <rpthibeault@...il.com>
Cc: marcel@...tmann.org, johan.hedberg@...il.com, 
	linux-bluetooth@...r.kernel.org, linux-kernel@...r.kernel.org, 
	syzbot+a9a4bedfca6aa9d7fa24@...kaller.appspotmail.com, 
	linux-kernel-mentees@...ts.linux.dev, skhan@...uxfoundation.org, 
	david.hunter.linux@...il.com, khalid@...nel.org
Subject: Re: [PATCH] Bluetooth: hci_event: validate HCI event packet Parameter
 Total Length

Hi Raphael,

On Wed, Oct 22, 2025 at 6:38 PM Raphael Pinsonneault-Thibeault
<rpthibeault@...il.com> wrote:
>
> There is a BUG: KMSAN: uninit-value in hci_cmd_complete_evt() due to a
> malformed HCI event packet received from userspace.
>
> The existing code in hci_event_packet() checks that the buffer is large
> enough to contain the event header, and checks that the hdr's Event Code
> is valid, but does not check the hdr's Parameter Total Length. So,
> syzbot’s event packet passes through and uses the un-init values in
> hci_event_func() => hci_cmd_complete_evt().

It does checks the length:

    if (skb->len < ev->min_len) {
        bt_dev_err(hdev, "unexpected event 0x%2.2x length: %u < %u",
               event, skb->len, ev->min_len);
        return;
    }

> Reported-by: syzbot+a9a4bedfca6aa9d7fa24@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=a9a4bedfca6aa9d7fa24
> Tested-by: syzbot+a9a4bedfca6aa9d7fa24@...kaller.appspotmail.com
> Fixes: a9de9248064bf ("[Bluetooth] Switch from OGF+OCF to using only opcodes")
> Signed-off-by: Raphael Pinsonneault-Thibeault <rpthibeault@...il.com>
> ---
>  net/bluetooth/hci_event.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index d790b0d4eb9a..5e1498cc04cd 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -7565,7 +7565,7 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
>         hci_req_complete_t req_complete = NULL;
>         hci_req_complete_skb_t req_complete_skb = NULL;
>         struct sk_buff *orig_skb = NULL;
> -       u8 status = 0, event, req_evt = 0;
> +       u8 status = 0, event, req_evt = 0, len;
>         u16 opcode = HCI_OP_NOP;
>
>         if (skb->len < sizeof(*hdr)) {
> @@ -7585,6 +7585,13 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
>                 goto done;
>         }
>
> +       len = hdr->plen;
> +       if (len != skb->len - HCI_EVENT_HDR_SIZE) {
> +               bt_dev_warn(hdev, "Unexpected HCI Parameter Length 0x%2.2x",
> +                           len);
> +               goto done;
> +       }

Looks like a big hammer for a uninitialized value, which I assume is
from the following code:

    if (i == ARRAY_SIZE(hci_cc_table)) {
        /* Unknown opcode, assume byte 0 contains the status, so
         * that e.g. __hci_cmd_sync() properly returns errors
         * for vendor specific commands send by HCI drivers.
         * If a vendor doesn't actually follow this convention we may
         * need to introduce a vendor CC table in order to properly set
         * the status.
         */
        *status = skb->data[0];
    }

That one is accessing skb->data without first checking it like
hci_cc_skb_pull like all other event handlers are doing, if that is
really the case then something like the following should make it go
away:

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index bae8c219341a..e71fbdebffae 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -4219,6 +4219,13 @@ static void hci_cmd_complete_evt(struct hci_dev
*hdev, void *data,
        }

        if (i == ARRAY_SIZE(hci_cc_table)) {
+               if (!skb->len) {
+                       bt_dev_err(hdev, "unexpected cc 0x%4.4x with no status",
+                                  *opcode);
+                       *status = HCI_ERROR_UNSPECIFIED;
+                       return;
+               }
+
                /* Unknown opcode, assume byte 0 contains the status, so
                 * that e.g. __hci_cmd_sync() properly returns errors
                 * for vendor specific commands send by HCI drivers.

> +
>         /* Only match event if command OGF is not for LE */
>         if (hdev->req_skb &&
>             hci_opcode_ogf(hci_skb_opcode(hdev->req_skb)) != 0x08 &&
> --
> 2.43.0
>


-- 
Luiz Augusto von Dentz