Re: [Xen-devel] [PATCH v6 01/12] xen: correct gnttab_get_status_frames()

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Xen-devel [mailto:xen-devel-bounces@xxxxxxxxxxxxx] On Behalf Of
> Juergen Gross
> Sent: 13 September 2017 08:47
> To: xen-devel@xxxxxxxxxxxxx
> Cc: Juergen Gross <jgross@xxxxxxxx>; sstabellini@xxxxxxxxxx; Wei Liu
> <wei.liu2@xxxxxxxxxx>; George Dunlap <George.Dunlap@xxxxxxxxxx>;
> Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Ian Jackson
> <Ian.Jackson@xxxxxxxxxx>; Tim (Xen.org) <tim@xxxxxxx>;
> julien.grall@xxxxxxx; jbeulich@xxxxxxxx; dgdegra@xxxxxxxxxxxxx
> Subject: [Xen-devel] [PATCH v6 01/12] xen: correct
> gnttab_get_status_frames()
> 
> In gnttab_get_status_frames() all accesses to nr_status_frames should
> be done with the grant table lock held.

Is this true? The value can only increase so what does the increase lock scope 
actually protect against?

  Paul

> 
> While at it correct coding style: labels should be indented by one
> space.
> 
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> ---
>  xen/common/grant_table.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> index c3895e6201..00ff075bd9 100644
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -2866,19 +2866,19 @@
> gnttab_get_status_frames(XEN_GUEST_HANDLE_PARAM(gnttab_get_statu
> s_frames_t) uop,
> 
>      gt = d->grant_table;
> 
> +    op.status = GNTST_okay;
> +
> +    grant_read_lock(gt);
> +
>      if ( unlikely(op.nr_frames > nr_status_frames(gt)) )
>      {
>          gdprintk(XENLOG_INFO, "Guest requested addresses for %d grant
> status "
>                   "frames, but only %d are available.\n",
>                   op.nr_frames, nr_status_frames(gt));
>          op.status = GNTST_general_error;
> -        goto out2;
> +        goto unlock;
>      }
> 
> -    op.status = GNTST_okay;
> -
> -    grant_read_lock(gt);
> -
>      for ( i = 0; i < op.nr_frames; i++ )
>      {
>          gmfn = gnttab_status_gmfn(d, gt, i);
> @@ -2886,10 +2886,11 @@
> gnttab_get_status_frames(XEN_GUEST_HANDLE_PARAM(gnttab_get_statu
> s_frames_t) uop,
>              op.status = GNTST_bad_virt_addr;
>      }
> 
> + unlock:
>      grant_read_unlock(gt);
> -out2:
> + out2:
>      rcu_unlock_domain(d);
> -out1:
> + out1:
>      if ( unlikely(__copy_field_to_guest(uop, &op, status)) )
>          return -EFAULT;
> 
> --
> 2.12.3
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> https://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel