qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH Kernel v18 4/7] vfio iommu: Implementation of ioctl for dirty


From: Kirti Wankhede
Subject: Re: [PATCH Kernel v18 4/7] vfio iommu: Implementation of ioctl for dirty pages tracking.
Date: Thu, 14 May 2020 01:56:33 +0530
User-agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0



On 5/6/2020 4:24 PM, Cornelia Huck wrote:
On Mon, 4 May 2020 21:28:56 +0530
Kirti Wankhede <address@hidden> wrote:

VFIO_IOMMU_DIRTY_PAGES ioctl performs three operations:
- Start dirty pages tracking while migration is active
- Stop dirty pages tracking.
- Get dirty pages bitmap. Its user space application's responsibility to
   copy content of dirty pages from source to destination during migration.

To prevent DoS attack, memory for bitmap is allocated per vfio_dma
structure. Bitmap size is calculated considering smallest supported page
size. Bitmap is allocated for all vfio_dmas when dirty logging is enabled

Bitmap is populated for already pinned pages when bitmap is allocated for
a vfio_dma with the smallest supported page size. Update bitmap from
pinning functions when tracking is enabled. When user application queries
bitmap, check if requested page size is same as page size used to
populated bitmap. If it is equal, copy bitmap, but if not equal, return
error.

Fixed below error by changing pgsize type from uint64_t to size_t.
Reported-by: kbuild test robot <address@hidden>

All errors:
drivers/vfio/vfio_iommu_type1.c:197: undefined reference to `__udivdi3'

drivers/vfio/vfio_iommu_type1.c:225: undefined reference to `__udivdi3'

Move that below the '---' delimiter so that it does not end up in the
commit? (Crediting the build bot is fine, but the details are not
really useful when you look at the code later.)


ok, removing errors.


Signed-off-by: Kirti Wankhede <address@hidden>
Reviewed-by: Neo Jia <address@hidden>
---
  drivers/vfio/vfio_iommu_type1.c | 266 +++++++++++++++++++++++++++++++++++++++-
  1 file changed, 260 insertions(+), 6 deletions(-)

@@ -2278,6 +2435,93 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
return copy_to_user((void __user *)arg, &unmap, minsz) ?
                        -EFAULT : 0;
+       } else if (cmd == VFIO_IOMMU_DIRTY_PAGES) {
+               struct vfio_iommu_type1_dirty_bitmap dirty;
+               uint32_t mask = VFIO_IOMMU_DIRTY_PAGES_FLAG_START |
+                               VFIO_IOMMU_DIRTY_PAGES_FLAG_STOP |
+                               VFIO_IOMMU_DIRTY_PAGES_FLAG_GET_BITMAP;
+               int ret = 0;
+
+               if (!iommu->v2)
+                       return -EACCES;
+
+               minsz = offsetofend(struct vfio_iommu_type1_dirty_bitmap,
+                                   flags);
+
+               if (copy_from_user(&dirty, (void __user *)arg, minsz))
+                       return -EFAULT;
+
+               if (dirty.argsz < minsz || dirty.flags & ~mask)
+                       return -EINVAL;
+
+               /* only one flag should be set at a time */
+               if (__ffs(dirty.flags) != __fls(dirty.flags))
+                       return -EINVAL;
+

Shouldn't you also check whether the flag that is set is actually
valid? (maybe dirty.flags & ~VFIO_IOMMU_DIRTY_PAGES_FLAG_MASK and do a
switch/case over dirty.flags & VFIO_IOMMU_DIRTY_PAGES_FLAG_MASK)


There is a check above this check, dirty.flags & ~mask, which makes sure that flag is valid.

Thanks,
Kirti




reply via email to

[Prev in Thread] Current Thread [Next in Thread]