[PATCH v4] Bluetooth: virtio: Fix virtbt_probe() init and cleanup

[PATCH v4] Bluetooth: virtio: Fix virtbt_probe() init and cleanup

[Haoxiang Li]

virtbt_probe() allocates vbt before setting up the virtqueues, but some
failure paths return without freeing it.

The probe path also registers the HCI device before the virtio transport
is opened. Since hci_register_dev() makes the HCI device visible and queues
power_on work, move it after virtio_device_ready() and virtbt_open_vdev()
so the transport is ready before the HCI core can use it.

On failures after DRIVER_OK, reset and close the virtio device before
deleting the virtqueues and freeing vbt. This also cancels pending rx work
before vbt is freed.

Fixes: afd2daa26c7a ("Bluetooth: Add support for virtio transport driver")
Fixes: dc65b4b0f90a ("Bluetooth: virtio_bt: fix device removal")
Cc: stable@vger.kernel.org
Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com>
---
Changes in v2:
 - Rework virtbt_probe() error paths into an unwind ladder.
 - Free vbt on probe failures.
 - Reset the virtio device and unregister the HCI device before freeing it
   when virtbt_open_vdev() fails.
 - Close the virtio device before unregistering the HCI device in remove().

   Thanks Dan for the suggestions. The blog is very helpful.

Changes in v3:
 - Remove virtio_reset_device() from the virtbt_open_vdev() failure path.

Changes in v4:
 - Move hci_register_dev() after virtio_device_ready() and virtbt_open_vdev().
 - Reset and close the virtio device on probe failures after DRIVER_OK. Thanks, Luiz!
---
 drivers/bluetooth/virtio_bt.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/bluetooth/virtio_bt.c b/drivers/bluetooth/virtio_bt.c
index 140ab55c9fc5..e7e79ba3c1f7 100644
--- a/drivers/bluetooth/virtio_bt.c
+++ b/drivers/bluetooth/virtio_bt.c
@@ -311,12 +311,12 @@ static int virtbt_probe(struct virtio_device *vdev)
 
 	err = virtio_find_vqs(vdev, VIRTBT_NUM_VQS, vbt->vqs, vqs_info, NULL);
 	if (err)
-		return err;
+		goto err_free_vbt;
 
 	hdev = hci_alloc_dev();
 	if (!hdev) {
 		err = -ENOMEM;
-		goto failed;
+		goto err_del_vqs;
 	}
 
 	vbt->hdev = hdev;
@@ -383,23 +383,28 @@ static int virtbt_probe(struct virtio_device *vdev)
 	if (virtio_has_feature(vdev, VIRTIO_BT_F_AOSP_EXT))
 		hci_set_aosp_capable(hdev);
 
-	if (hci_register_dev(hdev) < 0) {
-		hci_free_dev(hdev);
-		err = -EBUSY;
-		goto failed;
-	}
-
 	virtio_device_ready(vdev);
 	err = virtbt_open_vdev(vbt);
 	if (err)
-		goto open_failed;
+		goto err_close_vdev;
+
+	err = hci_register_dev(hdev);
+	if (err < 0) {
+		err = -EBUSY;
+		goto err_close_vdev;
+	}
 
 	return 0;
 
-open_failed:
+err_close_vdev:
+	virtio_reset_device(vdev);
+	virtbt_close_vdev(vbt);
 	hci_free_dev(hdev);
-failed:
+err_del_vqs:
 	vdev->config->del_vqs(vdev);
+err_free_vbt:
+	vdev->priv = NULL;
+	kfree(vbt);
 	return err;
 }
 
-- 
2.25.1

Re: [PATCH v4] Bluetooth: virtio: Fix virtbt_probe() init and cleanup

[Dan Carpenter]

