[libvirt] [PATCH] remote: Prevent race when closing a connection

[Daniel P. Berrange]

On Thu, Mar 14, 2013 at 01:26:55PM +0100, Viktor Mihajlovski wrote:
> A race condition can occur when virConnectClose is called parallel
> to the execution of the connection close callback in remoteClientCloseFunc.
> 
> The race happens if the connection object is destroyed (including
> the mutex) while remoteClientCloseFunc is waiting for the connection
> mutex. After the destruction of the (non error checking) mutex
> remoteClientCloseFunc starts to process the callbacks. However the
> operations can occur against a freed (or even worse, reallocated)
> object. Another issue is that the closeFreeCallback is invoked
> even if it's NULL (this is the case for virsh).
> 
> The solution is to clean out the callback pointers in virConnectDispose
> before destroying the mutex. This way remoteClientCloseFunc will
> return immediately after passing virMutexLock, thus avoiding potential
> data corruption. There's still the slight chance that the concluding
> virMutexUnlock could do harm on the freed connection object.
> This could be fixed using an error checking mutex which however has a
> much broader scope and impact.

No, this really isn't solving the problem. The virConnectDipose
function is the last thing to run on an object. Once virConnectDispose
is running absolutely nothing else may safely use that object pointer.
The thread that is not in virConnectDispose here is missing a reference
on the object, to prevent it being destroyed while it is still in use.

so NACk to this patch, it doesn't fix the problem, merely makes a SEGV
slightly less likely.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|