Re: [PATCH] virtio_blk: NULL out vqs to avoid double free on failed resume

[Cong Zhang <cong.zhang@oss.qualcomm.com>]

On 10/22/2025 4:02 PM, Cong Zhang wrote:
> 
> 
> On 10/22/2025 3:01 PM, Michael S. Tsirkin wrote:
>> On Wed, Oct 22, 2025 at 01:45:38AM -0400, Michael S. Tsirkin wrote:
>>> On Wed, Oct 22, 2025 at 12:19:19PM +0800, Jason Wang wrote:
>>>> On Tue, Oct 21, 2025 at 8:58 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>>>>>
>>>>> On Tue, Oct 21, 2025 at 07:07:56PM +0800, Cong Zhang wrote:
>>>>>> The vblk->vqs releases during freeze. If resume fails before vblk->vqs
>>>>>> is allocated, later freeze/remove may attempt to free vqs again.
>>>>>> Set vblk->vqs to NULL after freeing to avoid double free.
>>>>>>
>>>>>> Signed-off-by: Cong Zhang <cong.zhang@oss.qualcomm.com>
>>>>>> ---
>>>>>> The patch fixes a double free issue that occurs in virtio_blk during
>>>>>> freeze/resume.
>>>>>> The issue is caused by:
>>>>>> 1. During the first freeze, vblk->vqs is freed but pointer is not set to
>>>>>>    NULL.
>>>>>> 2. Virtio block device fails before vblk->vqs is allocated during resume.
>>>>>> 3. During the next freeze, vblk->vqs gets freed again, causing the
>>>>>>    double free crash.
>>>>>
>>>>> this part I don't get. if restore fails, how can freeze be called
>>>>> again?
>>>>
>>>> For example, could it be triggered by the user?
>>>>
>>>> Thanks
>>>
>>> I don't know - were you able to trigger it? how?
>>
>> Sorry I mean the submitter of course.
>>
> 
> Let me add more details:
> Autosleep is enabled in this case. When the system wakes up from
> suspend, it will call virtio_device_restore. The failure happens in the
> virtio_device_restore function before vblk->vqs has been allocated. The
> system still gets woken up after the failure happens. Since autosleep is
> enabled and there is no wakelock being held, the system will try to
> suspend again. Then virtio_device_freeze will be called and causes the
> issue.
> 

Gentle reminder.
Please let me know if you need any additional information.

>>>
>>>>>
>>>>>> ---
>>>>>>  drivers/block/virtio_blk.c | 13 ++++++++++++-
>>>>>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
>>>>>> index f061420dfb10c40b21765b173fab7046aa447506..746795066d7f56a01c9a9c0344d24f9fa06841eb 100644
>>>>>> --- a/drivers/block/virtio_blk.c
>>>>>> +++ b/drivers/block/virtio_blk.c
>>>>>> @@ -1026,8 +1026,13 @@ static int init_vq(struct virtio_blk *vblk)
>>>>>>  out:
>>>>>>       kfree(vqs);
>>>>>>       kfree(vqs_info);
>>>>>> -     if (err)
>>>>>> +     if (err) {
>>>>>>               kfree(vblk->vqs);
>>>>>> +             /*
>>>>>> +              * Set to NULL to prevent freeing vqs again during freezing.
>>>>>> +              */
>>>>>> +             vblk->vqs = NULL;
>>>>>> +     }
>>>>>>       return err;
>>>>>>  }
>>>>>>
>>>>>
>>>>>> @@ -1598,6 +1603,12 @@ static int virtblk_freeze_priv(struct virtio_device *vdev)
>>>>>>
>>>>>>       vdev->config->del_vqs(vdev);
>>>>>>       kfree(vblk->vqs);
>>>>>> +     /*
>>>>>> +      * Set to NULL to prevent freeing vqs again after a failed vqs
>>>>>> +      * allocation during resume. Note that kfree() already handles NULL
>>>>>> +      * pointers safely.
>>>>>> +      */
>>>>>> +     vblk->vqs = NULL;
>>>>>>
>>>>>>       return 0;
>>>>>>  }
>>>>>>
>>>>>> ---
>>>>>> base-commit: 8e2755d7779a95dd61d8997ebce33ff8b1efd3fb
>>>>>> change-id: 20250926-virtio_double_free-7ab880d82a17
>>>>>>
>>>>>> Best regards,
>>>>>> --
>>>>>> Cong Zhang <cong.zhang@oss.qualcomm.com>
>>>>>
>>
>