[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: [v2] Bluetooth: virtio_bt: fix cleanup paths

[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=1116220

---Test result---

Test Summary:
CheckPatch                    PASS      1.39 seconds
VerifyFixes                   PASS      0.34 seconds
VerifySignedoff               PASS      0.27 seconds
GitLint                       PASS      0.57 seconds
SubjectPrefix                 PASS      0.28 seconds
BuildKernel                   PASS      25.78 seconds
CheckAllWarning               PASS      28.53 seconds
CheckSparse                   PASS      27.60 seconds
BuildKernel32                 PASS      25.17 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      458.20 seconds
IncrementalBuild              PASS      25.61 seconds

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


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

---
Regards,
Linux Bluetooth

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