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

[Julien Grall <julien@xen.org>]

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