[PATCH] Bluetooth: virtio_bt: unregister HCI device on open failure

[PATCH] Bluetooth: virtio_bt: unregister HCI device on open failure

[Haoxiang Li]

virtbt_probe() registers the HCI device before calling
virtbt_open_vdev(). If opening the virtio Bluetooth
device fails, the error path frees the HCI device without
unregistering it.

Fixes: dc65b4b0f90a ("Bluetooth: virtio_bt: fix device removal")
Cc: stable@vger.kernel.org
Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com>
---
 drivers/bluetooth/virtio_bt.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/bluetooth/virtio_bt.c b/drivers/bluetooth/virtio_bt.c
index 140ab55c9fc5..bf6827431bb8 100644
--- a/drivers/bluetooth/virtio_bt.c
+++ b/drivers/bluetooth/virtio_bt.c
@@ -397,6 +397,7 @@ static int virtbt_probe(struct virtio_device *vdev)
 	return 0;
 
 open_failed:
+	hci_unregister_dev(hdev);
 	hci_free_dev(hdev);
 failed:
 	vdev->config->del_vqs(vdev);
-- 
2.25.1

Re: [PATCH] Bluetooth: virtio_bt: unregister HCI device on open failure

[Paul Menzel]

Dear Haoxiang,


Thank you for your patch.

Am 24.06.26 um 10:43 schrieb Haoxiang Li:
> virtbt_probe() registers the HCI device before calling
> virtbt_open_vdev(). If opening the virtio Bluetooth
> device fails, the error path frees the HCI device without
> unregistering it.

Should you resend, please re-flow for 72/75 characters, so only three 
lines are used.

> Fixes: dc65b4b0f90a ("Bluetooth: virtio_bt: fix device removal")
> Cc: stable@vger.kernel.org
> Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com>
> ---
>   drivers/bluetooth/virtio_bt.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/bluetooth/virtio_bt.c b/drivers/bluetooth/virtio_bt.c
> index 140ab55c9fc5..bf6827431bb8 100644
> --- a/drivers/bluetooth/virtio_bt.c
> +++ b/drivers/bluetooth/virtio_bt.c
> @@ -397,6 +397,7 @@ static int virtbt_probe(struct virtio_device *vdev)
>   	return 0;
>   
>   open_failed:
> +	hci_unregister_dev(hdev);
>   	hci_free_dev(hdev);
>   failed:
>   	vdev->config->del_vqs(vdev);

Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul


PS: gemini/gemini-3.1-pro-preview found an unrelated issue to the patch 
during review of this patch [1].

> This is a pre-existing issue, but does this error path safely clean up the
> active virtqueues? 
> Earlier in virtbt_probe(), virtio_device_ready(vdev) marks the device as
> active. If virtbt_open_vdev() subsequently fails, the code jumps to the
> open_failed label and eventually reaches here to call del_vqs(vdev).
> Deleting virtqueues without calling virtio_reset_device(vdev) first violates
> the VirtIO API contract for active devices. It could allow the host or
> hypervisor to access guest memory that has already been freed by del_vqs(),
> potentially leading to a use-after-free.
> Should virtio_reset_device(vdev) be called before tearing down the
> virtqueues in this error path?

No idea, how to best track these things.


[1]: 
https://sashiko.dev/#/patchset/20260624084333.2885144-1-haoxiang_li2024%40163.com

Re: [PATCH] Bluetooth: virtio_bt: unregister HCI device on open failure

[Dan Carpenter]

On Wed, Jun 24, 2026 at 04:43:33PM +0800, Haoxiang Li wrote:
> virtbt_probe() registers the HCI device before calling
> virtbt_open_vdev(). If opening the virtio Bluetooth
> device fails, the error path frees the HCI device without
> unregistering it.
> 
> Fixes: dc65b4b0f90a ("Bluetooth: virtio_bt: fix device removal")
> Cc: stable@vger.kernel.org
> Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com>
> ---
>  drivers/bluetooth/virtio_bt.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/bluetooth/virtio_bt.c b/drivers/bluetooth/virtio_bt.c
> index 140ab55c9fc5..bf6827431bb8 100644
> --- a/drivers/bluetooth/virtio_bt.c
> +++ b/drivers/bluetooth/virtio_bt.c
> @@ -397,6 +397,7 @@ static int virtbt_probe(struct virtio_device *vdev)
>  	return 0;
>  
>  open_failed:
> +	hci_unregister_dev(hdev);
>  	hci_free_dev(hdev);
>  failed:
>  	vdev->config->del_vqs(vdev);

I have written a blog about how to write error handling.
https://staticthinking.wordpress.com/2022/04/28/free-the-last-thing-style/

Originally this code using One Err style error handling where every
error path just did "goto fail".  It's also using ComeFrom label names
which don't say what the goto does only where the goto is...  Ideally if
hci_register_dev() failed it would use the unwind ladder to clean up it
instead calls hci_free_dev() and then goto fail.

The beauty of writing a normal kernel style unwind ladder is that it
writes the cleanup function automatically...  Let's look at the cleanup
function here.

   406  static void virtbt_remove(struct virtio_device *vdev)
   407  {
   408          struct virtio_bluetooth *vbt = vdev->priv;
   409          struct hci_dev *hdev = vbt->hdev;
   410  
   411          hci_unregister_dev(hdev);
   412          virtio_reset_device(vdev);
   413          virtbt_close_vdev(vbt);

I'm really uncomfortable with having the hci_unregister_dev()
before the close.  Potential use after free?

   414  
   415          hci_free_dev(hdev);
   416          vbt->hdev = NULL;
   417  
   418          vdev->config->del_vqs(vdev);
   419          kfree(vbt);

The probe function should free "vbt" but it doesn't so that's
another leak.

   420  }

So this fix is fine but it's also only a partial fix.

regards,
dan carpenter

RE: Bluetooth: virtio_bt: unregister HCI device on open failure

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1181 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=1115809

---Test result---

Test Summary:
CheckPatch                    PASS      0.77 seconds
VerifyFixes                   PASS      0.27 seconds
VerifySignedoff               PASS      0.17 seconds
GitLint                       PASS      0.47 seconds
SubjectPrefix                 PASS      0.20 seconds
BuildKernel                   PASS      20.84 seconds
CheckAllWarning               PASS      23.20 seconds
CheckSparse                   PASS      31.01 seconds
BuildKernel32                 PASS      20.39 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      414.24 seconds
IncrementalBuild              PASS      19.89 seconds

Details
##############################
Test: CheckKernelLLVM - SKIP
Desc: Build kernel with LLVM + context analysis
Output:
Clang not found


https://github.com/bluez/bluetooth-next/pull/346

---
Regards,
Linux Bluetooth