Re: [Xen-devel] [PATCH 1/3] x86: replace bad ASSERT() in xenmem_add_to_physmap_one()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 27/11/17 09:11, Jan Beulich wrote:
> There are no locks being held, i.e. it is possible to be triggered by
> racy hypercall invocations. Subsequent code doesn't really depend on the
> checked values, so this is not a security issue.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

> ---
> I'm up for to better suggestions for the EXDEV I've used; I certainly
> don't want to use -EINVAL, and -EAGAIN (afaik) generally is meant as a
> hint to re-invoke with the same arguments (which wouldn't help here).

I can't offer a better suggestion.

>
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -4143,8 +4143,12 @@ int xenmem_add_to_physmap_one(
>      /* Unmap from old location, if any. */
>      old_gpfn = get_gpfn_from_mfn(mfn_x(mfn));
>      ASSERT( old_gpfn != SHARED_M2P_ENTRY );
> -    if ( space == XENMAPSPACE_gmfn || space == XENMAPSPACE_gmfn_range )
> -        ASSERT( old_gpfn == gfn );
> +    if ( (space == XENMAPSPACE_gmfn || space == XENMAPSPACE_gmfn_range) &&
> +         old_gpfn != gfn )
> +    {
> +        rc = -EXDEV;
> +        goto put_both;
> +    }
>      if ( old_gpfn != INVALID_M2P_ENTRY )
>          rc = guest_physmap_remove_page(d, _gfn(old_gpfn), mfn, 
> PAGE_ORDER_4K);
>  
>
>
>


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