Re: [PATCH v3 5/7] xen: add new domctl get_changed_domain

[Jürgen Groß]

On 16.12.24 16:18, Jan Beulich wrote:

On 16.12.2024 16:03, Juergen Gross wrote:

On 16.12.24 11:41, Jan Beulich wrote:

On 13.12.2024 17:24, Juergen Gross wrote:

--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -193,6 +193,57 @@ static void domain_changed_state(const struct domain *d)
       spin_unlock(&dom_state_changed_lock);
   }

+                                  const struct domain *d)
+{
+    info->state = XEN_DOMCTL_GETDOMSTATE_STATE_EXIST;
+    if ( d->is_shut_down )
+        info->state |= XEN_DOMCTL_GETDOMSTATE_STATE_SHUTDOWN;
+    if ( d->is_dying == DOMDYING_dying )
+        info->state |= XEN_DOMCTL_GETDOMSTATE_STATE_DYING;
+    if ( d->is_dying == DOMDYING_dead )
+        info->state |= XEN_DOMCTL_GETDOMSTATE_STATE_DEAD;
+    info->unique_id = d->unique_id;
+}
+
+int get_domain_state(struct xen_domctl_get_domain_state *info, struct domain 
*d,
+                     domid_t *domid)
+{
+    unsigned int dom;
+
+    if ( info->pad0 || info->pad1 )
+        return -EINVAL;
+
+    if ( d )
+    {
+        set_domain_state_info(info, d);
+
+        return 0;
+    }
+
+    while ( (dom = find_first_bit(dom_state_changed, DOMID_MASK + 1)) <
+            DOMID_FIRST_RESERVED )
+    {
+        if ( test_and_clear_bit(dom, dom_state_changed) )

+static void set_domain_state_info(struct xen_domctl_get_domain_state *info,

For these two accesses to dom_state_changed don't you need to hold the
lock patch 4 introduces? Also didn't you say you'd constrain the new
sub-op to the sole domain having VIRQ_DOM_EXEC bound (which, ftaod,
isn't enough to eliminate the race)?

Just to be more specific regarding the race: I guess you mean that a domain
having registered for the VIRQ doesn't mean the calling component being in
that domain really is the one associated with the VIRQ.

While being true, even today it is possible for one dom0 user process to
"steal" a VIRQ from another process by using dirty tricks via the privcmd
driver.

In the end a process having the access rights to use the privcmd driver must
be trusted to not disturb other processes with the same rights.

Of course, but that's not exactly what I was getting at. I was trying to
point out that the vIRQ check alone is still insufficient to avoid potential
crashes here, by one vCPU calling here while another unbinds the vIRQ. Taking
the lock is required for Xen's safety; checking the vIRQ is bound is an extra
policy enforcement.

Ah, okay. I think V4 will cover your scenario.


Juergen

OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

OpenPGP_signature.asc
Description: OpenPGP digital signature