Fix handling of the entries-per-domain quota. Entries which are created by

the guest but deleted by dom0 were remaining accounted against the guest,
which meant that the guest would eventually run out of quota.

This patch also prevents unprivileged domains from changing the owner of a
node.  One guest could attack another by creating nodes and then transferring
them to the ownership of another, and though the accounting could be made to
work properly in this case, domains should never be transferring nodes in any
case, so it seems safer just to disallow the operation entirely.

Signed-off-by: Ewan Mellor <ewan@xensource.com>

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 3f304b0e417c94be23d2cbd0e65846916d7b9d78..5ac4dd3b0f12627579e9f8a84f028e9f563d77d9 100644 (file)
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -791,7 +791,7 @@ static void delete_node_single(struct connection *conn, struct node *node)
                corrupt(conn, "Could not delete '%s'", node->name);
                return;
        }
-       domain_entry_dec(conn);
+       domain_entry_dec(conn, node);
 }
 
 /* Must not be / */
@@ -842,7 +842,7 @@ static struct node *construct_node(struct connection *conn, const char *name)
        node->children = node->data = NULL;
        node->childlen = node->datalen = 0;
        node->parent = parent;
-       domain_entry_inc(conn);
+       domain_entry_inc(conn, node);
        return node;
 }
 
@@ -878,7 +878,7 @@ static struct node *create_node(struct connection *conn,
         * something goes wrong. */
        for (i = node; i; i = i->parent) {
                if (!write_node(conn, i)) {
-                       domain_entry_dec(conn);
+                       domain_entry_dec(conn, i);
                        return NULL;
                }
                talloc_set_destructor(i, destroy_node);
@@ -1108,6 +1108,7 @@ static void do_get_perms(struct connection *conn, const char *name)
 static void do_set_perms(struct connection *conn, struct buffered_data *in)
 {
        unsigned int num;
+       struct xs_permissions *perms;
        char *name, *permstr;
        struct node *node;
 
@@ -1129,12 +1130,24 @@ static void do_set_perms(struct connection *conn, struct buffered_data *in)
                return;
        }
 
-       node->perms = talloc_array(node, struct xs_permissions, num);
-       node->num_perms = num;
-       if (!xs_strings_to_perms(node->perms, num, permstr)) {
+       perms = talloc_array(node, struct xs_permissions, num);
+       if (!xs_strings_to_perms(perms, num, permstr)) {
                send_error(conn, errno);
                return;
        }
+
+       /* Unprivileged domains may not change the owner. */
+       if (domain_is_unprivileged(conn) &&
+           perms[0].id != node->perms[0].id) {
+               send_error(conn, EPERM);
+               return;
+       }
+
+       domain_entry_dec(conn, node);
+       node->perms = perms;
+       node->num_perms = num;
+       domain_entry_inc(conn, node);
+
        if (!write_node(conn, node)) {
                send_error(conn, errno);
                return;

diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
index 40cc20386ad018f955f0c1bb58b5a17aa8f06b1e..d21ae7b9c70c40afba6c073dcf8c9faba83a176e 100644 (file)
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -501,18 +501,35 @@ int domain_init(void)
        return xce_handle;
 }
 
-void domain_entry_inc(struct connection *conn)
+void domain_entry_inc(struct connection *conn, struct node *node)
 {
-       if (!conn || !conn->domain)
+       struct domain *d;
+
+       if (!conn)
                return;
-       conn->domain->nbentry++;
+
+       if (node->perms && node->perms[0].id != conn->id) {
+               d = find_domain_by_domid(node->perms[0].id);
+               if (d)
+                       d->nbentry++;
+       }
+       else if (conn->domain) {
+               conn->domain->nbentry++;
+       }
 }
 
-void domain_entry_dec(struct connection *conn)
+void domain_entry_dec(struct connection *conn, struct node *node)
 {
-       if (!conn || !conn->domain)
+       struct domain *d;
+
+       if (!conn)
                return;
-       if (conn->domain->nbentry)
+
+       if (node->perms && node->perms[0].id != conn->id) {
+               d = find_domain_by_domid(node->perms[0].id);
+               if (d && d->nbentry)
+                       d->nbentry--;
+       } else if (conn->domain && conn->domain->nbentry)
                conn->domain->nbentry--;
 }
 

diff --git a/tools/xenstore/xenstored_domain.h b/tools/xenstore/xenstored_domain.h
index 38f26b52de0632f7e6ac17a71552d599995a2eee..4acf61bbacc4deabbd77a1e36bf1d6ed74f4510a 100644 (file)
--- a/tools/xenstore/xenstored_domain.h
+++ b/tools/xenstore/xenstored_domain.h
@@ -50,8 +50,8 @@ bool domain_can_write(struct connection *conn);
 bool domain_is_unprivileged(struct connection *conn);
 
 /* Quota manipulation */
-void domain_entry_inc(struct connection *conn);
-void domain_entry_dec(struct connection *conn);
+void domain_entry_inc(struct connection *conn, struct node *);
+void domain_entry_dec(struct connection *conn, struct node *);
 int domain_entry(struct connection *conn);
 void domain_watch_inc(struct connection *conn);
 void domain_watch_dec(struct connection *conn);