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
|
From 6f76ac2cd36f4cff22830cf34f8b8fecc6ce54e8 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Fri, 29 Apr 2016 16:19:28 +0100
Subject: [PATCH 01/14] libxl: Make copy of every xs backend in /libxl in
_generic_add
We want to stop libxl trustingly reading information from the backend
directory (since this is, of course, writeable by the backend, which
might be a semi-trusted driver domain).
In principle it is wrong in current libxl for anything to try to
divine virtual device configuration from xenstore: the JSON domain
config ought to supply that, and xenstore should only tell us which
devices actually exist.
However:
Firstly, there are several existing places where configuration
information is retrieved from xenstore rather than JSON. We do not
want to reen gineer this in a security patch.
Secondly, we want to make a security patch which can be backported to
versions of libxl without the JSON configuration machinery.
So we take the expedient approach of keeping a copy of the
configuration somewhere we trust, namely /libxl. This is obviously
fairly low-risk, although it does write significantly more keys in
xenstore.
In this patch we make this change in libxl__device_generic_add. This
is responsible for actually writing the vast majority of device
information to xenstore. There are a few loose ends which will be
dealt with in a moment.
Likewise, changes to readers to use the new location will appear in
further patches.
This is part of XSA-178.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
docs/misc/xenstore-paths.markdown | 4 ++++
tools/libxl/libxl_device.c | 23 +++++++++++++++++++++++
2 files changed, 27 insertions(+)
diff --git a/docs/misc/xenstore-paths.markdown b/docs/misc/xenstore-paths.markdown
index e018b55..c2e2842 100644
--- a/docs/misc/xenstore-paths.markdown
+++ b/docs/misc/xenstore-paths.markdown
@@ -396,6 +396,10 @@ Path in xenstore to the frontend, normally
Path in xenstore to the backend, normally
/local/domain/$BACKEND_DOMID/backend/$KIND/$DOMID/$DEVID
+#### /libxl/$DOMID/device/$KIND/$DEVID/$NODE
+
+Trustworthy copy of /local/domain/$DOMID/backend/$KIND/$DEVID/$NODE.
+
#### /libxl/$DOMID/dm-version ("qemu\_xen"|"qemu\_xen\_traditional") = [n,INTERNAL]
The device model version for a domain.
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index e7650d0..501a140 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -167,6 +167,29 @@ retry_transaction:
xs_write(ctx->xsh, t, GCSPRINTF("%s/frontend", backend_path),
frontend_path, strlen(frontend_path));
libxl__xs_writev(gc, t, backend_path, bents);
+
+ /*
+ * We make a copy of everything for the backend in the libxl
+ * path as well. This means we don't need to trust the
+ * backend. Ideally this information would not be used and we
+ * would use the information from the json configuration
+ * instead. But there are still places in libxl that try to
+ * reconstruct a config from xenstore.
+ *
+ * This duplication will typically produces duplicate keys
+ * which will go out of date, but that's OK because nothing
+ * reads those. For example, there is usually
+ * /libxl/$guest/device/$kind/$devid/state
+ * which starts out containing XenbusStateInitialising ("1")
+ * just like the copy in
+ * /local/domain/$driverdom/backend/$guest/$kind/$devid/state
+ * but which won't ever be updated.
+ *
+ * This duplication is superfluous and messy but as discussed
+ * the proper fix is more intrusive than we want to do now.
+ */
+ rc = libxl__xs_writev(gc, t, libxl_path, bents);
+ if (rc) goto out;
}
if (!create_transaction)
--
1.9.1
|
