[PATCH v2] Bluetooth: coredump: Use tmp buffer with dev_coredumpv

[PATCH v2] Bluetooth: coredump: Use tmp buffer with dev_coredumpv

[Ivan Pravdin]

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@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/67eaa688.050a0220.1547ec.014a.GAE@google.com
Fixes: b257e02ecc46 ("HCI: coredump: Log devcd dumps into the monitor")
Signed-off-by: Ivan Pravdin <ipravdin.official@gmail.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

Re: [PATCH v2] Bluetooth: coredump: Use tmp buffer with dev_coredumpv

[Paul Menzel]

Dear Ivan,


Thank you for your patch and fixing the report.

Am 16.07.25 um 02:37 schrieb Ivan Pravdin:

Personally, I’d start with the problem description.

> 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.`

You could use email/Markdown style citation by prepending the lines with 
`> `.

> As hdev->dump is used after dev_coredumpv, create temporary buffer to
> hold hdev->dump data.
> 
> Reported-by: syzbot+ac3c79181f6aecc5120c@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/all/67eaa688.050a0220.1547ec.014a.GAE@google.com

Add a trace excerpt to the commit message?

> Fixes: b257e02ecc46 ("HCI: coredump: Log devcd dumps into the monitor")
> Signed-off-by: Ivan Pravdin <ipravdin.official@gmail.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);
> +	}

Should it be logged, if allocation fails?

>   
>   	/* Send a copy to monitor as a diagnostic packet */
>   	skb = bt_skb_alloc(size, GFP_ATOMIC);


Kind regards,

Paul

Re: [PATCH v2] Bluetooth: coredump: Use tmp buffer with dev_coredumpv

[Luiz Augusto von Dentz]

Hi Ivan,

On Tue, Jul 15, 2025 at 8:38 PM Ivan Pravdin
<ipravdin.official@gmail.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@syzkaller.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@gmail.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

Re: [PATCH v2] Bluetooth: coredump: Use tmp buffer with dev_coredumpv

[Ivan Pravdin]

On Wed, Jul 16, 2025 at 05:06:38AM GMT, Paul Menzel wrote:
> Dear Ivan,
> 
> 
> Thank you for your patch and fixing the report.
> 
> Am 16.07.25 um 02:37 schrieb Ivan Pravdin:
> 
> Personally, I’d start with the problem description.

I will add more details in v3.

> 
> > 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.`
> 
> You could use email/Markdown style citation by prepending the lines with `> `.

I will fix it in v3.

> 
> > As hdev->dump is used after dev_coredumpv, create temporary buffer to
> > hold hdev->dump data.
> > 
> > Reported-by: syzbot+ac3c79181f6aecc5120c@syzkaller.appspotmail.com
> > Closes: https://lore.kernel.org/all/67eaa688.050a0220.1547ec.014a.GAE@google.com
> 
> Add a trace excerpt to the commit message?

I will add it in v3.

> 
> > Fixes: b257e02ecc46 ("HCI: coredump: Log devcd dumps into the monitor")
> > Signed-off-by: Ivan Pravdin <ipravdin.official@gmail.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);
> > +	}
> 
> Should it be logged, if allocation fails?

Right, I will add it in v3.

> 
> >   	/* Send a copy to monitor as a diagnostic packet */
> >   	skb = bt_skb_alloc(size, GFP_ATOMIC);
> 
> 
> Kind regards,
> 
> Paul

	Ivan Pravdin

Re: [PATCH v2] Bluetooth: coredump: Use tmp buffer with dev_coredumpv

[Ivan Pravdin]

On Wed, Jul 16, 2025 at 10:11:01AM GMT, Luiz Augusto von Dentz wrote:
> Hi Ivan,
> 
> On Tue, Jul 15, 2025 at 8:38 PM Ivan Pravdin
> <ipravdin.official@gmail.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@syzkaller.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

I will change it in v3.

> 
> > Fixes: b257e02ecc46 ("HCI: coredump: Log devcd dumps into the monitor")
> > Signed-off-by: Ivan Pravdin <ipravdin.official@gmail.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

Unfortunately this does not work. This was my initial attempt but it
still reproduces the error [2]. The reason for that is that
dev_coredumpv takes the ownership of the buffer. From the comment
above dev_coredumpm_timeout (which is used by dev_coredumpv):