On Thu, Jul 09, 2026 at 07:47:45PM +0800, Haoxiang Li wrote:
> virtbt_probe() allocates vbt before setting up the virtqueues, but some
> failure paths return without freeing it.
> 
> The probe path also registers the HCI device before the virtio transport
> is opened. Since hci_register_dev() makes the HCI device visible and queues
> power_on work, move it after virtio_device_ready() and virtbt_open_vdev()
> so the transport is ready before the HCI core can use it.
> 
> On failures after DRIVER_OK, reset and close the virtio device before
> deleting the virtqueues and freeing vbt. This also cancels pending rx work
> before vbt is freed.
> 
> Fixes: afd2daa26c7a ("Bluetooth: Add support for virtio transport driver")
> Fixes: dc65b4b0f90a ("Bluetooth: virtio_bt: fix device removal")
> Cc: stable@vger.kernel.org
> Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com>
> ---
> Changes in v2:
>  - Rework virtbt_probe() error paths into an unwind ladder.
>  - Free vbt on probe failures.
>  - Reset the virtio device and unregister the HCI device before freeing it
>    when virtbt_open_vdev() fails.
>  - Close the virtio device before unregistering the HCI device in remove().
> 
>    Thanks Dan for the suggestions. The blog is very helpful.
> 
> Changes in v3:
>  - Remove virtio_reset_device() from the virtbt_open_vdev() failure path.
> 
> Changes in v4:
>  - Move hci_register_dev() after virtio_device_ready() and virtbt_open_vdev().
>  - Reset and close the virtio device on probe failures after DRIVER_OK. Thanks, Luiz!

These are Sashiko warnings.  To be honest, I would feel really
uncomfortable blindly applying them without testing.  If someone
can test, then great.  Otherwise, I would probably apply v3.  The
stuff that Sashiko complained about was all pre-existing issues
even though for the last one it said it wasn't but it was.

regards,
dan carpenter

Re: [PATCH v4] Bluetooth: virtio: Fix virtbt_probe() init and cleanup

[Michael S. Tsirkin]

Thanks for the patch! one question:

On Thu, Jul 09, 2026 at 07:47:45PM +0800, Haoxiang Li wrote:
> virtbt_probe() allocates vbt before setting up the virtqueues, but some
> failure paths return without freeing it.
> 
> The probe path also registers the HCI device before the virtio transport
> is opened. Since hci_register_dev() makes the HCI device visible and queues
> power_on work, move it after virtio_device_ready() and virtbt_open_vdev()
> so the transport is ready before the HCI core can use it.

Sounds good, and yes it is a spec violation to kick vq before virtio_device_ready

> On failures after DRIVER_OK, reset and close the virtio device before
> deleting the virtqueues and freeing vbt. This also cancels pending rx work
> before vbt is freed.

what "this" cancels pending work? And how can we have work since device was
not registered?

> Fixes: afd2daa26c7a ("Bluetooth: Add support for virtio transport driver")
> Fixes: dc65b4b0f90a ("Bluetooth: virtio_bt: fix device removal")
> Cc: stable@vger.kernel.org
> Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com>
> ---
> Changes in v2:
>  - Rework virtbt_probe() error paths into an unwind ladder.
>  - Free vbt on probe failures.
>  - Reset the virtio device and unregister the HCI device before freeing it
>    when virtbt_open_vdev() fails.
>  - Close the virtio device before unregistering the HCI device in remove().
> 
>    Thanks Dan for the suggestions. The blog is very helpful.
> 
> Changes in v3:
>  - Remove virtio_reset_device() from the virtbt_open_vdev() failure path.
> 
> Changes in v4:
>  - Move hci_register_dev() after virtio_device_ready() and virtbt_open_vdev().
>  - Reset and close the virtio device on probe failures after DRIVER_OK. Thanks, Luiz!
> ---
>  drivers/bluetooth/virtio_bt.c | 27 ++++++++++++++++-----------
>  1 file changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/bluetooth/virtio_bt.c b/drivers/bluetooth/virtio_bt.c
> index 140ab55c9fc5..e7e79ba3c1f7 100644
> --- a/drivers/bluetooth/virtio_bt.c
> +++ b/drivers/bluetooth/virtio_bt.c
> @@ -311,12 +311,12 @@ static int virtbt_probe(struct virtio_device *vdev)
>  
>  	err = virtio_find_vqs(vdev, VIRTBT_NUM_VQS, vbt->vqs, vqs_info, NULL);
>  	if (err)
> -		return err;
> +		goto err_free_vbt;
>  
>  	hdev = hci_alloc_dev();
>  	if (!hdev) {
>  		err = -ENOMEM;
> -		goto failed;
> +		goto err_del_vqs;
>  	}
>  
>  	vbt->hdev = hdev;
> @@ -383,23 +383,28 @@ static int virtbt_probe(struct virtio_device *vdev)
>  	if (virtio_has_feature(vdev, VIRTIO_BT_F_AOSP_EXT))
>  		hci_set_aosp_capable(hdev);
>  
> -	if (hci_register_dev(hdev) < 0) {
> -		hci_free_dev(hdev);
> -		err = -EBUSY;
> -		goto failed;
> -	}
> -
>  	virtio_device_ready(vdev);
>  	err = virtbt_open_vdev(vbt);
>  	if (err)
> -		goto open_failed;
> +		goto err_close_vdev;
> +
> +	err = hci_register_dev(hdev);
> +	if (err < 0) {
> +		err = -EBUSY;
> +		goto err_close_vdev;
> +	}
>  
>  	return 0;
>  
> -open_failed:
> +err_close_vdev:
> +	virtio_reset_device(vdev);
> +	virtbt_close_vdev(vbt);
>  	hci_free_dev(hdev);
> -failed:
> +err_del_vqs:
>  	vdev->config->del_vqs(vdev);
> +err_free_vbt:
> +	vdev->priv = NULL;
> +	kfree(vbt);
>  	return err;
>  }
>  
> -- 
> 2.25.1

