* [PATCH] vhost-vdpa: reject zero-size unmap
@ 2026-07-16 3:00 Weimin Xiong
0 siblings, 0 replies; 4+ messages in thread
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 [flat|nested] 4+ messages in thread
* [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; 4+ 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] 4+ 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; 4+ 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] 4+ 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; 4+ 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] 4+ messages in thread
end of thread, other threads:[~2026-07-17 3:03 UTC | newest]
Thread overview: 4+ 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
-- strict thread matches above, loose matches on Subject: below --
2026-07-16 3:00 Weimin Xiong
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.