Re: [Xen-devel] [PATCH] tools/xenstore: domain can sometimes disappear when destroying connection

["Eftime, Petre" <epetre@xxxxxxxxxx>]

I didn't go extremely deep in my debugging, as the talloc library is a bit
difficult to debug, but under the do_introduce function you have these
two lines:

                 /* Now domain belongs to its connection. */
                 talloc_steal(domain->conn, domain);

After these happen, destroying the domain leads to a SIGSEGV in xenstored, as
when conn gets freed, so does domain, which ends up in a use-after-free.

I've posted the patch with the fixed text.

Best,
Petre

On 2018-11-29, 14:54, "Wei Liu" <wei.liu2@xxxxxxxxxx> wrote:

    On Mon, Nov 26, 2018 at 01:22:04PM +0000, Petre Eftime wrote:
    > There is a circular link formed between domain and a connection. In 
certain
    > circustances, when conn is freed, domain is also freed, which leads to use
    > after free when trying to set the conn field in domain to null.
    
    Actually, can you provide more context on this? When will the circular
    link happen?
    
    Wei.
    
    > 
    > Signed-off-by: Petre Eftime <epetre@xxxxxxxxxx>
    > ---
    >  tools/xenstore/xenstored_domain.c | 9 ++++++++-
    >  1 file changed, 8 insertions(+), 1 deletion(-)
    > 
    > diff --git a/tools/xenstore/xenstored_domain.c 
b/tools/xenstore/xenstored_domain.c
    > index fa6655033a..f085d40476 100644
    > --- a/tools/xenstore/xenstored_domain.c
    > +++ b/tools/xenstore/xenstored_domain.c
    > @@ -222,6 +222,7 @@ static void domain_cleanup(void)
    >  {
    >   xc_dominfo_t dominfo;
    >   struct domain *domain;
    > + struct connection *tmp_conn;
    >   int notify = 0;
    >  
    >   again:
    > @@ -238,8 +239,14 @@ static void domain_cleanup(void)
    >                           continue;
    >           }
    >           if (domain->conn) {
    > -                 talloc_unlink(talloc_autofree_context(), domain->conn);
    > +                 /*
    > +                  * In certain circumstances conn owns domain and
    > +                  * domain will be freed when conn is unlinked.
    > +                  */
    > +                 tmp_conn = domain->conn;
    >                   domain->conn = NULL;
    > +
    > +                 talloc_unlink(talloc_autofree_context(), tmp_conn);
    >                   notify = 0; /* destroy_domain() fires the watch */
    >                   goto again;
    >           }
    > -- 
    > 2.16.5
    > 
    > 
    > 
    > 
    > Amazon Development Center (Romania) S.R.L. registered office: 27A Sf. 
Lazar Street, UBC5, floor 2, Iasi, Iasi County, 700045, Romania. Registered in 
Romania. Registration number J22/2621/2005.
    > 
    




Amazon Development Center (Romania) S.R.L. registered office: 27A Sf. Lazar 
Street, UBC5, floor 2, Iasi, Iasi County, 700045, Romania. Registered in 
Romania. Registration number J22/2621/2005.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel