Kernel KVM virtualization development
 help / color / mirror / Atom feed
* [PATCH] vhost-vdpa: reject zero-size unmap
@ 2026-07-16  3:02 Weimin Xiong
  2026-07-16 10:04 ` Eugenio Perez Martin
  2026-07-17  3:03 ` sashiko-bot
  0 siblings, 2 replies; 3+ messages in thread
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	[flat|nested] 3+ messages in thread

* Re: [PATCH] vhost-vdpa: reject zero-size unmap
  2026-07-16  3:02 [PATCH] vhost-vdpa: reject zero-size unmap Weimin Xiong
@ 2026-07-16 10:04 ` Eugenio Perez Martin
  2026-07-17  3:03 ` sashiko-bot
  1 sibling, 0 replies; 3+ messages in thread
From: Eugenio Perez Martin @ 2026-07-16 10:04 UTC (permalink / raw)
  To: Weimin Xiong; +Cc: virtualization, mst, jasowangio, netdev, kvm, xiongweimin

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	[flat|nested] 3+ messages in thread

* Re: [PATCH] vhost-vdpa: reject zero-size unmap
  2026-07-16  3:02 [PATCH] vhost-vdpa: reject zero-size unmap Weimin Xiong
  2026-07-16 10:04 ` Eugenio Perez Martin
@ 2026-07-17  3:03 ` sashiko-bot
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-07-17  3:03 UTC (permalink / raw)
  To: Weimin Xiong; +Cc: kvm

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [High] Pinned page leak in vhost_vdpa_pa_map() error paths due to incorrect nchunks reset.
- [High] Missing vma->vm_start boundary check in vhost_vdpa_va_map() causes integer underflow for map offset calculation.
- [Medium] The core vhost ioctl handler for VHOST_IOTLB_INVALIDATE does not reject zero-size requests, leading to integer underflow.
- [Medium] vhost_vdpa_va_map silently skips VMAs that do not meet its backing criteria instead of returning an error, leaving holes in the mapping.
--

--- Patch [1]: [PATCH] vhost-vdpa: reject zero-size unmap ---
commit d3e42a0021551a09934ef567e35c3c64cdff596d
Author: xiongweimin <xiongweimin@kylinos.cn>
Subject: vhost-vdpa: reject zero-size unmap

This patch rejects unmap requests with a size of zero in vhost_vdpa_unmap.
This prevents an integer underflow that would cause the entire IOTLB range
to be incorrectly unmapped.

> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index ac55275fa0d0a..f1724bfdc46fb 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -1032,6 +1032,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);

[Severity: Medium]
This isn't a bug introduced by this patch, but while reviewing this 
zero-size fix, I noticed a similar pattern in the core vhost module.

Does vhost_process_iotlb_msg() suffer from the exact same flaw for
VHOST_IOTLB_INVALIDATE?

drivers/vhost/vhost.c:vhost_process_iotlb_msg()
    case VHOST_IOTLB_INVALIDATE:
        if (!dev->iotlb) {
            ret = -EFAULT;
            break;
        }
        vhost_vq_meta_reset(dev);
        vhost_iotlb_del_range(dev->iotlb, msg->iova,
                      msg->iova + msg->size - 1);
        break;

If userspace sends VHOST_IOTLB_INVALIDATE with size == 0, would this
similarly underflow and wipe out the entire IOTLB tree?

>  	if (ops->set_map) {
>  		if (!v->in_batch)
>  			ops->set_map(vdpa, asid, iotlb);

[Severity: High]
This is a pre-existing issue, but looking at the error rollback path in 
vhost_vdpa_va_map() mentioned in the commit message, is there a missing 
boundary check on vm_start?

drivers/vhost/vdpa.c:vhost_vdpa_va_map()
    vma = find_vma(dev->mm, uaddr);
    if (!vma) {
        ret = -EINVAL;
        break;
    }
    map_size = min(size, vma->vm_end - uaddr);

find_vma() returns the first VMA where vm_end > uaddr, which doesn't 
guarantee that vm_start <= uaddr. If uaddr falls in an unmapped hole 
before the VMA, what happens to the offset calculation later in the 
function?

drivers/vhost/vdpa.c:vhost_vdpa_va_map()
    offset = (vma->vm_pgoff << PAGE_SHIFT) + uaddr - vma->vm_start;
    map_file->offset = offset;

Could subtracting vm_start from uaddr result in an integer underflow 
and produce an incorrect offset when uaddr < vm_start?


[Severity: Medium]
This is another pre-existing issue in the same function. Could 
vhost_vdpa_va_map() be silently dropping unsupported VMAs without 
returning an error?

drivers/vhost/vdpa.c:vhost_vdpa_va_map()
    if (!(vma->vm_file && (vma->vm_flags & VM_SHARED) &&
        !(vma->vm_flags & (VM_IO | VM_PFNMAP))))
        goto next;

When a VMA doesn't meet the backing criteria, the code skips the mapping 
logic and advances the pointers:

drivers/vhost/vdpa.c:vhost_vdpa_va_map()
next:
    size -= map_size;
    uaddr += map_size;
    map_iova += map_size;

Because the size decreases and the loop continues, the function can 
eventually return 0 (success). Will this give userspace a false guarantee 
that the entire requested IOVA range was mapped, leading to unexpected 
faults during DMA?


[Severity: High]
This is also a pre-existing issue, but I noticed a potential resource 
leak in the error paths for vhost_vdpa_pa_map().

When a discontiguous page triggers a successful map, nchunks is reset to 
0 and a new pending chunk begins at this_pfn:

drivers/vhost/vdpa.c:vhost_vdpa_pa_map()
            map_pfn = this_pfn;
            iova += csize;
            nchunks = 0;
        }

If there are remaining pages in the current batch, they are added to the 
pending chunk. If the next batch fails to pin, the code jumps to out. 
Because nchunks is only incremented after a successful pin, it remains 0:

drivers/vhost/vdpa.c:vhost_vdpa_pa_map()
out:
    if (ret) {
        if (nchunks) {
            unsigned long pfn;

            /*
             * Unpin the outstanding pages ...
             */
            WARN_ON(!last_pfn);
            for (pfn = map_pfn; pfn <= last_pfn; pfn++)
                unpin_user_page(pfn_to_page(pfn));

Since the if (nchunks) check evaluates to false, does this bypass the 
unpin loop entirely and permanently leak the pages in the pending chunk?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716030238.124368-1-xiongwm2026@163.com?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-17  3:03 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-16  3:02 [PATCH] vhost-vdpa: reject zero-size unmap Weimin Xiong
2026-07-16 10:04 ` Eugenio Perez Martin
2026-07-17  3:03 ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox