[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 4/6] xsm: flask: change the interface and default policy for xsm_map_gmfn_foregin


  • To: Jan Beulich <JBeulich@xxxxxxxx>, Zhongze Liu <blackskygg@xxxxxxxxx>
  • From: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
  • Date: Thu, 24 Aug 2017 10:30:27 -0400
  • Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Julien Grall <julien.grall@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxx
  • Delivery-date: Thu, 24 Aug 2017 14:31:18 +0000
  • Ironport-phdr: 9a23:famIxRG/FJ4/JznfExiwOZ1GYnF86YWxBRYc798ds5kLTJ76pc2/bnLW6fgltlLVR4KTs6sC0LuH9fi8EjNcqb+681k6OKRWUBEEjchE1ycBO+WiTXPBEfjxciYhF95DXlI2t1uyMExSBdqsLwaK+i764jEdAAjwOhRoLerpBIHSk9631+ev8JHPfglEnjSwbLd2IRmssQncucYajIltJ60s1hbHv3xEdvhMy2h1P1yThRH85smx/J5n7Stdvu8q+tBDX6vnYak2VKRUAzs6PW874s3rrgTDQhCU5nQASGUWkwFHDBbD4RrnQ5r+qCr6tu562CmHIc37SK0/VDq+46t3ThLjlTwKPCAl/m7JlsNwjbpboBO/qBx5347Ue5yeOP5ncq/AYd8WWW9NU8BfWCxbBoO3cpUBAewPM+1Fq4XxvkUCoQe7CQSqGejhyCJHhmXu0KMnzeohHwHI0g8uEd0Av3vbrsn6OqgJXOCpzqTF1ynPY+9Y1Dr/7oXDbxAvoeuLXbJ1acffx1MgFwXEjlqOrYzuIj2b2foQuGaa9epvT/igi2A6oAx2vzevydojhZfGhoIP0F/J7jl5wYYpKt24T053e9ikEIBKuC2AOIt2Rd0iTnhutS0nybMGoYa2cDUFxZko3RLSa+GLf5KW7h/sSuqdOyp0iXR4c7ylnRmy61KvyujkW8mx11ZFszRKn8HXtnAIyxzT8s+HSuZh/ku52TaAyQTT6uZcLEAoj6XbMZ8hwqMrlpYJrUTCHjP5mEXxjKOMcEUr5vOo5Pj9brXjp5+cM5d4igD4MqswhsyyGfk0PwcBUmSB+emwyafv8VP2TblUlPE6j7HVsJXAKsQaoq65DRVV0oEm6xunEjim38kXkmcILFJfYh2KlJTpOlHSL/D4CvezmVKskCxxyPzcMb3hBYvNImDZkLj9ZbZ991JcyA0rwN5b+p9bFKwBIPbyWkDttNzVFQQ5MxGvw+n5EtlyyoQeWWeXCK+DLKzSqUOI5v4oI+SUf4AVvCzyJOQm5/71jn84mVAdfaay0JsYbXC3BPVmI0GDbXXwhdcBFH8AvhAiQ+zylF2CTTlTam6wX6Ih4jE7CZypDYHZSoCimryOxiO7HplNa29cEFCMFG3keJmDW/cJcCiSONNukiQYVbi9TI8szQuhtAnnxLp9MOXV9DcUuo7k1Nhy/+3ciwsy+DJvAsuB0mGNU3t0nmIHRjMswK9/pkl9wE+Z0adkm/xYCcBT5/RRXwc1K5HcyPZ6C9/sVQ7bY9iJVVCmQtG8DjEpVd8+3cIOb1xhFNWjkhDDxSuqArAPm7OXA5w097rW32LtKMZl13bGyK4hgkE9QstUKW2pnLVw+BbXB47NkkWZkaeqeL8f3CHT7meDy3SBvEVCXA53S6XFUmgVZlHKotTh+kPCU7iuBKwoMwRfz86OM7ZFZcP3jVpYQPfuI9DeY2Oqm2esHhaE3LyNY5Tse2kH2yXdEkcEwEgv+iOkPA52LCq8p2PVDHQ6N3jifkft+ulWs26gQwk/yATcKwVT172z9QwYzdidVrtH1LYNsyQ6qCR7E36y2tvXD5yLoA83L4tGZtZozF5B1G/d/yBwdrO6JqlszgoSfAh6sFnn/wlmAYVH184xpTUlyxQkevHQ609Iaz7NhcO4AbbQMGSnuUn1M6M=
  • List-id: Xen developer discussion <xen-devel.lists.xen.org>

On 08/24/2017 08:39 AM, Jan Beulich wrote:
On 24.08.17 at 13:33, <blackskygg@xxxxxxxxx> wrote:
Hi Jan,

2017-08-24 14:37 GMT+08:00 Jan Beulich <JBeulich@xxxxxxxx>:
On 24.08.17 at 02:51, <blackskygg@xxxxxxxxx> wrote:
2017-08-23 17:55 GMT+08:00 Jan Beulich <JBeulich@xxxxxxxx>:
On 22.08.17 at 20:08, <blackskygg@xxxxxxxxx> wrote:
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -525,10 +525,12 @@ static XSM_INLINE int
xsm_remove_from_physmap(XSM_DEFAULT_ARG struct domain *d1,
      return xsm_default_action(action, d1, d2);
  }

-static XSM_INLINE int xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain
*d, struct domain *t)
+static XSM_INLINE int xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain
*cd,
+                                           struct domain *d, struct domain
*t)
  {
      XSM_ASSERT_ACTION(XSM_TARGET);
-    return xsm_default_action(action, d, t);
+    return xsm_default_action(action, cd, d) ||
+        xsm_default_action(action, cd, t);
  }

... you use "or" here and ...

This might be confusing. But think of returning 0 as "allowed", the
only condition where this
statement returns a 0 is when both calls return 0 -- so it's actually
an "and". (Think of de-morgan's law.)

But as Stefano has pointed out, I should preserve the error code.

Ah, right - the code as written suggests boolean return values,
which gives it the wrong look. You really mean ?: instead of ||.
Why do you, btw, pass in current->domain (as cd) instead of
obtaining it here (just like various other hooks do)?

This was my original plan, but I'm not very sure about this, so I turn
to Julien for help, and...
Here is part of the irc log from a discussion with Julien on
#xendevel, where Julien said:

    blackskygg: I think you want to pass the current domain in
parameter, i.e having 3 domains argument.
    because your solution only works when XSM is not enabled (this is
the dummy callback)
    when XSM is enabled, the policy would be specificed by the administrator
    he needs to be able to know which domain was doing the configuration.

in flask/hooks.c there are quite a few uses of
avc_current_has_perm() in such cases, so I would think not
handing current->domain through the hook should be fine. But
of course Daniel may tell you I'm completely wrong here.

Jan

This is really just a choice of whatever looks better.  There's a very minor
performance penalty from not calling current->domain over and over, but there
might also be a performance gain if current_has_perm is not inlined and the
call results in smaller code size.

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.