tools/xenstore/xenstored_core.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
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@suse.com>
---
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;
return get_node(conn, ctx, name, perm);
+ }
return get_node_canonicalized(conn, ctx, name, canonical_name, perm);
}
--
2.35.3
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@suse.com> > --- > 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
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@suse.com> >> --- >> 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
© 2016 - 2024 Red Hat, Inc.