* [PATCH] virtio_blk: NULL out vqs to avoid double free on failed resume @ 2025-10-21 11:07 Cong Zhang 2025-10-21 12:57 ` Michael S. Tsirkin 2025-11-06 23:33 ` Jens Axboe 0 siblings, 2 replies; 9+ messages in thread From: Cong Zhang @ 2025-10-21 11:07 UTC (permalink / raw) To: Michael S. Tsirkin, Jason Wang, Xuan Zhuo, Eugenio Pérez, Paolo Bonzini, Stefan Hajnoczi, Jens Axboe Cc: linux-arm-msm, virtualization, linux-block, linux-kernel, pavan.kondeti, Cong Zhang 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> ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] virtio_blk: NULL out vqs to avoid double free on failed resume 2025-10-21 11:07 [PATCH] virtio_blk: NULL out vqs to avoid double free on failed resume Cong Zhang @ 2025-10-21 12:57 ` Michael S. Tsirkin 2025-10-22 4:19 ` Jason Wang 2025-11-06 23:33 ` Jens Axboe 1 sibling, 1 reply; 9+ messages in thread From: Michael S. Tsirkin @ 2025-10-21 12:57 UTC (permalink / raw) To: Cong Zhang Cc: Jason Wang, Xuan Zhuo, Eugenio Pérez, Paolo Bonzini, Stefan Hajnoczi, Jens Axboe, linux-arm-msm, virtualization, linux-block, linux-kernel, pavan.kondeti 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> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] virtio_blk: NULL out vqs to avoid double free on failed resume 2025-10-21 12:57 ` Michael S. Tsirkin @ 2025-10-22 4:19 ` Jason Wang 2025-10-22 5:45 ` Michael S. Tsirkin 0 siblings, 1 reply; 9+ messages in thread From: Jason Wang @ 2025-10-22 4:19 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Cong Zhang, Xuan Zhuo, Eugenio Pérez, Paolo Bonzini, Stefan Hajnoczi, Jens Axboe, linux-arm-msm, virtualization, linux-block, linux-kernel, pavan.kondeti 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> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] virtio_blk: NULL out vqs to avoid double free on failed resume 2025-10-22 4:19 ` Jason Wang @ 2025-10-22 5:45 ` Michael S. Tsirkin 2025-10-22 7:01 ` Michael S. Tsirkin 0 siblings, 1 reply; 9+ messages in thread From: Michael S. Tsirkin @ 2025-10-22 5:45 UTC (permalink / raw) To: Jason Wang Cc: Cong Zhang, Xuan Zhuo, Eugenio Pérez, Paolo Bonzini, Stefan Hajnoczi, Jens Axboe, linux-arm-msm, virtualization, linux-block, linux-kernel, pavan.kondeti 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> > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] virtio_blk: NULL out vqs to avoid double free on failed resume 2025-10-22 5:45 ` Michael S. Tsirkin @ 2025-10-22 7:01 ` Michael S. Tsirkin 2025-10-22 8:02 ` Cong Zhang 0 siblings, 1 reply; 9+ messages in thread From: Michael S. Tsirkin @ 2025-10-22 7:01 UTC (permalink / raw) To: Jason Wang Cc: Cong Zhang, Xuan Zhuo, Eugenio Pérez, Paolo Bonzini, Stefan Hajnoczi, Jens Axboe, linux-arm-msm, virtualization, linux-block, linux-kernel, pavan.kondeti 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> > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] virtio_blk: NULL out vqs to avoid double free on failed resume 2025-10-22 7:01 ` Michael S. Tsirkin @ 2025-10-22 8:02 ` Cong Zhang 2025-11-05 8:56 ` Cong Zhang 0 siblings, 1 reply; 9+ messages in thread From: Cong Zhang @ 2025-10-22 8:02 UTC (permalink / raw) To: Michael S. Tsirkin, Jason Wang Cc: Xuan Zhuo, Eugenio Pérez, Paolo Bonzini, Stefan Hajnoczi, Jens Axboe, linux-arm-msm, virtualization, linux-block, linux-kernel, pavan.kondeti 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> >>>> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] virtio_blk: NULL out vqs to avoid double free on failed resume 2025-10-22 8:02 ` Cong Zhang @ 2025-11-05 8:56 ` Cong Zhang 2025-11-06 4:06 ` Jason Wang 0 siblings, 1 reply; 9+ messages in thread From: Cong Zhang @ 2025-11-05 8:56 UTC (permalink / raw) To: Michael S. Tsirkin, Jason Wang Cc: Xuan Zhuo, Eugenio Pérez, Paolo Bonzini, Stefan Hajnoczi, Jens Axboe, linux-arm-msm, virtualization, linux-block, linux-kernel, pavan.kondeti 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> >>>>> >> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] virtio_blk: NULL out vqs to avoid double free on failed resume 2025-11-05 8:56 ` Cong Zhang @ 2025-11-06 4:06 ` Jason Wang 0 siblings, 0 replies; 9+ messages in thread From: Jason Wang @ 2025-11-06 4:06 UTC (permalink / raw) To: Cong Zhang Cc: Michael S. Tsirkin, Xuan Zhuo, Eugenio Pérez, Paolo Bonzini, Stefan Hajnoczi, Jens Axboe, linux-arm-msm, virtualization, linux-block, linux-kernel, pavan.kondeti 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> > >>>>> > >> > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] virtio_blk: NULL out vqs to avoid double free on failed resume 2025-10-21 11:07 [PATCH] virtio_blk: NULL out vqs to avoid double free on failed resume Cong Zhang 2025-10-21 12:57 ` Michael S. Tsirkin @ 2025-11-06 23:33 ` Jens Axboe 1 sibling, 0 replies; 9+ messages in thread From: Jens Axboe @ 2025-11-06 23:33 UTC (permalink / raw) To: Michael S. Tsirkin, Jason Wang, Xuan Zhuo, Eugenio Pérez, Paolo Bonzini, Stefan Hajnoczi, Cong Zhang Cc: linux-arm-msm, virtualization, linux-block, linux-kernel, pavan.kondeti 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-11-06 23:33 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-10-21 11:07 [PATCH] virtio_blk: NULL out vqs to avoid double free on failed resume Cong Zhang 2025-10-21 12:57 ` Michael S. Tsirkin 2025-10-22 4:19 ` Jason Wang 2025-10-22 5:45 ` Michael S. Tsirkin 2025-10-22 7:01 ` Michael S. Tsirkin 2025-10-22 8:02 ` Cong Zhang 2025-11-05 8:56 ` Cong Zhang 2025-11-06 4:06 ` Jason Wang 2025-11-06 23:33 ` Jens Axboe
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).