audio/jack: fix use after free segfault

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 <geoff@hostfission.com>
Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Message-Id: <20201108063351.35804-2-geoff@hostfission.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
This commit is contained in:
Geoffrey McRae 2020-11-08 17:33:50 +11:00 committed by Gerd Hoffmann
parent cb5d19e829
commit a6e037390d
1 changed files with 37 additions and 13 deletions

View File

@ -25,6 +25,7 @@
#include "qemu/osdep.h" #include "qemu/osdep.h"
#include "qemu/module.h" #include "qemu/module.h"
#include "qemu/atomic.h" #include "qemu/atomic.h"
#include "qemu/main-loop.h"
#include "qemu-common.h" #include "qemu-common.h"
#include "audio.h" #include "audio.h"
@ -63,6 +64,7 @@ typedef struct QJackClient {
QJackState state; QJackState state;
jack_client_t *client; jack_client_t *client;
jack_nframes_t freq; jack_nframes_t freq;
QEMUBH *shutdown_bh;
struct QJack *j; struct QJack *j;
int nchannels; int nchannels;
@ -87,6 +89,7 @@ QJackIn;
static int qjack_client_init(QJackClient *c); static int qjack_client_init(QJackClient *c);
static void qjack_client_connect_ports(QJackClient *c); static void qjack_client_connect_ports(QJackClient *c);
static void qjack_client_fini(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) static void qjack_buffer_create(QJackBuffer *buffer, int channels, int frames)
{ {
@ -306,21 +309,27 @@ static int qjack_xrun(void *arg)
return 0; return 0;
} }
static void qjack_shutdown_bh(void *opaque)
{
QJackClient *c = (QJackClient *)opaque;
qjack_client_fini(c);
}
static void qjack_shutdown(void *arg) static void qjack_shutdown(void *arg)
{ {
QJackClient *c = (QJackClient *)arg; QJackClient *c = (QJackClient *)arg;
c->state = QJACK_STATE_SHUTDOWN; c->state = QJACK_STATE_SHUTDOWN;
qemu_bh_schedule(c->shutdown_bh);
} }
static void qjack_client_recover(QJackClient *c) static void qjack_client_recover(QJackClient *c)
{ {
if (c->state == QJACK_STATE_SHUTDOWN) { if (c->state != QJACK_STATE_DISCONNECTED) {
qjack_client_fini(c); return;
} }
/* packets is used simply to throttle this */ /* packets is used simply to throttle this */
if (c->state == QJACK_STATE_DISCONNECTED && if (c->packets % 100 == 0) {
c->packets % 100 == 0) {
/* if enabled then attempt to recover */ /* if enabled then attempt to recover */
if (c->enabled) { if (c->enabled) {
@ -489,15 +498,16 @@ static int qjack_init_out(HWVoiceOut *hw, struct audsettings *as,
QJackOut *jo = (QJackOut *)hw; QJackOut *jo = (QJackOut *)hw;
Audiodev *dev = (Audiodev *)drv_opaque; Audiodev *dev = (Audiodev *)drv_opaque;
qjack_client_fini(&jo->c);
jo->c.out = true; jo->c.out = true;
jo->c.enabled = false; jo->c.enabled = false;
jo->c.nchannels = as->nchannels; jo->c.nchannels = as->nchannels;
jo->c.opt = dev->u.jack.out; 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); int ret = qjack_client_init(&jo->c);
if (ret != 0) { if (ret != 0) {
qemu_bh_delete(jo->c.shutdown_bh);
return ret; return ret;
} }
@ -525,15 +535,16 @@ static int qjack_init_in(HWVoiceIn *hw, struct audsettings *as,
QJackIn *ji = (QJackIn *)hw; QJackIn *ji = (QJackIn *)hw;
Audiodev *dev = (Audiodev *)drv_opaque; Audiodev *dev = (Audiodev *)drv_opaque;
qjack_client_fini(&ji->c);
ji->c.out = false; ji->c.out = false;
ji->c.enabled = false; ji->c.enabled = false;
ji->c.nchannels = as->nchannels; ji->c.nchannels = as->nchannels;
ji->c.opt = dev->u.jack.in; 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); int ret = qjack_client_init(&ji->c);
if (ret != 0) { if (ret != 0) {
qemu_bh_delete(ji->c.shutdown_bh);
return ret; return ret;
} }
@ -555,7 +566,7 @@ static int qjack_init_in(HWVoiceIn *hw, struct audsettings *as,
return 0; return 0;
} }
static void qjack_client_fini(QJackClient *c) static void qjack_client_fini_locked(QJackClient *c)
{ {
switch (c->state) { switch (c->state) {
case QJACK_STATE_RUNNING: case QJACK_STATE_RUNNING:
@ -564,28 +575,40 @@ static void qjack_client_fini(QJackClient *c)
case QJACK_STATE_SHUTDOWN: case QJACK_STATE_SHUTDOWN:
jack_client_close(c->client); jack_client_close(c->client);
c->client = NULL;
qjack_buffer_free(&c->fifo);
g_free(c->port);
c->state = QJACK_STATE_DISCONNECTED;
/* fallthrough */ /* fallthrough */
case QJACK_STATE_DISCONNECTED: case QJACK_STATE_DISCONNECTED:
break; break;
} }
}
qjack_buffer_free(&c->fifo); static void qjack_client_fini(QJackClient *c)
g_free(c->port); {
qemu_mutex_lock(&qjack_shutdown_lock);
c->state = QJACK_STATE_DISCONNECTED; qjack_client_fini_locked(c);
qemu_mutex_unlock(&qjack_shutdown_lock);
} }
static void qjack_fini_out(HWVoiceOut *hw) static void qjack_fini_out(HWVoiceOut *hw)
{ {
QJackOut *jo = (QJackOut *)hw; QJackOut *jo = (QJackOut *)hw;
qjack_client_fini(&jo->c); qjack_client_fini(&jo->c);
qemu_bh_delete(jo->c.shutdown_bh);
} }
static void qjack_fini_in(HWVoiceIn *hw) static void qjack_fini_in(HWVoiceIn *hw)
{ {
QJackIn *ji = (QJackIn *)hw; QJackIn *ji = (QJackIn *)hw;
qjack_client_fini(&ji->c); qjack_client_fini(&ji->c);
qemu_bh_delete(ji->c.shutdown_bh);
} }
static void qjack_enable_out(HWVoiceOut *hw, bool enable) 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) static void register_audio_jack(void)
{ {
qemu_mutex_init(&qjack_shutdown_lock);
audio_driver_register(&jack_driver); audio_driver_register(&jack_driver);
jack_set_thread_creator(qjack_thread_creator); jack_set_thread_creator(qjack_thread_creator);
jack_set_error_function(qjack_error); jack_set_error_function(qjack_error);