Re: [Xen-devel] [PATCH v2 7/7] xen/arm: mm: Flush the TLBs even if a mapping failed in create_xen_entries

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 8 May 2019, Julien Grall wrote:
> At the moment, create_xen_entries will only flush the TLBs if the full
> range has successfully been updated. This may lead to leave unwanted
> entries in the TLBs if we fail to update some entries.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> Reviewed-by: Andrii Anisov <andrii_anisov@xxxxxxxx>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> ---
>     Changes in v2:
>         - Add Andrii's reviewed-by
> ---
>  xen/arch/arm/mm.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 8ee828d445..9d584e4cbf 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -984,7 +984,7 @@ static int create_xen_entries(enum xenmap_operation op,
>                                unsigned long nr_mfns,
>                                unsigned int flags)
>  {
> -    int rc;
> +    int rc = 0;
>      unsigned long addr = virt, addr_end = addr + nr_mfns * PAGE_SIZE;
>      lpae_t pte, *entry;
>      lpae_t *third = NULL;
> @@ -1013,7 +1013,8 @@ static int create_xen_entries(enum xenmap_operation op,
>                  {
>                      printk("%s: trying to replace an existing mapping 
> addr=%lx mfn=%"PRI_mfn"\n",
>                             __func__, addr, mfn_x(mfn));
> -                    return -EINVAL;
> +                    rc = -EINVAL;
> +                    goto out;
>                  }
>                  if ( op == RESERVE )
>                      break;
> @@ -1030,7 +1031,8 @@ static int create_xen_entries(enum xenmap_operation op,
>                  {
>                      printk("%s: trying to %s a non-existing mapping 
> addr=%lx\n",
>                             __func__, op == REMOVE ? "remove" : "modify", 
> addr);
> -                    return -EINVAL;
> +                    rc = -EINVAL;
> +                    goto out;
>                  }
>                  if ( op == REMOVE )
>                      pte.bits = 0;
> @@ -1043,7 +1045,8 @@ static int create_xen_entries(enum xenmap_operation op,
>                      {
>                          printk("%s: Incorrect combination for addr=%lx\n",
>                                 __func__, addr);
> -                        return -EINVAL;
> +                        rc = -EINVAL;
> +                        goto out;
>                      }
>                  }
>                  write_pte(entry, pte);
> @@ -1052,11 +1055,14 @@ static int create_xen_entries(enum xenmap_operation 
> op,
>                  BUG();
>          }
>      }
> +out:
> +    /*
> +     * Flush the TLBs even in case of failure because we may have
> +     * partially modified the PT. This will prevent any unexpected
> +     * behavior afterwards.
> +     */
>      flush_xen_tlb_range_va(virt, PAGE_SIZE * nr_mfns);
>  
> -    rc = 0;
> -
> -out:
>      return rc;
>  }
>  
> -- 
> 2.11.0
> 

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