Re: [PATCH] tools/xenstore: fix get_spec_node()

[Juergen Gross <jgross@xxxxxxxx>]

On 21.07.23 00:45, Julien Grall wrote:
> Hi Juergen,
>
> On 20/07/2023 16:08, Juergen Gross wrote:
> > In case get_spec_node() is being called for a special node starting
> > with '@' it won't set *canonical_name. This can result in a crash of
> > xenstored due to dereferencing the uninitialized name in
> > fire_watches().
> >
> > This is no security issue as it requires either a privileged caller or
> > ownership of the special node in question by an unprivileged caller
> > (which is questionable, as this would make the owner privileged in some
> > way).
> >
> > Fixes: d6bb63924fc2 ("tools/xenstore: introduce dummy nodes for special watch 
> > paths")
> > Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> > ---
> >   tools/xenstore/xenstored_core.c | 5 ++++-
> >   1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
> > index a1d3047e48..790c403904 100644
> > --- a/tools/xenstore/xenstored_core.c
> > +++ b/tools/xenstore/xenstored_core.c
> > @@ -1252,8 +1252,11 @@ static struct node *get_spec_node(struct connection 
> > *conn, const void *ctx,
> >                     const char *name, char **canonical_name,
> >                     unsigned int perm)
> >   {
> > -    if (name[0] == '@')
> > +    if (name[0] == '@') {
> > +        if (canonical_name)
> > +            *canonical_name = (char *)name;
> eww. Let's not continue the bad practice in Xenstored to cast away the const. I 
> will have a look to remove the const and you can rebase your patch on top.
I think it should be possible to make canonical_name const. I'll look into that.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature