]> xenbits.xensource.com Git - libvirt.git/commit
rpc: Fix connection close callback race condition and memory corruption/crash
authorPeter Krempa <pkrempa@redhat.com>
Fri, 29 Mar 2013 17:21:19 +0000 (18:21 +0100)
committerPeter Krempa <pkrempa@redhat.com>
Fri, 5 Apr 2013 08:36:03 +0000 (10:36 +0200)
commit8ad126e695e5cef5da9d62ccfde7338317041e84
tree3e309a15363599fc9162e50498a40c2b24a90b1c
parent69ab07560a134e82e36b6391be3c806d3dbdb16c
rpc: Fix connection close callback race condition and memory corruption/crash

The last Viktor's effort to fix the race and memory corruption unfortunately
wasn't complete in the case the close callback was not registered in an
connection. At that time, the trail of event's that I'll describe later could
still happen and corrupt the memory or cause a crash of the client (including
the daemon in case of a p2p migration).

Consider the following prerequisities and trail of events:
Let's have a remote connection to a hypervisor that doesn't have a close
callback registered and the client is using the event loop. The crash happens in
cooperation of 2 threads. Thread E is the event loop and thread W is the worker
that does some stuff. R denotes the remote client.

1.) W - The client finishes everything and sheds the last reference on the client
2.) W - The virObject stuff invokes virConnectDispose that invokes doRemoteClose
3.) W - the remote close method invokes the REMOTE_PROC_CLOSE RPC method.
4.) W - The thread is preempted at this point.
5.) R - The remote side receives the close and closes the socket.
6.) E - poll() wakes up due to the closed socket and invokes the close callback
7.) E - The event loop is preempted right before remoteClientCloseFunc is called
8.) W - The worker now finishes, and frees the conn object.
9.) E - The remoteClientCloseFunc accesses the now-freed conn object in the
        attempt to retrieve pointer for the real close callback.
10.) Kaboom, corrupted memory/segfault.

This patch tries to fix this by introducing a new object that survives the
freeing of the connection object. We can't increase the reference count on the
connection object itself or the connection would never be closed, as the
connection is closed only when the reference count reaches zero.

The new object - virConnectCloseCallbackData - is a lockable object that keeps
the pointers to the real user registered callback and ensures that the
connection callback is either not called if the connection was already freed or
that the connection isn't freed while this is being called.
src/datatypes.c
src/datatypes.h
src/libvirt.c
src/remote/remote_driver.c