Re: [PATCH v4] Bluetooth: virtio: Fix virtbt_probe() init and cleanup

[Michael S. Tsirkin]

On Thu, Jul 09, 2026 at 03:28:17PM +0300, Dan Carpenter wrote:
> On Thu, Jul 09, 2026 at 07:47:45PM +0800, Haoxiang Li wrote:
> > virtbt_probe() allocates vbt before setting up the virtqueues, but some
> > failure paths return without freeing it.
> > 
> > The probe path also registers the HCI device before the virtio transport
> > is opened. Since hci_register_dev() makes the HCI device visible and queues
> > power_on work, move it after virtio_device_ready() and virtbt_open_vdev()
> > so the transport is ready before the HCI core can use it.
> > 
> > On failures after DRIVER_OK, reset and close the virtio device before
> > deleting the virtqueues and freeing vbt. This also cancels pending rx work
> > before vbt is freed.
> > 
> > Fixes: afd2daa26c7a ("Bluetooth: Add support for virtio transport driver")
> > Fixes: dc65b4b0f90a ("Bluetooth: virtio_bt: fix device removal")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com>
> > ---
> > Changes in v2:
> >  - Rework virtbt_probe() error paths into an unwind ladder.
> >  - Free vbt on probe failures.
> >  - Reset the virtio device and unregister the HCI device before freeing it
> >    when virtbt_open_vdev() fails.
> >  - Close the virtio device before unregistering the HCI device in remove().
> > 
> >    Thanks Dan for the suggestions. The blog is very helpful.
> > 
> > Changes in v3:
> >  - Remove virtio_reset_device() from the virtbt_open_vdev() failure path.
> > 
> > Changes in v4:
> >  - Move hci_register_dev() after virtio_device_ready() and virtbt_open_vdev().
> >  - Reset and close the virtio device on probe failures after DRIVER_OK. Thanks, Luiz!
> 
> These are Sashiko warnings.  To be honest, I would feel really
> uncomfortable blindly applying them without testing.  If someone
> can test, then great.  Otherwise, I would probably apply v3.  The
> stuff that Sashiko complained about was all pre-existing issues
> even though for the last one it said it wasn't but it was.
> 
> regards,
> dan carpenter

why make changes at all if no one can test. in fact, why have a driver
then.

-- 
MST

Re: [PATCH v4] Bluetooth: virtio: Fix virtbt_probe() init and cleanup

[Dan Carpenter]

On Thu, Jul 09, 2026 at 08:36:32AM -0400, Michael S. Tsirkin wrote:
> 
> why make changes at all if no one can test. in fact, why have a driver
> then.

It would be interesting to see what proportion of kernel patches are
actually tested...  Testing the code is often impossible because you
need the hardware.

