[PATCH v2] Bluetooth: virtio_bt: fix cleanup paths

[PATCH v2] Bluetooth: virtio_bt: fix cleanup paths

[Haoxiang Li]

virtbt_probe() registers the HCI device before opening the virtio
Bluetooth device. If virtbt_open_vdev() fails, the error path frees
the HCI device without unregistering it first. The probe error paths
also leak the virtio_bluetooth structure after it has been allocated.

Rework the probe error handling into an unwind ladder so each failure
path releases the resources acquired earlier. Also close the virtio
device before unregistering the HCI device in virtbt_remove(), matching
the cleanup order used by the probe failure path.

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.
---
 drivers/bluetooth/virtio_bt.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/bluetooth/virtio_bt.c b/drivers/bluetooth/virtio_bt.c
index 140ab55c9fc5..4ca9b76f6410 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 = hci_register_dev(hdev);
+	if (err < 0) {
 		err = -EBUSY;
-		goto failed;
+		goto err_free_hdev;
 	}
 
 	virtio_device_ready(vdev);
 	err = virtbt_open_vdev(vbt);
 	if (err)
-		goto open_failed;
+		goto err_reset_vdev;
 
 	return 0;
 
-open_failed:
+err_reset_vdev:
+	virtio_reset_device(vdev);
+	hci_unregister_dev(hdev);
+err_free_hdev:
 	hci_free_dev(hdev);
-failed:
+err_del_vqs:
 	vdev->config->del_vqs(vdev);
+err_free_vbt:
+	kfree(vbt);
 	return err;
 }
 
@@ -408,10 +413,10 @@ static void virtbt_remove(struct virtio_device *vdev)
 	struct virtio_bluetooth *vbt = vdev->priv;
 	struct hci_dev *hdev = vbt->hdev;
 
-	hci_unregister_dev(hdev);
 	virtio_reset_device(vdev);
 	virtbt_close_vdev(vbt);
 
+	hci_unregister_dev(hdev);
 	hci_free_dev(hdev);
 	vbt->hdev = NULL;
 
-- 
2.25.1

Re: [PATCH v2] Bluetooth: virtio_bt: fix cleanup paths

[Dan Carpenter]

On Thu, Jun 25, 2026 at 10:01:59AM +0800, Haoxiang Li wrote:
> virtbt_probe() registers the HCI device before opening the virtio
> Bluetooth device. If virtbt_open_vdev() fails, the error path frees
> the HCI device without unregistering it first. The probe error paths
> also leak the virtio_bluetooth structure after it has been allocated.
> 
> Rework the probe error handling into an unwind ladder so each failure
> path releases the resources acquired earlier. Also close the virtio
> device before unregistering the HCI device in virtbt_remove(), matching
> the cleanup order used by the probe failure path.
> 
> 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.
> ---
>  drivers/bluetooth/virtio_bt.c | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/bluetooth/virtio_bt.c b/drivers/bluetooth/virtio_bt.c
> index 140ab55c9fc5..4ca9b76f6410 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 = hci_register_dev(hdev);
> +	if (err < 0) {
>  		err = -EBUSY;
> -		goto failed;
> +		goto err_free_hdev;
>  	}
>  
>  	virtio_device_ready(vdev);
>  	err = virtbt_open_vdev(vbt);
>  	if (err)
> -		goto open_failed;
> +		goto err_reset_vdev;
>  
>  	return 0;
>  
> -open_failed:
> +err_reset_vdev:
> +	virtio_reset_device(vdev);

I'm not sure that this reset is necessary.  I suspect that it isn't,
but I don't know for sure.  In a situation like this, I'd probably
err on the side of only fixing things which I know and leaving the
rest as a leak or whatever.

Otherwise it looks correct to me.

regards,
dan carpenter

Re: [PATCH v2] Bluetooth: virtio_bt: fix cleanup paths

[Michael S. Tsirkin]

On Thu, Jun 25, 2026 at 11:06:20AM +0300, Dan Carpenter wrote:
> On Thu, Jun 25, 2026 at 10:01:59AM +0800, Haoxiang Li wrote:
> > virtbt_probe() registers the HCI device before opening the virtio
> > Bluetooth device. If virtbt_open_vdev() fails, the error path frees
> > the HCI device without unregistering it first. The probe error paths
> > also leak the virtio_bluetooth structure after it has been allocated.
> > 
> > Rework the probe error handling into an unwind ladder so each failure
> > path releases the resources acquired earlier. Also close the virtio
> > device before unregistering the HCI device in virtbt_remove(), matching
> > the cleanup order used by the probe failure path.
> > 
> > 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.
> > ---
> >  drivers/bluetooth/virtio_bt.c | 23 ++++++++++++++---------
> >  1 file changed, 14 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/bluetooth/virtio_bt.c b/drivers/bluetooth/virtio_bt.c
> > index 140ab55c9fc5..4ca9b76f6410 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 = hci_register_dev(hdev);
> > +	if (err < 0) {
> >  		err = -EBUSY;
> > -		goto failed;
> > +		goto err_free_hdev;
> >  	}
> >  
> >  	virtio_device_ready(vdev);
> >  	err = virtbt_open_vdev(vbt);
> >  	if (err)
> > -		goto open_failed;
> > +		goto err_reset_vdev;
> >  
> >  	return 0;
> >  
> > -open_failed:
> > +err_reset_vdev:
> > +	virtio_reset_device(vdev);
> 
> I'm not sure that this reset is necessary.  I suspect that it isn't,
> but I don't know for sure.  In a situation like this, I'd probably
> err on the side of only fixing things which I know and leaving the
> rest as a leak or whatever.

if u called virtio_device_ready then yes you must reset.

> Otherwise it looks correct to me.
> 
> regards,
> dan carpenter