* [PATCH 08/12] vhost-scsi: improve CDB size validation and remove TODO comment
From: Weimin Xiong @ 2026-07-15 8:09 UTC (permalink / raw)
To: virtualization
From: xiongweimin <xiongwm2026@163.com>
Improve the CDB size validation by adding a zero-length check for
additional safety. Also remove the misleading TODO comment since
scsi_command_size() already correctly handles both fixed and
variable-length CDBs.
Signed-off-by: Weimin Xiong <xiongwm2026@163.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
---
drivers/vhost/scsi.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 219d9d881..2fbc328c9 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -1432,12 +1432,15 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
lun = vhost_buf_to_lun(v_req.lun);
}
/*
- * Check that the received CDB size does not exceeded our
- * hardcoded max for vhost-scsi, then get a pre-allocated
- * cmd descriptor for the new virtio-scsi tag.
- *
- * TODO what if cdb was too small for varlen cdb header?
+ * Check that the received CDB size is valid and does not
+ * exceed our hardcoded max for vhost-scsi, then get a
+ * pre-allocated cmd descriptor for the new virtio-scsi tag.
+ * scsi_command_size() handles both fixed and variable-length CDBs.
*/
+ if (unlikely(scsi_command_size(cdb) == 0)) {
+ vq_err(vq, "Received SCSI CDB with zero length\n");
+ goto err;
+ }
if (unlikely(scsi_command_size(cdb) > VHOST_SCSI_MAX_CDB_SIZE)) {
vq_err(vq, "Received SCSI CDB with command_size: %d that"
" exceeds SCSI_MAX_VARLEN_CDB_SIZE: %d\n",
--
2.43.0
^ permalink raw reply related
* [PATCH 02/12] vhost-scsi: remove dead commented code in vhost_scsi_make_tport
From: Weimin Xiong @ 2026-07-15 8:09 UTC (permalink / raw)
To: virtualization
From: xiongweimin <xiongwm2026@163.com>
Remove dead commented-out code that was left behind during previous
refactoring. The code checked for invalid WWN parsing but is no longer
used.
Signed-off-by: Weimin Xiong <xiongwm2026@163.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
---
drivers/vhost/scsi.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 81e905c4c..77e47f246 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -2841,9 +2841,6 @@ vhost_scsi_make_tport(struct target_fabric_configfs *tf,
u64 wwpn = 0;
int off = 0;
- /* if (vhost_scsi_parse_wwn(name, &wwpn, 1) < 0)
- return ERR_PTR(-EINVAL); */
-
tport = kzalloc_obj(*tport);
if (!tport) {
pr_err("Unable to allocate struct vhost_scsi_tport");
--
2.43.0
^ permalink raw reply related
* [PATCH 09/12] vhost-net: use sizeof(*featurep) instead of sizeof(u64)
From: Weimin Xiong @ 2026-07-15 8:09 UTC (permalink / raw)
To: virtualization
From: xiongweimin <xiongwm2026@163.com>
Use sizeof(*featurep) instead of sizeof(u64) for better type safety
and consistency with kernel coding style. While sizeof(u64) equals
sizeof(*featurep) on all supported platforms, using sizeof(*featurep)
makes the code more maintainable.
Signed-off-by: Weimin Xiong <xiongwm2026@163.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
---
drivers/vhost/net.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index c6854cba5..94028598f 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -1777,14 +1777,14 @@ static long vhost_net_ioctl(struct file *f, unsigned int ioctl,
return -EFAULT;
/* Copy the net features, up to the user-provided buffer size */
- argp += sizeof(u64);
+ argp += sizeof(*featurep);
copied = min(count, (u64)VIRTIO_FEATURES_U64S);
if (copy_to_user(argp, vhost_net_features,
- copied * sizeof(u64)))
+ copied * sizeof(*featurep)))
return -EFAULT;
/* Zero the trailing space provided by user-space, if any */
- if (clear_user(argp, size_mul(count - copied, sizeof(u64))))
+ if (clear_user(argp, size_mul(count - copied, sizeof(*featurep))))
return -EFAULT;
return 0;
case VHOST_SET_FEATURES_ARRAY:
@@ -1792,9 +1792,9 @@ static long vhost_net_ioctl(struct file *f, unsigned int ioctl,
return -EFAULT;
virtio_features_zero(all_features);
- argp += sizeof(u64);
+ argp += sizeof(*featurep);
copied = min(count, (u64)VIRTIO_FEATURES_U64S);
- if (copy_from_user(all_features, argp, copied * sizeof(u64)))
+ if (copy_from_user(all_features, argp, copied * sizeof(*featurep)))
return -EFAULT;
/*
--
2.43.0
^ permalink raw reply related
* [PATCH 07/12] vhost-scsi: improve TMF log allocation error handling comment
From: Weimin Xiong @ 2026-07-15 8:09 UTC (permalink / raw)
To: virtualization
From: xiongweimin <xiongwm2026@163.com>
Add a clarifying comment to explain the error handling flow when TMF
log allocation fails. The tmf structure is freed via vhost_scsi_release_tmf_res()
and then safely falls through to send_reject since it doesn't access tmf
after release.
Signed-off-by: Weimin Xiong <xiongwm2026@163.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
---
drivers/vhost/scsi.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 77e47f246..219d9d881 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -1620,6 +1620,9 @@ vhost_scsi_handle_tmf(struct vhost_scsi *vs, struct vhost_scsi_tpg *tpg,
tmf->tmf_log_num = log_num;
} else {
pr_err("vhost_scsi tmf log allocation error\n");
+ /* Free tmf and send reject response. The send_reject
+ * label does not access tmf after this release.
+ */
vhost_scsi_release_tmf_res(tmf);
goto send_reject;
}
--
2.43.0
^ permalink raw reply related
* [PATCH 04/12] vhost-vdpa: improve ASID not found error message and return code
From: Weimin Xiong @ 2026-07-15 8:09 UTC (permalink / raw)
To: virtualization
From: xiongweimin <xiongwm2026@163.com>
Improve the error message when IOTLB is not found for an ASID to clarify
that the ASID may not be allocated. Also change the return code from
-EINVAL to -ENOENT to better indicate a missing resource.
Signed-off-by: Weimin Xiong <xiongwm2026@163.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
---
drivers/vhost/vdpa.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index ac55275fa..a18ed439e 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -1275,8 +1275,10 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, u32 asid,
v->batch_asid, asid);
}
if (!iotlb)
- dev_err(&v->dev, "no iotlb for asid %d\n", asid);
- r = -EINVAL;
+ dev_err(&v->dev,
+ "IOTLB not found for ASID %d: ASID may not be allocated\n",
+ asid);
+ r = -ENOENT;
goto unlock;
}
--
2.43.0
^ permalink raw reply related
* [PATCH 03/12] vhost-net: improve TSO and checksum TODO comments
From: Weimin Xiong @ 2026-07-15 8:09 UTC (permalink / raw)
To: virtualization
From: xiongweimin <xiongwm2026@163.com>
Update TODO comments related to TSO (TCP Segmentation Offload) and
checksum handling to clearly document the current limitations and
what's needed for future implementation.
Signed-off-by: Weimin Xiong <xiongwm2026@163.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
---
drivers/vhost/net.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 77b59f49b..c6854cba5 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -644,7 +644,7 @@ static bool vhost_exceeds_maxpend(struct vhost_net *net)
static size_t init_iov_iter(struct vhost_virtqueue *vq, struct iov_iter *iter,
size_t hdr_size, int out)
{
- /* Skip header. TODO: support TSO. */
+ /* Skip header. TSO support requires parsing virtio_net_hdr gso_type */
size_t len = iov_length(vq->iov, out);
iov_iter_init(iter, ITER_SOURCE, vq->iov, out, len);
@@ -1240,8 +1240,8 @@ static void handle_rx(struct vhost_net *net)
iov_iter_init(&msg.msg_iter, ITER_DEST, vq->iov, in, vhost_len);
fixup = msg.msg_iter;
if (unlikely((vhost_hlen))) {
- /* We will supply the header ourselves
- * TODO: support TSO.
+ /* We will supply the header ourselves.
+ * Note: TSO requires handling gso_size/gso_type in virtio_net_hdr.
*/
iov_iter_advance(&msg.msg_iter, vhost_hlen);
}
@@ -1270,7 +1270,7 @@ static void handle_rx(struct vhost_net *net)
*/
iov_iter_advance(&fixup, sizeof(hdr));
}
- /* TODO: Should check and handle checksum. */
+ /* Note: Checksum offload handling not yet implemented */
num_buffers = cpu_to_vhost16(vq, headcount);
if (likely(set_num_buffers) &&
--
2.43.0
^ permalink raw reply related
* [PATCH 01/12] vhost-scsi: improve BIDI operation comments
From: Weimin Xiong @ 2026-07-15 8:09 UTC (permalink / raw)
To: virtualization
From: xiongweimin <xiongwm2026@163.com>
Replace FIXME comments with clearer Note comments documenting
that BIDI (bidirectional) operations are not yet supported.
Signed-off-by: Weimin Xiong <xiongwm2026@163.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
---
drivers/vhost/scsi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 9a1253b9d..81e905c4c 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -1047,7 +1047,7 @@ static void vhost_scsi_target_queue_cmd(struct vhost_scsi_nexus *nexus,
struct se_cmd *se_cmd = &cmd->tvc_se_cmd;
struct scatterlist *sg_ptr, *sg_prot_ptr = NULL;
- /* FIXME: BIDI operation */
+ /* Note: BIDI (bidirectional) operations are not yet supported */
if (cmd->tvc_sgl_count) {
sg_ptr = cmd->table.sgl;
@@ -1168,7 +1168,7 @@ vhost_scsi_get_desc(struct vhost_scsi *vs, struct vhost_virtqueue *vq,
/*
* Get the size of request and response buffers.
- * FIXME: Not correct for BIDI operation
+ * Note: Size calculation is not correct for BIDI operations.
*/
vc->out_size = iov_length(vq->iov, vc->out);
vc->in_size = iov_length(&vq->iov[vc->out], vc->in);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] virtio_net: fix infinite loop in virtnet_poll_cleantx when device is broken
From: Jinqian Yang @ 2026-07-15 2:45 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: jasowang, xuanzhuo, eperezma, andrew+netdev, davem, edumazet,
kuba, pabeni, netdev, virtualization, linux-kernel, liuyonglong,
wangzhou1, linuxarm
In-Reply-To: <20260714091622-mutt-send-email-mst@kernel.org>
Hi,
On 2026/7/14 21:17, Michael S. Tsirkin wrote:
> On Mon, Jul 13, 2026 at 09:20:25PM +0800, Jinqian Yang wrote:
>> virtnet_poll_cleantx() contains a do-while loop that cleans up
>> transmitted TX buffers and calls virtqueue_enable_cb_delayed() to check
>> whether more buffers need processing. When the virtio backend stops
>> responding during guest reboot, used->idx is never updated, so
>> virtqueue_enable_cb_delayed() always returns false and the loop never
>> terminates. Then it will block reboot process, and the guest will hang.
>>
>> The problem occurs during guest reboot under network traffic:
>>
>> 1. kernel_restart() -> device_shutdown() traverses the device list
>> 2. virtio_dev_shutdown() calls virtio_break_device() which sets
>> vq->broken = true
>> 3. virtio_dev_shutdown() then calls virtio_synchronize_cbs() to wait
>> for in-flight callbacks to complete
>> 4. A virtio interrupt fires, softirq is deferred to ksoftirqd which
>> calls net_rx_action() -> virtnet_poll() -> virtnet_poll_cleantx()
>> 5. virtnet_poll_cleantx() enters the do-while loop and never exits
>> because the QEMU backend has stopped updating used->idx, despite
>> vq->broken having been set to true in step 2.
>>
>> Since the loop runs inside ksoftirqd (a SCHED_OTHER kthread), it is
>> visible to the scheduler and does not trigger a hard lockup. However,
>> the kthread never leaves the loop, so RCU detects it as a CPU stall
>> and reports it periodically. Meanwhile, the reboot process remains
>> blocked in device_shutdown() because virtio_dev_shutdown() cannot
>> complete its synchronization step, and the guest hangs permanently.
>>
>> This can be reproduced on a guest with a virtio-net device: run iperf3
>> traffic in the guest, then trigger reboot. The reboot occasionally hangs
>> permanently with RCU stall on ksoftirqd.
>>
>> Observed on ARM64 KVM guest:
>>
>> CPU#1 RCU stall (ksoftirqd/1), repeated periodically:
>> virtqueue_enable_cb_delayed_split <- virtnet_poll <- __napi_poll <-
>> net_rx_action <- handle_softirqs <- run_ksoftirqd <-
>> smpboot_thread_fn <- kthread
>>
>> Fix by adding a virtqueue_is_broken() check to the loop condition, so
>> that the loop exits immediately when the device is broken, allowing
>> the device shutdown to proceed.
>>
>> Signed-off-by: Jinqian Yang <yangjinqian1@huawei.com>
>
> I'd expect lots of drivers have this issue? Wouldn't it make more sense
> to check virtqueue_is_broken in
> virtqueue_enable_cb_delayed/virtqueue_enable_cb? This way it works for
> all drivers.
>
In virtqueue_enable_cb->virtqueue_poll, a check for vq->broken is
performed, so other devices do not have this issue.
Indeed, it is more reasonable to check vq->broken inside
virtqueue_enable_cb_delayed. I will make the changes in v2.
Thanks,
Jinqian
>
>
>> ---
>> drivers/net/virtio_net.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 7d2eeb9b1226..c8d2d420c31d 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -2970,7 +2970,8 @@ static void virtnet_poll_cleantx(struct receive_queue *rq, int budget)
>> do {
>> virtqueue_disable_cb(sq->vq);
>> free_old_xmit(sq, txq, !!budget);
>> - } while (unlikely(!virtqueue_enable_cb_delayed(sq->vq)));
>> + } while (!virtqueue_is_broken(sq->vq) &&
>> + unlikely(!virtqueue_enable_cb_delayed(sq->vq)));
>>
>> if (sq->vq->num_free >= MAX_SKB_FRAGS + 2)
>> virtnet_tx_wake_queue(vi, sq);
>> --
>> 2.33.0
>
>
^ permalink raw reply
* Re: [RFC] virtio_balloon: fix Use-After-Free in page reporting during PM freeze
From: Michael S. Tsirkin @ 2026-07-14 18:52 UTC (permalink / raw)
To: David Rientjes
Cc: David Hildenbrand (Arm), Link Lin, Andrew Morton, Vlastimil Babka,
virtualization, linux-mm, linux-kernel, prasin, duenwen, jasowang,
xuanzhuo, Ammar Faizi, jiaqiyan, ahwilkins, Greg Thelen,
Alexander Duyck, stable
In-Reply-To: <5e18d7ec-a9d7-2cc3-7741-695ec3580ffa@google.com>
On Tue, Jul 14, 2026 at 11:21:33AM -0700, David Rientjes wrote:
> On Tue, 14 Jul 2026, Michael S. Tsirkin wrote:
>
> > > > diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> > > > index a1b2c3d4e5f6..45a90fb3abf8 100640
> > > > --- a/drivers/virtio/virtio_balloon.c
> > > > +++ b/drivers/virtio/virtio_balloon.c
> > > > @@ -1055,6 +1055,9 @@ static int virtballoon_freeze(struct virtio_device *vdev)
> > > > * The workqueue is already frozen by the PM core before this
> > > > * function is called.
> > > > */
> > > > + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING))
> > > > + page_reporting_unregister(&vb->pr_dev_info);
> > > > +
> > > > remove_common(vb);
> > > > return 0;
> > > > }
> > > >
> > > > static int virtballoon_restore(struct virtio_device *vdev)
> > > > {
> > > > struct virtio_balloon *vb = vdev->priv;
> > > > int ret;
> > > >
> > > > ret = init_vqs(vdev->priv);
> > > > if (ret)
> > > > return ret;
> > > >
> > > > virtio_device_ready(vdev);
> > > >
> > > > + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING)) {
> > > > + ret = page_reporting_register(&vb->pr_dev_info);
> > > > + if (ret)
> > > > + goto out_remove_vqs;
> > > > + }
> > >
> > > Hm, that failure handling is rather nasty.
> > >
> > >
> > > In virtballoon_freeze() we document:
> > >
> > > "The workqueue is already frozen by the PM core before this function is called"
> > >
> > > Your report states:
> > >
> > > "Workqueue: events page_reporting_process"
> > >
> > >
> > > I assume that workqueue is not frozen yet because ... it's not freezable :)
> > >
> > > So could we queue to system_freezable_wq instead, or define our own freezable
> > > workqueue there? Then a driver doesn't have to worry about that.
> > >
> > > --
> > > Cheers,
> > >
> > > David
> >
> > +1. Just system_freezable_wq will do the trick.
> >
>
> This makes sense.
>
> I'm curious why this bug hasn't popped up earlier, presumably any VM that
> has gone through suspend while reporting free pages through FPR is
> vulnerable to it and could have panicked as a result. Which would suggest
> maybe >99% of FPR is done by guests that never suspend?
Quite possible.
> While under
> pressure this issue seems reproducible.
^ permalink raw reply
* Re: [RFC] virtio_balloon: fix Use-After-Free in page reporting during PM freeze
From: David Rientjes @ 2026-07-14 18:21 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: David Hildenbrand (Arm), Link Lin, Andrew Morton, Vlastimil Babka,
virtualization, linux-mm, linux-kernel, prasin, duenwen, jasowang,
xuanzhuo, Ammar Faizi, jiaqiyan, ahwilkins, Greg Thelen,
Alexander Duyck, stable
In-Reply-To: <20260714092146-mutt-send-email-mst@kernel.org>
On Tue, 14 Jul 2026, Michael S. Tsirkin wrote:
> > > diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> > > index a1b2c3d4e5f6..45a90fb3abf8 100640
> > > --- a/drivers/virtio/virtio_balloon.c
> > > +++ b/drivers/virtio/virtio_balloon.c
> > > @@ -1055,6 +1055,9 @@ static int virtballoon_freeze(struct virtio_device *vdev)
> > > * The workqueue is already frozen by the PM core before this
> > > * function is called.
> > > */
> > > + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING))
> > > + page_reporting_unregister(&vb->pr_dev_info);
> > > +
> > > remove_common(vb);
> > > return 0;
> > > }
> > >
> > > static int virtballoon_restore(struct virtio_device *vdev)
> > > {
> > > struct virtio_balloon *vb = vdev->priv;
> > > int ret;
> > >
> > > ret = init_vqs(vdev->priv);
> > > if (ret)
> > > return ret;
> > >
> > > virtio_device_ready(vdev);
> > >
> > > + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING)) {
> > > + ret = page_reporting_register(&vb->pr_dev_info);
> > > + if (ret)
> > > + goto out_remove_vqs;
> > > + }
> >
> > Hm, that failure handling is rather nasty.
> >
> >
> > In virtballoon_freeze() we document:
> >
> > "The workqueue is already frozen by the PM core before this function is called"
> >
> > Your report states:
> >
> > "Workqueue: events page_reporting_process"
> >
> >
> > I assume that workqueue is not frozen yet because ... it's not freezable :)
> >
> > So could we queue to system_freezable_wq instead, or define our own freezable
> > workqueue there? Then a driver doesn't have to worry about that.
> >
> > --
> > Cheers,
> >
> > David
>
> +1. Just system_freezable_wq will do the trick.
>
This makes sense.
I'm curious why this bug hasn't popped up earlier, presumably any VM that
has gone through suspend while reporting free pages through FPR is
vulnerable to it and could have panicked as a result. Which would suggest
maybe >99% of FPR is done by guests that never suspend? While under
pressure this issue seems reproducible.
^ permalink raw reply
* Re: [RFC] virtio_balloon: fix Use-After-Free in page reporting during PM freeze
From: Link Lin @ 2026-07-14 18:05 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: David Hildenbrand (Arm), Andrew Morton, Vlastimil Babka,
virtualization, linux-mm, linux-kernel, prasin, rientjes, duenwen,
jasowang, xuanzhuo, Ammar Faizi, jiaqiyan, ahwilkins, Greg Thelen,
Alexander Duyck, stable
In-Reply-To: <20260714092146-mutt-send-email-mst@kernel.org>
On Mon, Jul 13, 2026 at 6:17 AM David Hildenbrand wrote:
> I assume that workqueue is not frozen yet because ... it's not freezable :)
> So could we queue to system_freezable_wq instead, or define our own freezable
> workqueue there? Then a driver doesn't have to worry about that.
Exactly. As noted in the RFC, the root cause is indeed that system_wq
lacks the WQ_FREEZABLE flag, leaving it active during suspend.
Switching the worker over to system_freezable_wq is a brilliant idea.
It's a much cleaner abstraction that solves the problem at the core,
saving individual drivers from having to micromanage this lifecycle
and handle failure unwinding during freeze/restore.
On Mon, Jul 13, 2026 at 6:26 AM Michael S. Tsirkin wrote:
> +1. Just system_freezable_wq will do the trick.
Thanks, David and Michael. I will drop the virtio-balloon specific
unregister logic and adopt this approach.
Per the earlier feedback to keep these fixes independent, I'll format
v2 as a 3-part patch series:
[PATCH v2 1/3] mm/page_reporting: use system_freezable_wq
[PATCH v2 2/3] virtio_balloon: fix shrinker UAF during PM freeze
[PATCH v2 3/3] virtio_balloon: fix OOM notifier UAF during PM freeze
I'll send that out shortly.
Thanks,
Link
^ permalink raw reply
* [PATCH v4 5/5] vhost/vsock: add VHOST_RESET_OWNER ioctl
From: Andrey Drobyshev @ 2026-07-14 15:16 UTC (permalink / raw)
To: linux-kernel
Cc: kvm, virtualization, netdev, sgarzare, mst, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den, andrey.drobyshev
In-Reply-To: <20260714151638.143019-1-andrey.drobyshev@virtuozzo.com>
From: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
This ioctl is needed for QEMU's CPR (checkpoint-restore) migration of
the guest with vhost-vsock device. For this to work, we need to reset
the device ownership on the source side by calling RESET_OWNER, and then
claim it on the dest side by calling SET_OWNER. We expect not to lose any
AF_VSOCK connection while this happens.
To that end, unlike the release path, RESET_OWNER keeps the guest CID
hashed: established connections survive, and host sends issued while
the device is between owners simply stay on send_pkt_queue until the
next device start drains them.
Since the device stays reachable through the CID hash, the lockless
send/cancel paths can race with the worker teardown in
vhost_workers_free(). The previous commit ("vhost: synchronize with
RCU readers when freeing workers") makes that safe.
Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
drivers/vhost/vsock.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index d5022d21120b..86f25ff80722 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -903,6 +903,29 @@ static int vhost_vsock_set_features(struct vhost_vsock *vsock, u64 features)
return -EFAULT;
}
+static long vhost_vsock_reset_owner(struct vhost_vsock *vsock)
+{
+ struct vhost_iotlb *umem;
+ long err;
+
+ mutex_lock(&vsock->dev.mutex);
+ err = vhost_dev_check_owner(&vsock->dev);
+ if (err)
+ goto done;
+ umem = vhost_dev_reset_owner_prepare();
+ if (!umem) {
+ err = -ENOMEM;
+ goto done;
+ }
+ vhost_vsock_drop_backends(vsock);
+ vhost_vsock_flush(vsock);
+ vhost_dev_stop(&vsock->dev);
+ vhost_dev_reset_owner(&vsock->dev, umem);
+done:
+ mutex_unlock(&vsock->dev.mutex);
+ return err;
+}
+
static long vhost_vsock_dev_ioctl(struct file *f, unsigned int ioctl,
unsigned long arg)
{
@@ -946,6 +969,8 @@ static long vhost_vsock_dev_ioctl(struct file *f, unsigned int ioctl,
return -EOPNOTSUPP;
vhost_set_backend_features(&vsock->dev, features);
return 0;
+ case VHOST_RESET_OWNER:
+ return vhost_vsock_reset_owner(vsock);
default:
mutex_lock(&vsock->dev.mutex);
r = vhost_dev_ioctl(&vsock->dev, ioctl, argp);
--
2.47.1
^ permalink raw reply related
* [PATCH v4 4/5] vhost: synchronize with RCU readers when freeing workers
From: Andrey Drobyshev @ 2026-07-14 15:16 UTC (permalink / raw)
To: linux-kernel
Cc: kvm, virtualization, netdev, sgarzare, mst, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den, andrey.drobyshev
In-Reply-To: <20260714151638.143019-1-andrey.drobyshev@virtuozzo.com>
vhost_vq_work_queue() only holds the RCU read lock while it dereferences
vq->worker and queues work on it. vhost_workers_free() however clears
the vq->worker pointers and immediately frees the workers, without
waiting for a grace period. A caller that fetched the worker right
before the pointer was cleared can therefore still be queueing work on
it while it is freed. And even when the queueing itself wins the race,
the work is never run, so its VHOST_WORK_QUEUED bit stays set and all
future attempts to queue it are silently skipped.
None of the current callers can actually hit this: net and scsi stop
their virtqueues before the workers are freed, and vsock unhashes the
device and does synchronize_rcu() of its own in vhost_vsock_dev_release()
before the workers go away. But the upcoming VHOST_RESET_OWNER support
in vhost-vsock keeps the device hashed while its workers are freed, so
the lockless send/cancel paths become able to race with the teardown.
Close this the way vhost_worker_killed() already does: clear the
vq->worker pointers, wait for a grace period, run whatever the last
readers may have queued, and only then free the workers. The
synchronize_rcu() is skipped if the device has no workers, so cleanup of
devices which never got an owner stays cheap.
Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
drivers/vhost/vhost.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 4c525b3e16ea..0d1414d40f4e 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -729,6 +729,21 @@ static void vhost_workers_free(struct vhost_dev *dev)
for (i = 0; i < dev->nvqs; i++)
rcu_assign_pointer(dev->vqs[i]->worker, NULL);
+
+ /*
+ * vhost_vq_work_queue() reads vq->worker under rcu_read_lock(), so a
+ * caller that fetched a worker before we cleared the pointers above
+ * may still be about to queue work on it. Wait for those RCU readers
+ * to finish before freeing the worker, then run whatever they queued
+ * so nothing is left with VHOST_WORK_QUEUED set. Mirrors
+ * vhost_worker_killed().
+ */
+ if (!xa_empty(&dev->worker_xa)) {
+ synchronize_rcu();
+ xa_for_each(&dev->worker_xa, i, worker)
+ vhost_run_work_list(worker);
+ }
+
/*
* Free the default worker we created and cleanup workers userspace
* created but couldn't clean up (it forgot or crashed).
--
2.47.1
^ permalink raw reply related
* [PATCH v4 3/5] vhost/vsock: re-scan TX virtqueue on device start
From: Andrey Drobyshev @ 2026-07-14 15:16 UTC (permalink / raw)
To: linux-kernel
Cc: kvm, virtualization, netdev, sgarzare, mst, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den, andrey.drobyshev
In-Reply-To: <20260714151638.143019-1-andrey.drobyshev@virtuozzo.com>
During QEMU CPR live-update (and VHOST_RESET_OWNER in general) the guest
keeps running while the host drops and later re-attaches vhost backends.
If the guest adds a buffer to the TX virtqueue (guest->host) and kicks
while the backend is temporarily NULL (between vhost_vsock_drop_backends()
and the next vhost_vsock_start()), then the kick is delivered to the
vhost worker, handle_tx_kick() sees a NULL backend and returns, and the
kick signal is consumed. The buffer is then left in the ring.
Then upon device start vhost_vsock_start() only re-kicks the RX send
worker, never the TX VQ, so the buffer is processed only if the guest
happens to kick again. But if the guest itself is now waiting for data
from the host, it will never kick TX VQ again, and we end up in a
deadlock.
The issue itself is pre-existing, but it only manifests during a device
pause caused by VHOST_RESET_OWNER. Namely, the deadlock is reproduced
during active host->guest socat data transfer under multiple consecutive
CPR live-update's.
To fix this, in vhost_vsock_start(), after kicking the RX send worker, also
queue the TX vq poll so any buffers the guest enqueued while we were paused
get scanned.
The VHOST_RESET_OWNER ioctl itself is implemented in the following
patch, thus this patch is a preparation to support VHOST_RESET_OWNER.
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
drivers/vhost/vsock.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index 27169a09e87e..d5022d21120b 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -646,6 +646,13 @@ static int vhost_vsock_start(struct vhost_vsock *vsock)
*/
vhost_vq_work_queue(&vsock->vqs[VSOCK_VQ_RX], &vsock->send_pkt_work);
+ /* The guest may have added TX buffers while the device was stopped
+ * (e.g. across VHOST_RESET_OWNER) and their kicks got consumed by
+ * the NULL-backend window. Re-scan the TX VQ, mirroring the RX
+ * send-worker kick above.
+ */
+ vhost_poll_queue(&vsock->vqs[VSOCK_VQ_TX].poll);
+
mutex_unlock(&vsock->dev.mutex);
return 0;
--
2.47.1
^ permalink raw reply related
* [PATCH v4 2/5] vhost/vsock: suppress EHOSTUNREACH fast-fail during CPR pause
From: Andrey Drobyshev @ 2026-07-14 15:16 UTC (permalink / raw)
To: linux-kernel
Cc: kvm, virtualization, netdev, sgarzare, mst, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den, andrey.drobyshev
In-Reply-To: <20260714151638.143019-1-andrey.drobyshev@virtuozzo.com>
Earlier commit bb26ed5f3a8b ("vhost/vsock: Refuse the connection
immediately when guest isn't ready") added a fast-fail in
vhost_transport_send_pkt(). It rejects every host send with -EHOSTUNREACH
until the destination calls SET_RUNNING(1). The fast-fail condition checks
whether device's backends are dropped, and if they're, the guest is
considered to be not ready.
However, there might be other reasons for backends to be nulled. In
particular, when QEMU is performing CPR (checkpoint-restore) migration,
device ownership is being RESET and SET again, which leads to backends
drop and reattach. If we end up connecting during this window, an
AF_VSOCK client gets -EHOSTUNREACH, which is wrong.
Add an 'ever_started' flag which is set once in vhost_vsock_start() and is
never cleared. The behaviour changes to:
* When device was never started -> flag is unset -> no listener can
exist yet -> fast-fail;
* Once the device starts -> flag is set -> we don't fast-fail ->
we queue and preserve during any later stop / CPR pause.
The VHOST_RESET_OWNER ioctl is implemented in a following patch, and
without RESET_OWNER the problem we fix here isn't manifesting - thus
this patch is a preparation to support RESET_OWNER.
Important caveat: after the first start, a connect during any stopped
window is queued instead of fast-failed. That was the behaviour before
the patch bb26ed5f3a8b, and we're restoring it now. However we still
keep the behaviour originally intended by that commit (i.e. fast-fail if
there's no real listener yet) while fixing the CPR path.
Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
drivers/vhost/vsock.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index b12221ce6faf..27169a09e87e 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -61,6 +61,7 @@ struct vhost_vsock {
u32 guest_cid;
bool seqpacket_allow;
+ bool ever_started; /* set on first SET_RUNNING(1); never cleared */
};
static u32 vhost_transport_get_local_cid(void)
@@ -302,17 +303,12 @@ vhost_transport_send_pkt(struct sk_buff *skb, struct net *net)
return -ENODEV;
}
- /* Fast-fail if the guest hasn't enabled the RX vq yet. Queuing the packet
- * and making the caller wait is pointless: even if the guest manages to init
- * within the timeout, it'll immediately reply with RST, because there's no
- * listener on the port yet.
- *
- * vhost_vq_get_backend() without vq->mutex is acceptable here: locking
- * the mutex would be too expensive in this hot path, and we already have
- * all the outcomes covered: if the backend becomes NULL right after the check,
- * vhost_transport_do_send_pkt() will check it under the mutex anyway.
+ /* Fast-fail until the guest first enables the device (SET_RUNNING(1)).
+ * Before that there is no listener, so queuing is pointless.
+ * 'ever_started' is never cleared, so once we're up we keep queuing
+ * across later stop / CPR-pause windows.
*/
- if (unlikely(!data_race(vhost_vq_get_backend(&vsock->vqs[VSOCK_VQ_RX])))) {
+ if (unlikely(!READ_ONCE(vsock->ever_started))) {
rcu_read_unlock();
kfree_skb(skb);
return -EHOSTUNREACH;
@@ -640,6 +636,11 @@ static int vhost_vsock_start(struct vhost_vsock *vsock)
mutex_unlock(&vq->mutex);
}
+ /* Set 'ever_started' flag on the first start; never cleared, so send_pkt
+ * keeps queuing (instead of fast-failing) on later stop / CPR pauses.
+ */
+ WRITE_ONCE(vsock->ever_started, true);
+
/* Some packets may have been queued before the device was started,
* let's kick the send worker to send them.
*/
@@ -728,6 +729,7 @@ static int vhost_vsock_dev_open(struct inode *inode, struct file *file)
vsock->guest_cid = 0; /* no CID assigned yet */
vsock->seqpacket_allow = false;
+ vsock->ever_started = false;
atomic_set(&vsock->queued_replies, 0);
--
2.47.1
^ permalink raw reply related
* [PATCH v4 1/5] vhost/vsock: split out vhost_vsock_drop_backends helper
From: Andrey Drobyshev @ 2026-07-14 15:16 UTC (permalink / raw)
To: linux-kernel
Cc: kvm, virtualization, netdev, sgarzare, mst, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den, andrey.drobyshev
In-Reply-To: <20260714151638.143019-1-andrey.drobyshev@virtuozzo.com>
From: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Split the actual backend dropping part from vhost_vsock_stop. We're
going to need it for the VHOST_RESET_OWNER implementation in the
following patch, when vsock->dev.mutex is already taken and owner is
checked.
Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
---
drivers/vhost/vsock.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index 9aaab6bb8061..b12221ce6faf 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -664,9 +664,24 @@ static int vhost_vsock_start(struct vhost_vsock *vsock)
return ret;
}
-static int vhost_vsock_stop(struct vhost_vsock *vsock, bool check_owner)
+static void vhost_vsock_drop_backends(struct vhost_vsock *vsock)
{
+ struct vhost_virtqueue *vq;
size_t i;
+
+ lockdep_assert_held(&vsock->dev.mutex);
+
+ for (i = 0; i < ARRAY_SIZE(vsock->vqs); i++) {
+ vq = &vsock->vqs[i];
+
+ mutex_lock(&vq->mutex);
+ vhost_vq_set_backend(vq, NULL);
+ mutex_unlock(&vq->mutex);
+ }
+}
+
+static int vhost_vsock_stop(struct vhost_vsock *vsock, bool check_owner)
+{
int ret = 0;
mutex_lock(&vsock->dev.mutex);
@@ -677,14 +692,7 @@ static int vhost_vsock_stop(struct vhost_vsock *vsock, bool check_owner)
goto err;
}
- for (i = 0; i < ARRAY_SIZE(vsock->vqs); i++) {
- struct vhost_virtqueue *vq = &vsock->vqs[i];
-
- mutex_lock(&vq->mutex);
- vhost_vq_set_backend(vq, NULL);
- mutex_unlock(&vq->mutex);
- }
-
+ vhost_vsock_drop_backends(vsock);
err:
mutex_unlock(&vsock->dev.mutex);
return ret;
--
2.47.1
^ permalink raw reply related
* [PATCH v4 0/5] vhost/vsock: add support for VHOST_RESET_OWNER and CPR migration
From: Andrey Drobyshev @ 2026-07-14 15:16 UTC (permalink / raw)
To: linux-kernel
Cc: kvm, virtualization, netdev, sgarzare, mst, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den, andrey.drobyshev
The main change since v3: as suggested by Stefano, the worker teardown
race is now fixed where it actually lives, i.e. in vhost_workers_free(),
instead of being guarded from the vsock side. The new patch 4 makes the
teardown wait out the RCU readers which might still be queueing work on
a worker, and then run whatever they queued, before the workers are
freed.
With the teardown itself made safe, queueing work on a stopped device
is harmless again: the work handlers check the backend under vq->mutex
and simply return. So the backend guards in send_pkt()/cancel_pkt()
and the synchronize_rcu() in reset_owner() from v3 are no longer needed
and are gone.
v3 -> v4:
* Patch 2:
- rename 'started' -> 'ever_started';
- reword commit message;
* Patch 3: reword commit message and code comment;
* Patch 4 (NEW) ("vhost: synchronize with RCU readers when freeing workers"):
- fix the vq->worker UAF and the stuck VHOST_WORK_QUEUED bit
generically in vhost_workers_free();
* Patch 4 -> 5 ("vhost/vsock: add VHOST_RESET_OWNER ioctl"):
- drop the backend guards in send_pkt()/cancel_pkt() and the
synchronize_rcu() in reset_owner() - covered by patch 4 now;
- make vhost_vsock_reset_owner() return long;
- reword commit message.
v3: https://lore.kernel.org/virtualization/20260625155416.480669-1-andrey.drobyshev@virtuozzo.com
Andrey Drobyshev (3):
vhost/vsock: suppress EHOSTUNREACH fast-fail during CPR pause
vhost/vsock: re-scan TX virtqueue on device start
vhost: synchronize with RCU readers when freeing workers
Pavel Tikhomirov (2):
vhost/vsock: split out vhost_vsock_drop_backends helper
vhost/vsock: add VHOST_RESET_OWNER ioctl
drivers/vhost/vhost.c | 15 ++++++++++
drivers/vhost/vsock.c | 80 +++++++++++++++++++++++++++++++++++++++------------
2 files changed, 76 insertions(+), 19 deletions(-)
--
2.47.1
^ permalink raw reply
* Re: [RFC] virtio_balloon: fix Use-After-Free in page reporting during PM freeze
From: Michael S. Tsirkin @ 2026-07-14 13:24 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Link Lin, Andrew Morton, Vlastimil Babka, virtualization,
linux-mm, linux-kernel, prasin, rientjes, duenwen, jasowang,
xuanzhuo, Ammar Faizi, jiaqiyan, ahwilkins, Greg Thelen,
Alexander Duyck, stable
In-Reply-To: <8d316b6c-41fb-4ae3-8923-3b649b92b33d@kernel.org>
On Tue, Jul 14, 2026 at 03:17:42PM +0200, David Hildenbrand (Arm) wrote:
> On 7/10/26 00:43, Link Lin wrote:
> > During system power management freeze (e.g. ACPI S3 suspend or S4
> > hibernation), virtballoon_freeze() calls remove_common() to reset the
> > virtio device and delete all virtqueues via vdev->config->del_vqs().
> > However, unlike virtballoon_remove(), virtballoon_freeze() fails to call
> > page_reporting_unregister(&vb->pr_dev_info).
> >
> > The comment in virtballoon_freeze() states:
> > /*
> > * The workqueue is already frozen by the PM core before this
> > * function is called.
> > */
> >
> > While this comment was accurate in 2011 for balloon-internal workqueues
> > (such as balloon_wq, which was created with WQ_FREEZABLE and is paused
> > by the PM freezer), it is invalid for Free Page Reporting.
> >
> > Free Page Reporting (mm/page_reporting.c) schedules its delayed work
> > (prdev->work) on the global system_wq. Because system_wq lacks the
> > WQ_FREEZABLE flag, the PM freezer (freeze_workqueues_busy()) explicitly
> > skips it. Consequently, page_reporting_process() on system_wq remains
> > active and unfrozen throughout device suspend.
> >
> > If memory is freed into the buddy allocator or a delayed work timer
> > expires while the device is being frozen, page_reporting_process() fires
> > on system_wq and calls virtballoon_free_page_report(). This function
> > passes vb->reporting_vq into virtqueue_add_inbuf() / virtqueue_add_split().
> > Because the virtqueues were already destroyed by del_vqs(), this results
> > in a Use-After-Free / General Protection Fault:
> >
> > [ 250.709271] general protection fault, probably for non-canonical address 0x7f728084daf08d5e: 0000 [#1] SMP PTI
> > [ 250.732967] CPU: 2 PID: 38 Comm: kworker/2:1 Not tainted 5.10.0-44-cloud-amd64 #1 Debian 5.10.257-1
> > [ 250.751575] Workqueue: events page_reporting_process
> > [ 250.756665] RIP: 0010:virtqueue_add_split+0x233/0x4c0 [virtio_ring]
> > ...
> > [ 250.867678] virtballoon_free_page_report+0x3a/0xe0 [virtio_balloon]
> > [ 250.883446] page_reporting_process+0x225/0x4f0
> >
> > (Note: The OOM Notifier and Shrinker/Free Page Hinting features suffer
> > from an identical lifecycle flaw and are also vulnerable to UAFs during
> > S4 hibernation when memory pressure spikes. This patch focuses on Free
> > Page Reporting, which runs periodically, to ensure clean backports to
> > stable kernels).
> >
> > Fix this by:
> > 1. Unregistering page reporting in virtballoon_freeze() prior to calling
> > remove_common(). This clears the RCU pr_dev_info pointer and flushes/
> > cancels prdev->work on system_wq via cancel_delayed_work_sync().
> > 2. Re-registering page reporting in virtballoon_restore() after the
> > virtqueues are re-initialized and virtio_device_ready() has been called.
> > 3. Unwinding virtqueue initialization via remove_common() in
> > virtballoon_restore() if page_reporting_register() fails.
> >
> > Fixes: 924a663f75e2 ("virtio-balloon: Reporting free page reservations")
> > Cc: stable@vger.kernel.org
> > Cc: jasowang@redhat.com
> > Cc: xuanzhuo@linux.alibaba.com
> > Cc: Ammar Faizi <ammarfaizi2@openresty.com>
> > Cc: jiaqiyan@google.com
> > Cc: ahwilkins@google.com
> > Cc: Greg Thelen <gthelen@google.com>
> > Cc: Alexander Duyck <alexander.duyck@gmail.com>
> > Signed-off-by: Link Lin <linkl@google.com>
> > ---
> > drivers/virtio/virtio_balloon.c | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> > index a1b2c3d4e5f6..45a90fb3abf8 100640
> > --- a/drivers/virtio/virtio_balloon.c
> > +++ b/drivers/virtio/virtio_balloon.c
> > @@ -1055,6 +1055,9 @@ static int virtballoon_freeze(struct virtio_device *vdev)
> > * The workqueue is already frozen by the PM core before this
> > * function is called.
> > */
> > + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING))
> > + page_reporting_unregister(&vb->pr_dev_info);
> > +
> > remove_common(vb);
> > return 0;
> > }
> >
> > static int virtballoon_restore(struct virtio_device *vdev)
> > {
> > struct virtio_balloon *vb = vdev->priv;
> > int ret;
> >
> > ret = init_vqs(vdev->priv);
> > if (ret)
> > return ret;
> >
> > virtio_device_ready(vdev);
> >
> > + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING)) {
> > + ret = page_reporting_register(&vb->pr_dev_info);
> > + if (ret)
> > + goto out_remove_vqs;
> > + }
>
> Hm, that failure handling is rather nasty.
>
>
> In virtballoon_freeze() we document:
>
> "The workqueue is already frozen by the PM core before this function is called"
>
> Your report states:
>
> "Workqueue: events page_reporting_process"
>
>
> I assume that workqueue is not frozen yet because ... it's not freezable :)
>
> So could we queue to system_freezable_wq instead, or define our own freezable
> workqueue there? Then a driver doesn't have to worry about that.
>
> --
> Cheers,
>
> David
+1. Just system_freezable_wq will do the trick.
--
MST
^ permalink raw reply
* Re: [PATCH] virtio_net: fix infinite loop in virtnet_poll_cleantx when device is broken
From: Michael S. Tsirkin @ 2026-07-14 13:17 UTC (permalink / raw)
To: Jinqian Yang
Cc: jasowang, xuanzhuo, eperezma, andrew+netdev, davem, edumazet,
kuba, pabeni, netdev, virtualization, linux-kernel, liuyonglong,
wangzhou1, linuxarm
In-Reply-To: <20260713132025.703147-1-yangjinqian1@huawei.com>
On Mon, Jul 13, 2026 at 09:20:25PM +0800, Jinqian Yang wrote:
> virtnet_poll_cleantx() contains a do-while loop that cleans up
> transmitted TX buffers and calls virtqueue_enable_cb_delayed() to check
> whether more buffers need processing. When the virtio backend stops
> responding during guest reboot, used->idx is never updated, so
> virtqueue_enable_cb_delayed() always returns false and the loop never
> terminates. Then it will block reboot process, and the guest will hang.
>
> The problem occurs during guest reboot under network traffic:
>
> 1. kernel_restart() -> device_shutdown() traverses the device list
> 2. virtio_dev_shutdown() calls virtio_break_device() which sets
> vq->broken = true
> 3. virtio_dev_shutdown() then calls virtio_synchronize_cbs() to wait
> for in-flight callbacks to complete
> 4. A virtio interrupt fires, softirq is deferred to ksoftirqd which
> calls net_rx_action() -> virtnet_poll() -> virtnet_poll_cleantx()
> 5. virtnet_poll_cleantx() enters the do-while loop and never exits
> because the QEMU backend has stopped updating used->idx, despite
> vq->broken having been set to true in step 2.
>
> Since the loop runs inside ksoftirqd (a SCHED_OTHER kthread), it is
> visible to the scheduler and does not trigger a hard lockup. However,
> the kthread never leaves the loop, so RCU detects it as a CPU stall
> and reports it periodically. Meanwhile, the reboot process remains
> blocked in device_shutdown() because virtio_dev_shutdown() cannot
> complete its synchronization step, and the guest hangs permanently.
>
> This can be reproduced on a guest with a virtio-net device: run iperf3
> traffic in the guest, then trigger reboot. The reboot occasionally hangs
> permanently with RCU stall on ksoftirqd.
>
> Observed on ARM64 KVM guest:
>
> CPU#1 RCU stall (ksoftirqd/1), repeated periodically:
> virtqueue_enable_cb_delayed_split <- virtnet_poll <- __napi_poll <-
> net_rx_action <- handle_softirqs <- run_ksoftirqd <-
> smpboot_thread_fn <- kthread
>
> Fix by adding a virtqueue_is_broken() check to the loop condition, so
> that the loop exits immediately when the device is broken, allowing
> the device shutdown to proceed.
>
> Signed-off-by: Jinqian Yang <yangjinqian1@huawei.com>
I'd expect lots of drivers have this issue? Wouldn't it make more sense
to check virtqueue_is_broken in
virtqueue_enable_cb_delayed/virtqueue_enable_cb? This way it works for
all drivers.
> ---
> drivers/net/virtio_net.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 7d2eeb9b1226..c8d2d420c31d 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2970,7 +2970,8 @@ static void virtnet_poll_cleantx(struct receive_queue *rq, int budget)
> do {
> virtqueue_disable_cb(sq->vq);
> free_old_xmit(sq, txq, !!budget);
> - } while (unlikely(!virtqueue_enable_cb_delayed(sq->vq)));
> + } while (!virtqueue_is_broken(sq->vq) &&
> + unlikely(!virtqueue_enable_cb_delayed(sq->vq)));
>
> if (sq->vq->num_free >= MAX_SKB_FRAGS + 2)
> virtnet_tx_wake_queue(vi, sq);
> --
> 2.33.0
^ permalink raw reply
* Re: [RFC] virtio_balloon: fix Use-After-Free in page reporting during PM freeze
From: David Hildenbrand (Arm) @ 2026-07-14 13:17 UTC (permalink / raw)
To: Link Lin, Andrew Morton, Vlastimil Babka, Michael S . Tsirkin
Cc: virtualization, linux-mm, linux-kernel, prasin, rientjes, duenwen,
jasowang, xuanzhuo, Ammar Faizi, jiaqiyan, ahwilkins, Greg Thelen,
Alexander Duyck, stable
In-Reply-To: <20260709224330.946683-1-linkl@google.com>
On 7/10/26 00:43, Link Lin wrote:
> During system power management freeze (e.g. ACPI S3 suspend or S4
> hibernation), virtballoon_freeze() calls remove_common() to reset the
> virtio device and delete all virtqueues via vdev->config->del_vqs().
> However, unlike virtballoon_remove(), virtballoon_freeze() fails to call
> page_reporting_unregister(&vb->pr_dev_info).
>
> The comment in virtballoon_freeze() states:
> /*
> * The workqueue is already frozen by the PM core before this
> * function is called.
> */
>
> While this comment was accurate in 2011 for balloon-internal workqueues
> (such as balloon_wq, which was created with WQ_FREEZABLE and is paused
> by the PM freezer), it is invalid for Free Page Reporting.
>
> Free Page Reporting (mm/page_reporting.c) schedules its delayed work
> (prdev->work) on the global system_wq. Because system_wq lacks the
> WQ_FREEZABLE flag, the PM freezer (freeze_workqueues_busy()) explicitly
> skips it. Consequently, page_reporting_process() on system_wq remains
> active and unfrozen throughout device suspend.
>
> If memory is freed into the buddy allocator or a delayed work timer
> expires while the device is being frozen, page_reporting_process() fires
> on system_wq and calls virtballoon_free_page_report(). This function
> passes vb->reporting_vq into virtqueue_add_inbuf() / virtqueue_add_split().
> Because the virtqueues were already destroyed by del_vqs(), this results
> in a Use-After-Free / General Protection Fault:
>
> [ 250.709271] general protection fault, probably for non-canonical address 0x7f728084daf08d5e: 0000 [#1] SMP PTI
> [ 250.732967] CPU: 2 PID: 38 Comm: kworker/2:1 Not tainted 5.10.0-44-cloud-amd64 #1 Debian 5.10.257-1
> [ 250.751575] Workqueue: events page_reporting_process
> [ 250.756665] RIP: 0010:virtqueue_add_split+0x233/0x4c0 [virtio_ring]
> ...
> [ 250.867678] virtballoon_free_page_report+0x3a/0xe0 [virtio_balloon]
> [ 250.883446] page_reporting_process+0x225/0x4f0
>
> (Note: The OOM Notifier and Shrinker/Free Page Hinting features suffer
> from an identical lifecycle flaw and are also vulnerable to UAFs during
> S4 hibernation when memory pressure spikes. This patch focuses on Free
> Page Reporting, which runs periodically, to ensure clean backports to
> stable kernels).
>
> Fix this by:
> 1. Unregistering page reporting in virtballoon_freeze() prior to calling
> remove_common(). This clears the RCU pr_dev_info pointer and flushes/
> cancels prdev->work on system_wq via cancel_delayed_work_sync().
> 2. Re-registering page reporting in virtballoon_restore() after the
> virtqueues are re-initialized and virtio_device_ready() has been called.
> 3. Unwinding virtqueue initialization via remove_common() in
> virtballoon_restore() if page_reporting_register() fails.
>
> Fixes: 924a663f75e2 ("virtio-balloon: Reporting free page reservations")
> Cc: stable@vger.kernel.org
> Cc: jasowang@redhat.com
> Cc: xuanzhuo@linux.alibaba.com
> Cc: Ammar Faizi <ammarfaizi2@openresty.com>
> Cc: jiaqiyan@google.com
> Cc: ahwilkins@google.com
> Cc: Greg Thelen <gthelen@google.com>
> Cc: Alexander Duyck <alexander.duyck@gmail.com>
> Signed-off-by: Link Lin <linkl@google.com>
> ---
> drivers/virtio/virtio_balloon.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index a1b2c3d4e5f6..45a90fb3abf8 100640
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -1055,6 +1055,9 @@ static int virtballoon_freeze(struct virtio_device *vdev)
> * The workqueue is already frozen by the PM core before this
> * function is called.
> */
> + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING))
> + page_reporting_unregister(&vb->pr_dev_info);
> +
> remove_common(vb);
> return 0;
> }
>
> static int virtballoon_restore(struct virtio_device *vdev)
> {
> struct virtio_balloon *vb = vdev->priv;
> int ret;
>
> ret = init_vqs(vdev->priv);
> if (ret)
> return ret;
>
> virtio_device_ready(vdev);
>
> + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING)) {
> + ret = page_reporting_register(&vb->pr_dev_info);
> + if (ret)
> + goto out_remove_vqs;
> + }
Hm, that failure handling is rather nasty.
In virtballoon_freeze() we document:
"The workqueue is already frozen by the PM core before this function is called"
Your report states:
"Workqueue: events page_reporting_process"
I assume that workqueue is not frozen yet because ... it's not freezable :)
So could we queue to system_freezable_wq instead, or define our own freezable
workqueue there? Then a driver doesn't have to worry about that.
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH] virtio_mem: fix typo in comment
From: David Hildenbrand (Arm) @ 2026-07-14 8:44 UTC (permalink / raw)
To: weimin xiong, mst; +Cc: jasowangio, virtualization, xiongweimin
In-Reply-To: <20260714032417.201353-1-xiongwm2026@163.com>
On 7/14/26 05:24, weimin xiong wrote:
> From: xiongweimin <xiongweimin@kylinos.cn>
>
> Correct "actipn" to "action".
>
> Signed-off-by: xiongweimin <xiongweimin@kylinos.cn>
> ---
> drivers/virtio/virtio_mem.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
> index 11c441501..90d8ef0b9 100644
> --- a/drivers/virtio/virtio_mem.c
> +++ b/drivers/virtio/virtio_mem.c
> @@ -1080,7 +1080,7 @@ static int virtio_mem_memory_notifier_cb(struct notifier_block *nb,
> atomic64_sub(size, &vm->offline_size);
> /*
> * Start adding more memory once we onlined half of our
> - * threshold. Don't trigger if it's possibly due to our actipn
> + * threshold. Don't trigger if it's possibly due to our action
> * (e.g., us adding memory which gets onlined immediately from
> * the core).
> */
Acked-by: David Hildenbrand (Arm) <david@kernel.org>
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH v1 20/20] gpio: Unify style of various *_device_id arrays
From: Andy Shevchenko @ 2026-07-14 8:37 UTC (permalink / raw)
To: Uwe Kleine-König (The Capable Hub)
Cc: Linus Walleij, Bartosz Golaszewski, Lixu Zhang, Sakari Ailus,
Enrico Weigelt, metux IT consult, Viresh Kumar, Mika Westerberg,
linux-gpio, linux-kernel, virtualization, linux-acpi
In-Reply-To: <1f2d39342995533857417eb890628f7643b9a159.1784013063.git.u.kleine-koenig@baylibre.com>
On Tue, Jul 14, 2026 at 09:24:21AM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> Update the various *_device_id arrays to conform to the most used and
> generally recommended coding style. That is:
>
> - no comma after the list terminator;
> - a comma after an initializer if (and only if) the closing } is not
> directly following;
> - no explicit zeros in the list terminator;
> - a space after an opening { and before a closing }, a single space in
> the list terminator;
>
> Adapt the few offenders accordingly.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v1 20/20] gpio: Unify style of various *_device_id arrays
From: Sakari Ailus @ 2026-07-14 8:12 UTC (permalink / raw)
To: Uwe Kleine-König (The Capable Hub)
Cc: Linus Walleij, Bartosz Golaszewski, Lixu Zhang,
Enrico Weigelt, metux IT consult, Viresh Kumar, Mika Westerberg,
Andy Shevchenko, linux-gpio, linux-kernel, virtualization,
linux-acpi
In-Reply-To: <1f2d39342995533857417eb890628f7643b9a159.1784013063.git.u.kleine-koenig@baylibre.com>
On Tue, Jul 14, 2026 at 09:24:21AM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> Update the various *_device_id arrays to conform to the most used and
> generally recommended coding style. That is:
>
> - no comma after the list terminator;
> - a comma after an initializer if (and only if) the closing } is not
> directly following;
> - no explicit zeros in the list terminator;
> - a space after an opening { and before a closing }, a single space in
> the list terminator;
>
> Adapt the few offenders accordingly.
>
> Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
> ---
> drivers/gpio/gpio-ljca.c | 2 +-
For gpio-ljca:
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
--
Sakari Ailus
^ permalink raw reply
* Re: [PATCH v1 20/20] gpio: Unify style of various *_device_id arrays
From: Viresh Kumar @ 2026-07-14 8:10 UTC (permalink / raw)
To: Uwe Kleine-König (The Capable Hub)
Cc: Linus Walleij, Bartosz Golaszewski, Lixu Zhang, Sakari Ailus,
Enrico Weigelt, metux IT consult, Viresh Kumar, Mika Westerberg,
Andy Shevchenko, linux-gpio, linux-kernel, virtualization,
linux-acpi
In-Reply-To: <1f2d39342995533857417eb890628f7643b9a159.1784013063.git.u.kleine-koenig@baylibre.com>
On 14-07-26, 09:24, Uwe Kleine-König (The Capable Hub) wrote:
> diff --git a/drivers/gpio/gpio-virtio.c b/drivers/gpio/gpio-virtio.c
> index 42871db05ec1..062c70fe4671 100644
> --- a/drivers/gpio/gpio-virtio.c
> +++ b/drivers/gpio/gpio-virtio.c
> @@ -647,7 +647,7 @@ static void virtio_gpio_remove(struct virtio_device *vdev)
>
> static const struct virtio_device_id id_table[] = {
> { .device = VIRTIO_ID_GPIO, .vendor = VIRTIO_DEV_ANY_ID },
> - {},
> + { }
> };
> MODULE_DEVICE_TABLE(virtio, id_table);
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
^ permalink raw reply
* Re: [PATCH v1 13/20] gpio: virtio: Use a named initializer for virtio_device_id array
From: Viresh Kumar @ 2026-07-14 8:10 UTC (permalink / raw)
To: Uwe Kleine-König (The Capable Hub)
Cc: Linus Walleij, Bartosz Golaszewski,
Enrico Weigelt, metux IT consult, Viresh Kumar, linux-gpio,
virtualization, linux-kernel
In-Reply-To: <1f764e00e8cbe7624f4c9f3d8e5c368fd9a13e08.1784013063.git.u.kleine-koenig@baylibre.com>
On 14-07-26, 09:24, Uwe Kleine-König (The Capable Hub) wrote:
> While being less compact, using named initializers allows to more easily
> see which members of the structs are assigned which value without having
> to lookup the declaration of the struct. And it's also more robust
> against changes to the struct definition.
>
> This patch doesn't modify the compiled array, only its representation in
> source form benefits.
>
> Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
> ---
> drivers/gpio/gpio-virtio.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpio-virtio.c b/drivers/gpio/gpio-virtio.c
> index ed6e0e90fa8a..42871db05ec1 100644
> --- a/drivers/gpio/gpio-virtio.c
> +++ b/drivers/gpio/gpio-virtio.c
> @@ -646,7 +646,7 @@ static void virtio_gpio_remove(struct virtio_device *vdev)
> }
>
> static const struct virtio_device_id id_table[] = {
> - { VIRTIO_ID_GPIO, VIRTIO_DEV_ANY_ID },
> + { .device = VIRTIO_ID_GPIO, .vendor = VIRTIO_DEV_ANY_ID },
> {},
> };
> MODULE_DEVICE_TABLE(virtio, id_table);
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox