Re: [Xen-devel] [PATCH] x86: put_page_from_l2e() should honor _PAGE_RW

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Tue, Oct 09, 2018 at 12:43:05AM -0600, Jan Beulich wrote:
> 56fff3e5e9 ("x86: nuke PV superpage option and code") has introduced a
> (luckily latent only) bug here, in that it didn't make reference

It seems that the bug was from the original superpage code -- see
put_spage_pages.

> dropping dependent on whether the page was mapped writable. The only
> current source of large page mappings for PV domains is the Dom0
> builder, which only produces writeable ones.
> 
> Take the opportunity and also convert to bool both put_data_page()'s
> respective parameter and the argument put_page_from_l3e() passes.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Reviewed-by: Wei Liu <wei.liu2@xxxxxxxxxx>

> 
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -663,8 +663,7 @@ static int get_page_and_type_from_mfn(
>      return rc;
>  }
>  
> -static void put_data_page(
> -    struct page_info *page, int writeable)
> +static void put_data_page(struct page_info *page, bool writeable)
>  {
>      if ( writeable )
>          put_page_and_type(page);
> @@ -1289,10 +1288,13 @@ static int put_page_from_l2e(l2_pgentry_
>      if ( l2e_get_flags(l2e) & _PAGE_PSE )
>      {
>          struct page_info *page = l2e_get_page(l2e);
> +        bool writeable = l2e_get_flags(l2e) & _PAGE_RW;
>          unsigned int i;
>  
> +        ASSERT(!(mfn_x(page_to_mfn(page)) &
> +                 ((1UL << (L2_PAGETABLE_SHIFT - PAGE_SHIFT)) - 1)));
>          for ( i = 0; i < (1u << PAGETABLE_ORDER); i++, page++ )
> -            put_page_and_type(page);
> +            put_data_page(page, writeable);
>      }
>      else
>      {
> @@ -1318,7 +1320,7 @@ static int put_page_from_l3e(l3_pgentry_
>      if ( unlikely(l3e_get_flags(l3e) & _PAGE_PSE) )
>      {
>          unsigned long mfn = l3e_get_pfn(l3e);
> -        int writeable = l3e_get_flags(l3e) & _PAGE_RW;
> +        bool writeable = l3e_get_flags(l3e) & _PAGE_RW;
>  
>          ASSERT(!(mfn & ((1UL << (L3_PAGETABLE_SHIFT - PAGE_SHIFT)) - 1)));
>          do {
> 
> 
> 
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel