linux-kernel - Re: [PATCH v2] Bluetooth: coredump: Use tmp buffer with dev

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CABBYNZLOTLVz4kCGMK7kw1RE-KdWciOYwkn6tN_=GZUk92ySdA@mail.gmail.com>
Date: Wed, 16 Jul 2025 10:11:01 -0400
From: Luiz Augusto von Dentz <luiz.dentz@...il.com>
To: Ivan Pravdin <ipravdin.official@...il.com>
Cc: marcel@...tmann.org, johan.hedberg@...il.com, 
	linux-bluetooth@...r.kernel.org, linux-kernel@...r.kernel.org, 
	syzbot+ac3c79181f6aecc5120c@...kaller.appspotmail.com
Subject: Re: [PATCH v2] Bluetooth: coredump: Use tmp buffer with dev_coredumpv

Hi Ivan,

On Tue, Jul 15, 2025 at 8:38 PM Ivan Pravdin
<ipravdin.official@...il.com> wrote:
>
> Create and use new vmalloc'ed buffer with dev_coredumpv. From
> dev_coredumpv documentation:
>
> `This function takes ownership of the vmalloc'ed data and will free
> it when it is no longer used.`
>
> As hdev->dump is used after dev_coredumpv, create temporary buffer to
> hold hdev->dump data.
>
> Reported-by: syzbot+ac3c79181f6aecc5120c@...kaller.appspotmail.com
> Closes: https://lore.kernel.org/all/67eaa688.050a0220.1547ec.014a.GAE@google.com

It should probably close the syzkaller issue, which I assume is this one:

https://syzkaller.appspot.com/bug?extid=ac3c79181f6aecc5120c

> Fixes: b257e02ecc46 ("HCI: coredump: Log devcd dumps into the monitor")
> Signed-off-by: Ivan Pravdin <ipravdin.official@...il.com>
> ---
> v1 -> v2: Changed subject prefix to Bluetooth:
>
>  net/bluetooth/coredump.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/coredump.c b/net/bluetooth/coredump.c
> index 819eacb38762..1232c9a94f95 100644
> --- a/net/bluetooth/coredump.c
> +++ b/net/bluetooth/coredump.c
> @@ -243,6 +243,7 @@ static void hci_devcd_handle_pkt_pattern(struct hci_dev *hdev,
>  static void hci_devcd_dump(struct hci_dev *hdev)
>  {
>         struct sk_buff *skb;
> +       char *coredump;
>         u32 size;
>
>         bt_dev_dbg(hdev, "state %d", hdev->dump.state);
> @@ -250,7 +251,11 @@ static void hci_devcd_dump(struct hci_dev *hdev)
>         size = hdev->dump.tail - hdev->dump.head;
>
>         /* Emit a devcoredump with the available data */
> -       dev_coredumpv(&hdev->dev, hdev->dump.head, size, GFP_KERNEL);
> +       coredump = vmalloc(size);
> +       if (coredump) {
> +               memcpy(coredump, hdev->dump.head, size);
> +               dev_coredumpv(&hdev->dev, coredump, size, GFP_KERNEL);
> +       }
>
>
>         /* Send a copy to monitor as a diagnostic packet */
>         skb = bt_skb_alloc(size, GFP_ATOMIC);
> --
> 2.45.2
>

What is wrong here the is code that attempt to send a copy to the
monitor uses dump.head _after_ dev_coredumpv has freed it, so just
changing the order shall make it work properly:

diff --git a/net/bluetooth/coredump.c b/net/bluetooth/coredump.c
index 819eacb38762..720cb79adf96 100644
--- a/net/bluetooth/coredump.c
+++ b/net/bluetooth/coredump.c
@@ -249,15 +249,15 @@ static void hci_devcd_dump(struct hci_dev *hdev)

        size = hdev->dump.tail - hdev->dump.head;

-       /* Emit a devcoredump with the available data */
-       dev_coredumpv(&hdev->dev, hdev->dump.head, size, GFP_KERNEL);
-
        /* 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);
 }

 static void hci_devcd_handle_pkt_complete(struct hci_dev *hdev


-- 
Luiz Augusto von Dentz