Re: [Xen-devel] [PATCH v3 for-4.10 1/2] x86/mm: fix potential race conditions in map_pages_to_xen().

[Yu Zhang <yu.c.zhang@xxxxxxxxxxxxxxx>]

On 11/14/2017 8:32 PM, Julien Grall wrote:
> Hi,
>
> On 14/11/17 08:20, Jan Beulich wrote:
> > > > > On 14.11.17 at 07:53, <yu.c.zhang@xxxxxxxxxxxxxxx> wrote:
> > > From: Min He <min.he@xxxxxxxxx>
> > >
> > > In map_pages_to_xen(), a L2 page table entry may be reset to point to
> > > a superpage, and its corresponding L1 page table need be freed in such
> > > scenario, when these L1 page table entries are mapping to consecutive
> > > page frames and having the same mapping flags.
> > >
> > > However, variable `pl1e` is not protected by the lock before L1 page 
> > > table
> > > is enumerated. A race condition may happen if this code path is invoked
> > > simultaneously on different CPUs.
> > >
> > > For example, `pl1e` value on CPU0 may hold an obsolete value, pointing
> > > to a page which has just been freed on CPU1. Besides, before this page
> > > is reused, it will still be holding the old PTEs, referencing 
> > > consecutive
> > > page frames. Consequently the `free_xen_pagetable(l2e_to_l1e(ol2e))` 
> > > will
> > > be triggered on CPU0, resulting the unexpected free of a normal page.
> > >
> > > This patch fixes the above problem by protecting the `pl1e` with the 
> > > lock.
> > > Also, there're other potential race conditions. For instance, the L2/L3
> > > entry may be modified concurrently on different CPUs, by routines 
> > > such as
> > > map_pages_to_xen(), modify_xen_mappings() etc. To fix this, this 
> > > patch will
> > > check the _PAGE_PRESENT and _PAGE_PSE flags, after the spinlock is 
> > > obtained,
> > > for the corresponding L2/L3 entry.
> > >
> > > Signed-off-by: Min He <min.he@xxxxxxxxx>
> > > Signed-off-by: Yi Zhang <yi.z.zhang@xxxxxxxxx>
> > > Signed-off-by: Yu Zhang <yu.c.zhang@xxxxxxxxxxxxxxx>
> > Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> Please try to have a cover letter in the future when you have multiple 
> patches. This will make easier to give comments/release-ack for the 
> all the patches. Anyway for the 2 patches:
Oh, got it. Thanks for the suggestion. :-)

Yu

> Release-acked-by: Julien Grall <julien.grall@xxxxxxxxxx>
>
> Cheers,

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