Skip to content

Commit bee0e77

Browse files
torvaldstorvalds
committed
Merge tag 'for-linus-iommufd' of git://git.kernel.org/pub/scm/linux/kernel/git/jgg/iommufd
Pull iommufd fixes from Jason Gunthorpe: - A small fix for the dirty tracking self test to fail correctly if the code is buggy - Fix a tricky syzkaller race UAF with object reference counting * tag 'for-linus-iommufd' of git://git.kernel.org/pub/scm/linux/kernel/git/jgg/iommufd: iommufd: Do not UAF during iommufd_put_object() iommufd: Add iommufd_ctx to iommufd_put_object() iommufd/selftest: Fix _test_mock_dirty_bitmaps()
2 parents 1e53574 + 6f9c4d8 commit bee0e77

File tree

8 files changed

+177
-120
lines changed

8 files changed

+177
-120
lines changed

‎drivers/iommu/iommufd/device.c

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,7 @@ iommufd_device_auto_get_domain(struct iommufd_device *idev,
571571
continue;
572572
destroy_hwpt = (*do_attach)(idev, hwpt);
573573
if (IS_ERR(destroy_hwpt)) {
574-
iommufd_put_object(&hwpt->obj);
574+
iommufd_put_object(idev->ictx, &hwpt->obj);
575575
/*
576576
* -EINVAL means the domain is incompatible with the
577577
* device. Other error codes should propagate to
@@ -583,7 +583,7 @@ iommufd_device_auto_get_domain(struct iommufd_device *idev,
583583
goto out_unlock;
584584
}
585585
*pt_id = hwpt->obj.id;
586-
iommufd_put_object(&hwpt->obj);
586+
iommufd_put_object(idev->ictx, &hwpt->obj);
587587
goto out_unlock;
588588
}
589589

@@ -652,15 +652,15 @@ static int iommufd_device_change_pt(struct iommufd_device *idev, u32 *pt_id,
652652
destroy_hwpt = ERR_PTR(-EINVAL);
653653
goto out_put_pt_obj;
654654
}
655-
iommufd_put_object(pt_obj);
655+
iommufd_put_object(idev->ictx, pt_obj);
656656

657657
/* This destruction has to be after we unlock everything */
658658
if (destroy_hwpt)
659659
iommufd_hw_pagetable_put(idev->ictx, destroy_hwpt);
660660
return 0;
661661

662662
out_put_pt_obj:
663-
iommufd_put_object(pt_obj);
663+
iommufd_put_object(idev->ictx, pt_obj);
664664
return PTR_ERR(destroy_hwpt);
665665
}
666666

@@ -792,7 +792,7 @@ static int iommufd_access_change_ioas_id(struct iommufd_access *access, u32 id)
792792
if (IS_ERR(ioas))
793793
return PTR_ERR(ioas);
794794
rc = iommufd_access_change_ioas(access, ioas);
795-
iommufd_put_object(&ioas->obj);
795+
iommufd_put_object(access->ictx, &ioas->obj);
796796
return rc;
797797
}
798798

@@ -941,7 +941,7 @@ void iommufd_access_notify_unmap(struct io_pagetable *iopt, unsigned long iova,
941941

942942
access->ops->unmap(access->data, iova, length);
943943

944-
iommufd_put_object(&access->obj);
944+
iommufd_put_object(access->ictx, &access->obj);
945945
xa_lock(&ioas->iopt.access_list);
946946
}
947947
xa_unlock(&ioas->iopt.access_list);
@@ -1243,6 +1243,6 @@ int iommufd_get_hw_info(struct iommufd_ucmd *ucmd)
12431243
out_free:
12441244
kfree(data);
12451245
out_put:
1246-
iommufd_put_object(&idev->obj);
1246+
iommufd_put_object(ucmd->ictx, &idev->obj);
12471247
return rc;
12481248
}

‎drivers/iommu/iommufd/hw_pagetable.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -318,9 +318,9 @@ int iommufd_hwpt_alloc(struct iommufd_ucmd *ucmd)
318318
if (ioas)
319319
mutex_unlock(&ioas->mutex);
320320
out_put_pt:
321-
iommufd_put_object(pt_obj);
321+
iommufd_put_object(ucmd->ictx, pt_obj);
322322
out_put_idev:
323-
iommufd_put_object(&idev->obj);
323+
iommufd_put_object(ucmd->ictx, &idev->obj);
324324
return rc;
325325
}
326326

@@ -345,7 +345,7 @@ int iommufd_hwpt_set_dirty_tracking(struct iommufd_ucmd *ucmd)
345345
rc = iopt_set_dirty_tracking(&ioas->iopt, hwpt_paging->common.domain,
346346
enable);
347347

