[PATCH 2/4] tools/xenstored: add support for "all domains" node permission

Juergen Gross posted 4 patches 3 months, 3 weeks ago
There is a newer version of this series
[PATCH 2/4] tools/xenstored: add support for "all domains" node permission
Posted by Juergen Gross 3 months, 3 weeks ago
Add support for using DOMID_ANY in node permissions to indicate that
all domains are allowed to access the node.

Add a new feature bit for indicating the support of DOMID_ANY.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 docs/man/xl.cfg.5.pod.in        |  4 ++++
 tools/xenstored/core.c          | 19 ++++++++++++++-----
 tools/xenstored/domain.c        | 16 ++++++++++++++--
 tools/xenstored/domain.h        |  3 ++-
 xen/include/public/io/xs_wire.h |  2 ++
 5 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index 2f77016ecf..d34951edb9 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -746,6 +746,10 @@ Xenstore supports to set watches with a limited depth (depth 0 matches
 only the watched node, depth 1 matches the node and its direct children,
 etc.).
 
+=item B<0x00000008>
+
+Xenstore supports the B<all domains> node access permission.
+
 =back
 
 The features supported by the running Xenstore instance can be retrieved
diff --git a/tools/xenstored/core.c b/tools/xenstored/core.c
index f92fca6e9e..5a621f53ba 100644
--- a/tools/xenstored/core.c
+++ b/tools/xenstored/core.c
@@ -882,6 +882,16 @@ static int write_node(struct connection *conn, struct node *node,
 	return ret;
 }
 
+/* Check one node permission to match a connection. */
+static bool perm_allows_conn(const struct connection *conn,
+			     const struct xs_permissions *p)
+{
+	if (p->id == conn->id || (conn->target && p->id == conn->target->id))
+		return true;
+
+	return p->id == DOMID_ANY;
+}
+
 unsigned int perm_for_conn(struct connection *conn,
 			   const struct node_perms *perms)
 {
@@ -889,14 +899,13 @@ unsigned int perm_for_conn(struct connection *conn,
 	unsigned int mask = XS_PERM_READ|XS_PERM_WRITE|XS_PERM_OWNER;
 
 	/* Owners and tools get it all... */
-	if (!domain_is_unprivileged(conn) || perms->p[0].id == conn->id
-                || (conn->target && perms->p[0].id == conn->target->id))
+	if (!domain_is_unprivileged(conn) ||
+	    perm_allows_conn(conn, perms->p))
 		return (XS_PERM_READ|XS_PERM_WRITE|XS_PERM_OWNER) & mask;
 
 	for (i = 1; i < perms->num; i++)
 		if (!(perms->p[i].perms & XS_PERM_IGNORE) &&
-		    (perms->p[i].id == conn->id ||
-		     (conn->target && perms->p[i].id == conn->target->id)))
+		    perm_allows_conn(conn, perms->p + i))
 			return perms->p[i].perms & mask;
 
 	return perms->p[0].perms & mask;
@@ -1832,7 +1841,7 @@ static int do_set_perms(const void *ctx, struct connection *conn,
 	if (!xenstore_strings_to_perms(perms.p, perms.num, permstr))
 		return errno;
 
-	if (domain_alloc_permrefs(&perms))
+	if (domain_alloc_permrefs(conn, &perms))
 		return ENOMEM;
 	if (perms.p[0].perms & XS_PERM_IGNORE)
 		return ENOENT;
diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c
index 00875d6b5c..0bd2a1891a 100644
--- a/tools/xenstored/domain.c
+++ b/tools/xenstored/domain.c
@@ -44,7 +44,8 @@
 #endif
 
 #define XENSTORE_FEATURES	(XENSTORE_SERVER_FEATURE_ERROR |	\
-				 XENSTORE_SERVER_FEATURE_WATCHDEPTH)
+				 XENSTORE_SERVER_FEATURE_WATCHDEPTH |	\
+				 XENSTORE_SERVER_FEATURE_DOMID_ANY)
 
 static xenmanage_handle *xm_handle;
 xengnttab_handle **xgt_handle;
@@ -1754,8 +1755,12 @@ static bool chk_domain_generation(unsigned int domid, uint64_t gen)
  * Allocate all missing struct domain referenced by a permission set.
  * Any permission entries for not existing domains will be marked to be
  * ignored.
+ * An DOMID_ANY entry will be marked to be ignored, if the writing
+ * domain doesn't have the XENSTORE_SERVER_FEATURE_DOMID_ANY enabled. Note
+ * that Xen tools will never set DOMID_ANY for a guest owned node.
  */
-int domain_alloc_permrefs(struct node_perms *perms)
+int domain_alloc_permrefs(const struct connection *conn,
+			  struct node_perms *perms)
 {
 	unsigned int i, domid;
 	struct domain *d;
@@ -1763,6 +1768,12 @@ int domain_alloc_permrefs(struct node_perms *perms)
 
 	for (i = 0; i < perms->num; i++) {
 		domid = perms->p[i].id;
+		if (domid == DOMID_ANY) {
+			if (!(conn->domain->features &
+			      XENSTORE_SERVER_FEATURE_DOMID_ANY))
+				perms->p[i].perms |= XS_PERM_IGNORE;
+			continue;
+		}
 		d = find_domain_struct(domid);
 		if (!d) {
 			if (xenmanage_get_domain_info(xm_handle, domid, NULL,
@@ -1788,6 +1799,7 @@ int domain_adjust_node_perms(struct node *node)
 
 	for (i = 1; i < node->hdr.num_perms; i++) {
 		if ((perms[i].perms & XS_PERM_IGNORE) ||
+		    perms[i].id == DOMID_ANY ||
 		    chk_domain_generation(perms[i].id, node->hdr.generation))
 			continue;
 
diff --git a/tools/xenstored/domain.h b/tools/xenstored/domain.h
index b1cfb5cd82..7dad4849a0 100644
--- a/tools/xenstored/domain.h
+++ b/tools/xenstored/domain.h
@@ -116,7 +116,8 @@ const char *get_implicit_path(const struct connection *conn);
  */
 int domain_adjust_node_perms(struct node *node);
 
-int domain_alloc_permrefs(struct node_perms *perms);
+int domain_alloc_permrefs(const struct connection *conn,
+			  struct node_perms *perms);
 
 /* Quota manipulation */
 int domain_nbentry_inc(struct connection *conn, unsigned int domid);
diff --git a/xen/include/public/io/xs_wire.h b/xen/include/public/io/xs_wire.h
index 2e763bc877..d6533a8452 100644
--- a/xen/include/public/io/xs_wire.h
+++ b/xen/include/public/io/xs_wire.h
@@ -126,6 +126,8 @@ struct xenstore_domain_interface {
 #define XENSTORE_SERVER_FEATURE_ERROR        2
 /* The XS_WATCH command can be used with a <depth> parameter */
 #define XENSTORE_SERVER_FEATURE_WATCHDEPTH   4
+/* The capability to use DOMID_ANY for node permissions */
+#define XENSTORE_SERVER_FEATURE_DOMID_ANY    8
 
 /* Valid values for the connection field */
 #define XENSTORE_CONNECTED 0 /* the steady-state */
-- 
2.53.0
Re: [PATCH 2/4] tools/xenstored: add support for "all domains" node permission
Posted by Jason Andryuk 3 months, 3 weeks ago
On 2026-04-23 04:08, Juergen Gross wrote:
> Add support for using DOMID_ANY in node permissions to indicate that
> all domains are allowed to access the node.
> 
> Add a new feature bit for indicating the support of DOMID_ANY.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>


> @@ -1754,8 +1755,12 @@ static bool chk_domain_generation(unsigned int domid, uint64_t gen)
>    * Allocate all missing struct domain referenced by a permission set.
>    * Any permission entries for not existing domains will be marked to be
>    * ignored.
> + * An DOMID_ANY entry will be marked to be ignored, if the writing

a/An/A/

> + * domain doesn't have the XENSTORE_SERVER_FEATURE_DOMID_ANY enabled. Note
> + * that Xen tools will never set DOMID_ANY for a guest owned node.

I'm not sure about stating "Xen tools will never" here since it may 
change in the future, and will not be updated.  Maybe "Note that Xen 
tools don't currently set DOMID_ANY for a guest owned node, and they 
aren't expected in the future."?  But maybe just drop it since I'm not 
seeing much value.

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

Thanks,
Jason
Re: [PATCH 2/4] tools/xenstored: add support for "all domains" node permission
Posted by Jürgen Groß 3 months, 2 weeks ago
On 28.04.26 00:00, Jason Andryuk wrote:
> On 2026-04-23 04:08, Juergen Gross wrote:
>> Add support for using DOMID_ANY in node permissions to indicate that
>> all domains are allowed to access the node.
>>
>> Add a new feature bit for indicating the support of DOMID_ANY.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
> 
> 
>> @@ -1754,8 +1755,12 @@ static bool chk_domain_generation(unsigned int domid, 
>> uint64_t gen)
>>    * Allocate all missing struct domain referenced by a permission set.
>>    * Any permission entries for not existing domains will be marked to be
>>    * ignored.
>> + * An DOMID_ANY entry will be marked to be ignored, if the writing
> 
> a/An/A/

Yes.

> 
>> + * domain doesn't have the XENSTORE_SERVER_FEATURE_DOMID_ANY enabled. Note
>> + * that Xen tools will never set DOMID_ANY for a guest owned node.
> 
> I'm not sure about stating "Xen tools will never" here since it may change in 
> the future, and will not be updated.  Maybe "Note that Xen tools don't currently 
> set DOMID_ANY for a guest owned node, and they aren't expected in the future."?  
> But maybe just drop it since I'm not seeing much value.

The main idea is that a guest not knowing about the DOMID_ANY feature should
never see it in the permissions of a Xenstore node it is owning.

So IMO it is a mandatory restriction for Xen tools not to set it for nodes
the guest is owning.

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

Thanks,


Juergen