Re: [PATCH] xen: arm: zero EL2 pagetable pages before use

[Julien Grall <julien.grall@arm.com>]

Hi,

On 11/03/2016 20:24, Andrew Cooper wrote:
> On 11/03/16 13:13, Jan Beulich wrote:
>>>>> On 11.03.16 at 13:56, <andrew.cooper3@citrix.com> wrote:
>>> On 11/03/16 11:29, Jan Beulich wrote:
>>>>>>> On 10.03.16 at 23:00, <shankerd@codeaurora.org> wrote:
>>>>> @@ -771,6 +772,7 @@ void __init setup_frametable_mappings(paddr_t ps, paddr_t pe)
>>>>>       nr_second = frametable_size >> SECOND_SHIFT;
>>>>>       second_base = alloc_boot_pages(nr_second, 1);
>>>>>       second = mfn_to_virt(second_base);
>>>>> +    memset(second, 0, nr_second * PAGE_SIZE);
>>>>>       for ( i = 0; i < nr_second; i++ )
>>>>>       {
>>>>>           pte = mfn_to_xen_entry(second_base + i, WRITEALLOC);
>>>> Along those lines here - use clear_page(), presumably by moving it
>>>> into the loop.
>>> This need only initialise the entries which are not filled by the loop,
>>> which will only be the rounding size up to the next 2M or 32M boundary.
>>>
>>> Most of the content of 'second' is explicitly initialised, so zeroing it
>>> all first is redundant.
>> Well, I certainly don't know all the details of how this works on
>> ARM, but the way I remember the original problem description
>> (sent a few days ago) the problem was with bogus translations
>> to be visible transiently. Of course all depends on whether the
>> page tables that are being modified here are live ones, which
>> I simply don't know.
>
> Looking at the code here, second is hooked into the live pagetables
> immediately after the loop.  Therefore, bogus translations will only be
> present for the untouched PTEs which make up the alignment space.

The frame table size is always aligned to 2MB/32MB. However, the frame 
table may not use all the entries in a level 2 page table (which cover 
1GB of memory). Those unused entries will be unknown if we don't clear them.

Keeping them unknown is not a problem as long as nobody is trying to 
access the underlying virtual address.

In the case of setup_frametable_mappings, Xen is still running with a 
single processor and the frame_table is not access until after 
create_mappings is called. The function should nuke all the TLBs at the 
end, so it looks like to me that zeroed the entries will hide the real 
problem.

Nonetheless, I would invalidate all the entries in the table to avoid 
polluting the TLBs with bogus entries and get a better crash.

Regards,

-- 
Julien Grall

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