Re: [PATCH v2 2/2] x86/mm: fix a potential race condition in modify_xen_mappings().

[Yu Zhang <yu.c.zhang@linux.intel.com>]

On 11/10/2017 5:57 PM, Jan Beulich wrote:
>>>> On 10.11.17 at 08:18, <yu.c.zhang@linux.intel.com> wrote:
>> --- a/xen/arch/x86/mm.c
>> +++ b/xen/arch/x86/mm.c
>> @@ -5097,6 +5097,17 @@ int modify_xen_mappings(unsigned long s, unsigned long e, unsigned int nf)
>>                */
>>               if ( (nf & _PAGE_PRESENT) || ((v != e) && (l1_table_offset(v) != 0)) )
>>                   continue;
>> +            if ( locking )
>> +                spin_lock(&map_pgdir_lock);
>> +
>> +            /* L2E may be cleared on another CPU. */
>> +            if ( !(l2e_get_flags(*pl2e) & _PAGE_PRESENT) )
> I think you also need a PSE check here, or else the l2e_to_l1e() below
> may be illegal.

Hmm, interesting point, and thanks! :-)
I did not check the PSE, because modify_xen_mappings() will not do the 
re-consolidation, and
concurrent invokes of this routine will not change this flag. But now I 
believe this presumption
shall not be made, because the paging structures may be modified by 
other routines, like
map_pages_to_xen() on other CPUs.

So yes, I think a _PAGE_PSE check is necessary here. And I suggest we 
also check the _PAGE_PRESENT
flag as well, for the re-consolidation part in my first patch for 
map_pages_to_xen(). Do you agree?

>> @@ -5105,11 +5116,16 @@ int modify_xen_mappings(unsigned long s, unsigned long e, unsigned int nf)
>>               {
>>                   /* Empty: zap the L2E and free the L1 page. */
>>                   l2e_write_atomic(pl2e, l2e_empty());
>> +                if ( locking )
>> +                    spin_unlock(&map_pgdir_lock);
>>                   flush_area(NULL, FLUSH_TLB_GLOBAL); /* flush before free */
>>                   free_xen_pagetable(pl1e);
>>               }
>> +            else if ( locking )
>> +                spin_unlock(&map_pgdir_lock);
>>           }
>>   
>> +check_l3:
> Labels indented by at least one space please.

Got it . Thanks.

Yu
>
> Jan
>
>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel