Re: [Xen-devel] [PATCH] Extra check in grant table code for mapping of shared frame

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 20.09.12 at 17:30, Andres Lagar-Cavilla <andreslc@xxxxxxxxxxxxxx> wrote:
> On Sep 19, 2012, at 11:35 AM, Jan Beulich wrote:
> 
>>>>> On 13.09.12 at 17:27, Andres Lagar-Cavilla <andres@xxxxxxxxxxxxxxxx> 
>>>>> wrote:
>>> --- a/xen/common/grant_table.c
>>> +++ b/xen/common/grant_table.c
>>> @@ -649,9 +649,12 @@ __gnttab_map_grant_ref(
>>>     }
>>>     else if ( owner == rd || owner == dom_cow )
>>>     {
>>> -        if ( gnttab_host_mapping_get_page_type(op, ld, rd) &&
>>> -             !get_page_type(pg, PGT_writable_page) )
>>> -            goto could_not_pin;
>>> +        if ( gnttab_host_mapping_get_page_type(op, ld, rd) )
>>> +        {
>>> +            if ( (owner == dom_cow) ||
>>> +                 !get_page_type(pg, PGT_writable_page) )
>>> +                goto could_not_pin;
>>> +        }
>>> 
>>>         nr_gets++;
>>>         if ( op->flags & GNTMAP_host_map )
>> 
>> Isn't that only half of it, in that the error/unmap paths need to
>> also consider that get_page_type() wasn't called? There's
>> quite a few calls to gnttab_host_mapping_get_page_type()/
>> put_page_type() sequences there.
> 
> I think this is covered. could_not_pin will cascade into undo_out, and 
> nr_gets remains at zero at this point. Then:
>  undo_out:
>     if ( nr_gets > 1 )
>     {
>        â
>     }
>     if ( nr_gets > 0 )
>     {
>         if ( gnttab_host_mapping_get_page_type(op, ld, rd) )
>             put_page_type(pg);
>             ...
> 
> i.e. put_page_type will not be called. This is really tricky code!

Okay, that path indeed looks safe through this nr_gets use.

Oh, and I see, the other cases are of no concern because
the check you added leads directly to the failure path.

Thanks for clarifying,
Jan

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