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
