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

Pre-existing issues:
- [High] Missing validation for `msg.size == 0` during `VHOST_IOTLB_INVALIDATE` 
leads to integer underflow and unintended interval tree queries.
--

commit b0f07892733bab641caa79a8f76f81c291b22034
Author: Linfeng Sun <[email protected]>

vhost: reject invalid IOTLB update permissions

This commit adds validation for the permission field of IOTLB update
messages in vhost_chr_write_iter(). It rejects messages with empty or
invalid permissions to prevent warnings when these are dispatched to the
vDPA backend.

> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 44cac11b68d2a..0adf0fea97b41 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -1750,7 +1750,8 @@ ssize_t vhost_chr_write_iter(struct vhost_dev *dev,
>               goto done;
>       }
>  
> -     if (msg.type == VHOST_IOTLB_UPDATE && msg.size == 0) {
> +     if (msg.type == VHOST_IOTLB_UPDATE &&
> +         (!msg.size || !msg.perm || (msg.perm & ~VHOST_ACCESS_RW))) {
>               ret = -EINVAL;
>               goto done;
>       }

[Severity: High]
This is a pre-existing issue, but since we are validating message sizes
here, is there a missing size == 0 check for VHOST_IOTLB_INVALIDATE messages
that can lead to an integer underflow?

If a userspace process sends a VHOST_IOTLB_INVALIDATE message with size == 0,
the generic vhost_process_iotlb_msg() handler explicitly checks for this and
returns -EINVAL. However, the custom vDPA handler does not:

drivers/vhost/vdpa.c:vhost_vdpa_process_iotlb_msg() {
    ...
        case VHOST_IOTLB_INVALIDATE:
                vhost_vdpa_unmap(v, iotlb, msg->iova, msg->size);
                break;
    ...
}

This gets passed to vhost_vdpa_unmap() which performs this calculation:

drivers/vhost/vdpa.c:vhost_vdpa_unmap() {
    ...
        vhost_vdpa_iotlb_unmap(v, iotlb, iova, iova + size - 1, asid);
    ...
}

Does this calculation underflow when size is 0? 

If iova == 0, the last index (iova + size - 1) would wrap to ULLONG_MAX,
silently unmapping the entire IOTLB. If iova > 0, it creates an inverted
interval where start > last. The interval tree incorrectly handles this by
finding and unmapping any valid intervals that straddle both start and last,
corrupting the device's internal tree representation.

Should we add a similar size == 0 check for VHOST_IOTLB_INVALIDATE either
here in vhost_chr_write_iter() or directly inside
vhost_vdpa_process_iotlb_msg()?

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to