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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CACGkMEuD43oH7zshX-Jf+nvxZmkw730vcX4fcFH9+2z6b_8O-g@mail.gmail.com>
Date: Thu, 6 Nov 2025 12:06:56 +0800
From: Jason Wang <jasowang@...hat.com>
To: Cong Zhang <cong.zhang@....qualcomm.com>
Cc: "Michael S. Tsirkin" <mst@...hat.com>, Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, 
	Eugenio Pérez <eperezma@...hat.com>, 
	Paolo Bonzini <pbonzini@...hat.com>, Stefan Hajnoczi <stefanha@...hat.com>, Jens Axboe <axboe@...nel.dk>, 
	linux-arm-msm@...r.kernel.org, virtualization@...ts.linux.dev, 
	linux-block@...r.kernel.org, linux-kernel@...r.kernel.org, 
	pavan.kondeti@....qualcomm.com
Subject: Re: [PATCH] virtio_blk: NULL out vqs to avoid double free on failed resume

On Wed, Nov 5, 2025 at 4:56 PM Cong Zhang <cong.zhang@....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@...hat.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@....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@...hat.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@....qualcomm.com>
> >>>>>
> >>
> >
>