On Mon, Oct 23, 2023 at 09:15:32PM +0100, Joao Martins wrote:
> External email: Use caution opening links or attachments
>
>
> On 23/10/2023 21:08, Nicolin Chen wrote:
> > On Fri, Oct 20, 2023 at 11:28:02PM +0100, Joao Martins wrote:
> >
> >> +static int iommufd_test_dirty(struct iommufd_ucmd *ucmd,
> >> + unsigned int mockpt_id, unsigned long iova,
> >> + size_t length, unsigned long page_size,
> >> + void __user *uptr, u32 flags)
> >> +{
> >> + unsigned long i, max = length / page_size;
> >> + struct iommu_test_cmd *cmd = ucmd->cmd;
> >> + struct iommufd_hw_pagetable *hwpt;
> >> + struct mock_iommu_domain *mock;
> >> + int rc, count = 0;
> >> +
> >> + if (iova % page_size || length % page_size ||
> >> + (uintptr_t)uptr % page_size)
> >> + return -EINVAL;
> >> +
> >> + hwpt = get_md_pagetable(ucmd, mockpt_id, &mock);
> >> + if (IS_ERR(hwpt))
> >> + return PTR_ERR(hwpt);
> >> +
> >> + if (!(mock->flags & MOCK_DIRTY_TRACK)) {
> >> + rc = -EINVAL;
> >> + goto out_put;
> >> + }
> >> +
> >> + for (i = 0; i < max; i++) {
> >> + unsigned long cur = iova + i * page_size;
> >> + void *ent, *old;
> >> +
> >> + if (!test_bit(i, (unsigned long *) uptr))
> >> + continue;
> >
> > Is it okay to test_bit on a user pointer/page? Should we call
> > get_user_pages or so?
> >
> Arggh, let me fix that.
>
> This is where it is failing the selftest for you?
>
> If so, I should paste a snippet for you to test.
Yea, the crash seems to be caused by this. Possibly some memory
debugging feature that I turned on caught this?
I tried a test fix and the crash is gone (attaching at EOM).
However, I still see other failures:
# # RUN iommufd_dirty_tracking.domain_dirty128M.get_dirty_bitmap ...
# # iommufd_utils.h:292:get_dirty_bitmap:Expected nr (32768) == out_dirty (13648)
# # get_dirty_bitmap: Test terminated by assertion
# # FAIL iommufd_dirty_tracking.domain_dirty128M.get_dirty_bitmap
# not ok 147 iommufd_dirty_tracking.domain_dirty128M.get_dirty_bitmap
# # RUN iommufd_dirty_tracking.domain_dirty256M.enforce_dirty ...
# # OK iommufd_dirty_tracking.domain_dirty256M.enforce_dirty
# ok 148 iommufd_dirty_tracking.domain_dirty256M.enforce_dirty
# # RUN iommufd_dirty_tracking.domain_dirty256M.set_dirty_tracking ...
# # OK iommufd_dirty_tracking.domain_dirty256M.set_dirty_tracking
# ok 149 iommufd_dirty_tracking.domain_dirty256M.set_dirty_tracking
# # RUN iommufd_dirty_tracking.domain_dirty256M.device_dirty_capability ...
# # OK iommufd_dirty_tracking.domain_dirty256M.device_dirty_capability
# ok 150 iommufd_dirty_tracking.domain_dirty256M.device_dirty_capability
# # RUN iommufd_dirty_tracking.domain_dirty256M.get_dirty_bitmap ...
# # iommufd_utils.h:292:get_dirty_bitmap:Expected nr (65536) == out_dirty (8923)
Maybe page_size isn't the right size?
-------------attaching copy_from_user------------
diff --git a/drivers/iommu/iommufd/selftest.c b/drivers/iommu/iommufd/selftest.c
index 8a2c7df85441..daa198809d61 100644
--- a/drivers/iommu/iommufd/selftest.c
+++ b/drivers/iommu/iommufd/selftest.c
@@ -1103,8 +1103,9 @@ static int iommufd_test_dirty(struct iommufd_ucmd *ucmd, unsigned int mockpt_id,
struct iommufd_hw_pagetable *hwpt;
struct mock_iommu_domain *mock;
int rc, count = 0;
+ void *tmp;
- if (iova % page_size || length % page_size ||
+ if (iova % page_size || length % page_size || !uptr ||
(uintptr_t)uptr % page_size)
return -EINVAL;
@@ -1117,11 +1118,22 @@ static int iommufd_test_dirty(struct iommufd_ucmd *ucmd, unsigned int mockpt_id,
goto out_put;
}
+ tmp = kvzalloc(page_size, GFP_KERNEL_ACCOUNT);
+ if (!tmp) {
+ rc = -ENOMEM;
+ goto out_put;
+ }
+
for (i = 0; i < max; i++) {
unsigned long cur = iova + i * page_size;
void *ent, *old;
- if (!test_bit(i, (unsigned long *)uptr))
+ if (copy_from_user(tmp, uptr, page_size)) {
+ rc = -EFAULT;
+ goto out_free;
+ }
+
+ if (!test_bit(i, (unsigned long *)tmp))
continue;
ent = xa_load(&mock->pfns, cur / page_size);
@@ -1138,6 +1150,8 @@ static int iommufd_test_dirty(struct iommufd_ucmd *ucmd, unsigned int mockpt_id,
cmd->dirty.out_nr_dirty = count;
rc = iommufd_ucmd_respond(ucmd, sizeof(*cmd));
+out_free:
+ kvfree(tmp);
out_put:
iommufd_put_object(&hwpt->obj);
return rc;
