[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 3/6] xen: add new domctl get_changed_domain
On 24.10.24 15:35, Daniel P. Smith wrote: On 10/24/24 05:13, Jürgen Groß wrote:On 23.10.24 17:55, Daniel P. Smith wrote:On 10/23/24 09:10, Juergen Gross wrote:Add a new domctl sub-function to get data of a domain having changed state (this is needed by Xenstore). The returned state just contains the domid, the domain unique id, and some flags (existing, shutdown, dying). In order to enable Xenstore stubdom being built for multiple Xen versions, make this domctl stable. For stable domctls the interface_version is specific to the respective domctl op and it is an in/out parameter: On input the caller is specifying the desired version of the op, while on output the hypervisor will return the used version (this will be at max the caller supplied version, but might be lower in case the hypervisor doesn't support this version). Signed-off-by: Juergen Gross <jgross@xxxxxxxx> --- V1: - use a domctl subop for the new interface (Jan Beulich) --- tools/flask/policy/modules/dom0.te | 2 +- xen/common/domain.c | 51 +++++++++++++++++++++++++++++ xen/common/domctl.c | 19 ++++++++++- xen/common/event_channel.c | 9 ++++- xen/include/public/domctl.h | 33 +++++++++++++++++++ xen/include/xen/event.h | 6 ++++ xen/include/xen/sched.h | 2 ++ xen/include/xsm/dummy.h | 8 +++++ xen/include/xsm/xsm.h | 6 ++++ xen/xsm/dummy.c | 1 + xen/xsm/flask/hooks.c | 7 ++++ xen/xsm/flask/policy/access_vectors | 2 ++ 12 files changed, 143 insertions(+), 3 deletions(-)diff --git a/tools/flask/policy/modules/dom0.te b/tools/flask/policy/modules/dom0.teindex 16b8c9646d..6043c01b12 100644 --- a/tools/flask/policy/modules/dom0.te +++ b/tools/flask/policy/modules/dom0.te @@ -40,7 +40,7 @@ allow dom0_t dom0_t:domain { }; allow dom0_t dom0_t:domain2 { set_cpu_policy gettsc settsc setscheduler set_vnumainfo - get_vnumainfo psr_cmt_op psr_alloc get_cpu_policy + get_vnumainfo psr_cmt_op psr_alloc get_cpu_policy get_domain_stateI don't think that is where you want it, as that restricts dom0 to only being able to make that call against dom0. The question I have is, are you looking for this permission to be explicitly assigned to dom0 or to the domain type that was allowed to create the domain. IMHO, I think you would want the latter, so not only should the permission go here, but also added to xen.if:create_domain_common.Additionally, I would also recommend adding the following to xenstore.te: allow xenstore_t domain_type:domain get_domain_stateOkay, but shouldn't this be: allow xenstore_t domain_type:domain2 get_domain_state;Apologies, yes that was a typo on my part.allow dom0_t dom0_t:resource { add remove };...@@ -866,6 +873,16 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)__HYPERVISOR_domctl, "h", u_domctl); break; + case XEN_DOMCTL_get_domain_state: + ret = xsm_get_domain_state(XSM_HOOK, d);XSM_HOOK will allow any domain to make this call on any domain. What I think you want here is XSM_XS_PRIV. That will allow either a domain flagged as the xenstore domain or dom0 to make the call.I thought so, too, but looking at the "getdomaininfo" example it seems to be okay this way, too. Especially with the addition to xsm_domctl() checking for XSM_XS_PRIV.I know this has been done in the past, but imho it is not a very good practice. Access checks really should always restrict equal or down. There should be a strong reason, which probably would deserves a code comment, to allow a check to relax up. Restricting equal should not reduce access, and if it does, then it means there is an unintended access path which is now exposed. Sounds logical. I'll check if it works the way you are suggesting. If so, I'll send a patch fixing the getdomaininfo case. Juergen Attachment:
OpenPGP_0xB0DE9DD628BF132F.asc Attachment:
OpenPGP_signature.asc
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |