Re: [PULL 25/32] crypto: fix lifecycle handling of gnutls credentials objects

["Daniel P. Berrange" <berrange@redhat.com>]

On Fri, Apr 17, 2026 at 03:31:02PM +0200, Maciej S. Szmigiero wrote:
> On 3.04.2026 20:25, Maciej S. Szmigiero wrote:
> > On 3.11.2025 14:37, Daniel P. Berrangé wrote:
> > > As described in the previous commit, the gnutls credentials need to
> > > be kept alive for as long as the gnutls session object exists. Convert
> > > the QCryptoTLSCreds objects to use QCryptoTLSCredsBox and holding the
> > > gnutls credential objects. When loading the credentials into a gnutls
> > > session, store a reference to the box into the QCryptoTLSSession object.
> > > 
> > > This has the useful side effect that the QCryptoTLSSession code no
> > > longer needs to know about all the different credential types, it can
> > > use the generic pointer stored in the box.
> > > 
> > > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > > ---
> > (..)
> > > diff --git a/crypto/tlscredsanon.c b/crypto/tlscredsanon.c
> > > index 5c55b07b2f..0a728ccbf6 100644
> > > --- a/crypto/tlscredsanon.c
> > > +++ b/crypto/tlscredsanon.c
> > > @@ -36,6 +36,7 @@ static int
> > >   qcrypto_tls_creds_anon_load(QCryptoTLSCredsAnon *creds,
> > >                               Error **errp)
> > >   {
> > > +    g_autoptr(QCryptoTLSCredsBox) box = NULL;
> > >       g_autofree char *dhparams = NULL;
> > >       int ret;
> > > @@ -43,6 +44,8 @@ qcrypto_tls_creds_anon_load(QCryptoTLSCredsAnon *creds,
> > >               creds->parent_obj.dir ? creds->parent_obj.dir : "<nodir>");
> > >       if (creds->parent_obj.endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) {
> > > +        box = qcrypto_tls_creds_box_new_server(GNUTLS_CRD_ANON);
> > > +
> > >           if (creds->parent_obj.dir &&
> > >               qcrypto_tls_creds_get_path(&creds->parent_obj,
> > >                                          QCRYPTO_TLS_CREDS_DH_PARAMS,
> > > @@ -50,7 +53,7 @@ qcrypto_tls_creds_anon_load(QCryptoTLSCredsAnon *creds,
> > >               return -1;
> > >           }
> > > -        ret = gnutls_anon_allocate_server_credentials(&creds->data.server);
> > > +        ret = gnutls_anon_allocate_server_credentials(&box->data.anonserver);
> > >           if (ret < 0) {
> > >               error_setg(errp, "Cannot allocate credentials: %s",
> > >                          gnutls_strerror(ret));
> > > @@ -58,42 +61,26 @@ qcrypto_tls_creds_anon_load(QCryptoTLSCredsAnon *creds,
> > >           }
> > >           if (qcrypto_tls_creds_get_dh_params_file(&creds->parent_obj, dhparams,
> > > -                                                 &creds->parent_obj.dh_params,
> > > -                                                 errp) < 0) {
> > > +                                                 &box->dh_params, errp) < 0) {
> > >               return -1;
> > >           }
> > > -        gnutls_anon_set_server_dh_params(creds->data.server,
> > > -                                         creds->parent_obj.dh_params);
> > > +        gnutls_anon_set_server_dh_params(box->data.anonserver,
> > > +                                         box->dh_params);
> > >       } else {
> > > -        ret = gnutls_anon_allocate_client_credentials(&creds->data.client);
> > > +        ret = gnutls_anon_allocate_client_credentials(&box->data.anonclient);
> > 
> > Pointed question - where's the object pointed to by "box" allocated in this case
> > of an anonymous TLS client being loaded? :)
> 
> To be clear, that was supposed to be a humorous report of a regression introduced
> by the above commit - sorry if it wasn't communicated well.

No problem. I understood your message, I simply forget to
reply to it :-(

> Reproducer:
> > $ qemu-system-x86_64 -object tls-creds-anon,endpoint=client,id=test
> > Segmentation fault
> 
> The above code branch really needs something like
> "box = qcrypto_tls_creds_box_new_client(GNUTLS_CRD_ANON)".

Yep, and even more importantly we needed a test case in test-crypto-tlssession
to validate this works !

I've CC'd you on the proposal fix