348-
iommufd_put_object(&hwpt_paging->common.obj);
348+
iommufd_put_object(ucmd->ictx, &hwpt_paging->common.obj);
349349
return rc;
350350
}
351351

@@ -368,6 +368,6 @@ int iommufd_hwpt_get_dirty_bitmap(struct iommufd_ucmd *ucmd)
368368
rc = iopt_read_and_clear_dirty_data(
369369
&ioas->iopt, hwpt_paging->common.domain, cmd->flags, cmd);
370370

371-
iommufd_put_object(&hwpt_paging->common.obj);
371+
iommufd_put_object(ucmd->ictx, &hwpt_paging->common.obj);
372372
return rc;
373373
}

‎drivers/iommu/iommufd/ioas.c

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ int iommufd_ioas_iova_ranges(struct iommufd_ucmd *ucmd)
105105
rc = -EMSGSIZE;
106106
out_put:
107107
up_read(&ioas->iopt.iova_rwsem);
108-
iommufd_put_object(&ioas->obj);
108+
iommufd_put_object(ucmd->ictx, &ioas->obj);
109109
return rc;
110110
}
111111

@@ -175,7 +175,7 @@ int iommufd_ioas_allow_iovas(struct iommufd_ucmd *ucmd)
175175
interval_tree_remove(node, &allowed_iova);
176176
kfree(container_of(node, struct iopt_allowed, node));
177177
}
178-
iommufd_put_object(&ioas->obj);
178+
iommufd_put_object(ucmd->ictx, &ioas->obj);
179179
return rc;
180180
}
181181

@@ -228,7 +228,7 @@ int iommufd_ioas_map(struct iommufd_ucmd *ucmd)
228228
cmd->iova = iova;
229229
rc = iommufd_ucmd_respond(ucmd, sizeof(*cmd));
230230
out_put:
231-
iommufd_put_object(&ioas->obj);
231+
iommufd_put_object(ucmd->ictx, &ioas->obj);
232232
return rc;
233233
}
234234

@@ -258,7 +258,7 @@ int iommufd_ioas_copy(struct iommufd_ucmd *ucmd)
258258
return PTR_ERR(src_ioas);
259259
rc = iopt_get_pages(&src_ioas->iopt, cmd->src_iova, cmd->length,
260260
&pages_list);
261-
iommufd_put_object(&src_ioas->obj);
261+
iommufd_put_object(ucmd->ictx, &src_ioas->obj);
262262
if (rc)
263263
return rc;
264264

@@ -279,7 +279,7 @@ int iommufd_ioas_copy(struct iommufd_ucmd *ucmd)
279279
cmd->dst_iova = iova;
280280
rc = iommufd_ucmd_respond(ucmd, sizeof(*cmd));
281281
out_put_dst:
282-
iommufd_put_object(&dst_ioas->obj);
282+
iommufd_put_object(ucmd->ictx, &dst_ioas->obj);
283283
out_pages:
284284
iopt_free_pages_list(&pages_list);
285285
return rc;
@@ -315,7 +315,7 @@ int iommufd_ioas_unmap(struct iommufd_ucmd *ucmd)
315315
rc = iommufd_ucmd_respond(ucmd, sizeof(*cmd));
316316

317317
out_put:
318-
iommufd_put_object(&ioas->obj);
318+
iommufd_put_object(ucmd->ictx, &ioas->obj);
319319
return rc;
320320
}
321321

@@ -393,6 +393,6 @@ int iommufd_ioas_option(struct iommufd_ucmd *ucmd)
393393
rc = -EOPNOTSUPP;
394394
}
395395

396-
iommufd_put_object(&ioas->obj);
396+
iommufd_put_object(ucmd->ictx, &ioas->obj);
397397
return rc;
398398
}

‎drivers/iommu/iommufd/iommufd_private.h

