* Re: iommu/vt-d: Use-after-free on sva unbind device with pending IOPFs
2025-07-11 13:03 iommu/vt-d: Use-after-free on sva unbind device with pending IOPFs Ethan MILON
@ 2025-07-14 7:20 ` Baolu Lu
0 siblings, 0 replies; 2+ messages in thread
From: Baolu Lu @ 2025-07-14 7:20 UTC (permalink / raw)
To: Ethan MILON, iommu@lists.linux.dev
Cc: CLEMENT MATHIEU--DRIF, Zhenzhong Duan, Kevin Tian, Yi Liu
On 7/11/25 21:03, Ethan MILON wrote:
> Hi,
>
> While testing SVA in QEMU with Intel IOMMU, I encountered a double free in
> the kernel when unbinding a device with page requests still in flight. This
> results in a use-after-free error and eventually a kernel panic:
>
> [ 48.227596] refcount_t: underflow; use-after-free.
> [ 48.227949] WARNING: CPU: 3 PID: 313 at lib/refcount.c:28 refcount_warn_saturate+0xd8/0xe0
> [ 48.228531] Modules linked in: sva(OE) xfs(E) sr_mod(E) sd_mod(E) cdrom(E) sg(E) ahci(E) libahci(E) libata(E) e1000(E) dm_mod(E)
> [ 48.229460] CPU: 3 UID: 0 PID: 313 Comm: kworker/u16:4 Kdump: loaded Tainted: G OE 6.16.0-rc5+ #59 PREEMPT(voluntary)
> [ 48.230231] Tainted: [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
> [ 48.230568] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
> [ 48.231278] Workqueue: iopf_queue/dmar0-iopfq iommu_sva_handle_iopf
> [ 48.231697] RIP: 0010:refcount_warn_saturate+0xd8/0xe0
> [ 48.232028] Code: ff 48 c7 c7 58 eb 2e 82 c6 05 40 2f 35 01 01 e8 ae e8 ab ff 0f 0b c3 48 c7 c7 00 eb 2e 82 c6 05 2c 2f 35 01 01 e8 98 e8 ab ff <0f> 0b c3 0f 1f 44 00 003
> [ 48.233202] RSP: 0018:ffffc900008afe50 EFLAGS: 00010286
> [ 48.233541] RAX: 0000000000000000 RBX: ffff888113dccb40 RCX: 0000000000000000
> [ 48.234007] RDX: ffff888861daa480 RSI: 0000000000000001 RDI: ffff888861d9c140
> [ 48.234461] RBP: ffff8881118e0000 R08: 0000000000000000 R09: ffffc900008afcf8
> [ 48.234942] R10: ffffc900008afcf0 R11: ffffffff829dfde8 R12: ffff888100f5c000
> [ 48.235396] R13: ffff888100051c00 R14: ffff888100f5c005 R15: ffff88810c480000
> [ 48.235860] FS: 0000000000000000(0000) GS:ffff8888dea4f000(0000) knlGS:0000000000000000
> [ 48.236381] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 48.236763] CR2: 0000558328aa6468 CR3: 0000000111332002 CR4: 0000000000770ef0
> [ 48.237222] PKRU: 55555554
> [ 48.237397] Call Trace:
> [ 48.237561] <TASK>
> [ 48.237727] iopf_free_group+0xe/0x20
> [ 48.237978] process_one_work+0x197/0x3d0
> [ 48.238239] worker_thread+0x23a/0x350
> [ 48.238487] ? rescuer_thread+0x4a0/0x4a0
> [ 48.238760] kthread+0xf8/0x230
> [ 48.238972] ? finish_task_switch.isra.0+0x81/0x260
> [ 48.239290] ? kthreads_online_cpu+0x110/0x110
> [ 48.239598] ? kthreads_online_cpu+0x110/0x110
> [ 48.239899] ret_from_fork+0x13b/0x170
> [ 48.240155] ? kthreads_online_cpu+0x110/0x110
> [ 48.240434] ret_from_fork_asm+0x11/0x20
> [ 48.240711] </TASK>
> [ 48.240864] ---[ end trace 0000000000000000 ]---
>
> The trace show that it's caused by a double free of the same iopf group :
>
> irq/24-d-66 2..... : iommu_report_device_fault <-prq_event_thread
> irq/24-d-66 2..... : iopf_get_dev_fault_param.isra.0 <-iommu_report_device_fault
> irq/24-d-66 2..... : iopf_group_alloc <-iommu_report_device_fault
> irq/24-d-66 2..... : iommu_sva_iopf_handler <-iommu_report_device_fault
> test-1633 0..... : iommu_sva_unbind_device <-test_handle.isra.0
> test-1633 0..... : iommu_detach_device_pasid <-iommu_sva_unbind_device
> test-1633 0..... : iopf_queue_remove_device <-blocking_domain_set_dev_pasid
> test-1633 0..... : iopf_free_group <-iopf_queue_remove_device
> test-1633 0..... : iopf_put_dev_fault_param <-iopf_free_group
> test-1633 0..... : iopf_put_dev_fault_param <-iopf_queue_remove_device
> test-1633 0..... : iommu_domain_free <-iommu_sva_unbind_device
> kworker/-313 3..... : iommu_sva_handle_iopf <-process_one_work
> kworker/-313 3..... : iopf_group_response <-iommu_sva_handle_iopf
> kworker/-313 3..... : iopf_free_group <-process_one_work
> kworker/-313 3..... : iopf_put_dev_fault_param <-iopf_free_group
>
> Unbinding a device while page requests are still in flight is likely an edge
> case that shouldn't occur, but investigating it has led me to two questions:
>
> 1. The issue appear at the commit 17fce9d2336d ("iommu/vt-d: Put iopf
> enablement in domain attach path"), where device are now dynamically added and
> removed from the iopf queue. This commit add a call to the new
> iopf_for_domain_remove() function when removing pasid :
>
>> @@ -4041,6 +4053,7 @@ static int blocking_domain_set_dev_pasid(struct iommu_domain *domain,
>> {
>> struct device_domain_info *info = dev_iommu_priv_get(dev);
>>
>> + iopf_for_domain_remove(old, dev);
>> intel_pasid_tear_down_entry(info->iommu, dev, pasid, false);
>> domain_remove_dev_pasid(old, dev, pasid);
>>
>> return 0;
>> }
>
> Moving iopf_for_domain_remove() after domain_remove_dev_pasid() fix the issue
> above.
This probably is the right fix.
The iommu driver must ensure that all in-flight page faults have been
flushed before removing the device from the fault queue. Otherwise, a
race condition will occur between the SVA unbind path and the fault
handling path.
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index ddb981ad0e19..b1f2a2fb7e06 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4075,8 +4075,8 @@ static int blocking_domain_set_dev_pasid(struct
iommu_domain *domain,
{
struct device_domain_info *info = dev_iommu_priv_get(dev);
- iopf_for_domain_remove(old, dev);
intel_pasid_tear_down_entry(info->iommu, dev, pasid, false);
+ iopf_for_domain_remove(old, dev);
domain_remove_dev_pasid(old, dev, pasid);
return 0;
The intel_pasid_tear_down_entry() function is responsible for blocking
hardware from generating new page faults and flushing all in-flight
ones. Therefore, moving iopf_for_domain_remove() after this function
should resolve this race condition.
Thanks,
baolu
>
> Shouldn't iopf_for_domain_remove() be called after
> intel_pasid_tear_down_entry() or domain_remove_dev_pasid() to ensure the device
> can no longer send page requests and the queue has been flushed? Is the current
> ordering intentional?
>
> 2. Regarding commit 9759ae2cee7c ("iommu: Fix potential memory leak in
> iopf_queue_remove_device()"): the commit frees the pending IOPF groups, but in
> the scenario above, direct references to the groups still exists either in the
> handler or in the workqueue awaiting processing, leading to a double free.
> Reverting this commit fix the issue above.
>
> I am not sure to see where is the potential memory leak, is there a case where
> a reference to a group exists only in the iommu_fault_param::faults that I'm
> missing?
>
> Thanks,
> Ethan
^ permalink raw reply related [flat|nested] 2+ messages in thread