The TLS code will create a GSource for tracking completion of the
handshake process, passing a QIOChannelTLSData struct that contains
various data items. The data struct is freed by the callback when
it completes, which means when a source is cancelled, nothing is
free'ing the data struct or its contents.
Switch to provide a data free callback to the GSource, which ensures
the QIOChannelTLSData struct is always freed even when the main event
callback never fires.
Fixes: https://gitlab.com/qemu-project/qemu/-/issues/3114
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
io/channel-tls.c | 68 ++++++++++++++++++++++++++++++------------------
1 file changed, 43 insertions(+), 25 deletions(-)
diff --git a/io/channel-tls.c b/io/channel-tls.c
index 07274c12df..940fc3c6d1 100644
--- a/io/channel-tls.c
+++ b/io/channel-tls.c
@@ -153,13 +153,32 @@ struct QIOChannelTLSData {
};
typedef struct QIOChannelTLSData QIOChannelTLSData;
+static void qio_channel_tls_io_data_free(gpointer user_data)
+{
+ QIOChannelTLSData *data = user_data;
+ /*
+ * Usually 'task' will be NULL since the GSource
+ * callback will either complete the task or pass
+ * it on to a new GSource. We'll see a non-NULL
+ * task here only if the GSource was released before
+ * its callback triggers
+ */
+ if (data->task) {
+ qio_task_free(data->task);
+ }
+ if (data->context) {
+ g_main_context_unref(data->context);
+ }
+ g_free(data);
+}
+
static gboolean qio_channel_tls_handshake_io(QIOChannel *ioc,
GIOCondition condition,
gpointer user_data);
-static void qio_channel_tls_handshake_task(QIOChannelTLS *ioc,
- QIOTask *task,
- GMainContext *context)
+static gboolean qio_channel_tls_handshake_task(QIOChannelTLS *ioc,
+ QIOTask *task,
+ GMainContext *context)
{
Error *err = NULL;
int status;
@@ -170,8 +189,7 @@ static void qio_channel_tls_handshake_task(QIOChannelTLS *ioc,
trace_qio_channel_tls_handshake_fail(ioc);
qio_task_set_error(task, err);
qio_task_complete(task);
- qio_task_free(task);
- return;
+ return TRUE;
}
if (status == QCRYPTO_TLS_HANDSHAKE_COMPLETE) {
@@ -184,7 +202,7 @@ static void qio_channel_tls_handshake_task(QIOChannelTLS *ioc,
trace_qio_channel_tls_credentials_allow(ioc);
}
qio_task_complete(task);
- qio_task_free(task);
+ return TRUE;
} else {
GIOCondition condition;
QIOChannelTLSData *data = g_new0(typeof(*data), 1);
@@ -208,8 +226,9 @@ static void qio_channel_tls_handshake_task(QIOChannelTLS *ioc,
condition,
qio_channel_tls_handshake_io,
data,
- NULL,
+ qio_channel_tls_io_data_free,
context);
+ return FALSE;
}
}
@@ -225,11 +244,9 @@ static gboolean qio_channel_tls_handshake_io(QIOChannel *ioc,
qio_task_get_source(task));
tioc->hs_ioc_tag = 0;
- g_free(data);
- qio_channel_tls_handshake_task(tioc, task, context);
-
- if (context) {
- g_main_context_unref(context);
+ if (!qio_channel_tls_handshake_task(tioc, task, context)) {
+ /* task is kept by new GSource so must not be released yet */
+ data->task = NULL;
}
return FALSE;
@@ -252,14 +269,16 @@ void qio_channel_tls_handshake(QIOChannelTLS *ioc,
func, opaque, destroy);
trace_qio_channel_tls_handshake_start(ioc);
- qio_channel_tls_handshake_task(ioc, task, context);
+ if (qio_channel_tls_handshake_task(ioc, task, context)) {
+ qio_task_free(task);
+ }
}
static gboolean qio_channel_tls_bye_io(QIOChannel *ioc, GIOCondition condition,
gpointer user_data);
-static void qio_channel_tls_bye_task(QIOChannelTLS *ioc, QIOTask *task,
- GMainContext *context)
+static gboolean qio_channel_tls_bye_task(QIOChannelTLS *ioc, QIOTask *task,
+ GMainContext *context)
{
GIOCondition condition;
QIOChannelTLSData *data;
@@ -272,14 +291,12 @@ static void qio_channel_tls_bye_task(QIOChannelTLS *ioc, QIOTask *task,
trace_qio_channel_tls_bye_fail(ioc);
qio_task_set_error(task, err);
qio_task_complete(task);
- qio_task_free(task);
- return;
+ return TRUE;
}
if (status == QCRYPTO_TLS_BYE_COMPLETE) {
qio_task_complete(task);
- qio_task_free(task);
- return;
+ return TRUE;
}
data = g_new0(typeof(*data), 1);
@@ -299,7 +316,10 @@ static void qio_channel_tls_bye_task(QIOChannelTLS *ioc, QIOTask *task,
trace_qio_channel_tls_bye_pending(ioc, status);
ioc->bye_ioc_tag = qio_channel_add_watch_full(ioc->master, condition,
qio_channel_tls_bye_io,
- data, NULL, context);
+ data,
+ qio_channel_tls_io_data_free,
+ context);
+ return FALSE;
}
@@ -312,11 +332,9 @@ static gboolean qio_channel_tls_bye_io(QIOChannel *ioc, GIOCondition condition,
QIOChannelTLS *tioc = QIO_CHANNEL_TLS(qio_task_get_source(task));
tioc->bye_ioc_tag = 0;
- g_free(data);
- qio_channel_tls_bye_task(tioc, task, context);
-
- if (context) {
- g_main_context_unref(context);
+ if (!qio_channel_tls_bye_task(tioc, task, context)) {
+ /* task is kept by new GSource so must not be released yet */
+ data->task = NULL;
}
return FALSE;
--
2.53.0