Lines changed: 57 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ struct iommufd_ctx {
2121
struct file *file;
2222
struct xarray objects;
2323
struct xarray groups;
24+
wait_queue_head_t destroy_wait;
2425

2526
u8 account_mode;
2627
/* Compatibility with VFIO no iommu */
@@ -135,47 +136,90 @@ enum iommufd_object_type {
135136

136137
/* Base struct for all objects with a userspace ID handle. */
137138
struct iommufd_object {
138-
struct rw_semaphore destroy_rwsem;
139+
refcount_t shortterm_users;
139140
refcount_t users;
140141
enum iommufd_object_type type;
141142
unsigned int id;
142143
};
143144

144145
static inline bool iommufd_lock_obj(struct iommufd_object *obj)
145146
{
146-
if (!down_read_trylock(&obj->destroy_rwsem))
147+
if (!refcount_inc_not_zero(&obj->users))
147148
return false;
148-
if (!refcount_inc_not_zero(&obj->users)) {
149-
up_read(&obj->destroy_rwsem);
149+
if (!refcount_inc_not_zero(&obj->shortterm_users)) {
150+
/*
151+
* If the caller doesn't already have a ref on obj this must be
152+
* called under the xa_lock. Otherwise the caller is holding a
153+
* ref on users. Thus it cannot be one before this decrement.
154+
*/
155+
refcount_dec(&obj->users);
150156
return false;
151157
}
152158
return true;
153159
}
154160

155161
struct iommufd_object *iommufd_get_object(struct iommufd_ctx *ictx, u32 id,
156162
enum iommufd_object_type type);
157-
static inline void iommufd_put_object(struct iommufd_object *obj)
163+
static inline void iommufd_put_object(struct iommufd_ctx *ictx,
164+
struct iommufd_object *obj)
158165
{
166+
/*
167+
* Users first, then shortterm so that REMOVE_WAIT_SHORTTERM never sees
168+
* a spurious !0 users with a 0 shortterm_users.
169+
*/
159170
refcount_dec(&obj->users);
160-
up_read(&obj->destroy_rwsem);
171+
if (refcount_dec_and_test(&obj->shortterm_users))
172+
wake_up_interruptible_all(&ictx->destroy_wait);
161173
}
162174

163175
void iommufd_object_abort(struct iommufd_ctx *ictx, struct iommufd_object *obj);
164176
void iommufd_object_abort_and_destroy(struct iommufd_ctx *ictx,
165177
struct iommufd_object *obj);
166178
void iommufd_object_finalize(struct iommufd_ctx *ictx,
167179
struct iommufd_object *obj);
168-
void __iommufd_object_destroy_user(struct iommufd_ctx *ictx,
169-
struct iommufd_object *obj, bool allow_fail);
180+
181+
enum {
182+
REMOVE_WAIT_SHORTTERM = 1,
183+
};
184+
int iommufd_object_remove(struct iommufd_ctx *ictx,
185+
struct iommufd_object *to_destroy, u32 id,
186+
unsigned int flags);
187+
188+
/*
189+
* The caller holds a users refcount and wants to destroy the object. At this
190+
* point the caller has no shortterm_users reference and at least the xarray
191+
* will be holding one.
192+
*/
170193
static inline void iommufd_object_destroy_user(struct iommufd_ctx *ictx,
171194
struct iommufd_object *obj)
172195
{
173-
__iommufd_object_destroy_user(ictx, obj, false);
196+
int ret;
197+
198+
ret = iommufd_object_remove(ictx, obj, obj->id, REMOVE_WAIT_SHORTTERM);
199+
200+
/*
201+
* If there is a bug and we couldn't destroy the object then we did put
202+
* back the caller's users refcount and will eventually try to free it
203+
* again during close.
204+
*/
205+
WARN_ON(ret);
174206
}
175-
static inline void iommufd_object_deref_user(struct iommufd_ctx *ictx,
176-
struct iommufd_object *obj)
207+
208+
/*
209+
* The HWPT allocated by autodomains is used in possibly many devices and
210+
* is automatically destroyed when its refcount reaches zero.
211+
*
212+
* If userspace uses the HWPT manually, even for a short term, then it will
213+
* disrupt this refcounting and the auto-free in the kernel will not work.
214+
* Userspace that tries to use the automatically allocated HWPT must be careful
215+
* to ensure that it is consistently destroyed, eg by not racing accesses
216+
* and by not attaching an automatic HWPT to a device manually.
217+
*/
218+
static inline void
219+
iommufd_object_put_and_try_destroy(struct iommufd_ctx *ictx,
220+
struct iommufd_object *obj)
177221
{
178-
__iommufd_object_destroy_user(ictx, obj, true);
222+
iommufd_object_remove(ictx, obj, obj->id, 0);
179223
}
180224

181225
struct iommufd_object *_iommufd_object_alloc(struct iommufd_ctx *ictx,
@@ -311,7 +355,7 @@ static inline void iommufd_hw_pagetable_put(struct iommufd_ctx *ictx,
311355
lockdep_assert_not_held(&hwpt_paging->ioas->mutex);
312356

313357
if (hwpt_paging->auto_domain) {
314-
iommufd_object_deref_user(ictx, &hwpt->obj);
358+
iommufd_object_put_and_try_destroy(ictx, &hwpt->obj);
315359
return;
316360
}
317361
}

0 commit comments

Comments
 (0)