[PATCH v4 0/1] audio/jack: fix use after free segfault

[PATCH v4 0/1] audio/jack: fix use after free segfault

[Geoffrey McRae]

v4:
  Use a bottom handler for shutdown as suggested by Gerd Hoffman

Geoffrey McRae (1):
  audio/jack: fix use after free segfault

 audio/jackaudio.c | 30 +++++++++++++++++++++---------
 configure         |  4 +++-
 2 files changed, 24 insertions(+), 10 deletions(-)

-- 
2.20.1

[PATCH v4 1/1] audio/jack: fix use after free segfault

[Geoffrey McRae]

This change registers a bottom handler to close the JACK client
connection when a server shutdown signal is recieved. Without this
libjack2 attempts to "clean up" old clients and causes a use after free
segfault.

Signed-off-by: Geoffrey McRae <geoff@hostfission.com>
---
 audio/jackaudio.c | 30 +++++++++++++++++++++---------
 configure         |  4 +++-
 2 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/audio/jackaudio.c b/audio/jackaudio.c
index 72ed7c4929..8ed70c3dbb 100644
--- a/audio/jackaudio.c
+++ b/audio/jackaudio.c
@@ -25,12 +25,14 @@
 #include "qemu/osdep.h"
 #include "qemu/module.h"
 #include "qemu/atomic.h"
+#include "qemu/main-loop.h"
 #include "qemu-common.h"
 #include "audio.h"
 
 #define AUDIO_CAP "jack"
 #include "audio_int.h"
 
+#include <dlfcn.h>
 #include <jack/jack.h>
 #include <jack/thread.h>
 
@@ -63,6 +65,7 @@ typedef struct QJackClient {
     QJackState      state;
     jack_client_t  *client;
     jack_nframes_t  freq;
+    QEMUBH         *shutdown_bh;
 
     struct QJack   *j;
     int             nchannels;
@@ -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;
+    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) {
@@ -417,6 +426,10 @@ static int qjack_client_init(QJackClient *c)
         options |= JackServerName;
     }
 
+    if (!c->shutdown_bh) {
+        c->shutdown_bh = qemu_bh_new(qjack_shutdown_bh, c);
+    }
+
     c->client = jack_client_open(client_name, options, &status,
       c->opt->server_name);
 
@@ -489,8 +502,6 @@ 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;
@@ -525,8 +536,6 @@ 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;
@@ -557,6 +566,8 @@ static int qjack_init_in(HWVoiceIn *hw, struct audsettings *as,
 
 static void qjack_client_fini(QJackClient *c)
 {
+    qemu_bh_cancel(c->shutdown_bh);
+
     switch (c->state) {
     case QJACK_STATE_RUNNING:
         jack_deactivate(c->client);
@@ -564,6 +575,7 @@ static void qjack_client_fini(QJackClient *c)
 
     case QJACK_STATE_SHUTDOWN:
         jack_client_close(c->client);
+        c->client = NULL;
         /* fallthrough */
 
     case QJACK_STATE_DISCONNECTED:
-- 
2.20.1