Skip to content

Commit cf7c278

Browse files
nicolincjgunthorpe
authored andcommitted
iommufd: Fix protection fault in iommufd_test_syz_conv_iova
Syzkaller reported the following bug: general protection fault, probably for non-canonical address 0xdffffc0000000038: 0000 [#1] SMP KASAN KASAN: null-ptr-deref in range [0x00000000000001c0-0x00000000000001c7] Call Trace: lock_acquire lock_acquire+0x1ce/0x4f0 down_read+0x93/0x4a0 iommufd_test_syz_conv_iova+0x56/0x1f0 iommufd_test_access_rw.isra.0+0x2ec/0x390 iommufd_test+0x1058/0x1e30 iommufd_fops_ioctl+0x381/0x510 vfs_ioctl __do_sys_ioctl __se_sys_ioctl __x64_sys_ioctl+0x170/0x1e0 do_syscall_x64 do_syscall_64+0x71/0x140 This is because the new iommufd_access_change_ioas() sets access->ioas to NULL during its process, so the lock might be gone in a concurrent racing context. Fix this by doing the same access->ioas sanity as iommufd_access_rw() and iommufd_access_pin_pages() functions do. Cc: stable@vger.kernel.org Fixes: 9227da7 ("iommufd: Add iommufd_access_change_ioas(_id) helpers") Link: https://lore.kernel.org/r/3f1932acaf1dd494d404c04364d73ce8f57f3e5e.1708636627.git.nicolinc@nvidia.com Reported-by: Jason Gunthorpe <jgg@nvidia.com> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
1 parent fde372d commit cf7c278

File tree

1 file changed

+21
-6
lines changed

1 file changed

+21
-6
lines changed

drivers/iommu/iommufd/selftest.c

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -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

@@ -1161,7 +1176,7 @@ static int iommufd_test_access_pages(struct iommufd_ucmd *ucmd,
11611176
}
11621177

11631178
if (flags & MOCK_FLAGS_ACCESS_SYZ)
1164-
iova = iommufd_test_syz_conv_iova(&staccess->access->ioas->iopt,
1179+
iova = iommufd_test_syz_conv_iova(staccess->access,
11651180
&cmd->access_pages.iova);
11661181

11671182
npages = (ALIGN(iova + length, PAGE_SIZE) -
@@ -1263,8 +1278,8 @@ static int iommufd_test_access_rw(struct iommufd_ucmd *ucmd,
12631278
}
12641279

12651280
if (flags & MOCK_FLAGS_ACCESS_SYZ)
1266-
iova = iommufd_test_syz_conv_iova(&staccess->access->ioas->iopt,
1267-
&cmd->access_rw.iova);
1281+
iova = iommufd_test_syz_conv_iova(staccess->access,
1282+
&cmd->access_rw.iova);
12681283

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

0 commit comments

Comments
 (0)