Re: [Xen-devel] [PATCHv14 1/2] gnttab: use per-VCPU maptrack free lists

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.06.15 at 12:02, <JBeulich@xxxxxxxx> wrote:
>>>> On 17.06.15 at 15:53, <david.vrabel@xxxxxxxxxx> wrote:
>> @@ -1474,6 +1519,17 @@ gnttab_setup_table(
>>      gt = d->grant_table;
>>      write_lock(&gt->lock);
>>  
>> +    /* Tracking of mapped foreign frames table */
>> +    gt->maptrack = vzalloc(max_maptrack_frames * sizeof(*gt->maptrack));
>> +    if ( gt->maptrack == NULL )
>> +        goto out3;
>> +    for_each_vcpu( d, v )
>> +    {
>> +        v->maptrack_head = MAPTRACK_TAIL;
>> +        v->maptrack_tail = MAPTRACK_TAIL;
>> +    }
>> +    gt->maptrack_limit = 0;
> 
> I'm sorry for noticing this only now, but this change adds a
> dependency on a domain invoking GNTTABOP_map_grant_ref to
> have invoked GNTTABOP_setup_table first. Not only did such a
> requirement not exist before afaict, you also don't verify that to
> be the case, and hence it looks like __get_maptrack_handle()
> could end up de-referencing NULL in such a case. IOW these
> sentinels need to be set during vCPU initialization instead, and
> gt->maptrack allocation needs to remain in grant_table_create().

And, not having said so explicitly, the setting of gt->maptrack_limit
needs to go away here too (and doesn't need to be done
explicitly in grant_table_create() due to the xzalloc() there).

All of this then also addresses the possibility of multiple
invocations of GNTTABOP_setup_table by a guest.

Jan


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