In drivers/staging probably very few patches are tested.  Every couple
years I look at the data from where the problems come from and it's
normally from complicated changes from the driver maintainer.  The
number of bugs introduced by checkpatch and static checker fixes is
really tiny.

It's about risk vs reward.  Fixing a security issue is a huge reward.
Cleaning up the code.  Fixing obvious leaks and static checker issues.
Those things are all valuable because they raise the standards and
they prevent copy and paste bugs.

I consider a few things:

1. Is it a security fix?  I recently fixed some memory corruption and
   broke a driver.  I tried to be careful, I wrote a long commit message
   describing my thinking, but I still messed up.  And that's okay
   because fixing security bugs is important.
2. Is the code new?  If it is then there are probably very few users,
   and the original developer is still around so it's pretty safe to
   change.
3. Is it an error path?  Code on error paths is hard to test in the
   best of times.  The risk is very low.
4. Is the change small and obvious?

On the other hand, I often leave known bugs.  In this case, we're talking
about a use after free if the driver fails to probe.  That's not a
security thing.  It's unlikely to ever affect anyone in real life.  The
fix affects the success path so it could easily cause the driver to stop
working.

regards,
dan carpenter

Re: [PATCH v4] Bluetooth: virtio: Fix virtbt_probe() init and cleanup

[Michael S. Tsirkin]

On Thu, Jul 09, 2026 at 04:44:31PM +0300, Dan Carpenter wrote:
> On Thu, Jul 09, 2026 at 08:36:32AM -0400, Michael S. Tsirkin wrote:
> > 
> > why make changes at all if no one can test. in fact, why have a driver
> > then.
> 
> It would be interesting to see what proportion of kernel patches are
> actually tested...  Testing the code is often impossible because you
> need the hardware.

Sure I agree - if I am refactoring kernel APIs I would often
compile the driver and that is it.
But that is different from poking at a driver specifically.
If I do that then yes I expect the patch to be tested.



> In drivers/staging probably very few patches are tested.  Every couple
> years I look at the data from where the problems come from and it's
> normally from complicated changes from the driver maintainer.  The
> number of bugs introduced by checkpatch and static checker fixes is
> really tiny.
> 
> It's about risk vs reward.  Fixing a security issue is a huge reward.
> Cleaning up the code.  Fixing obvious leaks and static checker issues.
> Those things are all valuable because they raise the standards and
> they prevent copy and paste bugs.
> 
> I consider a few things:
> 
> 1. Is it a security fix?  I recently fixed some memory corruption and
>    broke a driver.  I tried to be careful, I wrote a long commit message
>    describing my thinking, but I still messed up.  And that's okay
>    because fixing security bugs is important.
> 2. Is the code new?  If it is then there are probably very few users,
>    and the original developer is still around so it's pretty safe to
>    change.
> 3. Is it an error path?  Code on error paths is hard to test in the
>    best of times.  The risk is very low.
> 4. Is the change small and obvious?
> 
> On the other hand, I often leave known bugs.  In this case, we're talking
> about a use after free if the driver fails to probe.  That's not a
> security thing.  It's unlikely to ever affect anyone in real life.  The
> fix affects the success path so it could easily cause the driver to stop
> working.

Exactly, agree on all points.

> regards,
> dan carpenter

Re: [PATCH v4] Bluetooth: virtio: Fix virtbt_probe() init and cleanup

[Michael S. Tsirkin]

