Re: [Xen-devel] [PATCH v2 1/2] x86/mm: fix a potential race condition in map_pages_to_xen().

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.11.17 at 08:18, <yu.c.zhang@xxxxxxxxxxxxxxx> wrote:
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -4844,9 +4844,19 @@ int map_pages_to_xen(
>              {
>                  unsigned long base_mfn;
>  
> -                pl1e = l2e_to_l1e(*pl2e);
>                  if ( locking )
>                      spin_lock(&map_pgdir_lock);
> +
> +                /* Skip the re-consolidation if it's done on another CPU. */
> +                if ( l2e_get_flags(*pl2e) & _PAGE_PSE )
> +                {
> +                    if ( locking )
> +                        spin_unlock(&map_pgdir_lock);
> +                    goto check_l3;
> +                }
> +
> +                ol2e = *pl2e;

This wants moving up ahead of the if(), so you can use the local
variable inside that if().

> @@ -4880,6 +4889,15 @@ int map_pages_to_xen(
>  
>              if ( locking )
>                  spin_lock(&map_pgdir_lock);
> +
> +            /* Skip the re-consolidation if it's done on another CPU. */
> +            if ( l3e_get_flags(*pl3e) & _PAGE_PSE )
> +            {
> +                if ( locking )
> +                    spin_unlock(&map_pgdir_lock);
> +                continue;
> +            }
> +
>              ol3e = *pl3e;

Same here - move the if() below here and use ol3e in there.

With that
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

I'm not certain this is important enough a fix to consider for 4.10,
and you seem to think it's good enough if this gets applied only
after the tree would be branched, as you didn't Cc Julien. Please
indicate if you actually simply weren't aware, and you indeed
there's an important aspect to this that I'm overlooking.

Jan


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