On Wed, Jan 7, 2026 at 7:36 PM Daniel P. Berrangé <[email protected]> wrote: > > 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 > Signed-off-by: Daniel P. Berrangé <[email protected]>
Reviewed-by: Marc-André Lureau <[email protected]> > --- > io/channel-tls.c | 64 ++++++++++++++++++++++++++++++------------------ > 1 file changed, 40 insertions(+), 24 deletions(-) > > diff --git a/io/channel-tls.c b/io/channel-tls.c > index 07274c12df..8b70e95a4b 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; > @@ -258,8 +275,8 @@ void qio_channel_tls_handshake(QIOChannelTLS *ioc, > 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 +289,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 +314,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 +330,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.52.0 > > -- Marc-André Lureau