On Thu, Jul 09, 2026 at 03:28:17PM +0300, Dan Carpenter wrote:
> On Thu, Jul 09, 2026 at 07:47:45PM +0800, Haoxiang Li wrote:
> > virtbt_probe() allocates vbt before setting up the virtqueues, but some
> > failure paths return without freeing it.
> > 
> > The probe path also registers the HCI device before the virtio transport
> > is opened. Since hci_register_dev() makes the HCI device visible and queues
> > power_on work, move it after virtio_device_ready() and virtbt_open_vdev()
> > so the transport is ready before the HCI core can use it.
> > 
> > On failures after DRIVER_OK, reset and close the virtio device before
> > deleting the virtqueues and freeing vbt. This also cancels pending rx work
> > before vbt is freed.
> > 
> > Fixes: afd2daa26c7a ("Bluetooth: Add support for virtio transport driver")
> > Fixes: dc65b4b0f90a ("Bluetooth: virtio_bt: fix device removal")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com>
> > ---
> > Changes in v2:
> >  - Rework virtbt_probe() error paths into an unwind ladder.
> >  - Free vbt on probe failures.
> >  - Reset the virtio device and unregister the HCI device before freeing it
> >    when virtbt_open_vdev() fails.
> >  - Close the virtio device before unregistering the HCI device in remove().
> > 
> >    Thanks Dan for the suggestions. The blog is very helpful.
> > 
> > Changes in v3:
> >  - Remove virtio_reset_device() from the virtbt_open_vdev() failure path.
> > 
> > Changes in v4:
> >  - Move hci_register_dev() after virtio_device_ready() and virtbt_open_vdev().
> >  - Reset and close the virtio device on probe failures after DRIVER_OK. Thanks, Luiz!
> 
> These are Sashiko warnings.  To be honest, I would feel really
> uncomfortable blindly applying them without testing.

sashiko is right adding bufffers before driver ok is a spec violation.

>  If someone
> can test, then great.  Otherwise, I would probably apply v3.  The
> stuff that Sashiko complained about was all pre-existing issues
> even though for the last one it said it wasn't but it was.
> 
> regards,
> dan carpenter

Re: [PATCH v4] Bluetooth: virtio: Fix virtbt_probe() init and cleanup

[Igor Skalkin]

I tested this against real hardware (not just QEMU-internal loopback):
a MediaTek USB Bluetooth controller on the host, exposed to a guest
kernel through QEMU's virtio-bt-pci device via HCI_CHANNEL_USER. This
exercises the actual virtio transport and the real HCI core, not a
mock. No other local changes to drivers/bluetooth/virtio_bt.c were
present; this v4 patch was applied alone on top of plain v7.2-rc4.

Test setup:
- Host: Linux with a MediaTek USB BT controller (hci0), bluetooth.service
  stopped/masked and the adapter taken down for the duration of the test
  so QEMU could bind HCI_CHANNEL_USER exclusively.
- Guest: v7.2-rc4 kernel with CONFIG_DEBUG_KMEMLEAK=y, booted via QEMU's
  virtio-bt-pci device pointed at the host adapter.
- This patch (v4) applied alone, on top of plain v7.2-rc4, nothing else
  changed in virtio_bt.c; built as a loadable module for repeated
  bind/unbind testing.
- A module parameter (test-only, not part of this patch) let me force
  a failure at each of the four points virtbt_probe() can now fail at,
  to drive every branch of the new unwind ladder without needing to
  fault-inject the real kernel functions.

Cases run, each followed by an explicit kmemleak scan:

1. Happy path: probe succeeds, hci0 appears under
   /sys/class/bluetooth, remove() runs cleanly.
2. Five back-to-back insmod/rmmod cycles on the happy path, to catch
   leaks or use-after-free that only show up cumulatively.
3. Forced failure at virtio_find_vqs() -> err_free_vbt path.
4. Forced failure at hci_alloc_dev() -> err_del_vqs path.
5. Forced failure at virtbt_open_vdev(), i.e. after
   virtio_device_ready() (post-DRIVER_OK) -> err_close_vdev path.
6. Forced failure at hci_register_dev(), also post-DRIVER_OK and
   post-open -> err_close_vdev path.

Results for all six: no Oops/BUG, no lockdep or RCU-stall warnings,
rmmod always succeeded, hci0 was present under
/sys/class/bluetooth only when probe actually succeeded (cases 1-2),
and kmemleak reported zero unreferenced objects after every case and
in a final aggregate scan at the end of the run.

This covers the ordering fix Sashiko flagged (hci_register_dev()
moved after virtio_device_ready()/virtbt_open_vdev(), so no buffers
are kicked before DRIVER_OK) and the vbt-leak/priv-cleanup fix on the
virtio_find_vqs() failure path, both under a real transport rather
than a stub.

Tested-by: Igor Skalkin <igor.skalkin@oss.qualcomm.com>

Happy to share the QEMU/kernel config and the fault-injection harness
if useful for other reviewers.