xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Juergen Gross <jgross@suse.com>
To: xen-devel@lists.xen.org
Cc: Juergen Gross <jgross@suse.com>,
	wei.liu2@citrix.com, ian.jackson@eu.citrix.com
Subject: [PATCH v2 4/5] xenstore: add explicit memory context parameter to get_node()
Date: Mon, 18 Jul 2016 09:31:28 +0200	[thread overview]
Message-ID: <1468827089-9054-5-git-send-email-jgross@suse.com> (raw)
In-Reply-To: <1468827089-9054-1-git-send-email-jgross@suse.com>

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@suse.com>
---
 tools/xenstore/xenstored_core.c  | 33 ++++++++++++++++++---------------
 tools/xenstore/xenstored_core.h  |  1 +
 tools/xenstore/xenstored_watch.c |  2 +-
 3 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index e5c74f4..095ba00 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -517,13 +517,14 @@ static char *get_parent(const void *mem, const char *node)
 }
 
 /* What do parents say? */
-static enum xs_perm_type ask_parents(struct connection *conn, const char *name)
+static enum xs_perm_type ask_parents(struct connection *conn, const void *mem,
+				     const char *name)
 {
 	struct node *node;
 
 	do {
-		name = get_parent(name, name);
-		node = read_node(conn, name, name);
+		name = get_parent(mem, name);
+		node = read_node(conn, mem, name);
 		if (node)
 			break;
 	} while (!streq(name, "/"));
@@ -541,20 +542,22 @@ static enum xs_perm_type ask_parents(struct connection *conn, const char *name)
  * 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)
+static int errno_from_parents(struct connection *conn, const void *mem,
+			      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, mem, node) & perm)
 		return errnum;
 	return EACCES;
 }
 
 /* If it fails, returns NULL and sets errno. */
 struct node *get_node(struct connection *conn,
+		      const void *mem,
 		      const char *name,
 		      enum xs_perm_type perm)
 {
@@ -564,7 +567,7 @@ struct node *get_node(struct connection *conn,
 		errno = EINVAL;
 		return NULL;
 	}
-	node = read_node(conn, name, name);
+	node = read_node(conn, mem, 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)
@@ -575,7 +578,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, mem, name, errno, perm);
 	return node;
 }
 
@@ -768,7 +771,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;
@@ -783,7 +786,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;
@@ -920,7 +923,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) {
@@ -952,7 +955,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) {
@@ -1070,7 +1073,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) {
@@ -1107,7 +1110,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;
@@ -1139,7 +1142,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..f763e47 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 *mem,
 		      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@lists.xen.org
https://lists.xen.org/xen-devel

  parent reply	other threads:[~2016-07-18  7:31 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-18  7:31 [PATCH v2 0/5] xenstore: fix memory leak of xenstored Juergen Gross
2016-07-18  7:31 ` [PATCH v2 1/5] xenstore: call each xenstored command function with temporary context Juergen Gross
2016-07-19 10:04   ` Wei Liu
2016-07-19 10:35   ` Ian Jackson
2016-07-18  7:31 ` [PATCH v2 2/5] xenstore: add explicit memory context parameter to get_parent() Juergen Gross
2016-07-19 10:04   ` Wei Liu
2016-07-19 10:37     ` Ian Jackson
2016-07-19 10:37   ` Ian Jackson
2016-07-18  7:31 ` [PATCH v2 3/5] xenstore: add explicit memory context parameter to read_node() Juergen Gross
2016-07-19 10:04   ` Wei Liu
2016-07-19 10:38     ` Ian Jackson
2016-07-18  7:31 ` Juergen Gross [this message]
2016-07-19 10:05   ` [PATCH v2 4/5] xenstore: add explicit memory context parameter to get_node() Wei Liu
2016-07-19 10:39     ` Ian Jackson
2016-07-18  7:31 ` [PATCH v2 5/5] xenstore: use temporary memory context for firing watches Juergen Gross
2016-07-19 10:23   ` Wei Liu
2016-07-19 10:40     ` Ian Jackson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1468827089-9054-5-git-send-email-jgross@suse.com \
    --to=jgross@suse.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).