[Xen-devel] [PATCH v3 4/5] xenstore: add explicit memory context parameter to get_node()

[Juergen Gross <jgross@xxxxxxxx>]

Add a parameter to xenstored get_node() function to explicitly
specify the memory context to be used for allocations. This will make
it easier to avoid memory leaks by using a context which is freed
soon.

This requires adding the temporary context to errno_from_parents() and
ask_parents(), too.

When calling get_node() select a sensible memory context for the new
parameter by preferring a temporary one.

Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
Reviewed-by: Wei Liu <wei.liu2@xxxxxxxxxx>
Acked-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
---
 tools/xenstore/xenstored_core.c  | 50 +++++++++++++++++++++++++---------------
 tools/xenstore/xenstored_core.h  |  1 +
 tools/xenstore/xenstored_watch.c |  2 +-
 3 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index c462115..4239410 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -523,14 +523,18 @@ static char *get_parent(const void *ctx, const char *node)
        return talloc_asprintf(ctx, "%.*s", (int)(slash - node), node);
 }
 
-/* What do parents say? */
-static enum xs_perm_type ask_parents(struct connection *conn, const char *name)
+/*
+ * What do parents say?
+ * Temporary memory allocations are done with ctx.
+ */
+static enum xs_perm_type ask_parents(struct connection *conn, const void *ctx,
+                                    const char *name)
 {
        struct node *node;
 
        do {
-               name = get_parent(name, name);
-               node = read_node(conn, name, name);
+               name = get_parent(ctx, name);
+               node = read_node(conn, ctx, name);
                if (node)
                        break;
        } while (!streq(name, "/"));
@@ -544,24 +548,32 @@ static enum xs_perm_type ask_parents(struct connection 
*conn, const char *name)
        return perm_for_conn(conn, node->perms, node->num_perms);
 }
 
-/* We have a weird permissions system.  You can allow someone into a
+/*
+ * We have a weird permissions system.  You can allow someone into a
  * specific node without allowing it in the parents.  If it's going to
  * fail, however, we don't want the errno to indicate any information
- * about the node. */
-static int errno_from_parents(struct connection *conn, const char *node,
-                             int errnum, enum xs_perm_type perm)
+ * about the node.
+ * Temporary memory allocations are done with ctx.
+ */
+static int errno_from_parents(struct connection *conn, const void *ctx,
+                             const char *node, int errnum,
+                             enum xs_perm_type perm)
 {
        /* We always tell them about memory failures. */
        if (errnum == ENOMEM)
                return errnum;
 
-       if (ask_parents(conn, node) & perm)
+       if (ask_parents(conn, ctx, node) & perm)
                return errnum;
        return EACCES;
 }
 
-/* If it fails, returns NULL and sets errno. */
+/*
+ * If it fails, returns NULL and sets errno.
+ * Temporary memory allocations are done with ctx.
+ */
 struct node *get_node(struct connection *conn,
+                     const void *ctx,
                      const char *name,
                      enum xs_perm_type perm)
 {
@@ -571,7 +583,7 @@ struct node *get_node(struct connection *conn,
                errno = EINVAL;
                return NULL;
        }
-       node = read_node(conn, name, name);
+       node = read_node(conn, ctx, name);
        /* If we don't have permission, we don't have node. */
        if (node) {
                if ((perm_for_conn(conn, node->perms, node->num_perms) & perm)
@@ -582,7 +594,7 @@ struct node *get_node(struct connection *conn,
        }
        /* Clean up errno if they weren't supposed to know. */
        if (!node) 
-               errno = errno_from_parents(conn, name, errno, perm);
+               errno = errno_from_parents(conn, ctx, name, errno, perm);
        return node;
 }
 
@@ -775,7 +787,7 @@ static void send_directory(struct connection *conn, struct 
buffered_data *in)
        const char *name = onearg(in);
 
        name = canonicalize(conn, name);
-       node = get_node(conn, name, XS_PERM_READ);
+       node = get_node(conn, in, name, XS_PERM_READ);
        if (!node) {
                send_error(conn, errno);
                return;
@@ -790,7 +802,7 @@ static void do_read(struct connection *conn, struct 
buffered_data *in)
        const char *name = onearg(in);
 
        name = canonicalize(conn, name);
-       node = get_node(conn, name, XS_PERM_READ);
+       node = get_node(conn, in, name, XS_PERM_READ);
        if (!node) {
                send_error(conn, errno);
                return;
@@ -927,7 +939,7 @@ static void do_write(struct connection *conn, struct 
buffered_data *in)
        datalen = in->used - offset;
 
        name = canonicalize(conn, vec[0]);
-       node = get_node(conn, name, XS_PERM_WRITE);
+       node = get_node(conn, in, name, XS_PERM_WRITE);
        if (!node) {
                /* No permissions, invalid input? */
                if (errno != ENOENT) {
@@ -959,7 +971,7 @@ static void do_mkdir(struct connection *conn, struct 
buffered_data *in)
        const char *name = onearg(in);
 
        name = canonicalize(conn, name);
-       node = get_node(conn, name, XS_PERM_WRITE);
+       node = get_node(conn, in, name, XS_PERM_WRITE);
 
        /* If it already exists, fine. */
        if (!node) {
@@ -1077,7 +1089,7 @@ static void do_rm(struct connection *conn, struct 
buffered_data *in)
        const char *name = onearg(in);
 
        name = canonicalize(conn, name);
-       node = get_node(conn, name, XS_PERM_WRITE);
+       node = get_node(conn, in, name, XS_PERM_WRITE);
        if (!node) {
                /* Didn't exist already?  Fine, if parent exists. */
                if (errno == ENOENT) {
@@ -1114,7 +1126,7 @@ static void do_get_perms(struct connection *conn, struct 
buffered_data *in)
        unsigned int len;
 
        name = canonicalize(conn, name);
-       node = get_node(conn, name, XS_PERM_READ);
+       node = get_node(conn, in, name, XS_PERM_READ);
        if (!node) {
                send_error(conn, errno);
                return;
@@ -1146,7 +1158,7 @@ static void do_set_perms(struct connection *conn, struct 
buffered_data *in)
        num--;
 
        /* We must own node to do this (tools can do this too). */
-       node = get_node(conn, name, XS_PERM_WRITE|XS_PERM_OWNER);
+       node = get_node(conn, in, name, XS_PERM_WRITE|XS_PERM_OWNER);
        if (!node) {
                send_error(conn, errno);
                return;
diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
index 5dbf9c8..ecc614f 100644
--- a/tools/xenstore/xenstored_core.h
+++ b/tools/xenstore/xenstored_core.h
@@ -149,6 +149,7 @@ bool check_event_node(const char *node);
 
 /* Get this node, checking we have permissions. */
 struct node *get_node(struct connection *conn,
+                     const void *ctx,
                      const char *name,
                      enum xs_perm_type perm);
 
diff --git a/tools/xenstore/xenstored_watch.c b/tools/xenstore/xenstored_watch.c
index 8543999..beefd6c 100644
--- a/tools/xenstore/xenstored_watch.c
+++ b/tools/xenstore/xenstored_watch.c
@@ -57,7 +57,7 @@ static void add_event(struct connection *conn,
 
        if (!check_event_node(name)) {
                /* Can this conn load node, or see that it doesn't exist? */
-               struct node *node = get_node(conn, name, XS_PERM_READ);
+               struct node *node = get_node(conn, name, name, XS_PERM_READ);
                /*
                 * XXX We allow EACCES here because otherwise a non-dom0
                 * backend driver cannot watch for disappearance of a frontend
-- 
2.6.6


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel