[xen master] tools/xenstore: use accounting buffering for node accounting

[patchbot@xxxxxxx]

commit a4ffaa082458778306d16bf77c8cd099509ecab0
Author:     Juergen Gross <jgross@xxxxxxxx>
AuthorDate: Tue May 30 10:24:15 2023 +0200
Commit:     Julien Grall <jgrall@xxxxxxxxxx>
CommitDate: Wed Jun 7 12:10:31 2023 +0100

    tools/xenstore: use accounting buffering for node accounting
    
    Add the node accounting to the accounting information buffering in
    order to avoid having to undo it in case of failure.
    
    This requires to call domain_nbentry_dec() before any changes to the
    data base, as it can return an error now.
    
    Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
    Acked-by: Julien Grall <jgrall@xxxxxxxxxx>
---
 tools/xenstore/xenstored_core.c   | 37 ++++++++++++++-----------------------
 tools/xenstore/xenstored_domain.h |  4 ++--
 2 files changed, 16 insertions(+), 25 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 8392bdec9b..0a9c88ca67 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -1454,7 +1454,6 @@ static void destroy_node_rm(struct connection *conn, 
struct node *node)
 static int destroy_node(struct connection *conn, struct node *node)
 {
        destroy_node_rm(conn, node);
-       domain_nbentry_dec(conn, get_node_owner(node));
 
        /*
         * It is not possible to easily revert the changes in a transaction.
@@ -1645,9 +1644,12 @@ static int delnode_sub(const void *ctx, struct 
connection *conn,
        if (ret > 0)
                return WALK_TREE_SUCCESS_STOP;
 
+       if (domain_nbentry_dec(conn, get_node_owner(node)))
+               return WALK_TREE_ERROR_STOP;
+
        /* In case of error stop the walk. */
        if (!ret && do_tdb_delete(conn, &key, &node->acc))
-               return WALK_TREE_SUCCESS_STOP;
+               return WALK_TREE_ERROR_STOP;
 
        /*
         * Fire the watches now, when we can still see the node permissions.
@@ -1657,8 +1659,6 @@ static int delnode_sub(const void *ctx, struct connection 
*conn,
        watch_exact = strcmp(root, node->name);
        fire_watches(conn, ctx, node->name, node, watch_exact, NULL);
 
-       domain_nbentry_dec(conn, get_node_owner(node));
-
        return WALK_TREE_RM_CHILDENTRY;
 }
 
@@ -1679,6 +1679,12 @@ int rm_node(struct connection *conn, const void *ctx, 
const char *name)
        ret = walk_node_tree(ctx, conn, name, &walkfuncs, (void *)name);
        if (ret < 0) {
                if (ret == WALK_TREE_ERROR_STOP) {
+                       /*
+                        * This can't be triggered by an unprivileged guest,
+                        * so calling corrupt() is fine here.
+                        * In fact it is needed in order to fix a potential
+                        * accounting inconsistency.
+                        */
                        corrupt(conn, "error when deleting sub-nodes of %s\n",
                                name);
                        errno = EIO;
@@ -1797,29 +1803,14 @@ static int do_set_perms(const void *ctx, struct 
connection *conn,
                return EPERM;
 
        old_perms = node->perms;
-       domain_nbentry_dec(conn, get_node_owner(node));
+       if (domain_nbentry_dec(conn, get_node_owner(node)))
+               return ENOMEM;
        node->perms = perms;
-       if (domain_nbentry_inc(conn, get_node_owner(node))) {
-               node->perms = old_perms;
-               /*
-                * This should never fail because we had a reference on the
-                * domain before and Xenstored is single-threaded.
-                */
-               domain_nbentry_inc(conn, get_node_owner(node));
+       if (domain_nbentry_inc(conn, get_node_owner(node)))
                return ENOMEM;
-       }
 
-       if (write_node(conn, node, false)) {
-               int saved_errno = errno;
-
-               domain_nbentry_dec(conn, get_node_owner(node));
-               node->perms = old_perms;
-               /* No failure possible as above. */
-               domain_nbentry_inc(conn, get_node_owner(node));
-
-               errno = saved_errno;
+       if (write_node(conn, node, false))
                return errno;
-       }
 
        fire_watches(conn, ctx, name, node, false, &old_perms);
        send_ack(conn, XS_SET_PERMS);
diff --git a/tools/xenstore/xenstored_domain.h 
b/tools/xenstore/xenstored_domain.h
index e40657216b..466549709f 100644
--- a/tools/xenstore/xenstored_domain.h
+++ b/tools/xenstore/xenstored_domain.h
@@ -25,9 +25,9 @@
  * a per transaction array.
  */
 enum accitem {
+       ACC_NODES,
        ACC_REQ_N,              /* Number of elements per request. */
-       ACC_NODES = ACC_REQ_N,
-       ACC_TR_N,               /* Number of elements per transaction. */
+       ACC_TR_N = ACC_REQ_N,   /* Number of elements per transaction. */
        ACC_CHD_N = ACC_TR_N,   /* max(ACC_REQ_N, ACC_TR_N), for changed dom. */
        ACC_N = ACC_TR_N,       /* Number of elements per domain. */
 };
--
generated by git-patchbot for /home/xen/git/xen.git#master