Re: [for-4.15][PATCH v3 1/3] xen/iommu: x86: Clear the root page-table before freeing the page-tables

[Julien Grall <julien@xxxxxxx>]

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