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

[Dan Carpenter <error27@gmail.com>]

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