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

[Julien Grall <julien@xxxxxxx>]

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.
Cheers,

--
Julien Grall