[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[xen master] tools/xenstore: introduce dummy nodes for special watch paths



commit d6bb63924fc28a5bba53f5151db5c77f937879a5
Author:     Juergen Gross <jgross@xxxxxxxx>
AuthorDate: Wed Jan 18 10:50:04 2023 +0100
Commit:     Julien Grall <jgrall@xxxxxxxxxx>
CommitDate: Fri Jan 20 09:23:51 2023 +0000

    tools/xenstore: introduce dummy nodes for special watch paths
    
    Instead of special casing the permission handling and watch event
    firing for the special watch paths "@introduceDomain" and
    "@releaseDomain", use static dummy nodes added to the data base when
    starting Xenstore.
    
    The node accounting needs to reflect that change by adding the special
    nodes in the domain_entry_fix() call in setup_structure().
    
    Note that this requires to rework the calls of fire_watches() for the
    special events in order to avoid leaking memory.
    
    Move the check for a valid node name from get_node() to
    get_node_canonicalized(), as it allows to use get_node() for the
    special nodes, too.
    
    In order to avoid read and write accesses to the special nodes use a
    special variant for obtaining the current node data for the permission
    handling.
    
    This allows to simplify quite some code. In future sub-nodes of the
    special nodes will be possible due to this change, allowing more fine
    grained permission control of special events for specific domains.
    
    Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
    Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>
---
 tools/xenstore/xenstored_control.c |   3 -
 tools/xenstore/xenstored_core.c    |  94 +++++++++++++--------
 tools/xenstore/xenstored_domain.c  | 164 ++++++-------------------------------
 tools/xenstore/xenstored_domain.h  |   6 --
 tools/xenstore/xenstored_watch.c   |  17 +---
 5 files changed, 83 insertions(+), 201 deletions(-)

diff --git a/tools/xenstore/xenstored_control.c 
b/tools/xenstore/xenstored_control.c
index d1aaa00bf4..41e6992591 100644
--- a/tools/xenstore/xenstored_control.c
+++ b/tools/xenstore/xenstored_control.c
@@ -676,9 +676,6 @@ static const char *lu_dump_state(const void *ctx, struct 
connection *conn)
        if (ret)
                goto out;
        ret = dump_state_connections(fp);
-       if (ret)
-               goto out;
-       ret = dump_state_special_nodes(fp);
        if (ret)
                goto out;
        ret = dump_state_nodes(fp, ctx);
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index d30f35e642..c82fb6e3d5 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -597,12 +597,13 @@ static void get_acc_data(TDB_DATA *key, struct 
node_account_data *acc)
  * Per-transaction nodes need to be accounted for the transaction owner.
  * Those nodes are stored in the data base with the transaction generation
  * count prepended (e.g. 123/local/domain/...). So testing for the node's
- * key not to start with "/" is sufficient.
+ * key not to start with "/" or "@" is sufficient.
  */
 static unsigned int get_acc_domid(struct connection *conn, TDB_DATA *key,
                                  unsigned int domid)
 {
-       return (!conn || key->dptr[0] == '/') ? domid : conn->id;
+       return (!conn || key->dptr[0] == '/' || key->dptr[0] == '@')
+              ? domid : conn->id;
 }
 
 int do_tdb_write(struct connection *conn, TDB_DATA *key, TDB_DATA *data,
@@ -944,10 +945,6 @@ static struct node *get_node(struct connection *conn,
 {
        struct node *node;
 
-       if (!name || !is_valid_nodename(name)) {
-               errno = EINVAL;
-               return NULL;
-       }
        node = read_node(conn, ctx, name);
        /* If we don't have permission, we don't have node. */
        if (node) {
@@ -1236,9 +1233,23 @@ static struct node *get_node_canonicalized(struct 
connection *conn,
        *canonical_name = canonicalize(conn, ctx, name);
        if (!*canonical_name)
                return NULL;
+       if (!is_valid_nodename(*canonical_name)) {
+               errno = EINVAL;
+               return NULL;
+       }
        return get_node(conn, ctx, *canonical_name, perm);
 }
 
+static struct node *get_spec_node(struct connection *conn, const void *ctx,
+                                 const char *name, char **canonical_name,
+                                 unsigned int perm)
+{
+       if (name[0] == '@')
+               return get_node(conn, ctx, name, perm);
+
+       return get_node_canonicalized(conn, ctx, name, canonical_name, perm);
+}
+
 static int send_directory(const void *ctx, struct connection *conn,
                          struct buffered_data *in)
 {
@@ -1723,8 +1734,7 @@ static int do_get_perms(const void *ctx, struct 
connection *conn,
        char *strings;
        unsigned int len;
 
-       node = get_node_canonicalized(conn, ctx, onearg(in), NULL,
-                                     XS_PERM_READ);
+       node = get_spec_node(conn, ctx, onearg(in), NULL, XS_PERM_READ);
        if (!node)
                return errno;
 
@@ -1766,17 +1776,9 @@ static int do_set_perms(const void *ctx, struct 
connection *conn,
        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))
-                       return errno;
-               send_ack(conn, XS_SET_PERMS);
-               return 0;
-       }
-
        /* We must own node to do this (tools can do this too). */
-       node = get_node_canonicalized(conn, ctx, in->buffer, &name,
-                                     XS_PERM_WRITE | XS_PERM_OWNER);
+       node = get_spec_node(conn, ctx, in->buffer, &name,
+                            XS_PERM_WRITE | XS_PERM_OWNER);
        if (!node)
                return errno;
 
@@ -2374,7 +2376,9 @@ void setup_structure(bool live_update)
                manual_node("/", "tool");
                manual_node("/tool", "xenstored");
                manual_node("/tool/xenstored", NULL);
-               domain_entry_fix(dom0_domid, 3, true);
+               manual_node("@releaseDomain", NULL);
+               manual_node("@introduceDomain", NULL);
+               domain_entry_fix(dom0_domid, 5, true);
        }
 
        check_store();
@@ -3156,6 +3160,23 @@ static int dump_state_node(const void *ctx, struct 
connection *conn,
        return WALK_TREE_OK;
 }
 
+static int dump_state_special_node(FILE *fp, const void *ctx,
+                                  struct dump_node_data *data,
+                                  const char *name)
+{
+       struct node *node;
+       int ret;
+
+       node = read_node(NULL, ctx, name);
+       if (!node)
+               return dump_state_node_err(data, "Dump node read node error");
+
+       ret = dump_state_node(ctx, NULL, node, data);
+       talloc_free(node);
+
+       return ret;
+}
+
 const char *dump_state_nodes(FILE *fp, const void *ctx)
 {
        struct dump_node_data data = {
@@ -3167,6 +3188,11 @@ const char *dump_state_nodes(FILE *fp, const void *ctx)
        if (walk_node_tree(ctx, NULL, "/", &walkfuncs, &data))
                return data.err;
 
+       if (dump_state_special_node(fp, ctx, &data, "@releaseDomain"))
+               return data.err;
+       if (dump_state_special_node(fp, ctx, &data, "@introduceDomain"))
+               return data.err;
+
        return NULL;
 }
 
@@ -3340,25 +3366,21 @@ void read_state_node(const void *ctx, const void *state)
                node->perms.p[i].id = sn->perms[i].domid;
        }
 
-       if (strstarts(name, "@")) {
-               set_perms_special(&conn, name, &node->perms);
-               talloc_free(node);
-               return;
-       }
-
-       parentname = get_parent(node, name);
-       if (!parentname)
-               barf("allocation error restoring node");
-       parent = read_node(NULL, node, parentname);
-       if (!parent)
-               barf("read parent error restoring node");
+       if (!strstarts(name, "@")) {
+               parentname = get_parent(node, name);
+               if (!parentname)
+                       barf("allocation error restoring node");
+               parent = read_node(NULL, node, parentname);
+               if (!parent)
+                       barf("read parent error restoring node");
 
-       if (add_child(node, parent, name))
-               barf("allocation error restoring node");
+               if (add_child(node, parent, name))
+                       barf("allocation error restoring node");
 
-       set_tdb_key(parentname, &key);
-       if (write_node_raw(NULL, &key, parent, true))
-               barf("write parent error restoring node");
+               set_tdb_key(parentname, &key);
+               if (write_node_raw(NULL, &key, parent, true))
+                       barf("write parent error restoring node");
+       }
 
        set_tdb_key(name, &key);
        if (write_node_raw(NULL, &key, node, true))
diff --git a/tools/xenstore/xenstored_domain.c 
b/tools/xenstore/xenstored_domain.c
index 3ad1028edb..494694fd30 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -43,9 +43,6 @@ static evtchn_port_t virq_port;
 
 xenevtchn_handle *xce_handle = NULL;
 
-static struct node_perms dom_release_perms;
-static struct node_perms dom_introduce_perms;
-
 struct domain
 {
        /* The id of this domain */
@@ -225,27 +222,6 @@ static void unmap_interface(void *interface)
        xengnttab_unmap(*xgt_handle, interface, 1);
 }
 
-static void remove_domid_from_perm(struct node_perms *perms,
-                                  struct domain *domain)
-{
-       unsigned int cur, new;
-
-       if (perms->p[0].id == domain->domid)
-               perms->p[0].id = priv_domid;
-
-       for (cur = new = 1; cur < perms->num; cur++) {
-               if (perms->p[cur].id == domain->domid)
-                       continue;
-
-               if (new != cur)
-                       perms->p[new] = perms->p[cur];
-
-               new++;
-       }
-
-       perms->num = new;
-}
-
 static int domain_tree_remove_sub(const void *ctx, struct connection *conn,
                                  struct node *node, void *arg)
 {
@@ -297,8 +273,26 @@ static void domain_tree_remove(struct domain *domain)
                               "error when looking for orphaned nodes\n");
        }
 
-       remove_domid_from_perm(&dom_release_perms, domain);
-       remove_domid_from_perm(&dom_introduce_perms, domain);
+       walk_node_tree(domain, NULL, "@releaseDomain", &walkfuncs, domain);
+       walk_node_tree(domain, NULL, "@introduceDomain", &walkfuncs, domain);
+}
+
+static void fire_special_watches(const char *name)
+{
+       void *ctx = talloc_new(NULL);
+       struct node *node;
+
+       if (!ctx)
+               return;
+
+       node = read_node(NULL, ctx, name);
+
+       if (node)
+               fire_watches(NULL, ctx, name, node, true, NULL);
+       else
+               log("special node %s not found\n", name);
+
+       talloc_free(ctx);
 }
 
 static int destroy_domain(void *_domain)
@@ -326,7 +320,7 @@ static int destroy_domain(void *_domain)
                        unmap_interface(domain->interface);
        }
 
-       fire_watches(NULL, domain, "@releaseDomain", NULL, true, NULL);
+       fire_special_watches("@releaseDomain");
 
        wrl_domain_destroy(domain);
 
@@ -384,7 +378,7 @@ void check_domains(void)
                ;
 
        if (notify)
-               fire_watches(NULL, NULL, "@releaseDomain", NULL, true, NULL);
+               fire_special_watches("@releaseDomain");
 }
 
 /* We scan all domains rather than use the information given here. */
@@ -633,8 +627,7 @@ static struct domain *introduce_domain(const void *ctx,
                }
 
                if (!is_master_domain && !restore)
-                       fire_watches(NULL, ctx, "@introduceDomain", NULL,
-                                    true, NULL);
+                       fire_special_watches("@introduceDomain");
        } else {
                /* Use XS_INTRODUCE for recreating the xenbus event-channel. */
                if (domain->port)
@@ -840,59 +833,6 @@ const char *get_implicit_path(const struct connection 
*conn)
        return conn->domain->path;
 }
 
-static int set_dom_perms_default(struct node_perms *perms)
-{
-       perms->num = 1;
-       perms->p = talloc_array(NULL, struct xs_permissions, perms->num);
-       if (!perms->p)
-               return -1;
-       perms->p->id = 0;
-       perms->p->perms = XS_PERM_NONE;
-
-       return 0;
-}
-
-static struct node_perms *get_perms_special(const char *name)
-{
-       if (!strcmp(name, "@releaseDomain"))
-               return &dom_release_perms;
-       if (!strcmp(name, "@introduceDomain"))
-               return &dom_introduce_perms;
-       return NULL;
-}
-
-int set_perms_special(struct connection *conn, const char *name,
-                     struct node_perms *perms)
-{
-       struct node_perms *p;
-
-       p = get_perms_special(name);
-       if (!p)
-               return EINVAL;
-
-       if ((perm_for_conn(conn, p) & (XS_PERM_WRITE | XS_PERM_OWNER)) !=
-           (XS_PERM_WRITE | XS_PERM_OWNER))
-               return EACCES;
-
-       p->num = perms->num;
-       talloc_free(p->p);
-       p->p = perms->p;
-       talloc_steal(NULL, perms->p);
-
-       return 0;
-}
-
-bool check_perms_special(const char *name, struct connection *conn)
-{
-       struct node_perms *p;
-
-       p = get_perms_special(name);
-       if (!p)
-               return false;
-
-       return perm_for_conn(conn, p) & XS_PERM_READ;
-}
-
 void dom0_init(void)
 {
        evtchn_port_t port;
@@ -962,10 +902,6 @@ void domain_init(int evtfd)
        if (xce_handle == NULL)
                barf_perror("Failed to open evtchn device");
 
-       if (set_dom_perms_default(&dom_release_perms) ||
-           set_dom_perms_default(&dom_introduce_perms))
-               barf_perror("Failed to set special permissions");
-
        if ((rc = xenevtchn_bind_virq(xce_handle, VIRQ_DOM_EXC)) == -1)
                barf_perror("Failed to bind to domain exception virq port");
        virq_port = rc;
@@ -1535,60 +1471,6 @@ const char *dump_state_connections(FILE *fp)
        return ret;
 }
 
