Re: [Xen-devel] [PATCH] gnttab: lock the left grant table earlier in __gnttab_unmap_common()

[Anthony Liguori <anthony@xxxxxxxxxxxxx>]

On Mon, Nov 11, 2013 at 1:58 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>> On 11.11.13 at 03:14, Anthony Liguori <anthony@xxxxxxxxxxxxx> wrote:
>> On Sun, Nov 10, 2013 at 6:07 PM, Matt Wilson <msw@xxxxxxxxx> wrote:
>>> From: Matt Wilson <msw@xxxxxxxxxx>
>>>
>>> Luckily today maptrack_limit never shrinks. But if at some point in
>>> the future this were to change, checking maptrack_limit without
>>> holding the grant table spinlock would no longer be safe.
>>
>> Reviewed-by: Anthony Liguori <aliguori@xxxxxxxxxx>
>>
>> The commit doesn't mention it, but this problem could cause the
>> function to fail with "Bad handle" even when the handle is valid (in
>> theory at least).
>
> But only if the growing of the table races the operation here.
> Which ought to be impossible, as we're dealing with an unmap
> here (i.e. the handle must have been established earlier). And
> even if not impossible, it's pointless - checking against the
> lower (pre-growth) value is fine here, as guests are supposed
> to not issue requests resulting in growth in parallel with
> requests utilizing the grown table

There are no locks here so in between dropping the grant table lock,
anything can happen in theory including table growth and a larger
handle return.  I have seen horribly broken systems out there with SMM
routines that can run for almost a millisecond.

I agree, it's not at all practical and extremely unlikely to happen in
practice.  That's why I qualified the comment with "in theory at
least".

Regards,

Anthony Liguori

> (and they also should be
> unable to, as they can't possibly know of the "bigger" handle,
> which would be returned to them only after the growing
> completed).
>
> Or else - what am I overlooking?
>
> Jan
>

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