Re: [Xen-devel] [V3 PATCH 7/9] pvh: change xsm_add_to_physmap

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 27.11.13 at 21:29, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> wrote:
> On Wed, 27 Nov 2013 11:46:27 -0500
> Daniel De Graaf <dgdegra@xxxxxxxxxxxxx> wrote:
> 
>> On 11/26/2013 09:27 PM, Mukesh Rathor wrote:
>> > In preparation for the next patch, we update xsm_add_to_physmap to
>> > allow for checking of foreign domain. Thus, the current domain must
>> > have the right to update the mappings of target domain with pages
>> > from foreign domain.
>> >
>> > Signed-off-by: Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>
>> > CC: dgdegra@xxxxxxxxxxxxx 
>> > ---
>> >   xen/arch/x86/mm.c       |   16 +++++++++++++---
>> >   xen/include/xsm/dummy.h |   10 ++++++++--
>> >   xen/include/xsm/xsm.h   |    6 +++---
>> >   xen/xsm/flask/hooks.c   |   10 ++++++++--
>> >   4 files changed, 32 insertions(+), 10 deletions(-)
>> 
>> The XSM changes look good; however, the calling code needs a bit of
>> tweaking.  Currently, if domain 0 is specified as the foreign domain,
>> the check is skipped, and the check is also run unnecessarily when
>> foreign_domid is nonzero but the operation is not
>> XENMAPSPACE_gmfn_foreign. The locking in this version also implies a
>> potential TOCTOU bug, but which in reality is impossible to trigger
>> due to the existing RCU lock held on (d).  I would suggest passing
>> the foreign struct domain instead of the domid, as below.
>> 
>> An unrelated question about XENMAPSPACE_gmfn_foreign that came up
>> while looking at this: is the domain parameter (d) supposed to be
>> ignored here, with maps always modifying the current domain? I would
>> have expected this call to manipulate d's physmap, with the common
>> case being (d == current->domain).
> 
> Right, that is the assumption at present that PVH dom0 is creating
> user domains. Perhaps a debug assert would be a good idea. I can add 
> that. It's probably straightforward to just manipulate d's physmap,
> but I'd rather do it when it's really time for it, than to anticipate
> something I can't test right now.

I'm afraid that's not the right approach: Close to none of us would
ever test in a heavily disaggregated environment, yet nevertheless
the fundamentals for it are being put in place. So the PVH code
shouldn't be an exception here.

Jan


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