Grant table for console, xenstore pages

I'm working on moving xenstored into a dedicated, unprivileged domain. This is the first set of patches I'm sending out towards that goal. I understand there is currently a freeze, so I'm just looking for feedback at this point.

Each domU shares one of its pages with the xenstore daemon from its creation. The domain builder writes the mfn for this page in the domU's start info page. Then it sends the xenstore daemon an "introduce" command, giving it the new domU's domid, this mfn to map, and an unbound port in the domU to bind.

However, if the xenstore daemon resides in an unprivileged domain, it is not permitted to map an arbitrary mfn. Instead, it could use the existing grant table mechanism. In fact, the first 8 grant table entries for each domU are reserved for cases like this. (DomU's don't use the first 8 entries.)

Because the console and the xenstore mechanisms are so similar, these patches include analogous changes for console support as well.

The first patch claims one grant entry for the console and another for the xenstore. It modifies the builder to fill in the grant table entries for the console and the xenstore. At this stage, the grant entries just give access to domain 0 (addressed in a later patch).

The next two patches modify the xenstore daemon and the console daemon, respectively, to use xc_gnttab_map_grant_ref instead of xc_map_foreign_range.

The final two patches implement a way to determine in which domains the console and xenstore daemons reside. If each of the files /var/run/{console,xenstore}.did contains an integer, this integer is interpreted as the domain id for that daemon. The default or fallback is domid=0, of course. In patch 4, libxc is modified to use this mechanism for the grant table entries. In patch 5, xend is modified to use this mechanism for the allocated unbound ports.

To get the discussion going, what should be done about xenstore's /local/domain/#/device/{console,store}/ring-ref ? I don't think they're necessary anymore, but I've made no effort to remove them.

Thanks,
Diego Ongaro

This patch claims one grant entry for the console and another for the xenstore. It modifies the builder to fill in the grant table entries for the console and the xenstore. At this stage, the grant entries just give access to domain 0 (addressed in a later patch).

---

diff -r b61cab9b4213 tools/libxc/xc_dom.h
@@ -179,6 +179,7 @@
                            xen_pfn_t count);
 int xc_dom_boot_image(struct xc_dom_image *dom);
 int xc_dom_compat_check(struct xc_dom_image *dom);
+int xc_dom_gnttab_init(struct xc_dom_image *dom);
 
 /* --- debugging bits ---------------------------------------------- */
 

diff -r b61cab9b4213 tools/libxc/xc_dom_compat_linux.c
@@ -47,6 +47,8 @@
     if ( (rc = xc_dom_build_image(dom)) != 0 )
         goto out;
     if ( (rc = xc_dom_boot_image(dom)) != 0 )
+        goto out;
+    if ( (rc = xc_dom_gnttab_init(dom)) != 0)
         goto out;
 
     *console_mfn = xc_dom_p2m_host(dom, dom->console_pfn);

diff -r b61cab9b4213 xen/include/public/grant_table.h
@@ -101,6 +101,12 @@
     uint32_t frame;
 };
 typedef struct grant_entry grant_entry_t;
+
+/* External tools reserve first few grant table entries. */
+#define GNTTAB_NR_RESERVED_ENTRIES     8
+#define GNTTAB_RESERVED_CONSOLE        0
+#define GNTTAB_RESERVED_XENSTORE       1
+/* (the next 6 are reserved for future use) */
 
 /*
  * Type of grant entry.

This patch modifies the xenstore daemon to use xc_gnttab_map_grant_ref instead of xc_map_foreign_range.

Make sure you have linux-2.6.18-xen.hg changeset 600:1bd3dbfdaf0f before running this.

---

diff -r e4e473425d0f tools/xenstore/xenstored_domain.c
@@ -31,8 +31,10 @@
 #include "xenstored_watch.h"
 
 #include <xenctrl.h>
+#include <xen/grant_table.h>
 
 static int *xc_handle;
+static int *xcg_handle;
 static evtchn_port_t virq_port;
 
 int xce_handle = -1; 

@@ -343,13 +345,14 @@
 	domain = find_domain_by_domid(domid);
 
 	if (domain == NULL) {
-		interface = xc_map_foreign_range(
-			*xc_handle, domid,
-			getpagesize(), PROT_READ|PROT_WRITE, mfn);
+		interface = xc_gnttab_map_grant_ref(*xcg_handle, domid,
+		                                    GNTTAB_RESERVED_XENSTORE,
+		                                    PROT_READ|PROT_WRITE);
 		if (!interface) {
 			send_error(conn, errno);
 			return;
 		}
+
 		/* Hang domain off "in" until we're finished. */
 		domain = new_domain(in, domid, port);
 		if (!domain) {

@@ -542,6 +545,12 @@
 	return 0;
 }
 
+static int close_xcg_handle(void *_handle)
+{
+	xc_gnttab_close(*(int *)_handle);
+	return 0;
+}
+
 /* Returns the implicit path of a connection (only domains have this) */
 const char *get_implicit_path(const struct connection *conn)
 {

@@ -593,6 +602,16 @@
 		barf_perror("Failed to open connection to hypervisor");
 
 	talloc_set_destructor(xc_handle, close_xc_handle);
+	
+	xcg_handle = talloc(talloc_autofree_context(), int);
+	if (!xcg_handle)
+		barf_perror("Failed to allocate domain gnttab handle");
+
+	*xcg_handle = xc_gnttab_open();
+	if (*xcg_handle < 0)
+		barf_perror("Failed to open gnttab connection to hypervisor");
+
+	talloc_set_destructor(xcg_handle, close_xcg_handle);
 
 	xce_handle = xc_evtchn_open();

This patch modifies the console daemon to use xc_gnttab_map_grant_ref instead of xc_map_foreign_range.

Make sure you have linux-2.6.18-xen.hg changeset 600:1bd3dbfdaf0f before running this.

---

diff -r e0be727aa133 tools/console/daemon/io.c
@@ -25,6 +25,7 @@
 #include <xs.h>
 #include <xen/io/console.h>
 #include <xenctrl.h>
+#include <xen/grant_table.h>
 
 #include <stdlib.h>
 #include <errno.h>

@@ -540,10 +541,9 @@
 	if (ring_ref != dom->ring_ref) {
 		if (dom->interface != NULL)
 			munmap(dom->interface, getpagesize());
-		dom->interface = xc_map_foreign_range(
-			xc, dom->domid, getpagesize(),
-			PROT_READ|PROT_WRITE,
-			(unsigned long)ring_ref);
+		dom->interface = xc_gnttab_map_grant_ref(xcg, dom->domid,
+		                                         GNTTAB_RESERVED_CONSOLE,
+							 PROT_READ|PROT_WRITE);
 		if (dom->interface == NULL) {
 			err = EINVAL;
 			goto out;

diff -r e0be727aa133 tools/console/daemon/utils.c
@@ -37,8 +37,9 @@
 #include "xenctrl.h"
 #include "utils.h"
 
-struct xs_handle *xs;
-int xc;
+struct xs_handle *xs = NULL;
+int xc = -1;
+int xcg = -1;
 
 static void child_exit(int sig)
 {

@@ -122,6 +123,12 @@
 		goto out;
 	}
 
+	xcg = xc_gnttab_open();
+	if (xcg == -1) {
+		dolog(LOG_ERR, "Failed to open gnttab (%m)");
+		goto out;
+	}
+
 	if (!xs_watch(xs, "@introduceDomain", "domlist")) {
 		dolog(LOG_ERR, "xenstore watch on @introduceDomain fails.");
 		goto out;

@@ -139,6 +146,8 @@
 		xs_daemon_close(xs);
 	if (xc != -1)
 		xc_interface_close(xc);
+	if (xcg != -1)
+		xc_gnttab_close(xcg);
 	return false;
 }
 

diff -r e0be727aa133 tools/console/daemon/utils.h
@@ -32,6 +32,7 @@
 
 extern struct xs_handle *xs;
 extern int xc;
+extern int xcg;
 
 #if 1
 #define dolog(val, fmt, ...) do {				\

If each of the files /var/run/{console,xenstore}.did contains an integer, this integer is interpreted as the domain id for that daemon. The default or fallback is domid=0, of course. In this patch, libxc is modified to use this mechanism for the grant table entries.

---

diff -r 581c5525e1ac tools/libxc/xc_dom_boot.c
@@ -297,14 +297,40 @@
     return mfn;
 }
 
-/* TODO: don't hard-code these */
-#define CONSOLE_DOMID 0
-#define XENSTORE_DOMID 0
+static uint32_t read_domid(const char *service)
+{
+    char buf[64];
+    int n;
+    FILE *f = NULL;
+    uint32_t domid = 0;
+    
+    n = snprintf(buf, sizeof(buf), "/var/run/%s.did", service);
+    if ( n < 0 || n >= sizeof(buf) )
+        goto out;
+
+    f = fopen(buf, "r");
+    if ( f == NULL )
+        goto out;
+
+    if ( fscanf(f, "%d", &domid) != 1 )
+        goto out;
+
+out:
+    if ( f != NULL )
+        fclose(f);
+
+    return domid;
+}
 
 int xc_dom_gnttab_init(struct xc_dom_image *dom)
 {
+    uint32_t console_domid;
+    uint32_t xenstore_domid;
     unsigned long grant_table_mfn;
     grant_entry_t *grant_table;
+
+    console_domid = read_domid("console");
+    xenstore_domid = read_domid("xenstore");
 
     grant_table_mfn = xc_dom_gnttab_mfn(dom);
     if ( grant_table_mfn == -1 )

@@ -324,17 +350,17 @@
         return -1;
     }
 
-    if ( dom->guest_domid != CONSOLE_DOMID )
+    if ( dom->guest_domid != console_domid )
     {
         grant_table[GNTTAB_RESERVED_CONSOLE].flags = GTF_permit_access;
-        grant_table[GNTTAB_RESERVED_CONSOLE].domid = CONSOLE_DOMID;
+        grant_table[GNTTAB_RESERVED_CONSOLE].domid = console_domid;
         grant_table[GNTTAB_RESERVED_CONSOLE].frame = xc_dom_p2m_host(dom, dom->console_pfn);
     }
 
-    if ( dom->guest_domid != XENSTORE_DOMID )
+    if ( dom->guest_domid != xenstore_domid )
     {
         grant_table[GNTTAB_RESERVED_XENSTORE].flags = GTF_permit_access;
-        grant_table[GNTTAB_RESERVED_XENSTORE].domid = XENSTORE_DOMID;
+        grant_table[GNTTAB_RESERVED_XENSTORE].domid = xenstore_domid;
         grant_table[GNTTAB_RESERVED_XENSTORE].frame = xc_dom_p2m_host(dom, dom->xenstore_pfn);
     }

If each of the files /var/run/{console,xenstore}.did contains an integer, this integer is interpreted as the domain id for that daemon. The default or fallback is domid=0, of course. In this patch, xend is modified to use this mechanism for the allocated unbound ports.

---

diff -r 581c5525e1ac tools/python/xen/xend/XendDomainInfo.py
@@ -2427,17 +2427,26 @@
     def _createChannels(self):
         """Create the channels to the domain.
         """
-        self.store_port = self._createChannel()
-        self.console_port = self._createChannel()
+        def read_domid(service):
+            try:
+                f = open("/var/run/%s.did" % service)
+                try:
+                    return int(f.read())
+                finally:
+                    f.close()
+            except (IOError, ValueError):
+                return 0 # assume domain 0 is providing service
 
+        self.store_port = self._createChannel(read_domid("xenstore"))
+        self.console_port = self._createChannel(read_domid("console"))
 
-    def _createChannel(self):
+    def _createChannel(self, remote_dom):
         """Create an event channel to the domain.
         """
         try:
             if self.domid != None:
                 return xc.evtchn_alloc_unbound(domid = self.domid,
-                                               remote_dom = 0)
+                                               remote_dom = remote_dom)
         except:
             log.exception("Exception in alloc_unbound(%s)", str(self.domid))
             raise

Hi Diego,

I imagine you've already ready seen this, but the thread beginning here has some historical context:

http://lists.xensource.com/archives/html/xense-devel/2007-05/msg00004.html

So the main points to note will be:
* The patches won't work on Dom0s without gntdev support (e.g. at present Solaris, or older XenoLinux kernels). I'm not sure of the current status but a question mark still hangs over gntdev in pv_ops-Dom0, though your patches might give some impetus to find a solution.
* Have you tested with domain save/restore? Apparently (per
http://lists.xensource.com/archives/html/xense-devel/2007-05/msg00011.html
), the grant table is reset upon restore, so you may need to reinstall the grants for the console and XenStore.
* There was some argument against fixed grant references for the console and store pages, though this would require modifying the interface to the domain builder.

I would imagine, considering the previous discussion, that you may want to retain the existing ring-ref values, and it should be possible to continue with the old method. Apart from that, the idea and implementation look pretty sound!

Regards,

Derek Murray.

Just to let you know, Dereck Murray is a quite active academic person working on dom0 disaggregation ;)

Samuel

No, I didn't know about that thread. It's disappointing to see that I've duplicated your efforts in patches 1-3 of my series.

Did you ever finish polishing those patches? Based on the discussion, there were just a couple things left to clean up.

Have you also worked on this, Derek? I wouldn't want to keep working on something you've already done...

-Diego

On the contrary, I think this is a much nicer approach - especially as it doesn't require any modifications to the hypervisor. Therefore I think it will be a better fit for the mainline repository.

If I remember correctly, I did do some more work on those patches to make the use of gntdev optional, but I don't think they were ready for prime-time. I'll look them out and send them to you off list, in case they might be of any use.

I haven't worked on this myself, but I vaguely recall hearing of efforts to disaggregate XenStore - I don't think any of these are publicly available. Is the main aim of this work to enhance security or performance? If the former, how do you plan to launch the XenStore domain? From Dom0, or using another mechanism?

My personal inclination is to enhance Xen so that the tools no longer run as root (a conventional Unix-based privilege separation), which provides a low-cost improvement in Dom0 security. This would build on your patches to use gntdev for console and XenStore access, and use modifications to gntdev that allow non-root users to map certain explicitly-specified grants. This would provide a route to disaggregating all necessarily-trusted functionality on systems that would benefit from it (i.e. IOMMU-equipped systems). If you'd like, we could discuss this approach further.

