On Thu, Mar 24, 2022 at 03:09:46AM +0000, Tian, Kevin wrote:
> +             /*
> +              * Can't cross areas that are not aligned to the system page
> +              * size with this API.
> +              */
> +             if (cur_iova % PAGE_SIZE) {
> +                     rc = -EINVAL;
> +                     goto out_remove;
> +             }
> 
> Currently it's done after iopt_pages_add_user() but before cur_iova 
> is adjusted, which implies the last add_user() will not be reverted in
> case of failed check here.

Oh, yes that's right too..

The above is wrong even, it didn't get fixed when page_offset was
done.

So more like this:

diff --git a/drivers/iommu/iommufd/io_pagetable.c 
b/drivers/iommu/iommufd/io_pagetable.c
index 1c08ae9b848fcf..9505f119df982e 100644
--- a/drivers/iommu/iommufd/io_pagetable.c
+++ b/drivers/iommu/iommufd/io_pagetable.c
@@ -23,7 +23,7 @@ static unsigned long iopt_area_iova_to_index(struct iopt_area 
*area,
        if (IS_ENABLED(CONFIG_IOMMUFD_TEST))
                WARN_ON(iova < iopt_area_iova(area) ||
                        iova > iopt_area_last_iova(area));
-       return (iova - (iopt_area_iova(area) & PAGE_MASK)) / PAGE_SIZE;
+       return (iova - (iopt_area_iova(area) - area->page_offset)) / PAGE_SIZE;
 }
 
 static struct iopt_area *iopt_find_exact_area(struct io_pagetable *iopt,
@@ -436,31 +436,45 @@ int iopt_access_pages(struct io_pagetable *iopt, unsigned 
long iova,
                unsigned long index;
 
                /* Need contiguous areas in the access */
-               if (iopt_area_iova(area) < cur_iova || !area->pages) {
+               if (iopt_area_iova(area) > cur_iova || !area->pages) {
                        rc = -EINVAL;
                        goto out_remove;
                }
 
                index = iopt_area_iova_to_index(area, cur_iova);
                last_index = iopt_area_iova_to_index(area, last);
+
+               /*
+                * The API can only return aligned pages, so the starting point
+                * must be at a page boundary.
+                */
+               if ((cur_iova - (iopt_area_iova(area) - area->page_offset)) %
+                   PAGE_SIZE) {
+                       rc = -EINVAL;
+                       goto out_remove;
+               }
+
+               /*
+                * and an interior ending point must be at a page boundary
+                */
+               if (last != last_iova &&
+                   (iopt_area_last_iova(area) - cur_iova + 1) % PAGE_SIZE) {
+                       rc = -EINVAL;
+                       goto out_remove;
+               }
+
                rc = iopt_pages_add_user(area->pages, index, last_index,
                                         out_pages, write);
                if (rc)
                        goto out_remove;
                if (last == last_iova)
                        break;
-               /*
-                * Can't cross areas that are not aligned to the system page
-                * size with this API.
-                */
-               if (cur_iova % PAGE_SIZE) {
-                       rc = -EINVAL;
-                       goto out_remove;
-               }
                cur_iova = last + 1;
                out_pages += last_index - index;
                atomic_inc(&area->num_users);
        }
+       if (cur_iova != last_iova)
+               goto out_remove;
 
        up_read(&iopt->iova_rwsem);
        return 0;
diff --git a/tools/testing/selftests/iommu/iommufd.c 
b/tools/testing/selftests/iommu/iommufd.c
index 5c47d706ed9449..a46e0f0ae82553 100644
--- a/tools/testing/selftests/iommu/iommufd.c
+++ b/tools/testing/selftests/iommu/iommufd.c
@@ -1221,5 +1221,6 @@ TEST_F(vfio_compat_mock_domain, get_info)
 /* FIXME test VFIO_IOMMU_MAP_DMA */
 /* FIXME test VFIO_IOMMU_UNMAP_DMA */
 /* FIXME test 2k iova alignment */
+/* FIXME cover boundary cases for iopt_access_pages()  */
 
 TEST_HARNESS_MAIN
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to