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

Cong Zhang posted 1 patch 3 months, 2 weeks ago
drivers/block/virtio_blk.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
[PATCH] virtio_blk: NULL out vqs to avoid double free on failed resume
Posted by Cong Zhang 3 months, 2 weeks ago
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.
---
 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>
Re: [PATCH] virtio_blk: NULL out vqs to avoid double free on failed resume
Posted by Jens Axboe 3 months ago
On Tue, 21 Oct 2025 19:07:56 +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.
> 
> 

Applied, thanks!

[1/1] virtio_blk: NULL out vqs to avoid double free on failed resume
      commit: 0739c2c6a015604a7c01506bea28200a2cc2e08c

Best regards,
-- 
Jens Axboe
Re: [PATCH] virtio_blk: NULL out vqs to avoid double free on failed resume
Posted by Michael S. Tsirkin 3 months, 2 weeks ago
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?

> ---
>  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>
Re: [PATCH] virtio_blk: NULL out vqs to avoid double free on failed resume
Posted by Jason Wang 3 months, 2 weeks ago
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

>
> > ---
> >  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>
>
Re: [PATCH] virtio_blk: NULL out vqs to avoid double free on failed resume
Posted by Michael S. Tsirkin 3 months, 2 weeks ago
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?


> >
> > > ---
> > >  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>
> >

Re: [PATCH] virtio_blk: NULL out vqs to avoid double free on failed resume
Posted by Michael S. Tsirkin 3 months, 2 weeks ago
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.

> 
> > >
> > > > ---
> > > >  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>
> > >

Re: [PATCH] virtio_blk: NULL out vqs to avoid double free on failed resume
Posted by Cong Zhang 3 months, 2 weeks ago

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.

>>
>>>>
>>>>> ---
>>>>>  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>
>>>>
> 

Re: [PATCH] virtio_blk: NULL out vqs to avoid double free on failed resume
Posted by Cong Zhang 3 months ago

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>
>>>>>
>>
> 

Re: [PATCH] virtio_blk: NULL out vqs to avoid double free on failed resume
Posted by Jason Wang 3 months ago
On Wed, Nov 5, 2025 at 4:56 PM Cong Zhang <cong.zhang@oss.qualcomm.com> wrote:
>
>
>
> 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.

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks

>
> >>>
> >>>>>
> >>>>>> ---
> >>>>>>  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>
> >>>>>
> >>
> >
>