On Tue, Jul 13, 2021 at 04:46:52PM +0800, Xie Yongji wrote:
> @@ -613,37 +618,28 @@ static void vhost_vdpa_unmap(struct vhost_vdpa *v, u64 iova, u64 size)
> }
> }
>
> -static int vhost_vdpa_process_iotlb_update(struct vhost_vdpa *v,
> - struct vhost_iotlb_msg *msg)
> +static int vhost_vdpa_pa_map(struct vhost_vdpa *v,
> + u64 iova, u64 size, u64 uaddr, u32 perm)
> {
> struct vhost_dev *dev = &v->vdev;
> - struct vhost_iotlb *iotlb = dev->iotlb;
> struct page **page_list;
> unsigned long list_size = PAGE_SIZE / sizeof(struct page *);
> unsigned int gup_flags = FOLL_LONGTERM;
> unsigned long npages, cur_base, map_pfn, last_pfn = 0;
> unsigned long lock_limit, sz2pin, nchunks, i;
> - u64 iova = msg->iova;
> + u64 start = iova;
> long pinned;
> int ret = 0;
>
> - if (msg->iova < v->range.first ||
> - msg->iova + msg->size - 1 > v->range.last)
> - return -EINVAL;
This is not related to your patch, but can the "msg->iova + msg->size"
addition can have an integer overflow. From looking at the callers it
seems like it can. msg comes from:
vhost_chr_write_iter()
--> dev->msg_handler(dev, &msg);
--> vhost_vdpa_process_iotlb_msg()
--> vhost_vdpa_process_iotlb_update()
If I'm thinking of the right thing then these are allowed to overflow to
0 because of the " - 1" but not further than that. I believe the check
needs to be something like:
if (msg->iova < v->range.first ||
msg->iova - 1 > U64_MAX - msg->size ||
msg->iova + msg->size - 1 > v->range.last)
But writing integer overflow check correctly is notoriously difficult.
Do you think you could send a fix for that which is separate from the
patcheset? We'd want to backport it to stable.
regards,
dan carpenter
