| Message ID | 20200324101257.20781-1-jgross@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tools/xenstore: fix a use after free problem in xenstored | expand |
Ping? On 24.03.20 11:12, Juergen Gross wrote: > Commit 562a1c0f7ef3fb ("tools/xenstore: dont unlink connection object > twice") introduced a potential use after free problem in > domain_cleanup(): after calling talloc_unlink() for domain->conn > domain->conn is set to NULL. The problem is that domain is registered > as talloc child of domain->conn, so it might be freed by the > talloc_unlink() call. > > Fixes: 562a1c0f7ef3fb ("tools/xenstore: dont unlink connection object twice") > Signed-off-by: Juergen Gross <jgross@suse.com> > --- > tools/xenstore/xenstored_domain.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c > index baddaba5df..5858185211 100644 > --- a/tools/xenstore/xenstored_domain.c > +++ b/tools/xenstore/xenstored_domain.c > @@ -214,6 +214,7 @@ static void domain_cleanup(void) > { > xc_dominfo_t dominfo; > struct domain *domain; > + struct connection *conn; > int notify = 0; > > again: > @@ -230,8 +231,10 @@ static void domain_cleanup(void) > continue; > } > if (domain->conn) { > - talloc_unlink(talloc_autofree_context(), domain->conn); > + /* domain is a talloc child of domain->conn. */ > + conn = domain->conn; > domain->conn = NULL; > + talloc_unlink(talloc_autofree_context(), conn); > notify = 0; /* destroy_domain() fires the watch */ > goto again; > } >
diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c index baddaba5df..5858185211 100644 --- a/tools/xenstore/xenstored_domain.c +++ b/tools/xenstore/xenstored_domain.c @@ -214,6 +214,7 @@ static void domain_cleanup(void) { xc_dominfo_t dominfo; struct domain *domain; + struct connection *conn; int notify = 0; again: @@ -230,8 +231,10 @@ static void domain_cleanup(void) continue; } if (domain->conn) { - talloc_unlink(talloc_autofree_context(), domain->conn); + /* domain is a talloc child of domain->conn. */ + conn = domain->conn; domain->conn = NULL; + talloc_unlink(talloc_autofree_context(), conn); notify = 0; /* destroy_domain() fires the watch */ goto again; }
Commit 562a1c0f7ef3fb ("tools/xenstore: dont unlink connection object twice") introduced a potential use after free problem in domain_cleanup(): after calling talloc_unlink() for domain->conn domain->conn is set to NULL. The problem is that domain is registered as talloc child of domain->conn, so it might be freed by the talloc_unlink() call. Fixes: 562a1c0f7ef3fb ("tools/xenstore: dont unlink connection object twice") Signed-off-by: Juergen Gross <jgross@suse.com> --- tools/xenstore/xenstored_domain.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)