On Fri, Oct 23, 2009 at 01:01:32PM +0200, Chris Lalancette wrote:
When working with the TLS transport, I noticed that every single
time
a remote stream was closed, I would get a message like:
09:09:40.793: error : remoteIOReadBuffer:7328 : failed to read from TLS socket A TLS
packet with unexpected length was received.
libvir: QEMU error : failed to read from TLS socket A TLS packet with unexpected length
was received.
in the logs. This happens because of a race in libvirtd; one thread
handles the doRemoteClose(), which calls gnutls_bye() followed by close()
while another thread is poll()'ing on the same fd. Once the close()
happens, the poll returns with revents set to POLLIN, and we would poll
one more time for data from the now-closed fd. Fix this situation by
setting poll->session to NULL when we clean up, and check for that in
remoteIOHandleInput().
I'm not sure that this is correct. If the connection is being closed
while another thread is still using it, that sounds like a bug that
should be fixed, because whatever just called doRemoteClose() has
also free'd 'priv', and so whatever other thread is in remoteIOReadBuffer()
is now accessing free'd memory.
Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
---
src/remote/remote_driver.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index bf001eb..335f44b 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -1389,6 +1389,7 @@ doRemoteClose (virConnectPtr conn, struct private_data *priv)
if (priv->uses_tls && priv->session) {
gnutls_bye (priv->session, GNUTLS_SHUT_RDWR);
gnutls_deinit (priv->session);
+ priv->session = NULL;
}
#if HAVE_SASL
if (priv->saslconn)
@@ -7223,6 +7224,12 @@ remoteIOReadBuffer(virConnectPtr conn,
int ret;
if (priv->uses_tls) {
+ if (!priv->session) {
+ /* we may have reached here one more time after gnutls_bye()
+ * was called, so just return here
+ */
+ return 0;
+ }
If gnutls_bye() was just called, then 'priv' has also just been
freed. So this must surely be accessing freed memory.
tls_resend:
ret = gnutls_record_recv (priv->session, bytes, len);
if (ret == GNUTLS_E_INTERRUPTED)
--
1.6.0.6
Daniel
--
|: Red Hat, Engineering, London -o-
http://people.redhat.com/berrange/ :|
|:
http://libvirt.org -o-
http://virt-manager.org -o-
http://ovirt.org :|
|:
http://autobuild.org -o-
http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|