Re: [Xen-devel] [PATCHv4 13/14] xen/gntdev: mark userspace PTEs as special on x86 PV guests

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Mon, 26 Jan 2015, David Vrabel wrote:
> In an x86 PV guest, get_user_pages_fast() on a userspace address range
> containing foreign mappings does not work correctly because the M2P
> lookup of the MFN from a userspace PTE may return the wrong page.
> 
> Force get_user_pages_fast() to fail on such addresses by marking the PTEs
> as special.
> 
> If Xen has XENFEAT_gnttab_map_avail_bits (available since at least
> 4.0), we can do so efficiently in the grant map hypercall.  Otherwise,
> it needs to be done afterwards.  This is both inefficient and racy
> (the mapping is visible to the task before we fixup the PTEs), but
> will be fine for well-behaved applications that do not use the mapping
> until after the mmap() system call returns.
> 
> Guests with XENFEAT_auto_translated_physmap (ARM and x86 HVM or PVH)
> do not need this since get_user_pages() has always worked correctly
> for them.
> 
> Signed-off-by: David Vrabel <david.vrabel@xxxxxxxxxx>
> ---
>  drivers/xen/gntdev.c                |   32 ++++++++++++++++++++++++++++++--
>  include/xen/interface/features.h    |    6 ++++++
>  include/xen/interface/grant_table.h |    7 +++++++
>  3 files changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
> index 494bd06..8ea10eb 100644
> --- a/drivers/xen/gntdev.c
> +++ b/drivers/xen/gntdev.c
> @@ -244,11 +244,24 @@ static int find_grant_ptes(pte_t *pte, pgtable_t token,
>       BUG_ON(pgnr >= map->count);
>       pte_maddr = arbitrary_virt_to_machine(pte).maddr;
>  
> +     /*
> +      * Set the PTE as special to force get_user_pages_fast() fall
> +      * back to the slow path.  If this is not supported as part of
> +      * the grant map, it will be done afterwards.
> +      */
> +     if (xen_feature(XENFEAT_gnttab_map_avail_bits))
> +             flags |= (1 << _GNTMAP_guest_avail0);
> +
>       gnttab_set_map_op(&map->map_ops[pgnr], pte_maddr, flags,
>                         map->grants[pgnr].ref,
>                         map->grants[pgnr].domid);
> -     gnttab_set_unmap_op(&map->unmap_ops[pgnr], pte_maddr, flags,
> -                         -1 /* handle */);

Why are you removing the call to gnttab_set_unmap_op?


> +     return 0;
> +}
> +
> +static int set_grant_ptes_as_special(pte_t *pte, pgtable_t token,
> +                                  unsigned long addr, void *data)
> +{
> +     set_pte_at(current->mm, addr, pte, pte_mkspecial(*pte));
>       return 0;
>  }
>  
> @@ -842,6 +855,21 @@ static int gntdev_mmap(struct file *flip, struct 
> vm_area_struct *vma)
>                       if (err)
>                               goto out_put_map;
>               }
> +     } else {
> +             /*
> +              * If the PTEs were not made special by the grant map
> +              * hypercall, do so here.
> +              *
> +              * This is racy since the mapping is already visible
> +              * to userspace but userspace should be well-behaved
> +              * enough to not touch it until the mmap() call
> +              * returns.
> +              */
> +             if (!xen_feature(XENFEAT_gnttab_map_avail_bits)) {
> +                     apply_to_page_range(vma->vm_mm, vma->vm_start,
> +                                         vma->vm_end - vma->vm_start,
> +                                         set_grant_ptes_as_special, NULL);
> +             }
>       }
>  
>       return 0;
> diff --git a/include/xen/interface/features.h 
> b/include/xen/interface/features.h
> index 131a6cc..6ad3d11 100644
> --- a/include/xen/interface/features.h
> +++ b/include/xen/interface/features.h
> @@ -41,6 +41,12 @@
>  /* x86: Does this Xen host support the MMU_PT_UPDATE_PRESERVE_AD hypercall? 
> */
>  #define XENFEAT_mmu_pt_update_preserve_ad  5
>  
> +/*
> + * If set, GNTTABOP_map_grant_ref honors flags to be placed into guest kernel
> + * available pte bits.
> + */
> +#define XENFEAT_gnttab_map_avail_bits      7
> +
>  /* x86: Does this Xen host support the HVM callback vector type? */
>  #define XENFEAT_hvm_callback_vector        8
>  
> diff --git a/include/xen/interface/grant_table.h 
> b/include/xen/interface/grant_table.h
> index bcce564..56806bc 100644
> --- a/include/xen/interface/grant_table.h
> +++ b/include/xen/interface/grant_table.h
> @@ -526,6 +526,13 @@ DEFINE_GUEST_HANDLE_STRUCT(gnttab_cache_flush);
>  #define GNTMAP_contains_pte     (1<<_GNTMAP_contains_pte)
>  
>  /*
> + * Bits to be placed in guest kernel available PTE bits (architecture
> + * dependent; only supported when XENFEAT_gnttab_map_avail_bits is set).
> + */
> +#define _GNTMAP_guest_avail0    (16)
> +#define GNTMAP_guest_avail_mask ((uint32_t)~0 << _GNTMAP_guest_avail0)
> +
> +/*
>   * Values for error status returns. All errors are -ve.
>   */
>  #define GNTST_okay             (0)  /* Normal return.                        
> */
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel
> 

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