-static const char *dump_state_special_node(FILE *fp, const char *name,
-                                          const struct node_perms *perms)
-{
-       struct xs_state_record_header head;
-       struct xs_state_node sn;
-       unsigned int pathlen;
-       const char *ret;
-
-       pathlen = strlen(name) + 1;
-
-       head.type = XS_STATE_TYPE_NODE;
-       head.length = sizeof(sn);
-
-       sn.conn_id = 0;
-       sn.ta_id = 0;
-       sn.ta_access = 0;
-       sn.perm_n = perms->num;
-       sn.path_len = pathlen;
-       sn.data_len = 0;
-       head.length += perms->num * sizeof(*sn.perms);
-       head.length += pathlen;
-       head.length = ROUNDUP(head.length, 3);
-       if (fwrite(&head, sizeof(head), 1, fp) != 1)
-               return "Dump special node error";
-       if (fwrite(&sn, sizeof(sn), 1, fp) != 1)
-               return "Dump special node error";
-
-       ret = dump_state_node_perms(fp, perms->p, perms->num);
-       if (ret)
-               return ret;
-
-       if (fwrite(name, pathlen, 1, fp) != 1)
-               return "Dump special node path error";
-
-       ret = dump_state_align(fp);
-
-       return ret;
-}
-
-const char *dump_state_special_nodes(FILE *fp)
-{
-       const char *ret;
-
-       ret = dump_state_special_node(fp, "@releaseDomain",
-                                     &dom_release_perms);
-       if (ret)
-               return ret;
-
-       ret = dump_state_special_node(fp, "@introduceDomain",
-                                     &dom_introduce_perms);
-
-       return ret;
-}
-
 void read_state_connection(const void *ctx, const void *state)
 {
        const struct xs_state_connection *sc = state;
diff --git a/tools/xenstore/xenstored_domain.h 
b/tools/xenstore/xenstored_domain.h
index b38c82991d..630641d620 100644
--- a/tools/xenstore/xenstored_domain.h
+++ b/tools/xenstore/xenstored_domain.h
@@ -99,11 +99,6 @@ void domain_outstanding_domid_dec(unsigned int domid);
 int domain_get_quota(const void *ctx, struct connection *conn,
                     unsigned int domid);
 
-/* Special node permission handling. */
-int set_perms_special(struct connection *conn, const char *name,
-                     struct node_perms *perms);
-bool check_perms_special(const char *name, struct connection *conn);
-
 /* Write rate limiting */
 
 #define WRL_FACTOR   1000 /* for fixed-point arithmetic */
@@ -132,7 +127,6 @@ void wrl_apply_debit_direct(struct connection *conn);
 void wrl_apply_debit_trans_commit(struct connection *conn);
 
 const char *dump_state_connections(FILE *fp);
-const char *dump_state_special_nodes(FILE *fp);
 
 void read_state_connection(const void *ctx, const void *state);
 
diff --git a/tools/xenstore/xenstored_watch.c b/tools/xenstore/xenstored_watch.c
index 316c08b7f7..75748ac109 100644
--- a/tools/xenstore/xenstored_watch.c
+++ b/tools/xenstore/xenstored_watch.c
@@ -46,13 +46,6 @@ struct watch
        char *node;
 };
 
-static bool check_special_event(const char *name)
-{
-       assert(name);
-
-       return strstarts(name, "@");
-}
-
 /* Is child a subnode of parent, or equal? */
 static bool is_child(const char *child, const char *parent)
 {
@@ -153,14 +146,8 @@ void fire_watches(struct connection *conn, const void 
*ctx, const char *name,
 
        /* Create an event for each watch. */
        list_for_each_entry(i, &connections, list) {
-               /* introduce/release domain watches */
-               if (check_special_event(name)) {
-                       if (!check_perms_special(name, i))
-                               continue;
-               } else {
-                       if (!watch_permitted(i, ctx, name, node, perms))
-                               continue;
-               }
+               if (!watch_permitted(i, ctx, name, node, perms))
+                       continue;
 
                list_for_each_entry(watch, &i->watches, list) {
                        if (exact) {
--
generated by git-patchbot for /home/xen/git/xen.git#master



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.