On 13/09/17 18:01, Paul Durrant wrote:
>> -----Original Message-----
>> From: Xen-devel [mailto:xen-devel-boun...@lists.xen.org] On Behalf Of
>> Juergen Gross
>> Sent: 13 September 2017 08:47
>> To: xen-devel@lists.xen.org
>> Cc: Juergen Gross <jgr...@suse.com>; sstabell...@kernel.org; Wei Liu
>> <wei.l...@citrix.com>; George Dunlap <george.dun...@citrix.com>;
>> Andrew Cooper <andrew.coop...@citrix.com>; Ian Jackson
>> <ian.jack...@citrix.com>; Tim (Xen.org) <t...@xen.org>;
>> julien.gr...@arm.com; jbeul...@suse.com; dgde...@tycho.nsa.gov
>> 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?
The comment above nr_status_frames() says so. Either the comment or the
code is wrong.
Juergen
>
> Paul
>
>>
>> While at it correct coding style: labels should be indented by one
>> space.
>>
>> Signed-off-by: Juergen Gross <jgr...@suse.com>
>> ---
>> 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@lists.xen.org
>> https://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
