[xen master] tools/xenstore: merge is_valid_nodename() into canonicalize()

[patchbot@xxxxxxx]

commit f174147e63593af75ee00f4994acad84ac937540
Author:     Juergen Gross <jgross@xxxxxxxx>
AuthorDate: Mon Aug 14 09:47:03 2023 +0200
Commit:     Julien Grall <jgrall@xxxxxxxxxx>
CommitDate: Fri Aug 18 13:49:38 2023 +0100

    tools/xenstore: merge is_valid_nodename() into canonicalize()
    
    Today is_valid_nodename() is always called directly after calling
    canonicalize(), with the exception of do_unwatch(), where the call
    is missing (which is not correct, but results just in a wrong error
    reason being returned).
    
    Merge is_valid_nodename() into canonicalize().
    
    Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
    Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>
---
 tools/xenstore/xenstored_core.c  | 85 +++++++++++++++++++++-------------------
 tools/xenstore/xenstored_core.h  |  6 +--
 tools/xenstore/xenstored_watch.c | 16 ++------
 3 files changed, 50 insertions(+), 57 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 0ebe4bb7d2..69e147df2c 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -1219,33 +1219,6 @@ static bool valid_chars(const char *node)
                       "0123456789-/_@") == strlen(node));
 }
 
-bool is_valid_nodename(const struct connection *conn, const char *node,
-                      bool allow_special)
-{
-       int local_off = 0;
-       unsigned int domid;
-
-       /* Must start in / or - if special nodes are allowed - in @. */
-       if (!strstarts(node, "/") && (!allow_special || !strstarts(node, "@")))
-               return false;
-
-       /* Cannot end in / (unless it's just "/"). */
-       if (strends(node, "/") && !streq(node, "/"))
-               return false;
-
-       /* No double //. */
-       if (strstr(node, "//"))
-               return false;
-
-       if (sscanf(node, "/local/domain/%5u/%n", &domid, &local_off) != 1)
-               local_off = 0;
-
-       if (domain_max_chk(conn, ACC_PATHLEN, strlen(node) - local_off))
-               return false;
-
-       return valid_chars(node);
-}
-
 /* We expect one arg in the input: return NULL otherwise.
  * The payload must contain exactly one nul, at the end.
  */
@@ -1278,16 +1251,51 @@ static char *node_perms_to_strings(const struct node 
*node, unsigned int *len)
 }
 
 const char *canonicalize(struct connection *conn, const void *ctx,
-                        const char *node)
+                        const char *node, bool allow_special)
 {
-       const char *prefix;
+       const char *name;
+       int local_off = 0;
+       unsigned int domid;
 
-       if (!node || (node[0] == '/') || (node[0] == '@'))
-               return node;
-       prefix = get_implicit_path(conn);
-       if (prefix)
-               return talloc_asprintf(ctx, "%s/%s", prefix, node);
-       return node;
+       /*
+        * Invalid if any of:
+        * - no node at all
+        * - illegal character in node
+        * - starts with '@' but no special node allowed
+        */
+       errno = EINVAL;
+       if (!node ||
+           !valid_chars(node) ||
+           (node[0] == '@' && !allow_special))
+               return NULL;
+
+       if (node[0] != '/' && node[0] != '@') {
+               name = talloc_asprintf(ctx, "%s/%s", get_implicit_path(conn),
+                                      node);
+               if (!name)
+                       return NULL;
+       } else
+               name = node;
+
+       if (sscanf(name, "/local/domain/%5u/%n", &domid, &local_off) != 1)
+               local_off = 0;
+
+       /*
+        * Only valid if:
+        * - doesn't end in / (unless it's just "/")
+        * - no double //
+        * - not violating max allowed path length
+        */
+       if (!(strends(name, "/") && !streq(name, "/")) &&
+           !strstr(name, "//") &&
+           !domain_max_chk(conn, ACC_PATHLEN, strlen(name) - local_off))
+               return name;
+
+       /* Release the memory if 'name' was allocated by us. */
+       if (name != node)
+               talloc_free(name);
+
+       return NULL;
 }
 
 static struct node *get_node_canonicalized(struct connection *conn,
@@ -1301,13 +1309,10 @@ static struct node *get_node_canonicalized(struct 
connection *conn,
 
        if (!canonical_name)
                canonical_name = &tmp_name;
-       *canonical_name = canonicalize(conn, ctx, name);
+       *canonical_name = canonicalize(conn, ctx, name, allow_special);
        if (!*canonical_name)
                return NULL;
-       if (!is_valid_nodename(conn, *canonical_name, allow_special)) {
-               errno = EINVAL;
-               return NULL;
-       }
+
        return get_node(conn, ctx, *canonical_name, perm);
 }
 
diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
index 5575cc0689..ad87199042 100644
--- a/tools/xenstore/xenstored_core.h
+++ b/tools/xenstore/xenstored_core.h
@@ -241,7 +241,7 @@ void send_ack(struct connection *conn, enum 
xsd_sockmsg_type type);
 
 /* Canonicalize this path if possible. */
 const char *canonicalize(struct connection *conn, const void *ctx,
-                        const char *node);
+                        const char *node, bool allow_special);
 
 /* Get access permissions. */
 unsigned int perm_for_conn(struct connection *conn,
@@ -304,10 +304,6 @@ struct connection *get_connection_by_id(unsigned int 
conn_id);
 void check_store(void);
 void corrupt(struct connection *conn, const char *fmt, ...);
 
-/* Is this a valid node name? */
-bool is_valid_nodename(const struct connection *conn, const char *node,
-                      bool allow_special);
-
 /* Get name of parent node. */
 char *get_parent(const void *ctx, const char *node);
 
diff --git a/tools/xenstore/xenstored_watch.c b/tools/xenstore/xenstored_watch.c
index e695762c64..7d4d097cf9 100644
--- a/tools/xenstore/xenstored_watch.c
+++ b/tools/xenstore/xenstored_watch.c
@@ -164,17 +164,9 @@ static int check_watch_path(struct connection *conn, const 
void *ctx,
                            const char **path, bool *relative)
 {
        *relative = !strstarts(*path, "/") && !strstarts(*path, "@");
-       *path = canonicalize(conn, ctx, *path);
-       if (!*path)
-               return errno;
-       if (!is_valid_nodename(conn, *path, true))
-               goto inval;
-
-       return 0;
+       *path = canonicalize(conn, ctx, *path, true);
 
- inval:
-       errno = EINVAL;
-       return errno;
+       return *path ? 0 : errno;
 }
 
 static struct watch *add_watch(struct connection *conn, const char *path,
@@ -258,9 +250,9 @@ int do_unwatch(const void *ctx, struct connection *conn,
        if (get_strings(in, vec, ARRAY_SIZE(vec)) != ARRAY_SIZE(vec))
                return EINVAL;
 
-       node = canonicalize(conn, ctx, vec[0]);
+       node = canonicalize(conn, ctx, vec[0], true);
        if (!node)
-               return ENOMEM;
+               return errno;
        list_for_each_entry(watch, &conn->watches, list) {
                if (streq(watch->node, node) && streq(watch->token, vec[1])) {
                        list_del(&watch->list);
--
generated by git-patchbot for /home/xen/git/xen.git#master