diff mbox series

[v4,12/19] tools/xenstore: alloc new memory in domain_adjust_node_perms()

Message ID 20230814074707.27696-13-jgross@suse.com (mailing list archive)
State New, archived
Headers show
Series tools/xenstore: drop TDB | expand

Commit Message

Jürgen Groß Aug. 14, 2023, 7:47 a.m. UTC
In order to avoid modifying the node data in the data base in case a
domain is gone, let domain_adjust_node_perms() allocate new memory for
the permissions in case they need to be modified. As this should
happen only in very rare cases, it is fine to do this even when having
copied the node data already.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V3:
- new patch
V4:
- add comments (Julien Grall)
---
 tools/xenstore/xenstored_core.c   | 10 +++++-----
 tools/xenstore/xenstored_domain.c | 24 ++++++++++++++++++++----
 tools/xenstore/xenstored_domain.h |  8 +++++++-
 3 files changed, 32 insertions(+), 10 deletions(-)

Comments

Julien Grall Aug. 18, 2023, 10:59 a.m. UTC | #1
Hi Juergen,

On 14/08/2023 08:47, Juergen Gross wrote:
> In order to avoid modifying the node data in the data base in case a
> domain is gone, let domain_adjust_node_perms() allocate new memory for
> the permissions in case they need to be modified. As this should
> happen only in very rare cases, it is fine to do this even when having
> copied the node data already.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Reviewed-by: Julien Grall <jgrall@amazon.com>

Cheers,
diff mbox series

Patch

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index ab4714263d..c8c8c4b8f4 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -752,6 +752,11 @@  struct node *read_node(struct connection *conn, const void *ctx,
 		goto error;
 	}
 
+	/* Data is binary blob (usually ascii, no nul). */
+	node->data = node->perms + hdr->num_perms;
+	/* Children is strings, nul separated. */
+	node->children = node->data + node->hdr.datalen;
+
 	if (domain_adjust_node_perms(node))
 		goto error;
 
@@ -759,11 +764,6 @@  struct node *read_node(struct connection *conn, const void *ctx,
 	if (node->acc.domid != get_node_owner(node))
 		node->acc.memory = 0;
 
-	/* Data is binary blob (usually ascii, no nul). */
-	node->data = node->perms + hdr->num_perms;
-	/* Children is strings, nul separated. */
-	node->children = node->data + node->hdr.datalen;
-
 	if (access_node(conn, node, NODE_ACCESS_READ, NULL))
 		goto error;
 
diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
index fdf1095acb..c00ea397cf 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -1334,13 +1334,29 @@  int domain_alloc_permrefs(struct node_perms *perms)
 int domain_adjust_node_perms(struct node *node)
 {
 	unsigned int i;
+	struct xs_permissions *perms = node->perms;
+	bool copied = false;
 
 	for (i = 1; i < node->hdr.num_perms; i++) {
-		if (node->perms[i].perms & XS_PERM_IGNORE)
+		if ((perms[i].perms & XS_PERM_IGNORE) ||
+		    chk_domain_generation(perms[i].id, node->hdr.generation))
 			continue;
-		if (!chk_domain_generation(node->perms[i].id,
-					   node->hdr.generation))
-			node->perms[i].perms |= XS_PERM_IGNORE;
+
+		/*
+		 * Don't do a in-place modification, as the node might
+		 * reference data directly in the data base, which we don't
+		 * want to modify.
+		 */
+		if (!copied) {
+			perms = talloc_memdup(node, node->perms,
+					node->hdr.num_perms * sizeof(*perms));
+			if (!perms)
+				return ENOMEM;
+			node->perms = perms;
+			copied = true;
+		}
+
+		perms[i].perms |= XS_PERM_IGNORE;
 	}
 
 	return 0;
diff --git a/tools/xenstore/xenstored_domain.h b/tools/xenstore/xenstored_domain.h
index 4950b00aee..7625dca8cd 100644
--- a/tools/xenstore/xenstored_domain.h
+++ b/tools/xenstore/xenstored_domain.h
@@ -90,8 +90,14 @@  void ignore_connection(struct connection *conn, unsigned int err);
 /* Returns the implicit path of a connection (only domains have this) */
 const char *get_implicit_path(const struct connection *conn);
 
-/* Remove node permissions for no longer existing domains. */
+/*
+ * Remove node permissions for no longer existing domains.
+ * In case of a change of permissions the related array is reallocated in
+ * order to avoid a data base change when operating on a node directly
+ * referencing the data base contents.
+ */
 int domain_adjust_node_perms(struct node *node);
+
 int domain_alloc_permrefs(struct node_perms *perms);
 
 /* Quota manipulation */