From 67d5ecd609b8f12346eadb40e547cd7e01d825dc Mon Sep 17 00:00:00 2001

From: Juergen Gross <jgross@suse.com>

Date: Tue, 13 Sep 2022 07:35:10 +0200

Subject: tools/xenstore: fix checking node permissions

Today chk_domain_generation() is being used to check whether a node

permission entry is still valid or whether it is referring to a domain

no longer existing. This is done by comparing the node's and the

domain's generation count.

In case no struct domain is existing for a checked domain, but the

domain itself is valid, chk_domain_generation() assumes it is being

called due to the first node created for a new domain and it will

return success.

This might be wrong in case the checked permission is related to an

old domain, which has just been replaced with a new domain using the

same domid.

Fix that by letting chk_domain_generation() fail in case a struct

domain isn't found. In order to cover the case of the first node for

a new domain try to allocate the needed struct domain explicitly when

processing the related SET_PERMS command. In case a referenced domain

isn't existing, flag the related permission to be ignored right away.

This is XSA-417 / CVE-2022-42320.

Reported-by: Juergen Gross <jgross@suse.com>

Signed-off-by: Juergen Gross <jgross@suse.com>

Reviewed-by: Julien Grall <jgrall@amazon.com>

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c

index 66bbeaf6bfb0..a0c176fa203e 100644

--- a/tools/xenstore/xenstored_core.c

+++ b/tools/xenstore/xenstored_core.c

@@ -1753,6 +1753,11 @@ static int do_set_perms(const void *ctx, struct connection *conn,

 	if (!xs_strings_to_perms(perms.p, perms.num, permstr))

 		return errno;

+	if (domain_alloc_permrefs(&perms) < 0)

+		return ENOMEM;

+	if (perms.p[0].perms & XS_PERM_IGNORE)

+		return ENOENT;

+

 	/* First arg is node name. */

 	if (strstarts(in->buffer, "@")) {

 		if (set_perms_special(conn, in->buffer, &perms))

diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c

index b9ff4ded8360..98b401fdec30 100644

--- a/tools/xenstore/xenstored_domain.c

+++ b/tools/xenstore/xenstored_domain.c

@@ -907,7 +907,6 @@ int domain_entry_inc(struct connection *conn, struct node *node)

  * count (used for testing whether a node permission is older than a domain).

  *

  * Return values:

- * -1: error

  *  0: domain has higher generation count (it is younger than a node with the

  *     given count), or domain isn't existing any longer

  *  1: domain is older than the node

@@ -915,20 +914,38 @@ int domain_entry_inc(struct connection *conn, struct node *node)

 static int chk_domain_generation(unsigned int domid, uint64_t gen)

 {

 	struct domain *d;

-	xc_dominfo_t dominfo;

 	if (!xc_handle && domid == 0)

 		return 1;

 	d = find_domain_struct(domid);

-	if (d)

-		return (d->generation <= gen) ? 1 : 0;

-	if (!get_domain_info(domid, &dominfo))

-		return 0;

+	return (d && d->generation <= gen) ? 1 : 0;

+}

-	d = alloc_domain(NULL, domid);

-	return d ? 1 : -1;

+/*

+ * Allocate all missing struct domain referenced by a permission set.

+ * Any permission entries for not existing domains will be marked to be

+ * ignored.

+ */

+int domain_alloc_permrefs(struct node_perms *perms)

+{

+	unsigned int i, domid;

+	struct domain *d;

+	xc_dominfo_t dominfo;

+

+	for (i = 0; i < perms->num; i++) {

+		domid = perms->p[i].id;

+		d = find_domain_struct(domid);

+		if (!d) {

+			if (!get_domain_info(domid, &dominfo))

+				perms->p[i].perms |= XS_PERM_IGNORE;

+			else if (!alloc_domain(NULL, domid))

+				return ENOMEM;

+		}

+	}

+

+	return 0;

 }

 /*

@@ -941,8 +958,6 @@ int domain_adjust_node_perms(struct connection *conn, struct node *node)

 	int ret;

 	ret = chk_domain_generation(node->perms.p[0].id, node->generation);

-	if (ret < 0)

-		return errno;

 	/* If the owner doesn't exist any longer give it to priv domain. */

 	if (!ret) {

@@ -959,8 +974,6 @@ int domain_adjust_node_perms(struct connection *conn, struct node *node)

 			continue;

 		ret = chk_domain_generation(node->perms.p[i].id,

 					    node->generation);

-		if (ret < 0)

-			return errno;

 		if (!ret)

 			node->perms.p[i].perms |= XS_PERM_IGNORE;

 	}

diff --git a/tools/xenstore/xenstored_domain.h b/tools/xenstore/xenstored_domain.h

index 209442190911..7fe0a21d9e45 100644

--- a/tools/xenstore/xenstored_domain.h

+++ b/tools/xenstore/xenstored_domain.h

@@ -63,6 +63,7 @@ bool domain_is_unprivileged(struct connection *conn);

 /* Remove node permissions for no longer existing domains. */

 int domain_adjust_node_perms(struct connection *conn, struct node *node);

+int domain_alloc_permrefs(struct node_perms *perms);

 /* Quota manipulation */

 int domain_entry_inc(struct connection *conn, struct node *);