On Sonntag, 8. November 2020 07:33:50 CET Geoffrey McRae wrote: > This change registers a bottom handler to close the JACK client > connection when a server shutdown signal is received. Without this > libjack2 attempts to "clean up" old clients and causes a use after free > segfault. > > Signed-off-by: Geoffrey McRae <ge...@hostfission.com>
Reviewed-by: Christian Schoenebeck <qemu_...@crudebyte.com> Gerd, I think it would be Ok to queue this as fix for 5.2. > --- > audio/jackaudio.c | 50 +++++++++++++++++++++++++++++++++++------------ > 1 file changed, 37 insertions(+), 13 deletions(-) > > diff --git a/audio/jackaudio.c b/audio/jackaudio.c > index 1e714b30bc..3b7c18443d 100644 > --- a/audio/jackaudio.c > +++ b/audio/jackaudio.c > @@ -25,6 +25,7 @@ > #include "qemu/osdep.h" > #include "qemu/module.h" > #include "qemu/atomic.h" > +#include "qemu/main-loop.h" > #include "qemu-common.h" > #include "audio.h" > > @@ -63,6 +64,7 @@ typedef struct QJackClient { > QJackState state; > jack_client_t *client; > jack_nframes_t freq; > + QEMUBH *shutdown_bh; > > struct QJack *j; > int nchannels; > @@ -87,6 +89,7 @@ QJackIn; > static int qjack_client_init(QJackClient *c); > static void qjack_client_connect_ports(QJackClient *c); > static void qjack_client_fini(QJackClient *c); > +static QemuMutex qjack_shutdown_lock; > > static void qjack_buffer_create(QJackBuffer *buffer, int channels, int > frames) { > @@ -306,21 +309,27 @@ static int qjack_xrun(void *arg) > return 0; > } > > +static void qjack_shutdown_bh(void *opaque) > +{ > + QJackClient *c = (QJackClient *)opaque; > + qjack_client_fini(c); > +} > + > static void qjack_shutdown(void *arg) > { > QJackClient *c = (QJackClient *)arg; > c->state = QJACK_STATE_SHUTDOWN; > + qemu_bh_schedule(c->shutdown_bh); > } > > static void qjack_client_recover(QJackClient *c) > { > - if (c->state == QJACK_STATE_SHUTDOWN) { > - qjack_client_fini(c); > + if (c->state != QJACK_STATE_DISCONNECTED) { > + return; > } > > /* packets is used simply to throttle this */ > - if (c->state == QJACK_STATE_DISCONNECTED && > - c->packets % 100 == 0) { > + if (c->packets % 100 == 0) { > > /* if enabled then attempt to recover */ > if (c->enabled) { > @@ -489,15 +498,16 @@ static int qjack_init_out(HWVoiceOut *hw, struct > audsettings *as, QJackOut *jo = (QJackOut *)hw; > Audiodev *dev = (Audiodev *)drv_opaque; > > - qjack_client_fini(&jo->c); > - > jo->c.out = true; > jo->c.enabled = false; > jo->c.nchannels = as->nchannels; > jo->c.opt = dev->u.jack.out; > > + jo->c.shutdown_bh = qemu_bh_new(qjack_shutdown_bh, &jo->c); > + > int ret = qjack_client_init(&jo->c); > if (ret != 0) { > + qemu_bh_delete(jo->c.shutdown_bh); > return ret; > } > > @@ -525,15 +535,16 @@ static int qjack_init_in(HWVoiceIn *hw, struct > audsettings *as, QJackIn *ji = (QJackIn *)hw; > Audiodev *dev = (Audiodev *)drv_opaque; > > - qjack_client_fini(&ji->c); > - > ji->c.out = false; > ji->c.enabled = false; > ji->c.nchannels = as->nchannels; > ji->c.opt = dev->u.jack.in; > > + ji->c.shutdown_bh = qemu_bh_new(qjack_shutdown_bh, &ji->c); > + > int ret = qjack_client_init(&ji->c); > if (ret != 0) { > + qemu_bh_delete(ji->c.shutdown_bh); > return ret; > } > > @@ -555,7 +566,7 @@ static int qjack_init_in(HWVoiceIn *hw, struct > audsettings *as, return 0; > } > > -static void qjack_client_fini(QJackClient *c) > +static void qjack_client_fini_locked(QJackClient *c) > { > switch (c->state) { > case QJACK_STATE_RUNNING: > @@ -564,28 +575,40 @@ static void qjack_client_fini(QJackClient *c) > > case QJACK_STATE_SHUTDOWN: > jack_client_close(c->client); > + c->client = NULL; > + > + qjack_buffer_free(&c->fifo); > + g_free(c->port); > + > + c->state = QJACK_STATE_DISCONNECTED; > /* fallthrough */ > > case QJACK_STATE_DISCONNECTED: > break; > } > +} > > - qjack_buffer_free(&c->fifo); > - g_free(c->port); > - > - c->state = QJACK_STATE_DISCONNECTED; > +static void qjack_client_fini(QJackClient *c) > +{ > + qemu_mutex_lock(&qjack_shutdown_lock); > + qjack_client_fini_locked(c); > + qemu_mutex_unlock(&qjack_shutdown_lock); > } > > static void qjack_fini_out(HWVoiceOut *hw) > { > QJackOut *jo = (QJackOut *)hw; > qjack_client_fini(&jo->c); > + > + qemu_bh_delete(jo->c.shutdown_bh); > } > > static void qjack_fini_in(HWVoiceIn *hw) > { > QJackIn *ji = (QJackIn *)hw; > qjack_client_fini(&ji->c); > + > + qemu_bh_delete(ji->c.shutdown_bh); > } > > static void qjack_enable_out(HWVoiceOut *hw, bool enable) > @@ -662,6 +685,7 @@ static void qjack_info(const char *msg) > > static void register_audio_jack(void) > { > + qemu_mutex_init(&qjack_shutdown_lock); > audio_driver_register(&jack_driver); > jack_set_thread_creator(qjack_thread_creator); > jack_set_error_function(qjack_error); Best regards, Christian Schoenebeck