Re: [Xen-devel] Unable to start a domain with no disks

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2012-10-12 at 17:29 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [Xen-devel] Unable to start a domain with no 
> disks"):
> > On Fri, 2012-10-12 at 16:50 +0100, Ian Campbell wrote:
> > > On Fri, 2012-10-12 at 16:45 +0100, Ian Campbell wrote:
> > > > Trying to start a very simple domain with no disk from:
> 
> > > This makes the issue go away but since this function also free's the ao
> > > I think there must be a use after free somewhere.
> 
> In this case what happens is that the tty available event gets queued
> in the outer egc, which survives until after libxl__ao_inprogress has
> returned.  You only see the bug with no disks because with disks the
> events are only generated later in the event loop inside
> libxl__ao_inprogress.
> 
> Ian.
> 
> 
> Subject: [PATCH] libxl: ao: cope with fast ao completion with progess events
> 
> There are two egcs in an ao initiator: the one in the AO_CREATE
> function, and the one in libxl__ao_inprogress.  If synchronous ao
> operation generates progress events and completes immediately, the
> progress callbacks end up queued in the outer egc.  These callbacks
> are currently only called after libxl__ao_inprogress has returned, and
> keep the ao alive until they happen.  This is not good because the
> principle is that a synchronous ao is not supposed to survive beyond
> libxl__ao_inprogress's return.
> 
> The fix is to ensure that the callbacks queued in the outer egc are
> called early enough that they don't preserve the ao.  This is
> straightforward in the AO_INPROGRESS macro because AO_CREATE's egc is
> not used inside that macro other than to destroy it.  All we have to
> do is destroy it a bit sooner.
> 
> This involves unlocking and relocking the ctx since EGC_FREE expects
> to be called with the lock released but libxl__ao_inprogress needs it
> locked.  The is hole in our lock tenure is fine - libxl__ao_inprogress
> has such holes already.
> 
> It is still possible to use the CTX_LOCK macros for this unlock/lock
> because the gc we are using is destroyed only afterwards by
> libxl__ao_inprogress.
> 
> Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>

Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

> 
> diff -r 8aadb3204cfa tools/libxl/libxl_internal.h
> --- a/tools/libxl/libxl_internal.h    Thu Sep 06 21:41:27 2012 +0200
> +++ b/tools/libxl/libxl_internal.h    Fri Oct 12 17:20:10 2012 +0100
> @@ -1709,10 +1709,12 @@ _hidden void libxl__egc_cleanup(libxl__e
>  
>  #define AO_INPROGRESS ({                                        \
>          libxl_ctx *ao__ctx = libxl__gc_owner(&ao->gc);          \
> +        CTX_UNLOCK;                                             \
> +        EGC_FREE;                                               \
> +        CTX_LOCK;                                               \
>          int ao__rc = libxl__ao_inprogress(ao,                   \
>                                 __FILE__, __LINE__, __func__);   \
>          libxl__ctx_unlock(ao__ctx); /* gc is now invalid */     \
> -        EGC_FREE;                                               \
>          (ao__rc);                                               \
>     })
>  



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel