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
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
|
From 8271f0e8f23b63199caf0edcfe85ebc1c1412d1b Mon Sep 17 00:00:00 2001
From: Demi Marie Obenour <demi@invisiblethingslab.com>
Date: Tue, 21 May 2024 10:23:52 +0200
Subject: [PATCH 14/56] libxl: Fix handling XenStore errors in device creation
If xenstored runs out of memory it is possible for it to fail operations
that should succeed. libxl wasn't robust against this, and could fail
to ensure that the TTY path of a non-initial console was created and
read-only for guests. This doesn't qualify for an XSA because guests
should not be able to run xenstored out of memory, but it still needs to
be fixed.
Add the missing error checks to ensure that all errors are properly
handled and that at no point can a guest make the TTY path of its
frontend directory writable.
Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
master commit: 531d3bea5e9357357eaf6d40f5784a1b4c29b910
master date: 2024-05-11 00:13:43 +0100
---
tools/libs/light/libxl_console.c | 11 ++---
tools/libs/light/libxl_device.c | 72 ++++++++++++++++++++------------
tools/libs/light/libxl_xshelp.c | 13 ++++--
3 files changed, 60 insertions(+), 36 deletions(-)
diff --git a/tools/libs/light/libxl_console.c b/tools/libs/light/libxl_console.c
index cd7412a327..a563c9d3c7 100644
--- a/tools/libs/light/libxl_console.c
+++ b/tools/libs/light/libxl_console.c
@@ -351,11 +351,10 @@ int libxl__device_console_add(libxl__gc *gc, uint32_t domid,
flexarray_append(front, "protocol");
flexarray_append(front, LIBXL_XENCONSOLE_PROTOCOL);
}
- libxl__device_generic_add(gc, XBT_NULL, device,
- libxl__xs_kvs_of_flexarray(gc, back),
- libxl__xs_kvs_of_flexarray(gc, front),
- libxl__xs_kvs_of_flexarray(gc, ro_front));
- rc = 0;
+ rc = libxl__device_generic_add(gc, XBT_NULL, device,
+ libxl__xs_kvs_of_flexarray(gc, back),
+ libxl__xs_kvs_of_flexarray(gc, front),
+ libxl__xs_kvs_of_flexarray(gc, ro_front));
out:
return rc;
}
@@ -665,6 +664,8 @@ int libxl_device_channel_getinfo(libxl_ctx *ctx, uint32_t domid,
*/
if (!val) val = "/NO-SUCH-PATH";
channelinfo->u.pty.path = strdup(val);
+ if (channelinfo->u.pty.path == NULL)
+ abort();
break;
default:
break;
diff --git a/tools/libs/light/libxl_device.c b/tools/libs/light/libxl_device.c
index 13da6e0573..3035501f2c 100644
--- a/tools/libs/light/libxl_device.c
+++ b/tools/libs/light/libxl_device.c
@@ -177,8 +177,13 @@ int libxl__device_generic_add(libxl__gc *gc, xs_transaction_t t,
ro_frontend_perms[1].perms = backend_perms[1].perms = XS_PERM_READ;
retry_transaction:
- if (create_transaction)
+ if (create_transaction) {
t = xs_transaction_start(ctx->xsh);
+ if (t == XBT_NULL) {
+ LOGED(ERROR, device->domid, "xs_transaction_start failed");
+ return ERROR_FAIL;
+ }
+ }
/* FIXME: read frontend_path and check state before removing stuff */
@@ -195,42 +200,55 @@ retry_transaction:
if (rc) goto out;
}
- /* xxx much of this function lacks error checks! */
-
if (fents || ro_fents) {
- xs_rm(ctx->xsh, t, frontend_path);
- xs_mkdir(ctx->xsh, t, frontend_path);
+ if (!xs_rm(ctx->xsh, t, frontend_path) && errno != ENOENT)
+ goto out;
+ if (!xs_mkdir(ctx->xsh, t, frontend_path))
+ goto out;
/* Console 0 is a special case. It doesn't use the regular PV
* state machine but also the frontend directory has
* historically contained other information, such as the
* vnc-port, which we don't want the guest fiddling with.
*/
if ((device->kind == LIBXL__DEVICE_KIND_CONSOLE && device->devid == 0) ||
- (device->kind == LIBXL__DEVICE_KIND_VUART))
- xs_set_permissions(ctx->xsh, t, frontend_path,
- ro_frontend_perms, ARRAY_SIZE(ro_frontend_perms));
- else
- xs_set_permissions(ctx->xsh, t, frontend_path,
- frontend_perms, ARRAY_SIZE(frontend_perms));
- xs_write(ctx->xsh, t, GCSPRINTF("%s/backend", frontend_path),
- backend_path, strlen(backend_path));
- if (fents)
- libxl__xs_writev_perms(gc, t, frontend_path, fents,
- frontend_perms, ARRAY_SIZE(frontend_perms));
- if (ro_fents)
- libxl__xs_writev_perms(gc, t, frontend_path, ro_fents,
- ro_frontend_perms, ARRAY_SIZE(ro_frontend_perms));
+ (device->kind == LIBXL__DEVICE_KIND_VUART)) {
+ if (!xs_set_permissions(ctx->xsh, t, frontend_path,
+ ro_frontend_perms, ARRAY_SIZE(ro_frontend_perms)))
+ goto out;
+ } else {
+ if (!xs_set_permissions(ctx->xsh, t, frontend_path,
+ frontend_perms, ARRAY_SIZE(frontend_perms)))
+ goto out;
+ }
+ if (!xs_write(ctx->xsh, t, GCSPRINTF("%s/backend", frontend_path),
+ backend_path, strlen(backend_path)))
+ goto out;
+ if (fents) {
+ rc = libxl__xs_writev_perms(gc, t, frontend_path, fents,
+ frontend_perms, ARRAY_SIZE(frontend_perms));
+ if (rc) goto out;
+ }
+ if (ro_fents) {
+ rc = libxl__xs_writev_perms(gc, t, frontend_path, ro_fents,
+ ro_frontend_perms, ARRAY_SIZE(ro_frontend_perms));
+ if (rc) goto out;
+ }
}
if (bents) {
if (!libxl_only) {
- xs_rm(ctx->xsh, t, backend_path);
- xs_mkdir(ctx->xsh, t, backend_path);
- xs_set_permissions(ctx->xsh, t, backend_path, backend_perms,
- ARRAY_SIZE(backend_perms));
- xs_write(ctx->xsh, t, GCSPRINTF("%s/frontend", backend_path),
- frontend_path, strlen(frontend_path));
- libxl__xs_writev(gc, t, backend_path, bents);
+ if (!xs_rm(ctx->xsh, t, backend_path) && errno != ENOENT)
+ goto out;
+ if (!xs_mkdir(ctx->xsh, t, backend_path))
+ goto out;
+ if (!xs_set_permissions(ctx->xsh, t, backend_path, backend_perms,
+ ARRAY_SIZE(backend_perms)))
+ goto out;
+ if (!xs_write(ctx->xsh, t, GCSPRINTF("%s/frontend", backend_path),
+ frontend_path, strlen(frontend_path)))
+ goto out;
+ rc = libxl__xs_writev(gc, t, backend_path, bents);
+ if (rc) goto out;
}
/*
@@ -276,7 +294,7 @@ retry_transaction:
out:
if (create_transaction && t)
libxl__xs_transaction_abort(gc, &t);
- return rc;
+ return rc != 0 ? rc : ERROR_FAIL;
}
typedef struct {
diff --git a/tools/libs/light/libxl_xshelp.c b/tools/libs/light/libxl_xshelp.c
index 751cd942d9..a6e34ab10f 100644
--- a/tools/libs/light/libxl_xshelp.c
+++ b/tools/libs/light/libxl_xshelp.c
@@ -60,10 +60,15 @@ int libxl__xs_writev_perms(libxl__gc *gc, xs_transaction_t t,
for (i = 0; kvs[i] != NULL; i += 2) {
path = GCSPRINTF("%s/%s", dir, kvs[i]);
if (path && kvs[i + 1]) {
- int length = strlen(kvs[i + 1]);
- xs_write(ctx->xsh, t, path, kvs[i + 1], length);
- if (perms)
- xs_set_permissions(ctx->xsh, t, path, perms, num_perms);
+ size_t length = strlen(kvs[i + 1]);
+ if (length > UINT_MAX)
+ return ERROR_FAIL;
+ if (!xs_write(ctx->xsh, t, path, kvs[i + 1], length))
+ return ERROR_FAIL;
+ if (perms) {
+ if (!xs_set_permissions(ctx->xsh, t, path, perms, num_perms))
+ return ERROR_FAIL;
+ }
}
}
return 0;
--
2.45.2
|
GitWeb