> Creates a new device coredump for the given device. If a previous one hasn't
> been read yet, the new coredump is discarded. The data lifetime is determined
> by the device coredump framework and when it is no longer needed the @free
> function will be called to free the data.

If hci_devcd_dump is called multiple times, first time hdev->dump.head
will be kept. However, calling hci_devcd_dump second time before data is
read by userspace will free it, leading to uaf error like the one
observed in the report. 

As hci_devcd_dump need not be called only one time before hdev is freed, 
calling it multiple times will lead to uaf, no matter in what order
skb_put_data and dev_coredumpv occur in the function.

To solve this problem, a temporary buffer should be used. Such buffer
should copy data from hdev->dump.head and be surrendered to
dev_coredumpv without any possibility to be altered or freed.

For reference, here are some examples of using temporary buffer with
dev_coredumpv in Intel AVS driver:

[bluetoot-next 6f40d15c70bc] sound/soc/intel/avs/apl.c:183
[bluetoot-next 6f40d15c70bc] sound/soc/intel/avs/skl.c:147

[2] https://lore.kernel.org/all/6845dc6b.050a0220.daf97.0af4.GAE@google.com/

> 
> 
> -- 
> Luiz Augusto von Dentz

Thank you for your feedback.

	Ivan Pravdin

Re: [PATCH v2] Bluetooth: coredump: Use tmp buffer with dev_coredumpv

[Ivan Pravdin]

On Thu, Jul 17, 2025 at 12:11:16AM GMT, Ivan Pravdin wrote:
> On Wed, Jul 16, 2025 at 10:11:01AM GMT, Luiz Augusto von Dentz wrote:
> > Hi Ivan,
> > 
> > On Tue, Jul 15, 2025 at 8:38 PM Ivan Pravdin
> > <ipravdin.official@gmail.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@syzkaller.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
> 
> I will change it in v3.
> 
> > 
> > > Fixes: b257e02ecc46 ("HCI: coredump: Log devcd dumps into the monitor")
> > > Signed-off-by: Ivan Pravdin <ipravdin.official@gmail.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
> 
> Unfortunately this does not work. This was my initial attempt but it
> still reproduces the error [2]. The reason for that is that
> dev_coredumpv takes the ownership of the buffer. From the comment
> above dev_coredumpm_timeout (which is used by dev_coredumpv):
> 
> > Creates a new device coredump for the given device. If a previous one hasn't
> > been read yet, the new coredump is discarded. The data lifetime is determined
> > by the device coredump framework and when it is no longer needed the @free
> > function will be called to free the data.
> 
> If hci_devcd_dump is called multiple times, first time hdev->dump.head
> will be kept. However, calling hci_devcd_dump second time before data is
> read by userspace will free it, leading to uaf error like the one
> observed in the report. 
> 
> As hci_devcd_dump need not be called only one time before hdev is freed, 
> calling it multiple times will lead to uaf, no matter in what order
> skb_put_data and dev_coredumpv occur in the function.
> 
> To solve this problem, a temporary buffer should be used. Such buffer
> should copy data from hdev->dump.head and be surrendered to
> dev_coredumpv without any possibility to be altered or freed.
> 
> For reference, here are some examples of using temporary buffer with
> dev_coredumpv in Intel AVS driver:
> 
> [bluetoot-next 6f40d15c70bc] sound/soc/intel/avs/apl.c:183
> [bluetoot-next 6f40d15c70bc] sound/soc/intel/avs/skl.c:147
> 
> [2] https://lore.kernel.org/all/6845dc6b.050a0220.daf97.0af4.GAE@google.com/
> 

Nevermind... I was under impression that hdev->dump is allocated only
once and is reused until hdev is freed. The link I have copied actually
shows another error that is solved by another patch [1]. I have tested
your suggestion on top of unintegrated patch and it did not reproduce
any issues [2].

I will include your suggestion in v3.

Thanks.

[1] https://lore.kernel.org/all/20250716004252.125466-2-ipravdin.official@gmail.com/
[2] https://lore.kernel.org/all/gkct6i53onzjuzugsor6pnrhjfef7trwhgbjrgfie25xirebhp@dpqxvkguqdnf/

> > 
> > 
> > -- 
> > Luiz Augusto von Dentz
> 
> Thank you for your feedback.
> 
> 	Ivan Pravdin

	Ivan Pravdin