Wit unprivileged domains now capable to use the @releaseDomain watch,
there is no reason not to remove any node permissions which relate to
a domain which has been removed.

This resolves a complex scenario where a new domain could inherit the
permissions of an old one with the same domid.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/xenstored/domain.c | 61 ++++++++++++++++++++++++----------------
 1 file changed, 36 insertions(+), 25 deletions(-)

diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c
index 0bd2a1891a..6fbb5c061a 100644
--- a/tools/xenstored/domain.c
+++ b/tools/xenstored/domain.c
@@ -569,24 +569,10 @@ static int domain_tree_remove_sub(const void *ctx, struct connection *conn,
 				  struct node *node, void *arg)
 {
 	struct domain *domain = arg;
-	int ret = WALK_TREE_OK;
-
-	if (node->perms[0].id != domain->domid)
-		return WALK_TREE_OK;
+	bool node_changed = false;
+	unsigned int i;
 
-	if (keep_orphans) {
-		domain_nbentry_dec(NULL, domain->domid);
-		node->perms[0].id = priv_domid;
-		node->acc.memory = 0;
-		domain_nbentry_inc(NULL, priv_domid);
-		if (write_node_raw(NULL, node->name, node, NODE_MODIFY, true)) {
-			/* That's unfortunate. We only can try to continue. */
-			syslog(LOG_ERR,
-			       "error when moving orphaned node %s to dom0\n",
-			       node->name);
-		} else
-			trace("orphaned node %s moved to dom0\n", node->name);
-	} else {
+	if (node->perms[0].id == domain->domid && !keep_orphans) {
 		if (rm_node(NULL, ctx, node->name)) {
 			/* That's unfortunate. We only can try to continue. */
 			syslog(LOG_ERR,
@@ -596,10 +582,38 @@ static int domain_tree_remove_sub(const void *ctx, struct connection *conn,
 			trace("orphaned node %s deleted\n", node->name);
 
 		/* Skip children in all cases in order to avoid more errors. */
-		ret = WALK_TREE_SKIP_CHILDREN;
+		return WALK_TREE_SKIP_CHILDREN;
 	}
 
-	return domain->acc_val[ACC_NODES] ? ret : WALK_TREE_SUCCESS_STOP;
+	if (node->perms[0].id == domain->domid) {
+		domain_nbentry_dec(NULL, domain->domid);
+		node->perms[0].id = priv_domid;
+		node->acc.memory = 0;
+		domain_nbentry_inc(NULL, priv_domid);
+		trace("moving orphaned node %s to dom0\n", node->name);
+		node_changed = true;
+	}
+
+	for (i = 1; i < node->hdr.num_perms; i++) {
+		if (node->perms[i].id != domain->domid)
+			continue;
+		memmove(node->perms + i, node->perms + i + 1,
+			sizeof(*node->perms) * (node->hdr.num_perms - i - 1));
+		node->hdr.num_perms--;
+		i--;
+		node_changed = true;
+	}
+
+	if (node_changed) {
+		if (write_node_raw(NULL, node->name, node, NODE_MODIFY, true)) {
+			/* That's unfortunate. We only can try to continue. */
+			syslog(LOG_ERR,
+			       "error when writing modified node %s\n",
+			       node->name);
+		}
+	}
+
+	return WALK_TREE_OK;
 }
 
 static void domain_tree_remove(struct domain *domain)
@@ -607,12 +621,9 @@ static void domain_tree_remove(struct domain *domain)
 	int ret;
 	struct walk_funcs walkfuncs = { .enter = domain_tree_remove_sub };
 
-	if (domain->acc_val[ACC_NODES]) {
-		ret = walk_node_tree(domain, NULL, "/", &walkfuncs, domain);
-		if (ret == WALK_TREE_ERROR_STOP)
-			syslog(LOG_ERR,
-			       "error when looking for orphaned nodes\n");
-	}
+	ret = walk_node_tree(domain, NULL, "/", &walkfuncs, domain);
+	if (ret == WALK_TREE_ERROR_STOP)
+		syslog(LOG_ERR, "error when looking for orphaned nodes\n");
 
 	walk_node_tree(domain, NULL, "@releaseDomain", &walkfuncs, domain);
 	walk_node_tree(domain, NULL, "@introduceDomain", &walkfuncs, domain);
-- 
2.53.0

On 2026-04-23 04:08, Juergen Gross wrote:
> Wit unprivileged domains now capable to use the @releaseDomain watch,

s/Wit/With/

> there is no reason not to remove any node permissions which relate to
> a domain which has been removed.
> 
> This resolves a complex scenario where a new domain could inherit the
> permissions of an old one with the same domid.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

>   
> -	return domain->acc_val[ACC_NODES] ? ret : WALK_TREE_SUCCESS_STOP;
> +	if (node->perms[0].id == domain->domid) {
> +		domain_nbentry_dec(NULL, domain->domid);
> +		node->perms[0].id = priv_domid;
> +		node->acc.memory = 0;
> +		domain_nbentry_inc(NULL, priv_domid);
> +		trace("moving orphaned node %s to dom0\n", node->name);

Since you are touching this, maybe s/dom0/dom%u/ and priv_domid?

Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>

Thanks,
Jason

On 28.04.26 00:14, Jason Andryuk wrote:
> On 2026-04-23 04:08, Juergen Gross wrote:
>> Wit unprivileged domains now capable to use the @releaseDomain watch,
> 
> s/Wit/With/
> 
>> there is no reason not to remove any node permissions which relate to
>> a domain which has been removed.
>>
>> This resolves a complex scenario where a new domain could inherit the
>> permissions of an old one with the same domid.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
> 
>> -    return domain->acc_val[ACC_NODES] ? ret : WALK_TREE_SUCCESS_STOP;
>> +    if (node->perms[0].id == domain->domid) {
>> +        domain_nbentry_dec(NULL, domain->domid);
>> +        node->perms[0].id = priv_domid;
>> +        node->acc.memory = 0;
>> +        domain_nbentry_inc(NULL, priv_domid);
>> +        trace("moving orphaned node %s to dom0\n", node->name);
> 
> Since you are touching this, maybe s/dom0/dom%u/ and priv_domid?

Good idea.

> 
> Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>

Thanks,


Juergen