Re: [Xen-devel] Re: xen crash in tmem: checking a xen pfn for domain ownership

[Keir Fraser <keir.fraser@xxxxxxxxxxxxx>]

Oh, depending on what you want to do with the page you may well want to
get_page(current->domain, page). You don't hold a lock on the domain's p2m,
so page ownerships can change under your feet, and hence getting a reference
to the page, and checking the page's ownership at the same time, might be
wise. And if you want to modify the page you should probably use
get_page_and_type(..., PGT_writable_page).

 -- Keir


On 17/09/2010 18:25, "Keir Fraser" <keir.fraser@xxxxxxxxxxxxx> wrote:

> Gfn_to_mfn() takes a domain as a parameter. It looks up gfn in that domain's
> p2m. The only RAM-typed pfns that can be present in a domain's p2m, if it is
> not sharing pages via memshr, are the domain's own pages. As far as I know,
> at least. It does no harm for you to switch to gfn_to_mfn_unshare(), but I
> doubt this is the fix for your current problem.
> 
>  -- Keir
> 
> On 17/09/2010 17:48, "Dan Magenheimer" <dan.magenheimer@xxxxxxxxxx> wrote:
> 
>> Thanks for the reply, but I'm not sure I understand.
>> 
>> Ignore memory sharing for now...
>> 
>> Are you saying, yes, the ownership check IS performed?
>> E.g. if gpfn is a random number, NULL will always be
>> returned (unless of course the random number happens
>> to be a valid gfn for current->domain)?
>> 
>> Or are you saying its plausible that this IS the problem
>> (that I am not checking for ownership)?
>> 
>> Now bring memory sharing back in...
>> 
>> Since tmem and memory sharing are supposed to be complementary
>> (though I don't think anybody has ever tried using both
>> together), are you saying I should change this one
>> call from gfn_to_mfn() to gfn_to_mfn_unshare() for
>> some reason (e.g. maybe to avoid a race)?  Note
>> that this code is just getting a virtual address
>> to copy a page to/from the guest.
>> 
>> Thanks,
>> Dan
>> 
>>> -----Original Message-----
>>> From: Keir Fraser [mailto:keir.fraser@xxxxxxxxxxxxx]
>>> Sent: Friday, September 17, 2010 10:35 AM
>>> To: Dan Magenheimer; Jan Beulich
>>> Cc: Xen-devel
>>> Subject: Re: xen crash in tmem: checking a xen pfn for domain ownership
>>> 
>>> If you could be doing memory sharing then you might need to use
>>> gfn_to_mfn_unshare()? Otherwise it looks pretty plausible, and that one
>>> flaw
>>> is pretty minor as you're probably not using memshr.
>>> 
>>>  -- Keir
>>> 
>>> On 17/09/2010 17:29, "Dan Magenheimer" <dan.magenheimer@xxxxxxxxxx>
>>> wrote:
>>> 
>>>> Does the construct:
>>>> 
>>>>   xen_pfn_t gpfn;
>>>>   p2m_type_t t;
>>>>   unsigned long mfn;
>>>> 
>>>>   mfn = mfn_x(gfn_to_mfn(current->domain, gpfn, &t));
>>>>   if (t != p2m_ram_rw || cli_mfn == INVALID_MFN)
>>>>       return NULL; /* bad */
>>>>   return map_domain_page(mfn)
>>>> 
>>>> somehow check to ensure that pfn belongs to current->domain?
>>>> (See cli_mfn_to_va() in common/tmem_xen.c.)
>>>> 
>>>> If not, is there an easy way to perform that check?
>>>> (preferably one that works for both HVM and PV guests)
>>>> 
>>>> In debugging a tmem Linux-side guest patch, I discovered
>>>> that a bad mfn passed by the guest can crash Xen and
>>>> I think this assumption might be the problem.
>>>> 
>>>> Thanks,
>>>> Dan
>>> 
>>> 
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel