[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 22/25] tools/xenstore: merge get_spec_node() into get_node_canonicalized()





On 04/08/2023 10:34, Juergen Gross wrote:
On 04.08.23 11:21, Julien Grall wrote:
Hi,

On 04/08/2023 10:17, Juergen Gross wrote:
On 03.08.23 23:36, Julien Grall wrote:
Hi,

On 24/07/2023 12:02, Juergen Gross wrote:
diff --git a/tools/xenstore/xenstored_watch.c b/tools/xenstore/xenstored_watch.c
index 86cf8322b4..2662a3fa49 100644
--- a/tools/xenstore/xenstored_watch.c
+++ b/tools/xenstore/xenstored_watch.c
@@ -166,19 +166,12 @@ static int destroy_watch(void *_watch)
  static int check_watch_path(struct connection *conn, const void *ctx,
                  const char **path, bool *relative)
  {
-    /* Check if valid event. */
-    if (strstarts(*path, "@")) {
-        *relative = false;
-        if (strlen(*path) > XENSTORE_REL_PATH_MAX)
-            goto inval;

I can't find an exact matching check in is_valid_nodename(). The call also seems to put more restriction on '@' node. Can you clarify?

The call of domain_max_chk() in is_valid_nodename() will check the length of the node name (at least for unprivileged callers, which is the important
case).

Right, but from my understanding, this may not check against XENSTORE_REL_PATH_MAX but whatever limit the user set.

Yes, and that's what should be tested, right? I don't see why special nodes
should not adhere to the same limits as other nodes. In case an unprivileged
user should have access to special nodes, the limits shouldn't hinder the
user to access those nodes (setting a limit below 15 would be ridiculous
anyway, and that is the length of longest special node name today).
I don't mind you want to test against a different value. My point is more that you didn't mention that the limit will be changed.

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.