[Xen-changelog] [xen master] tools/xenstore: fix a use after free problem in xenstored

[patchbot@xxxxxxx]

commit bb2a34fd740e9a26be9e2244f1a5b4cef439e5a8
Author:     Juergen Gross <jgross@xxxxxxxx>
AuthorDate: Fri Apr 3 13:03:40 2020 +0100
Commit:     Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
CommitDate: Fri Apr 3 17:57:51 2020 +0100

    tools/xenstore: fix a use after free problem in xenstored
    
    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.
    
    With Xenstore being single threaded there are normally no concurrent
    memory allocations running and freeing a virtual memory area normally
    doesn't result in that area no longer being accessible. A problem
    could occur only in case either a signal received results in some
    memory allocation done in the signal handler (SIGHUP is a primary
    candidate leading to reopening the log file), or in case the talloc
    framework would do some internal memory allocation during freeing of
    the memory (which would lead to clobbering of the freed domain
    structure).
    
    Fixes: 562a1c0f7ef3fb ("tools/xenstore: dont unlink connection object 
twice")
    Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
    Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>
---
 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;
                }
--
generated by git-patchbot for /home/xen/git/xen.git#master

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog