* Re: [PATCH 2/4] virtio_input: validate device-reported multitouch slot count
From: Dmitry Torokhov @ 2026-07-16 17:33 UTC (permalink / raw)
To: Hari Mishal
Cc: Michael S. Tsirkin, Amit Shah, Arnd Bergmann, Greg Kroah-Hartman,
Gerd Hoffmann, Jason Wang, David Hildenbrand, Henrik Rydberg,
Xuan Zhuo, Eugenio Pérez, virtualization, linux-kernel,
linux-input
In-Reply-To: <alkTnRb9qhgcMGGi@google.com>
[ Just realized that CC was dropped in the email I was replying to, so
restoring and resending... ]
On Thu, Jul 16, 2026 at 10:28:36AM -0700, Dmitry Torokhov wrote:
> On Thu, Jul 16, 2026 at 12:34:57PM +0200, Hari Mishal wrote:
> > > What is the failure mode if we keep the ABS_MT_SLOT capability? Does the
> > > kernel crash? And if this can cause crash then we should fix
> > > input_mt_init_slots() to reject requests for 0 slots with -EINVAL.
> > >
> >
> > No, it doesn't crash. I think every place in the input core that touches
> > dev->mt guards against it being NULL: input_handle_abs_event() and
> > the mt_slots check in input.c, and evdev's EVIOCGMTSLOTS ioctl
> > handler all explicitly check for NULL and degrade cleanly instead of
> > dereferencing. From my understanding, the worst case is what the
> > original commit message already covers: the device advertises
> > multitouch support it can't back.
>
> So what? I still do not see the problem. Let's say I have a device that
> properly supports multitouch and has slots, but then never sends any
> events because firmware is buggy. How would that affect anything?
>
> If there is no crash that I would leave the driver alone.
>
> And we need to remember that we are dealing with a hypervisor here that
> normally had higher level of trust than the VM. If it messes up we do
> not have to clean up after it. This scenario is different from user
> attaching a malicious USB device to their system and getting owned.
>
> Thanks.
>
--
Dmitry
^ permalink raw reply
* [PATCH 6.12] vsock/virtio: fix zerocopy completion for multi-skb sends
From: Alexander Martyniuk @ 2026-07-16 16:35 UTC (permalink / raw)
To: stable, Greg Kroah-Hartman
Cc: Alexander Martyniuk, lvc-project, Michael S. Tsirkin, Jason Wang,
Xuan Zhuo, Eugenio Pérez, Stefan Hajnoczi,
Stefano Garzarella, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Arseniy Krasnov, virtualization, kvm,
netdev, linux-kernel, Maher Azzouzi
From: Stefano Garzarella <sgarzare@redhat.com>
commit ae38d9179190a956e2a87a69ef1dd6f451b51c4d upstream.
When a large message is fragmented into multiple skbs, the zerocopy
uarg is only allocated and attached to the last skb in the loop.
Non-final skbs carry pinned user pages with no completion tracking,
so the kernel has no way to notify userspace when those pages are safe
to reuse. If the loop breaks early the uarg is never allocated at all,
leaking pinned pages with no completion notification.
Fix this by following the approach used by TCP: allocate the zerocopy
uarg (if not provided by the caller) before the send loop and attach
it to every skb via skb_zcopy_set(), which takes a reference per skb.
Each skb's completion properly decrements the refcount, and the
notification only fires after the last skb is freed.
On failure, if no data was sent, the uarg is cleanly aborted via
net_zcopy_put_abort().
This issue was initially discovered by sashiko while reviewing commit
1cb36e252211 ("vsock/virtio: fix MSG_ZEROCOPY pinned-pages accounting")
but was pre-existing.
Fixes: 581512a6dc93 ("vsock/virtio: MSG_ZEROCOPY flag support")
Closes: https://sashiko.dev/#/patchset/20260420132051.217589-1-sgarzare%40redhat.com
Reported-by: Maher Azzouzi <maherazz04@gmail.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Arseniy Krasnov <avkrasnov@salutedevices.com>
Link: https://patch.msgid.link/20260514092948.268720-1-sgarzare@redhat.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Alexander Martyniuk <alexevgmart@gmail.com>
---
Backport fix for CVE-2026-53365
net/vmw_vsock/virtio_transport_common.c | 78 +++++++++++--------------
1 file changed, 34 insertions(+), 44 deletions(-)
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 95170c7be758..aeb205e84bd3 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -72,35 +72,6 @@ static bool virtio_transport_can_zcopy(const struct virtio_transport *t_ops,
return true;
}
-static int virtio_transport_init_zcopy_skb(struct vsock_sock *vsk,
- struct sk_buff *skb,
- struct msghdr *msg,
- bool zerocopy)
-{
- struct ubuf_info *uarg;
-
- if (msg->msg_ubuf) {
- uarg = msg->msg_ubuf;
- net_zcopy_get(uarg);
- } else {
- struct iov_iter *iter = &msg->msg_iter;
- struct ubuf_info_msgzc *uarg_zc;
-
- uarg = msg_zerocopy_realloc(sk_vsock(vsk),
- iter->count,
- NULL);
- if (!uarg)
- return -1;
-
- uarg_zc = uarg_to_msgzc(uarg);
- uarg_zc->zerocopy = zerocopy ? 1 : 0;
- }
-
- skb_zcopy_init(skb, uarg);
-
- return 0;
-}
-
static int virtio_transport_fill_skb(struct sk_buff *skb,
struct virtio_vsock_pkt_info *info,
size_t len,
@@ -321,8 +292,10 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
u32 src_cid, src_port, dst_cid, dst_port;
const struct virtio_transport *t_ops;
struct virtio_vsock_sock *vvs;
+ struct ubuf_info *uarg = NULL;
u32 pkt_len = info->pkt_len;
bool can_zcopy = false;
+ bool have_uref = false;
u32 rest_len;
int ret;
@@ -364,6 +337,25 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
if (can_zcopy)
max_skb_len = min_t(u32, VIRTIO_VSOCK_MAX_PKT_BUF_SIZE,
(MAX_SKB_FRAGS * PAGE_SIZE));
+
+ if (info->msg->msg_flags & MSG_ZEROCOPY &&
+ info->op == VIRTIO_VSOCK_OP_RW) {
+ uarg = info->msg->msg_ubuf;
+
+ if (!uarg) {
+ uarg = msg_zerocopy_realloc(sk_vsock(vsk),
+ pkt_len, NULL);
+ if (!uarg) {
+ virtio_transport_put_credit(vvs, pkt_len);
+ return -ENOMEM;
+ }
+
+ if (!can_zcopy)
+ uarg_to_msgzc(uarg)->zerocopy = 0;
+
+ have_uref = true;
+ }
+ }
}
rest_len = pkt_len;
@@ -382,21 +374,7 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
break;
}
- /* We process buffer part by part, allocating skb on
- * each iteration. If this is last skb for this buffer
- * and MSG_ZEROCOPY mode is in use - we must allocate
- * completion for the current syscall.
- */
- if (info->msg && info->msg->msg_flags & MSG_ZEROCOPY &&
- skb_len == rest_len && info->op == VIRTIO_VSOCK_OP_RW) {
- if (virtio_transport_init_zcopy_skb(vsk, skb,
- info->msg,
- can_zcopy)) {
- kfree_skb(skb);
- ret = -ENOMEM;
- break;
- }
- }
+ skb_zcopy_set(skb, uarg, NULL);
virtio_transport_inc_tx_pkt(vvs, skb);
@@ -420,6 +398,18 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
virtio_transport_put_credit(vvs, rest_len);
+ /* msg_zerocopy_realloc() initializes the ubuf_info refcnt to 1.
+ * skb_zcopy_set() increases it for each skb, so we can drop that
+ * initial reference to keep it balanced.
+ */
+ if (have_uref) {
+ if (rest_len == pkt_len)
+ /* No data sent, abort the notification. */
+ net_zcopy_put_abort(uarg, true);
+ else
+ net_zcopy_put(uarg);
+ }
+
/* Return number of bytes, if any data has been sent. */
if (rest_len != pkt_len)
ret = pkt_len - rest_len;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v4 4/5] vhost: synchronize with RCU readers when freeing workers
From: Stefano Garzarella @ 2026-07-16 16:13 UTC (permalink / raw)
To: Andrey Drobyshev
Cc: linux-kernel, kvm, virtualization, netdev, mst, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den
In-Reply-To: <2f680236-f4c1-418b-8401-4dea1230caf0@virtuozzo.com>
On Thu, Jul 16, 2026 at 06:39:48PM +0300, Andrey Drobyshev wrote:
>On 7/16/26 11:57 AM, Stefano Garzarella wrote:
>> On Tue, Jul 14, 2026 at 06:16:37PM +0300, Andrey Drobyshev wrote:
>>> 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.
>>>
>>
>> Do we need a Fixes tag for this?
>>
>
>I'm guessing it should be:
>
>Fixes: 228a27cf78af ("vhost: Allow worker switching while work is queueing")
>
>> Thanks for pointing out that the issue wasn't occurring, but I think we
>> should add it because it's a sneaky problem we discovered by chance.
>> IMO the code should already have `synchronize_rcu()` after
>> `rcu_assign_pointer()` loop.
>>
>> @Michael, what do you think?
>>
>>> 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);
>>> + }
>>> +
>>
>> Following sashiko review [1], I tried to undersand why we need this, but
>> TBH I'm really confused. That said, this seems wrong also because it
>> will work only with vhost_tasks, and not with kthreads.
>>
>> IIUC vhost_worker_killed() will be called anyway when calling
>> vhost_worker_destroy(). For vhost_tasks, it will call
>> vhost_task_do_stop() that calls vhost_task_stop(). This sets
>> VHOST_TASK_FLAGS_STOP and wait the worker on vtsk->exited before freeing
>> stuff. The worker breaks the loop and calls vtsk->handle_sigkill() that
>> is exactly vhost_worker_killed() you mentioned we are mirroring here.
>>
>
>Hmm, are we sure it's the case for our codepath? Looking at the
>vhost_task loop function:
>
>> static int vhost_task_fn(void *data)
>> {
>> for (;;) {
>> if (signal_pending(current)) {
>> if (get_signal(&ksig))
>> break;
>> }
>> ...
>> if (test_bit(VHOST_TASK_FLAGS_STOP, &vtsk->flags)) {
>> __set_current_state(TASK_RUNNING);
>> break;
>> }
>> did_work = vtsk->fn(vtsk->data);
>> ...
>> }
>>
>> ...
>>
>> if (!test_bit(VHOST_TASK_FLAGS_STOP, &vtsk->flags)) {
>> set_bit(VHOST_TASK_FLAGS_KILLED, &vtsk->flags);
>> vtsk->handle_sigkill(vtsk->data);
>> }
>> ...
>> }
>
>AFAICT, we exit the loop in 2 cases: signal delivery or STOP bit
>setting. Like you said, STOP is set by vhost_task_stop. E.g. for our
>RESET_OWNER case:
>
>vhost_vsock_reset_owner()
> vhost_dev_reset_owner()
> vhost_dev_cleanup()
> vhost_workers_free()
> vhost_worker_destroy()
> vhost_task_stop() // for vhost_task_ops backend
> set_bit(VHOST_TASK_FLAGS_STOP)
>
>So, first of all, actual work by .fn() callback is done after the exit
>checks, therefore we skip it - no chance to drain there.
>
>Secondly, the handle_sigkill() callback is deliberately NOT called in
>the STOP case and only called on fatal signal delivery. And for
>vhost_task backend the .handle_sigkill() callback is exactly
>vhost_worker_killed().
>
>So my understanding is: if we only call synchronize_rcu() here and leave
>this path undrained, then whatever work which was put by send_pkt() for
>the worker currently being freed - will be lost. Please correct me if
>I'm wrong.
Yep, your right. But what will be the issue of loosing them?
IIUC we are not loosing any data, just avoiding some works that will be
handled later when/if will set a new owner.
>
>That said, I agree that vhost_run_work_list() will only work with
>vhost_task backend, not with kthreads backend. If we do
>vhost_worker_flush() instead - I guess it'll keep the drain here, yet
>become backend-agnostic. I.e.:
>
>> + if (!xa_empty(&dev->worker_xa)) {
>> + synchronize_rcu();
>> + xa_for_each(&dev->worker_xa, i, worker)
>> + vhost_worker_flush(worker);
>> + }
>
>With the last 2 lines being equivalent to just calling
>vhost_dev_flush(dev). And once we become backend-agnostic here, I'm
>guessing the warning reported by Sashiko should be dealt with as well.
I'd avoid `if !xa_empty(&dev->worker_xa)` at all, and call
synchronize_rcu() in any case.
About vhost_dev_flush(), we are calling it in several places, and maybe
we should re-check them. E.g. we call in vhost_vsock_flush(), but it's
also called by vhost_dev_stop(), maybe we can avoid to call
vhost_vsock_flush() if we call vhost_dev_stop().
I'm not sure we really need another one here, but if you think some
other works can be queued between the vhost_dev_stop() and the
synchronize_rcu() we are adding here, then okay, it may have sense.
Thanks,
Stefano
^ permalink raw reply
* Re: [PATCH v2 1/4] virtio-mem: validate device-reported block size
From: David Hildenbrand (Arm) @ 2026-07-16 15:59 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Hari Mishal, Michael S . Tsirkin, Jason Wang, Xuan Zhuo,
Eugenio Pérez, virtualization, linux-kernel
In-Reply-To: <2026071635-relive-flogging-2a81@gregkh>
On 7/16/26 16:18, Greg Kroah-Hartman wrote:
> On Thu, Jul 16, 2026 at 10:55:42AM +0200, David Hildenbrand (Arm) wrote:
>> On 7/15/26 18:41, Hari Mishal wrote:
>>> The device_block_size read from the virtio-mem config space is used as a
>>> divisor and also in ALIGN_DOWN() further down the code path in the
>>> driver without further validation. A zero value leads to a division by
>>> zero, and a non-power-of-two value corrupts the ALIGN_DOWN() bitmask
>>> arithmetic leading to a misreporting of guest-usable ram, post crash.
>>>
>>> Reject both at init time instead of trusting the device.
>>>
>>> Signed-off-by: Hari Mishal <harimishal1@gmail.com>
>>> ---
>>> v2: dropped the redundant explicit zero check, since
>>> is_power_of_2(0) already returns false.
>>>
>>> drivers/virtio/virtio_mem.c | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>>
>>> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
>>> index 11c441501582..0e04fec458af 100644
>>> --- a/drivers/virtio/virtio_mem.c
>>> +++ b/drivers/virtio/virtio_mem.c
>>> @@ -2847,6 +2847,12 @@ static int virtio_mem_init(struct virtio_mem *vm)
>>> &vm->plugged_size);
>>> virtio_cread_le(vm->vdev, struct virtio_mem_config, block_size,
>>> &vm->device_block_size);
>>> + if (!is_power_of_2(vm->device_block_size)) {
>>> + dev_err(&vm->vdev->dev,
>>> + "invalid device block size: 0x%llx\n",
>>> + (unsigned long long)vm->device_block_size);
>>> + return -EINVAL;
>>> + }
>>
>> The spec states "The device MUST set block_size to a power of two."
>>
>> I'm missing the point here.
>
> So what happens if we have a non-spec-compliant device? Shouldn't we be
> attempting to verify this before doing something with the data?
The problem I see is that there are plenty of other devices where a
non-compliant device might cause problems.
Like, we request to hotplug some memory block and our device ACKs it, but it
simply didn't do that.
Or we request to hotunplug a memory block and instead it hotplugs some random
other memory block.
Or we sense whether a memory block is plugged and the device lies to us.
>
> Or do we just always trust virtio mem devices explicitly?
It's hard for me to understand where we draw the line, really.
But maybe MST can clarify what we care about in virtio world where the
hypervisor is fully in charge of the device,
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH v4 4/5] vhost: synchronize with RCU readers when freeing workers
From: Andrey Drobyshev @ 2026-07-16 15:39 UTC (permalink / raw)
To: Stefano Garzarella
Cc: linux-kernel, kvm, virtualization, netdev, mst, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den
In-Reply-To: <alibLtcrC7o5r4Dh@sgarzare-redhat>
On 7/16/26 11:57 AM, Stefano Garzarella wrote:
> On Tue, Jul 14, 2026 at 06:16:37PM +0300, Andrey Drobyshev wrote:
>> 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.
>>
>
> Do we need a Fixes tag for this?
>
I'm guessing it should be:
Fixes: 228a27cf78af ("vhost: Allow worker switching while work is queueing")
> Thanks for pointing out that the issue wasn't occurring, but I think we
> should add it because it's a sneaky problem we discovered by chance.
> IMO the code should already have `synchronize_rcu()` after
> `rcu_assign_pointer()` loop.
>
> @Michael, what do you think?
>
>> 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);
>> + }
>> +
>
> Following sashiko review [1], I tried to undersand why we need this, but
> TBH I'm really confused. That said, this seems wrong also because it
> will work only with vhost_tasks, and not with kthreads.
>
> IIUC vhost_worker_killed() will be called anyway when calling
> vhost_worker_destroy(). For vhost_tasks, it will call
> vhost_task_do_stop() that calls vhost_task_stop(). This sets
> VHOST_TASK_FLAGS_STOP and wait the worker on vtsk->exited before freeing
> stuff. The worker breaks the loop and calls vtsk->handle_sigkill() that
> is exactly vhost_worker_killed() you mentioned we are mirroring here.
>
Hmm, are we sure it's the case for our codepath? Looking at the
vhost_task loop function:
> static int vhost_task_fn(void *data)
> {
> for (;;) {
> if (signal_pending(current)) {
> if (get_signal(&ksig))
> break;
> }
> ...
> if (test_bit(VHOST_TASK_FLAGS_STOP, &vtsk->flags)) {
> __set_current_state(TASK_RUNNING);
> break;
> }
> did_work = vtsk->fn(vtsk->data);
> ...
> }
>
> ...
>
> if (!test_bit(VHOST_TASK_FLAGS_STOP, &vtsk->flags)) {
> set_bit(VHOST_TASK_FLAGS_KILLED, &vtsk->flags);
> vtsk->handle_sigkill(vtsk->data);
> }
> ...
> }
AFAICT, we exit the loop in 2 cases: signal delivery or STOP bit
setting. Like you said, STOP is set by vhost_task_stop. E.g. for our
RESET_OWNER case:
vhost_vsock_reset_owner()
vhost_dev_reset_owner()
vhost_dev_cleanup()
vhost_workers_free()
vhost_worker_destroy()
vhost_task_stop() // for vhost_task_ops backend
set_bit(VHOST_TASK_FLAGS_STOP)
So, first of all, actual work by .fn() callback is done after the exit
checks, therefore we skip it - no chance to drain there.
Secondly, the handle_sigkill() callback is deliberately NOT called in
the STOP case and only called on fatal signal delivery. And for
vhost_task backend the .handle_sigkill() callback is exactly
vhost_worker_killed().
So my understanding is: if we only call synchronize_rcu() here and leave
this path undrained, then whatever work which was put by send_pkt() for
the worker currently being freed - will be lost. Please correct me if
I'm wrong.
That said, I agree that vhost_run_work_list() will only work with
vhost_task backend, not with kthreads backend. If we do
vhost_worker_flush() instead - I guess it'll keep the drain here, yet
become backend-agnostic. I.e.:
> + if (!xa_empty(&dev->worker_xa)) {
> + synchronize_rcu();
> + xa_for_each(&dev->worker_xa, i, worker)
> + vhost_worker_flush(worker);
> + }
With the last 2 lines being equivalent to just calling
vhost_dev_flush(dev). And once we become backend-agnostic here, I'm
guessing the warning reported by Sashiko should be dealt with as well.
WDYT?
Andrey
> So, why we need this?
>
> Should be enough to call synchronize_rcu() in any case after the
> rcu_assign_pointer() loop?
>
> Thanks,
> Stefano
>
> [1]
> https://sashiko.dev/#/patchset/20260714151638.143019-1-andrey.drobyshev@virtuozzo.com?part=4
>
^ permalink raw reply
* Re: [PATCH v2 1/4] virtio-mem: validate device-reported block size
From: Greg Kroah-Hartman @ 2026-07-16 14:18 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Hari Mishal, Michael S . Tsirkin, Jason Wang, Xuan Zhuo,
Eugenio Pérez, virtualization, linux-kernel
In-Reply-To: <b2176313-b444-4f66-a71c-647b92cc16a8@kernel.org>
On Thu, Jul 16, 2026 at 10:55:42AM +0200, David Hildenbrand (Arm) wrote:
> On 7/15/26 18:41, Hari Mishal wrote:
> > The device_block_size read from the virtio-mem config space is used as a
> > divisor and also in ALIGN_DOWN() further down the code path in the
> > driver without further validation. A zero value leads to a division by
> > zero, and a non-power-of-two value corrupts the ALIGN_DOWN() bitmask
> > arithmetic leading to a misreporting of guest-usable ram, post crash.
> >
> > Reject both at init time instead of trusting the device.
> >
> > Signed-off-by: Hari Mishal <harimishal1@gmail.com>
> > ---
> > v2: dropped the redundant explicit zero check, since
> > is_power_of_2(0) already returns false.
> >
> > drivers/virtio/virtio_mem.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
> > index 11c441501582..0e04fec458af 100644
> > --- a/drivers/virtio/virtio_mem.c
> > +++ b/drivers/virtio/virtio_mem.c
> > @@ -2847,6 +2847,12 @@ static int virtio_mem_init(struct virtio_mem *vm)
> > &vm->plugged_size);
> > virtio_cread_le(vm->vdev, struct virtio_mem_config, block_size,
> > &vm->device_block_size);
> > + if (!is_power_of_2(vm->device_block_size)) {
> > + dev_err(&vm->vdev->dev,
> > + "invalid device block size: 0x%llx\n",
> > + (unsigned long long)vm->device_block_size);
> > + return -EINVAL;
> > + }
>
> The spec states "The device MUST set block_size to a power of two."
>
> I'm missing the point here.
So what happens if we have a non-spec-compliant device? Shouldn't we be
attempting to verify this before doing something with the data?
Or do we just always trust virtio mem devices explicitly?
thanks,
greg k-h
^ permalink raw reply
* [PATCH] vdpa: Remove redundant dev_err()
From: Pan Chuang @ 2026-07-16 14:13 UTC (permalink / raw)
To: reviewer:MARVELL OCTEON ENDPOINT VIRTIO DATA PATH ACCELE...,commit_signer:2/8=25%,authored:2/8=25%,added_lines:97/119=82%,added_lines:17/119=14%,removed_lines:16/25=64%,removed_lines:3/25=12%,
reviewer:MARVELL OCTEON ENDPOINT VIRTIO DATA PATH ACCELE...,commit_signer:2/8=25%,authored:2/8=25%,added_lines:97/119=82%,added_lines:17/119=14%,removed_lines:16/25=64%,removed_lines:3/25=12%,
Michael S. Tsirkin, Jason Wang, Xuan Zhuo, Eugenio Pérez,
Pan Chuang, Miaoqian Lin, Kees Cook, open list:VIRTIO CORE,
open list
Since commit 55b48e23f5c4 ("genirq/devres: Add error handling in
devm_request_*_irq()"), devm_request_irq() automatically logs
detailed error messages on failure. Remove the now-redundant
driver-specific dev_err() calls.
Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
drivers/vdpa/octeon_ep/octep_vdpa_main.c | 4 +---
drivers/vdpa/virtio_pci/vp_vdpa.c | 12 +++---------
2 files changed, 4 insertions(+), 12 deletions(-)
diff --git a/drivers/vdpa/octeon_ep/octep_vdpa_main.c b/drivers/vdpa/octeon_ep/octep_vdpa_main.c
index 5b35993750f5..6e32925bc57a 100644
--- a/drivers/vdpa/octeon_ep/octep_vdpa_main.c
+++ b/drivers/vdpa/octeon_ep/octep_vdpa_main.c
@@ -170,10 +170,8 @@ static int octep_request_irqs(struct octep_hw *oct_hw, irqreturn_t (*irq_handler
irq = pci_irq_vector(pdev, idx);
ret = devm_request_irq(&pdev->dev, irq, irq_handler, 0, dev_name(&pdev->dev),
oct_hw);
- if (ret) {
- dev_err(&pdev->dev, "Failed to register interrupt handler\n");
+ if (ret)
goto free_irqs;
- }
oct_hw->irqs[idx] = irq;
}
oct_hw->requested_irqs = nb_irqs;
diff --git a/drivers/vdpa/virtio_pci/vp_vdpa.c b/drivers/vdpa/virtio_pci/vp_vdpa.c
index 51ffc245a038..f2eb654b1665 100644
--- a/drivers/vdpa/virtio_pci/vp_vdpa.c
+++ b/drivers/vdpa/virtio_pci/vp_vdpa.c
@@ -189,11 +189,8 @@ static int vp_vdpa_request_irq(struct vp_vdpa *vp_vdpa)
vp_vdpa_vq_handler,
0, vp_vdpa->vring[i].msix_name,
&vp_vdpa->vring[i]);
- if (ret) {
- dev_err(&pdev->dev,
- "vp_vdpa: fail to request irq for vq %d\n", i);
+ if (ret)
goto err;
- }
vp_modern_queue_vector(mdev, i, msix_vec);
vp_vdpa->vring[i].irq = irq;
msix_vec++;
@@ -204,11 +201,8 @@ static int vp_vdpa_request_irq(struct vp_vdpa *vp_vdpa)
irq = pci_irq_vector(pdev, msix_vec);
ret = devm_request_irq(&pdev->dev, irq, vp_vdpa_config_handler, 0,
vp_vdpa->msix_name, vp_vdpa);
- if (ret) {
- dev_err(&pdev->dev,
- "vp_vdpa: fail to request irq for config: %d\n", ret);
- goto err;
- }
+ if (ret)
+ goto err;
vp_modern_config_vector(mdev, msix_vec);
vp_vdpa->config_irq = irq;
--
2.34.1
^ permalink raw reply related
* [PATCH v3] virtio_ring: fix infinite loop in virtnet_poll_cleantx when device is broken
From: Jinqian Yang @ 2026-07-16 11:59 UTC (permalink / raw)
To: mst, jasowang, xuanzhuo, eperezma, andrew+netdev, davem, edumazet,
kuba, pabeni
Cc: netdev, virtualization, linux-kernel, liuyonglong, wangzhou1,
linuxarm, Jinqian Yang
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 vq->broken check in virtqueue_enable_cb_delayed(), 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>
---
Changes in v2:
- Moved vq->broken check to virtqueue_enable_cb_delayed().
Changes in v3:
- Updated the patch subject prefix.
v1: https://lore.kernel.org/lkml/20260713132025.703147-1-yangjinqian1@huawei.com/
v2: https://lore.kernel.org/lkml/20260716035201.3736582-1-yangjinqian1@huawei.com/
---
drivers/virtio/virtio_ring.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index b438dc2ce1b8..5c169fbb418a 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -3233,6 +3233,14 @@ bool virtqueue_enable_cb_delayed(struct virtqueue *_vq)
{
struct vring_virtqueue *vq = to_vvq(_vq);
+ /*
+ * When the device is broken there is no point in polling used->idx,
+ * the backend will never update it. Return true to let callers
+ * exit their cleanup loops instead of spinning forever.
+ */
+ if (unlikely(vq->broken))
+ return true;
+
if (vq->event_triggered)
data_race(vq->event_triggered = false);
--
2.33.0
^ permalink raw reply related
* Re: [PATCH] vhost-vdpa: reject zero-size unmap
From: Eugenio Perez Martin @ 2026-07-16 10:04 UTC (permalink / raw)
To: Weimin Xiong; +Cc: virtualization, mst, jasowangio, netdev, kvm, xiongweimin
In-Reply-To: <20260716030238.124368-1-xiongwm2026@163.com>
On Thu, Jul 16, 2026 at 5:02 AM Weimin Xiong <xiongwm2026@163.com> wrote:
>
> From: xiongweimin <xiongweimin@kylinos.cn>
>
> Reject unmap requests with size == 0 to prevent iova + size - 1
> from underflowing to U64_MAX, which would incorrectly unmap the
> entire IOTLB range.
>
> This fix also covers the error rollback path in vhost_vdpa_va_map:
> when the first VMA lookup fails, map_iova equals iova, resulting
> in a zero-size unmap that would otherwise clear the whole IOTLB.
>
> Signed-off-by: xiongweimin <xiongweimin@kylinos.cn>
Acked-by: Eugenio Pérez <eperezma@redhat.com>
Thanks!
> ---
> drivers/vhost/vdpa.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index bb96b1aa5..f49bf1cfb 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -1035,6 +1035,9 @@ static void vhost_vdpa_unmap(struct vhost_vdpa *v,
> const struct vdpa_config_ops *ops = vdpa->config;
> u32 asid = iotlb_to_asid(iotlb);
>
> + if (!size)
> + return;
> +
> vhost_vdpa_iotlb_unmap(v, iotlb, iova, iova + size - 1, asid);
>
> if (ops->set_map) {
> --
> 2.39.3
>
^ permalink raw reply
* Re: [PATCH] vhost-vdpa: propagate set_map error to caller
From: Eugenio Perez Martin @ 2026-07-16 10:01 UTC (permalink / raw)
To: Weimin Xiong; +Cc: virtualization, mst, jasowangio, netdev, kvm, xiongweimin
In-Reply-To: <20260716030242.124455-1-xiongwm2026@163.com>
On Thu, Jul 16, 2026 at 5:02 AM Weimin Xiong <xiongwm2026@163.com> wrote:
>
> From: xiongweimin <xiongweimin@kylinos.cn>
>
> The return value of ops->set_map() is currently ignored when handling
> VHOST_IOTLB_BATCH_END. If the backend fails to program the IOTLB,
> the VMM incorrectly believes the operation succeeded and may continue
> with stale or incorrect mappings.
>
> Save and propagate the error from ops->set_map() in BATCH_END.
>
> Signed-off-by: xiongweimin <xiongweimin@kylinos.cn>
> ---
> 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 bb96b1aa5..ffbb10a92 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -1297,8 +1297,10 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, u32 asid,
> v->in_batch = true;
> break;
> case VHOST_IOTLB_BATCH_END:
> - if (v->in_batch && ops->set_map)
> - ops->set_map(vdpa, asid, iotlb);
> + if (v->in_batch && ops->set_map) {
> + r = ops->set_map(vdpa, asid, iotlb);
> + break;
> + }
> v->in_batch = false;
Maybe this needs to be discussed, but v->in_batch should be set to
false even in failure case.
> break;
> default:
> --
> 2.39.3
>
^ permalink raw reply
* Re: [PATCH] vhost: reject zero-size IOTLB INVALIDATE
From: Eugenio Perez Martin @ 2026-07-16 9:41 UTC (permalink / raw)
To: Weimin Xiong; +Cc: virtualization, mst, jasowangio, netdev, kvm, xiongweimin
In-Reply-To: <20260716030236.124322-1-xiongwm2026@163.com>
On Thu, Jul 16, 2026 at 5:02 AM Weimin Xiong <xiongwm2026@163.com> wrote:
>
> From: xiongweimin <xiongweimin@kylinos.cn>
>
> Reject VHOST_IOTLB_INVALIDATE messages with size == 0 to prevent
> iova + size - 1 from underflowing to U64_MAX, which would
> incorrectly delete the entire IOTLB.
>
> Signed-off-by: xiongweimin <xiongweimin@kylinos.cn>
> ---
> drivers/vhost/vhost.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 3c080c454e374cabd7321416ed92c5f7d3135254..xxxxxxxxxx 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -1656,6 +1656,10 @@ static int vhost_process_iotlb_msg(struct vhost_dev *dev, u32 asid,
> if (!dev->iotlb) {
> ret = -EFAULT;
> break;
> + }
> + if (!msg->size) {
> + ret = -EINVAL;
> + break;
> }
I think the issue is real, but how about adding the condition to the
caller vhost_chr_write_iter? It is already the
if (msg.type == VHOST_IOTLB_UPDATE && msg.size == 0) {
ret = -EINVAL;
goto done;
}
So it should be somthing in the line of:
if ((msg.type == VHOST_IOTLB_UPDATE || msg.type ==
VHOST_IOTLB_INVALIDATE) && msg.size == 0) {
ret = -EINVAL;
goto done;
}
With that, please add my acked-by.
> vhost_vq_meta_reset(dev);
> vhost_iotlb_del_range(dev->iotlb, msg->iova,
> --
> 2.39.3
>
^ permalink raw reply
* Re: [PATCH v4 4/5] vhost: synchronize with RCU readers when freeing workers
From: Stefano Garzarella @ 2026-07-16 8:57 UTC (permalink / raw)
To: Andrey Drobyshev
Cc: linux-kernel, kvm, virtualization, netdev, mst, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den
In-Reply-To: <20260714151638.143019-5-andrey.drobyshev@virtuozzo.com>
On Tue, Jul 14, 2026 at 06:16:37PM +0300, Andrey Drobyshev wrote:
>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.
>
Do we need a Fixes tag for this?
Thanks for pointing out that the issue wasn't occurring, but I think we
should add it because it's a sneaky problem we discovered by chance.
IMO the code should already have `synchronize_rcu()` after
`rcu_assign_pointer()` loop.
@Michael, what do you think?
>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);
>+ }
>+
Following sashiko review [1], I tried to undersand why we need this, but
TBH I'm really confused. That said, this seems wrong also because it
will work only with vhost_tasks, and not with kthreads.
IIUC vhost_worker_killed() will be called anyway when calling
vhost_worker_destroy(). For vhost_tasks, it will call
vhost_task_do_stop() that calls vhost_task_stop(). This sets
VHOST_TASK_FLAGS_STOP and wait the worker on vtsk->exited before freeing
stuff. The worker breaks the loop and calls vtsk->handle_sigkill() that
is exactly vhost_worker_killed() you mentioned we are mirroring here.
So, why we need this?
Should be enough to call synchronize_rcu() in any case after the
rcu_assign_pointer() loop?
Thanks,
Stefano
[1]
https://sashiko.dev/#/patchset/20260714151638.143019-1-andrey.drobyshev@virtuozzo.com?part=4
^ permalink raw reply
* Re: [PATCH v2 1/4] virtio-mem: validate device-reported block size
From: David Hildenbrand (Arm) @ 2026-07-16 8:55 UTC (permalink / raw)
To: Hari Mishal, Michael S . Tsirkin, Jason Wang
Cc: Greg Kroah-Hartman, Xuan Zhuo, Eugenio Pérez, virtualization,
linux-kernel
In-Reply-To: <20260715164139.40957-1-harimishal1@gmail.com>
On 7/15/26 18:41, Hari Mishal wrote:
> The device_block_size read from the virtio-mem config space is used as a
> divisor and also in ALIGN_DOWN() further down the code path in the
> driver without further validation. A zero value leads to a division by
> zero, and a non-power-of-two value corrupts the ALIGN_DOWN() bitmask
> arithmetic leading to a misreporting of guest-usable ram, post crash.
>
> Reject both at init time instead of trusting the device.
>
> Signed-off-by: Hari Mishal <harimishal1@gmail.com>
> ---
> v2: dropped the redundant explicit zero check, since
> is_power_of_2(0) already returns false.
>
> drivers/virtio/virtio_mem.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
> index 11c441501582..0e04fec458af 100644
> --- a/drivers/virtio/virtio_mem.c
> +++ b/drivers/virtio/virtio_mem.c
> @@ -2847,6 +2847,12 @@ static int virtio_mem_init(struct virtio_mem *vm)
> &vm->plugged_size);
> virtio_cread_le(vm->vdev, struct virtio_mem_config, block_size,
> &vm->device_block_size);
> + if (!is_power_of_2(vm->device_block_size)) {
> + dev_err(&vm->vdev->dev,
> + "invalid device block size: 0x%llx\n",
> + (unsigned long long)vm->device_block_size);
> + return -EINVAL;
> + }
The spec states "The device MUST set block_size to a power of two."
I'm missing the point here.
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH v2] virtio_net: fix infinite loop in virtnet_poll_cleantx when device is broken
From: Jinqian Yang @ 2026-07-16 6:05 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: <20260716011312-mutt-send-email-mst@kernel.org>
Hi,
On 2026/7/16 13:14, Michael S. Tsirkin wrote:
> On Thu, Jul 16, 2026 at 11:52:01AM +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>
>
>
> Good thanks! Just the subject needs change so it's clear
> we are changing virtio core not virtio net.
>
Thanks for the catch. Will change the subject prefix to "virtio_ring:"
and send v3 shortly.
Thanks,
Jinqian
>> ---
>> Changes in v2:
>> - Moved vq->broken check to virtqueue_enable_cb_delayed().
>>
>> v1: https://lore.kernel.org/lkml/20260713132025.703147-1-yangjinqian1@huawei.com/
>> ---
>> drivers/virtio/virtio_ring.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
>> index b438dc2ce1b8..5c169fbb418a 100644
>> --- a/drivers/virtio/virtio_ring.c
>> +++ b/drivers/virtio/virtio_ring.c
>> @@ -3233,6 +3233,14 @@ bool virtqueue_enable_cb_delayed(struct virtqueue *_vq)
>> {
>> struct vring_virtqueue *vq = to_vvq(_vq);
>>
>> + /*
>> + * When the device is broken there is no point in polling used->idx,
>> + * the backend will never update it. Return true to let callers
>> + * exit their cleanup loops instead of spinning forever.
>> + */
>> + if (unlikely(vq->broken))
>> + return true;
>> +
>> if (vq->event_triggered)
>> data_race(vq->event_triggered = false);
>>
>> --
>> 2.33.0
>
>
^ permalink raw reply
* [PATCH v2] vdpa/mlx5: roll back MR update after VQ setup failure
From: Weimin Xiong @ 2026-07-16 5:43 UTC (permalink / raw)
To: Dragos Tatulea
Cc: Michael S . Tsirkin, Jason Wang, Xuan Zhuo, Eugenio Perez,
virtualization, linux-kernel, Weimin Xiong
mlx5_vdpa_change_map() must install the new MR before rebuilding or
resuming virtqueues, because both paths read the MR keys from
mvdev->mres.mr[].
If rebuilding the virtqueue resources fails, the new MR must not remain
installed after its reference is released. Keep an extra reference to
the old MR before replacing it. On setup failure, restore the old MR;
the saved reference then becomes the map reference, while replacing the
new MR drops its map reference.
Make mlx5_vdpa_change_map() consume new_mr on all error paths so that
set_map_data() does not release an MR already released during rollback.
v2:
- Keep the new MR installed while virtqueues are rebuilt.
- Restore the old MR only after setup_vq_resources() fails.
Signed-off-by: Weimin Xiong <xiongwm2026@163.com>
---
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index ad0d5fbbb..25bf1c5cd 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -3055,18 +3055,24 @@ static int mlx5_vdpa_change_map(struct mlx5_vdpa_dev *mvdev,
unsigned int asid)
{
struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
+ struct mlx5_vdpa_mr *old_mr;
bool teardown = !is_resumable(ndev);
int err;
suspend_vqs(ndev, 0, ndev->cur_num_vqs);
if (teardown) {
err = save_channels_info(ndev);
- if (err)
+ if (err) {
+ mlx5_vdpa_put_mr(mvdev, new_mr);
return err;
+ }
teardown_vq_resources(ndev);
}
+ /* Keep the old MR alive in case rebuilding the VQs fails. */
+ old_mr = mvdev->mres.mr[asid];
+ mlx5_vdpa_get_mr(mvdev, old_mr);
mlx5_vdpa_update_mr(mvdev, new_mr, asid);
for (int i = 0; i < mvdev->max_vqs; i++)
@@ -3074,17 +3080,22 @@ static int mlx5_vdpa_change_map(struct mlx5_vdpa_dev *mvdev,
MLX5_VIRTQ_MODIFY_MASK_DESC_GROUP_MKEY;
if (!(mvdev->status & VIRTIO_CONFIG_S_DRIVER_OK) || mvdev->suspended)
- return 0;
+ goto out;
if (teardown) {
restore_channels_info(ndev);
err = setup_vq_resources(ndev, true);
- if (err)
+ if (err) {
+ /* The saved reference becomes the restored map reference. */
+ mlx5_vdpa_update_mr(mvdev, old_mr, asid);
return err;
+ }
}
resume_vqs(ndev, 0, ndev->cur_num_vqs);
+out:
+ mlx5_vdpa_put_mr(mvdev, old_mr);
return 0;
}
@@ -3368,15 +3379,11 @@ static int set_map_data(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb,
err = mlx5_vdpa_change_map(mvdev, new_mr, asid);
if (err) {
mlx5_vdpa_err(mvdev, "change map failed(%d)\n", err);
- goto out_err;
+ return err;
}
}
return mlx5_vdpa_update_cvq_iotlb(mvdev, iotlb, asid);
-
-out_err:
- mlx5_vdpa_put_mr(mvdev, new_mr);
- return err;
}
static int mlx5_vdpa_set_map(struct vdpa_device *vdev, unsigned int asid,
^ permalink raw reply related
* Re: [PATCH v2] virtio_net: fix infinite loop in virtnet_poll_cleantx when device is broken
From: Michael S. Tsirkin @ 2026-07-16 5:14 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: <20260716035201.3736582-1-yangjinqian1@huawei.com>
On Thu, Jul 16, 2026 at 11:52:01AM +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>
Good thanks! Just the subject needs change so it's clear
we are changing virtio core not virtio net.
> ---
> Changes in v2:
> - Moved vq->broken check to virtqueue_enable_cb_delayed().
>
> v1: https://lore.kernel.org/lkml/20260713132025.703147-1-yangjinqian1@huawei.com/
> ---
> drivers/virtio/virtio_ring.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index b438dc2ce1b8..5c169fbb418a 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -3233,6 +3233,14 @@ bool virtqueue_enable_cb_delayed(struct virtqueue *_vq)
> {
> struct vring_virtqueue *vq = to_vvq(_vq);
>
> + /*
> + * When the device is broken there is no point in polling used->idx,
> + * the backend will never update it. Return true to let callers
> + * exit their cleanup loops instead of spinning forever.
> + */
> + if (unlikely(vq->broken))
> + return true;
> +
> if (vq->event_triggered)
> data_race(vq->event_triggered = false);
>
> --
> 2.33.0
^ permalink raw reply
* [PATCH v2] virtio_net: fix infinite loop in virtnet_poll_cleantx when device is broken
From: Jinqian Yang @ 2026-07-16 3:52 UTC (permalink / raw)
To: mst, jasowang, xuanzhuo, eperezma, andrew+netdev, davem, edumazet,
kuba, pabeni
Cc: netdev, virtualization, linux-kernel, liuyonglong, wangzhou1,
linuxarm, Jinqian Yang
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>
---
Changes in v2:
- Moved vq->broken check to virtqueue_enable_cb_delayed().
v1: https://lore.kernel.org/lkml/20260713132025.703147-1-yangjinqian1@huawei.com/
---
drivers/virtio/virtio_ring.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index b438dc2ce1b8..5c169fbb418a 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -3233,6 +3233,14 @@ bool virtqueue_enable_cb_delayed(struct virtqueue *_vq)
{
struct vring_virtqueue *vq = to_vvq(_vq);
+ /*
+ * When the device is broken there is no point in polling used->idx,
+ * the backend will never update it. Return true to let callers
+ * exit their cleanup loops instead of spinning forever.
+ */
+ if (unlikely(vq->broken))
+ return true;
+
if (vq->event_triggered)
data_race(vq->event_triggered = false);
--
2.33.0
^ permalink raw reply related
* [PATCH] vhost-vdpa: propagate set_map error to caller
From: Weimin Xiong @ 2026-07-16 3:02 UTC (permalink / raw)
To: virtualization; +Cc: mst, jasowangio, eperezma, netdev, kvm, xiongweimin
From: xiongweimin <xiongweimin@kylinos.cn>
The return value of ops->set_map() is currently ignored when handling
VHOST_IOTLB_BATCH_END. If the backend fails to program the IOTLB,
the VMM incorrectly believes the operation succeeded and may continue
with stale or incorrect mappings.
Save and propagate the error from ops->set_map() in BATCH_END.
Signed-off-by: xiongweimin <xiongweimin@kylinos.cn>
---
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 bb96b1aa5..ffbb10a92 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -1297,8 +1297,10 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, u32 asid,
v->in_batch = true;
break;
case VHOST_IOTLB_BATCH_END:
- if (v->in_batch && ops->set_map)
- ops->set_map(vdpa, asid, iotlb);
+ if (v->in_batch && ops->set_map) {
+ r = ops->set_map(vdpa, asid, iotlb);
+ break;
+ }
v->in_batch = false;
break;
default:
--
2.39.3
^ permalink raw reply related
* [PATCH] vdpa/mlx5: fix MR state corruption on setup_vq_resources failure
From: Weimin Xiong @ 2026-07-16 3:02 UTC (permalink / raw)
To: virtualization; +Cc: mst, jasowangio, dtatulea, kvm, xiongweimin
From: xiongweimin <xiongweimin@kylinos.cn>
In mlx5_vdpa_change_map(), mlx5_vdpa_update_mr() is called before
setup_vq_resources(), which releases the old MR and updates
mres.mr[asid] to point to new_mr. If setup_vq_resources() then
fails, the function returns an error but the MR pointer is already
updated, leaving mres.mr[asid] pointing to a potentially invalid
state.
Fix by calling setup_vq_resources() before updating the MR pointer.
On failure, the old MR is still intact and can be restored by the
caller.
Signed-off-by: xiongweimin <xiongweimin@kylinos.cn>
---
drivers/vdpa/mlx5/net/mlx5_vnet.c | 24 ++++++++++++++++----------
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index ad0d5fbbb..8a6ddc00b 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -3067,24 +3067,28 @@ static int mlx5_vdpa_change_map(struct mlx5_vdpa_dev *mvdev,
teardown_vq_resources(ndev);
}
- mlx5_vdpa_update_mr(mvdev, new_mr, asid);
-
for (int i = 0; i < mvdev->max_vqs; i++)
ndev->vqs[i].modified_fields |= MLX5_VIRTQ_MODIFY_MASK_VIRTIO_Q_MKEY |
MLX5_VIRTQ_MODIFY_MASK_DESC_GROUP_MKEY;
if (!(mvdev->status & VIRTIO_CONFIG_S_DRIVER_OK) || mvdev->suspended)
- return 0;
+ goto done;
if (teardown) {
restore_channels_info(ndev);
err = setup_vq_resources(ndev, true);
if (err)
- return err;
+ goto out_err;
}
resume_vqs(ndev, 0, ndev->cur_num_vqs);
+ goto done;
+out_err:
+ return err;
+
+done:
+ mlx5_vdpa_update_mr(mvdev, new_mr, asid);
return 0;
}
--
2.39.3
^ permalink raw reply related
* [PATCH] vhost-vdpa: reject zero-size unmap
From: Weimin Xiong @ 2026-07-16 3:02 UTC (permalink / raw)
To: virtualization; +Cc: mst, jasowangio, eperezma, netdev, kvm, xiongweimin
From: xiongweimin <xiongweimin@kylinos.cn>
Reject unmap requests with size == 0 to prevent iova + size - 1
from underflowing to U64_MAX, which would incorrectly unmap the
entire IOTLB range.
This fix also covers the error rollback path in vhost_vdpa_va_map:
when the first VMA lookup fails, map_iova equals iova, resulting
in a zero-size unmap that would otherwise clear the whole IOTLB.
Signed-off-by: xiongweimin <xiongweimin@kylinos.cn>
---
drivers/vhost/vdpa.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index bb96b1aa5..f49bf1cfb 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -1035,6 +1035,9 @@ static void vhost_vdpa_unmap(struct vhost_vdpa *v,
const struct vdpa_config_ops *ops = vdpa->config;
u32 asid = iotlb_to_asid(iotlb);
+ if (!size)
+ return;
+
vhost_vdpa_iotlb_unmap(v, iotlb, iova, iova + size - 1, asid);
if (ops->set_map) {
--
2.39.3
^ permalink raw reply related
* [PATCH] vhost: reject zero-size IOTLB INVALIDATE
From: Weimin Xiong @ 2026-07-16 3:02 UTC (permalink / raw)
To: virtualization; +Cc: mst, jasowangio, eperezma, netdev, kvm, xiongweimin
From: xiongweimin <xiongweimin@kylinos.cn>
Reject VHOST_IOTLB_INVALIDATE messages with size == 0 to prevent
iova + size - 1 from underflowing to U64_MAX, which would
incorrectly delete the entire IOTLB.
Signed-off-by: xiongweimin <xiongweimin@kylinos.cn>
---
drivers/vhost/vhost.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 3c080c454e374cabd7321416ed92c5f7d3135254..xxxxxxxxxx 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1656,6 +1656,10 @@ static int vhost_process_iotlb_msg(struct vhost_dev *dev, u32 asid,
if (!dev->iotlb) {
ret = -EFAULT;
break;
+ }
+ if (!msg->size) {
+ ret = -EINVAL;
+ break;
}
vhost_vq_meta_reset(dev);
vhost_iotlb_del_range(dev->iotlb, msg->iova,
--
2.39.3
^ permalink raw reply
* [PATCH] vhost-vdpa: propagate set_map error to caller
From: Weimin Xiong @ 2026-07-16 3:00 UTC (permalink / raw)
To: virtualization; +Cc: mst, jasowangio, eperezma, netdev, xiongweimin
From: xiongweimin <xiongweimin@kylinos.cn>
The return value of ops->set_map() is currently ignored when handling
VHOST_IOTLB_BATCH_END. If the backend fails to program the IOTLB,
the VMM incorrectly believes the operation succeeded and may continue
with stale or incorrect mappings.
Save and propagate the error from ops->set_map() in BATCH_END.
Signed-off-by: xiongweimin <xiongweimin@kylinos.cn>
---
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 bb96b1aa5..ffbb10a92 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -1297,8 +1297,10 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, u32 asid,
v->in_batch = true;
break;
case VHOST_IOTLB_BATCH_END:
- if (v->in_batch && ops->set_map)
- ops->set_map(vdpa, asid, iotlb);
+ if (v->in_batch && ops->set_map) {
+ r = ops->set_map(vdpa, asid, iotlb);
+ break;
+ }
v->in_batch = false;
break;
default:
--
2.39.3
^ permalink raw reply related
* [PATCH] vdpa/mlx5: fix MR state corruption on setup_vq_resources failure
From: Weimin Xiong @ 2026-07-16 3:00 UTC (permalink / raw)
To: virtualization; +Cc: mst, jasowangio, dtatulea, xiongweimin
From: xiongweimin <xiongweimin@kylinos.cn>
In mlx5_vdpa_change_map(), mlx5_vdpa_update_mr() is called before
setup_vq_resources(), which releases the old MR and updates
mres.mr[asid] to point to new_mr. If setup_vq_resources() then
fails, the function returns an error but the MR pointer is already
updated, leaving mres.mr[asid] pointing to a potentially invalid
state.
Fix by calling setup_vq_resources() before updating the MR pointer.
On failure, the old MR is still intact and can be restored by the
caller.
Signed-off-by: xiongweimin <xiongweimin@kylinos.cn>
---
drivers/vdpa/mlx5/net/mlx5_vnet.c | 24 ++++++++++++++++----------
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index ad0d5fbbb..8a6ddc00b 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -3067,24 +3067,28 @@ static int mlx5_vdpa_change_map(struct mlx5_vdpa_dev *mvdev,
teardown_vq_resources(ndev);
}
- mlx5_vdpa_update_mr(mvdev, new_mr, asid);
-
for (int i = 0; i < mvdev->max_vqs; i++)
ndev->vqs[i].modified_fields |= MLX5_VIRTQ_MODIFY_MASK_VIRTIO_Q_MKEY |
MLX5_VIRTQ_MODIFY_MASK_DESC_GROUP_MKEY;
if (!(mvdev->status & VIRTIO_CONFIG_S_DRIVER_OK) || mvdev->suspended)
- return 0;
+ goto done;
if (teardown) {
restore_channels_info(ndev);
err = setup_vq_resources(ndev, true);
if (err)
- return err;
+ goto out_err;
}
resume_vqs(ndev, 0, ndev->cur_num_vqs);
+ goto done;
+out_err:
+ return err;
+
+done:
+ mlx5_vdpa_update_mr(mvdev, new_mr, asid);
return 0;
}
--
2.39.3
^ permalink raw reply related
* [PATCH] vhost-vdpa: reject zero-size unmap
From: Weimin Xiong @ 2026-07-16 3:00 UTC (permalink / raw)
To: virtualization; +Cc: mst, jasowangio, eperezma, netdev, xiongweimin
From: xiongweimin <xiongweimin@kylinos.cn>
Reject unmap requests with size == 0 to prevent iova + size - 1
from underflowing to U64_MAX, which would incorrectly unmap the
entire IOTLB range.
This fix also covers the error rollback path in vhost_vdpa_va_map:
when the first VMA lookup fails, map_iova equals iova, resulting
in a zero-size unmap that would otherwise clear the whole IOTLB.
Signed-off-by: xiongweimin <xiongweimin@kylinos.cn>
---
drivers/vhost/vdpa.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index bb96b1aa5..f49bf1cfb 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -1035,6 +1035,9 @@ static void vhost_vdpa_unmap(struct vhost_vdpa *v,
const struct vdpa_config_ops *ops = vdpa->config;
u32 asid = iotlb_to_asid(iotlb);
+ if (!size)
+ return;
+
vhost_vdpa_iotlb_unmap(v, iotlb, iova, iova + size - 1, asid);
if (ops->set_map) {
--
2.39.3
^ permalink raw reply related
* [PATCH] vhost: reject zero-size IOTLB INVALIDATE
From: Weimin Xiong @ 2026-07-16 3:00 UTC (permalink / raw)
To: virtualization; +Cc: mst, jasowangio, eperezma, netdev, xiongweimin
From: xiongweimin <xiongweimin@kylinos.cn>
Reject VHOST_IOTLB_INVALIDATE messages with size == 0 to prevent
iova + size - 1 from underflowing to U64_MAX, which would
incorrectly delete the entire IOTLB.
Signed-off-by: xiongweimin <xiongweimin@kylinos.cn>
---
drivers/vhost/vhost.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 3c080c454e374cabd7321416ed92c5f7d3135254..xxxxxxxxxx 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1656,6 +1656,10 @@ static int vhost_process_iotlb_msg(struct vhost_dev *dev, u32 asid,
if (!dev->iotlb) {
ret = -EFAULT;
break;
+ }
+ if (!msg->size) {
+ ret = -EINVAL;
+ break;
}
vhost_vq_meta_reset(dev);
vhost_iotlb_del_range(dev->iotlb, msg->iova,
--
2.39.3
^ 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