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
