Skip to content

Commit e613c90

Browse files
committed
Merge tag 'for-linus-iommufd' of git://git.kernel.org/pub/scm/linux/kernel/git/jgg/iommufd
Pull iommufd fixes from Jason Gunthorpe: "Four syzkaller found bugs: - Corruption during error unwind in iommufd_access_change_ioas() - Overlapping IDs in the test suite due to out of order destruction - Missing locking for access->ioas in the test suite - False failures in the test suite validation logic with huge pages" * tag 'for-linus-iommufd' of git://git.kernel.org/pub/scm/linux/kernel/git/jgg/iommufd: iommufd/selftest: Don't check map/unmap pairing with HUGE_PAGES iommufd: Fix protection fault in iommufd_test_syz_conv_iova iommufd/selftest: Fix mock_dev_num bug iommufd: Fix iopt_access_list_id overwrite bug
2 parents fb54efc + bb04d13 commit e613c90

File tree

2 files changed

+54
-24
lines changed

2 files changed

+54
-24
lines changed

drivers/iommu/iommufd/io_pagetable.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1330,20 +1330,23 @@ int iopt_disable_large_pages(struct io_pagetable *iopt)
13301330

13311331
int iopt_add_access(struct io_pagetable *iopt, struct iommufd_access *access)
13321332
{
1333+
u32 new_id;
13331334
int rc;
13341335

13351336
down_write(&iopt->domains_rwsem);
13361337
down_write(&iopt->iova_rwsem);
1337-
rc = xa_alloc(&iopt->access_list, &access->iopt_access_list_id, access,
1338-
xa_limit_16b, GFP_KERNEL_ACCOUNT);
1338+
rc = xa_alloc(&iopt->access_list, &new_id, access, xa_limit_16b,
1339+
GFP_KERNEL_ACCOUNT);
1340+
13391341
if (rc)
13401342
goto out_unlock;
13411343

13421344
rc = iopt_calculate_iova_alignment(iopt);
13431345
if (rc) {
1344-
xa_erase(&iopt->access_list, access->iopt_access_list_id);
1346+
xa_erase(&iopt->access_list, new_id);
13451347
goto out_unlock;
13461348
}
1349+
access->iopt_access_list_id = new_id;
13471350

13481351
out_unlock:
13491352
up_write(&iopt->iova_rwsem);

drivers/iommu/iommufd/selftest.c

Lines changed: 48 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ static struct mock_bus_type iommufd_mock_bus_type = {
3636
},
3737
};
3838

39-
static atomic_t mock_dev_num;
39+
static DEFINE_IDA(mock_dev_ida);
4040

