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 8012324cb9e676bd342a5adfda1700525f195e2e Mon Sep 17 00:00:00 2001
From: Juergen Gross <jgross@suse.com>
Date: Tue, 13 Sep 2022 07:35:10 +0200
Subject: [PATCH 110/126] 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 806f24bbab8b..8aecd425f274 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -1723,6 +1723,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 d262f4e9dbdf..8b503c2dfe07 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -881,7 +881,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
@@ -889,20 +888,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;
}
/*
@@ -915,8 +932,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) {
@@ -933,8 +948,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 da513443cd46..0b4f56b8146c 100644
--- a/tools/xenstore/xenstored_domain.h
+++ b/tools/xenstore/xenstored_domain.h
@@ -66,6 +66,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
|
GitWeb