Re: [PATCH v2 3/3] [GSOC14] FIX:- Race condition between initializing shared ring and mempaging.

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 23/07/14 11:35, Ian Campbell wrote:
> On Wed, 2014-07-23 at 11:32 +0100, Andrew Cooper wrote:
>> On 23/07/14 11:14, Ian Campbell wrote:
>>> On Mon, 2014-07-21 at 15:40 +0100, Andrew Cooper wrote:
>>>
>>>>> Note that the XSA commmit, 6ae2df93c27, does exactly that. Enable
>>>>> paging/access/sharing, and only after that decrease reservation (and
>>>>> after that unpause). So the window is open ... methinks?
>>>> No - it does a domain pause around this set of critical operations, so
>>>> the guest is guaranteed not to be running, and therefore cannot
>>>> interfere.
>>> Shouldn't there be a memset in here somewhere? To clear out any bogus
>>> material in the ring? (maybe the caller of this code always clears the
>>> ring itself, I didn't check that)
>>>
>>> Ian.
>>>
>> Erm.  There probably should be.  Xen sets up the ring frontend
>> correctly, but nothing I can spot cleans up any stale backend state
>> which was preexisting in the page.
>>
>> It seems possible for a guest can map the affected pages and leave some
>> crafted backend replies which will be picked up as soon as mem_events
>> are enabled.  Combined with the lack of validation of responses in Xen,
>> this is quite bad.
> Indeed!
>
> BTW, I wonder why we don't make the initial P2M mapping R/O. That would
> still allow the tools to use the gpfn as a handle to pickup the page
> later but would stop the guest messing with it in the meantime.
>
> Ian.
>

Because the nominated pfn is in a RW hvmparam, so free to be altered by
the guest before mem_events are set up.

The correct solution to a whole slue of problems along these lines is
for an explicit notion of emulator pages for a domain, accounted to that
domain, but have never been part of the guest p2m.  Also, far more
restrictions on hvmparams. 

~Andrew