4141
enum {
4242
MOCK_DIRTY_TRACK = 1,
@@ -63,8 +63,8 @@ enum {
6363
* In syzkaller mode the 64 bit IOVA is converted into an nth area and offset
6464
* value. This has a much smaller randomization space and syzkaller can hit it.
6565
*/
66-
static unsigned long iommufd_test_syz_conv_iova(struct io_pagetable *iopt,
67-
u64 *iova)
66+
static unsigned long __iommufd_test_syz_conv_iova(struct io_pagetable *iopt,
67+
u64 *iova)
6868
{
6969
struct syz_layout {
7070
__u32 nth_area;
@@ -88,6 +88,21 @@ static unsigned long iommufd_test_syz_conv_iova(struct io_pagetable *iopt,
8888
return 0;
8989
}
9090

91+
static unsigned long iommufd_test_syz_conv_iova(struct iommufd_access *access,
92+
u64 *iova)
93+
{
94+
unsigned long ret;
95+
96+
mutex_lock(&access->ioas_lock);
97+
if (!access->ioas) {
98+
mutex_unlock(&access->ioas_lock);
99+
return 0;
100+
}
101+
ret = __iommufd_test_syz_conv_iova(&access->ioas->iopt, iova);
102+
mutex_unlock(&access->ioas_lock);
103+
return ret;
104+
}
105+
91106
void iommufd_test_syz_conv_iova_id(struct iommufd_ucmd *ucmd,
92107
unsigned int ioas_id, u64 *iova, u32 *flags)
93108
{
@@ -100,7 +115,7 @@ void iommufd_test_syz_conv_iova_id(struct iommufd_ucmd *ucmd,
100115
ioas = iommufd_get_ioas(ucmd->ictx, ioas_id);
101116
if (IS_ERR(ioas))
102117
return;
103-
*iova = iommufd_test_syz_conv_iova(&ioas->iopt, iova);
118+
*iova = __iommufd_test_syz_conv_iova(&ioas->iopt, iova);
104119
iommufd_put_object(ucmd->ictx, &ioas->obj);
105120
}
106121

@@ -123,6 +138,7 @@ enum selftest_obj_type {
123138
struct mock_dev {
124139
struct device dev;
125140
unsigned long flags;
141+
int id;
126142
};
127143

128144
struct selftest_obj {
@@ -430,20 +446,27 @@ static size_t mock_domain_unmap_pages(struct iommu_domain *domain,
430446

431447
/*
432448
* iommufd generates unmaps that must be a strict
433-
* superset of the map's performend So every starting
434-
* IOVA should have been an iova passed to map, and the
449+
* superset of the map's performend So every
450+
* starting/ending IOVA should have been an iova passed
451+
* to map.
435452
*
436-
* First IOVA must be present and have been a first IOVA
437-
* passed to map_pages
453+
* This simple logic doesn't work when the HUGE_PAGE is
454+
* turned on since the core code will automatically
455+
* switch between the two page sizes creating a break in
456+
* the unmap calls. The break can land in the middle of
457+
* contiguous IOVA.
438458
*/
439-
if (first) {
440-
WARN_ON(ent && !(xa_to_value(ent) &
441-
MOCK_PFN_START_IOVA));
442-
first = false;
459+
if (!(domain->pgsize_bitmap & MOCK_HUGE_PAGE_SIZE)) {
460+
if (first) {
461+
WARN_ON(ent && !(xa_to_value(ent) &
462+
MOCK_PFN_START_IOVA));
463+
first = false;
464+
}
465+
if (pgcount == 1 &&
466+
cur + MOCK_IO_PAGE_SIZE == pgsize)
467+
WARN_ON(ent && !(xa_to_value(ent) &
468+
MOCK_PFN_LAST_IOVA));
443469
}
444-
if (pgcount == 1 && cur + MOCK_IO_PAGE_SIZE == pgsize)
445-
WARN_ON(ent && !(xa_to_value(ent) &
446-
MOCK_PFN_LAST_IOVA));
447470

448471
iova += MOCK_IO_PAGE_SIZE;
449472
ret += MOCK_IO_PAGE_SIZE;
@@ -631,7 +654,7 @@ static void mock_dev_release(struct device *dev)
631654
{
632655
struct mock_dev *mdev = container_of(dev, struct mock_dev, dev);
633656

634-
atomic_dec(&mock_dev_num);
657+
ida_free(&mock_dev_ida, mdev->id);
635658
kfree(mdev);
636659
}
637660

@@ -653,8 +676,12 @@ static struct mock_dev *mock_dev_create(unsigned long dev_flags)
653676
mdev->dev.release = mock_dev_release;
654677
mdev->dev.bus = &iommufd_mock_bus_type.bus;
655678

656-
rc = dev_set_name(&mdev->dev, "iommufd_mock%u",
657-
atomic_inc_return(&mock_dev_num));
679+
rc = ida_alloc(&mock_dev_ida, GFP_KERNEL);
680+
if (rc < 0)
681+
goto err_put;
682+
mdev->id = rc;
683+
684+
rc = dev_set_name(&mdev->dev, "iommufd_mock%u", mdev->id);
658685
if (rc)
659686
goto err_put;
660687

@@ -1156,7 +1183,7 @@ static int iommufd_test_access_pages(struct iommufd_ucmd *ucmd,
11561183
}
11571184

11581185
if (flags & MOCK_FLAGS_ACCESS_SYZ)
1159-
iova = iommufd_test_syz_conv_iova(&staccess->access->ioas->iopt,
1186+
iova = iommufd_test_syz_conv_iova(staccess->access,
11601187
&cmd->access_pages.iova);
11611188

11621189
npages = (ALIGN(iova + length, PAGE_SIZE) -
@@ -1258,8 +1285,8 @@ static int iommufd_test_access_rw(struct iommufd_ucmd *ucmd,
12581285
}
12591286

12601287
if (flags & MOCK_FLAGS_ACCESS_SYZ)
1261-
iova = iommufd_test_syz_conv_iova(&staccess->access->ioas->iopt,
1262-
&cmd->access_rw.iova);
1288+
iova = iommufd_test_syz_conv_iova(staccess->access,
1289+
&cmd->access_rw.iova);
12631290

12641291
rc = iommufd_access_rw(staccess->access, iova, tmp, length, flags);
12651292
if (rc)

0 commit comments

Comments
 (0)