On 17/02/2021 15:17, Jan Beulich wrote:
On 17.02.2021 16:00, Julien Grall wrote:
Hi Jan,

On 17/02/2021 14:54, Jan Beulich wrote:
On 17.02.2021 15:24, Julien Grall wrote:
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -267,6 +267,15 @@ int iommu_free_pgtables(struct domain *d)
       struct page_info *pg;
       unsigned int done = 0;
+ if ( !is_iommu_enabled(d) )
+        return 0;
+
+    /*
+     * Pages will be moved to the free list below. So we want to
+     * clear the root page-table to avoid any potential use after-free.
+     */
+    hd->platform_ops->clear_root_pgtable(d);

Taking amd_iommu_alloc_root() as example, is this really correct
prior to what is now patch 2?

Yes, there are no more use-after-free...

And this is because of ...? The necessary lock isn't being held
here, so on another CPU allocation of a new root and then of new
page tables could happen before you make enough progress here,
and hence it looks to me as if there might then still be pages
which get freed while present in the page tables (and hence
accessible by devices).

Ah yes. I forgot that now patch #3 is not first anymore. I can move again patch #3 first, although I know you dislike the approach taken there...

Cheers,

--
Julien Grall

Reply via email to