[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH for-4.15 3/4] [RFC] xen/iommu: x86: Clear the root page-table before freeing the page-tables
Hi Jan,
On 23/12/2020 14:12, Jan Beulich wrote:
On 22.12.2020 16:43, Julien Grall wrote:
From: Julien Grall <jgrall@xxxxxxxxxx>
The new per-domain IOMMU page-table allocator will now free the
page-tables when domain's resources are relinquished. However, the root
page-table (i.e. hd->arch.pg_maddr) will not be cleared.
Xen may access the IOMMU page-tables afterwards at least in the case of
PV domain:
(XEN) Xen call trace:
(XEN) [<ffff82d04025b4b2>] R iommu.c#addr_to_dma_page_maddr+0x12e/0x1d8
(XEN) [<ffff82d04025b695>] F iommu.c#intel_iommu_unmap_page+0x5d/0xf8
(XEN) [<ffff82d0402695f3>] F iommu_unmap+0x9c/0x129
(XEN) [<ffff82d0402696a6>] F iommu_legacy_unmap+0x26/0x63
(XEN) [<ffff82d04033c5c7>] F mm.c#cleanup_page_mappings+0x139/0x144
(XEN) [<ffff82d04033c61d>] F put_page+0x4b/0xb3
(XEN) [<ffff82d04033c87f>] F put_page_from_l1e+0x136/0x13b
(XEN) [<ffff82d04033cada>] F devalidate_page+0x256/0x8dc
(XEN) [<ffff82d04033d396>] F mm.c#_put_page_type+0x236/0x47e
(XEN) [<ffff82d04033d64d>] F mm.c#put_pt_page+0x6f/0x80
(XEN) [<ffff82d04033d8d6>] F mm.c#put_page_from_l2e+0x8a/0xcf
(XEN) [<ffff82d04033cc27>] F devalidate_page+0x3a3/0x8dc
(XEN) [<ffff82d04033d396>] F mm.c#_put_page_type+0x236/0x47e
(XEN) [<ffff82d04033d64d>] F mm.c#put_pt_page+0x6f/0x80
(XEN) [<ffff82d04033d807>] F mm.c#put_page_from_l3e+0x8a/0xcf
(XEN) [<ffff82d04033cdf0>] F devalidate_page+0x56c/0x8dc
(XEN) [<ffff82d04033d396>] F mm.c#_put_page_type+0x236/0x47e
(XEN) [<ffff82d04033d64d>] F mm.c#put_pt_page+0x6f/0x80
(XEN) [<ffff82d04033d6c7>] F mm.c#put_page_from_l4e+0x69/0x6d
(XEN) [<ffff82d04033cf24>] F devalidate_page+0x6a0/0x8dc
(XEN) [<ffff82d04033d396>] F mm.c#_put_page_type+0x236/0x47e
(XEN) [<ffff82d04033d92e>] F put_page_type_preemptible+0x13/0x15
(XEN) [<ffff82d04032598a>] F domain.c#relinquish_memory+0x1ff/0x4e9
(XEN) [<ffff82d0403295f2>] F domain_relinquish_resources+0x2b6/0x36a
(XEN) [<ffff82d040205cdf>] F domain_kill+0xb8/0x141
(XEN) [<ffff82d040236cac>] F do_domctl+0xb6f/0x18e5
(XEN) [<ffff82d04031d098>] F pv_hypercall+0x2f0/0x55f
(XEN) [<ffff82d04039b432>] F lstar_enter+0x112/0x120
This will result to a use after-free and possibly an host crash or
memory corruption.
Freeing the page-tables further down in domain_relinquish_resources()
would not work because pages may not be released until later if another
domain hold a reference on them.
Once all the PCI devices have been de-assigned, it is actually pointless
to access modify the IOMMU page-tables. So we can simply clear the root
page-table address.
Fixes: 3eef6d07d722 ("x86/iommu: convert VT-d code to use new page table
allocator")
Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>
---
This is an RFC because it would break AMD IOMMU driver. One option would
be to move the call to the teardown callback earlier on. Any opinions?
We already have
static void amd_iommu_domain_destroy(struct domain *d)
{
dom_iommu(d)->arch.amd.root_table = NULL;
}
and this function is AMD's teardown handler. Hence I suppose
doing the same for VT-d would be quite reasonable. And really
VT-d's iommu_domain_teardown() also already has
hd->arch.vtd.pgd_maddr = 0;
Let me have a look if that works.
I guess what's missing is prevention of the root table
getting re-setup.
This is taken care in the follow-up patch by forbidding page-table
allocation. I can mention it in the commit message.
This, I guess, would be helped by the
previously suggested preventing of any further modifications
to the p2m and alike for dying domains. Note how e.g. the
handling of XEN_DOMCTL_assign_device already includes a
"dying" check.
Jan
--
Julien Grall
|
