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

[Julien Grall <julien@xxxxxxx>]

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