Re: [PATCH v5 07/18] iommufd: Add IOMMU_HWPT_GET_DIRTY_BITMAP

[Jason Gunthorpe <jgg@nvidia.com>]

On Mon, Oct 23, 2023 at 04:56:01PM +0100, Joao Martins wrote:
> On 23/10/2023 13:41, Arnd Bergmann wrote:
> > On Mon, Oct 23, 2023, at 14:10, Jason Gunthorpe wrote:
> >> On Mon, Oct 23, 2023 at 10:28:13AM +0100, Joao Martins wrote:
> >>>> so it's probably
> >>>> best to add a range check plus type cast, rather than an
> >>>> expensive div_u64() here.
> >>>
> >>> OK
> >>
> >> Just keep it simple, we don't need to optimize for 32 bit. div_u64
> >> will make the compiler happy.
> > 
> > Fair enough. FWIW, I tried adding just the range check to see
> > if that would make the compiler turn it into a 32-bit division,
> > but that didn't work.
> > 
> > Some type of range check might still be good to have for
> > unrelated reasons.
> 
> I can reproduce the arm32 build problem and I'm applying this diff below to this
> patch to fix it. It essentially moves all the checks to
> iommufd_check_iova_range(), including range-check and adding div_u64.
> 
> Additionally, perhaps should also move the iommufd_check_iova_range() invocation
> via io_pagetable.c code rather than hw-pagetable code? It seems to make more
> sense as there's nothing hw-pagetable specific that needs to be in here.

Don't you need the IOAS though?

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

Jason