On 2026-02-19 07:15, Daniel P. Smith wrote:
On 2/18/26 18:04, Jason Andryuk wrote:
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 29a7726d32d0..2eedc639c72a 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -860,12 +860,9 @@ long
do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
break;
case XEN_DOMCTL_get_domain_state:
- ret = xsm_get_domain_state(XSM_XS_PRIV, d);
With the initial NULL deref issue, I wondered if this wouldn't be
better off modified to drop the d param -
xsm_get_domain_state(XSM_XS_PRIV) - and changing flask permissions like:
allow dom0_t xen_t:xen get_domain_state;
That would gate the external call, and individual domain permissions
could be checked with xsm_getdomaininfo(), or a new hook if you don't
want to re-use.
But as your approach avoids passing NULL, it seems okay to me. It
also doesn't change the flask policy, which is nice.
That's a plain nack from me. Whether it is viewed as a pipe dream or
not, my goal continues to be to work towards enabling the ability to
have a truly disaggregated platform. In the original architecture, it
was envisioned to have multiple toolstack domains, each responsible for
a distinct set of domains. In terms of implementation, that would mean
multiple xenstored instances, each with a purview over a subset of domains.
I don't think what I wrote is at odds with your pipe dream.
My main concern is passing NULL as a domain. I think that is wrong,
beyond the fault seen on ARM. In domain_target_sid(), I think the NULL
dst was mistakenly matched to dom0's NULL src->target. src->target_sid
is 0 from zalloc, which is not otherwise initialized and invalid. That
is returned. Later when sidtab_search() can't find it, it is remapped
to SECINITSID_UNLABELED and returned. That silent remapping is dubious,
and it points toward unlabled_t should never be allowed in any rule.
Maybe it should remap unknown sids to a dedicated invalid_t, but maybe
invalid_t is already supposed to be that?
Regards,
Jason
