On 23/10/2023 17:34, Jason Gunthorpe wrote:
> On Mon, Oct 23, 2023 at 05:31:22PM +0100, Joao Martins wrote:
>>> Write it like this:
>>>
>>> int iommufd_check_iova_range(struct iommufd_ioas *ioas,
>>> struct iommu_hwpt_get_dirty_bitmap *bitmap)
>>> {
>>> size_t iommu_pgsize = ioas->iopt.iova_alignment;
>>> u64 last_iova;
>>>
>>> if (check_add_overflow(bitmap->iova, bitmap->length - 1, &last_iova))
>>> return -EOVERFLOW;
>>>
>>> if (bitmap->iova > ULONG_MAX || last_iova > ULONG_MAX)
>>> return -EOVERFLOW;
>>>
>>> if ((bitmap->iova & (iommu_pgsize - 1)) ||
>>> ((last_iova + 1) & (iommu_pgsize - 1)))
>>> return -EINVAL;
>>> return 0;
>>> }
>>>
>>> And if 0 should really be rejected then check iova == last_iova
>>
>> It should; Perhaps extending the above and replicate that second the ::page_size
>> alignment check is important as it's what's used by the bitmap e.g.
>
> That makes sense, much clearer what it is trying to do that way
In regards to clearity, I am still checking if bitmap::page_size being 0 or not,
to avoid being so implicitly in the last check i.e. (bitmap->iova &
(bitmap->page_size - 1)) being an and to 0xfffffffff.
