On Wed, Nov 5, 2025 at 4:56 PM Cong Zhang <[email protected]> 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 <[email protected]> > >>>> 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 <[email protected]> > >>>>>> --- > >>>>>> 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 <[email protected]> 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 <[email protected]> > >>>>> > >> > > >
