summaryrefslogtreecommitdiff
blob: 7cfb08bacb32c2f8a170d523a9d665d699b2f4df (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
From 036fa8717b316a10b67ea8cf4d5dd200ac2b29af Mon Sep 17 00:00:00 2001
From: Juergen Gross <jgross@suse.com>
Date: Tue, 13 Sep 2022 07:35:10 +0200
Subject: [PATCH 71/87] 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.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit ab128218225d3542596ca3a02aee80d55494bef8)
---
 tools/xenstore/xenstored_core.c   |  5 +++++
 tools/xenstore/xenstored_domain.c | 37 +++++++++++++++++++++----------
 tools/xenstore/xenstored_domain.h |  1 +
 3 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 411cc0e44714..c676ee4e4e4f 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -1757,6 +1757,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 fb732d0a14c3..e2f1b09c6037 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -875,7 +875,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
@@ -883,20 +882,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;
 }
 
 /*
@@ -909,8 +926,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) {
@@ -927,8 +942,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 b9e152890149..40fe5f690900 100644
--- a/tools/xenstore/xenstored_domain.h
+++ b/tools/xenstore/xenstored_domain.h
@@ -62,6 +62,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 *);
-- 
2.37.4