Re: [PATCH v3 2/7] xsm: flask: change the dummy xsm policy and flask hook for map_gmfn_foregin

From: Daniel De Graaf <dgdegra@tycho.nsa.gov>
To: Jan Beulich <JBeulich@suse.com>, Zhongze Liu <blackskygg@gmail.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>,
	Julien Grall <julien.grall@arm.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	xen-devel@lists.xen.org
Subject: Re: [PATCH v3 2/7] xsm: flask: change the dummy xsm policy and flask hook for map_gmfn_foregin
Date: Thu, 19 Oct 2017 13:36:30 -0400	[thread overview]
Message-ID: <f3e12d04-ffcb-fe4a-4666-d147b292d141@tycho.nsa.gov> (raw)
In-Reply-To: <59E8AF7A020000780018815A@prv-mh.provo.novell.com>

On 10/19/2017 07:58 AM, Jan Beulich wrote:
>>>> On 19.10.17 at 04:36, <blackskygg@gmail.com> wrote:
>> --- a/xen/include/xsm/dummy.h
>> +++ b/xen/include/xsm/dummy.h
>> @@ -516,7 +516,8 @@ static XSM_INLINE int xsm_remove_from_physmap(XSM_DEFAULT_ARG struct domain *d1,
>>   static XSM_INLINE int xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain *d, struct domain *t)
>>   {
>>       XSM_ASSERT_ACTION(XSM_TARGET);
>> -    return xsm_default_action(action, d, t);
>> +    return xsm_default_action(action, current->domain, d) ?:
>> +        xsm_default_action(action, current->domain, t);
>>   }
> 
> When all three domains are different, how does the changed
> policy reflect the original "d has privilege over t" requirement?
> I understand you want to relax the current condition, but this
> shouldn't come at the price of granting access when access
> should be denied. Nor the inverse - the current domain not
> having privilege over both does also not mean d doesn't have
> the necessary privilege over t.
> 
> I continue to think that you can't validly retrofit the new
> intended functionality onto the existing hypercall, even if
> nothing except the permission check needs to be different.
> 
> Jan

If this operation is going to be allowed at all (and I agree it has
valid use cases), then there won't be a privilege relationship between
(d) and (t) to check - they'll both be (somewhat related) domUs as far
as Xen can tell.  If this hypercall isn't used, adding a new hypercall
(subop) is the only way I'd see to do it - and that seems very redundant
as it'd need to do all the same checks except for the one about the
relationship between (d) and (t).  I don't see the reason why the
existing hypercall should deny being used for that purpose once it's
possible using other means.

The only possible problem that springs to mind is a restricted kernel
interface (such as the one used by QEMU in dom0 that restricts to a
single target domain) that now doesn't realize it's relaying an
operation that also requires permission over (t) after only checking
that the origin is allowed to modify (d).

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