Regards,

Derek Murray.

Enhancing security is one aim of this work.

I'm launching the XenStore domain using a small program in dom0 that just makes the necessary libxc calls. I couldn't really use xend, xm, or xenconsoled as they all depend on xenstore. (However, I ripped out the main loop of xenconsoled so that I'd be able to get at a console.)

I think that approach definitely makes sense for something like the console daemon, which I would argue should stay in dom0. On the other hand, I don't see any technical reasons why XenStore needs to stay in dom0, and I don't think it's such a high-cost improvement to move it out.

-Diego

This has been looked at by others as well (I had a very similar set of internal patches that created a pre-dom0 domain but for running the vTPM Manager). The trickiest part of deciding where and when to launch xenstore is that it is required for the paravirt drivers to communicate. So if you have any front ends, then xenstore needs to be running before they can connect to their back ends.

Joe

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Diego Ongaro
Sent: Monday, July 14, 2008 8:42 AM
To: [email protected]
Cc: [email protected]
Subject: Re: [Xen-devel] [PATCH RFC 0/5] Grant table for console, xenstorepages

domain.

on

Enhancing security is one aim of this work.

I'm launching the XenStore domain using a small program in dom0 that just makes the necessary libxc calls. I couldn't really use xend, xm, or xenconsoled as they all depend on xenstore. (However, I ripped out the main loop of xenconsoled so that I'd be able to get at a console.)

I think that approach definitely makes sense for something like the console daemon, which I would argue should stay in dom0. On the other hand, I don't see any technical reasons why XenStore needs to stay in dom0, and I don't think it's such a high-cost improvement to move it out.

-Diego

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

I'm launching the xenstore domain before starting any other domains and before starting xend. I just had to make dom0's xenbus driver finish its initialization later.

-Diego