Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
audio/jackaudio: Avoid dynamic stack allocation in qjack_process()
Avoid a dynamic stack allocation in qjack_process().  Since this
function is a JACK process callback, we are not permitted to malloc()
here, so we allocate a working buffer in qjack_client_init() instead.

The codebase has very few VLAs, and if we can get rid of them all we
can make the compiler error on new additions.  This is a defensive
measure against security bugs where an on-stack dynamic allocation
isn't correctly size-checked (e.g.  CVE-2021-3527).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com>
Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Message-id: 20230818155846.1651287-3-peter.maydell@linaro.org
  • Loading branch information
pm215 committed Sep 21, 2023
1 parent d71c3d3 commit 07ffc4b
Showing 1 changed file with 11 additions and 5 deletions.
16 changes: 11 additions & 5 deletions audio/jackaudio.c
Expand Up @@ -70,6 +70,9 @@ typedef struct QJackClient {
int buffersize;
jack_port_t **port;
QJackBuffer fifo;

/* Used as workspace by qjack_process() */
float **process_buffers;
}
QJackClient;

Expand Down Expand Up @@ -267,22 +270,21 @@ static int qjack_process(jack_nframes_t nframes, void *arg)
}

/* get the buffers for the ports */
float *buffers[c->nchannels];
for (int i = 0; i < c->nchannels; ++i) {
buffers[i] = jack_port_get_buffer(c->port[i], nframes);
c->process_buffers[i] = jack_port_get_buffer(c->port[i], nframes);
}

if (c->out) {
if (likely(c->enabled)) {
qjack_buffer_read_l(&c->fifo, buffers, nframes);
qjack_buffer_read_l(&c->fifo, c->process_buffers, nframes);
} else {
for (int i = 0; i < c->nchannels; ++i) {
memset(buffers[i], 0, nframes * sizeof(float));
memset(c->process_buffers[i], 0, nframes * sizeof(float));
}
}
} else {
if (likely(c->enabled)) {
qjack_buffer_write_l(&c->fifo, buffers, nframes);
qjack_buffer_write_l(&c->fifo, c->process_buffers, nframes);
}
}

Expand Down Expand Up @@ -448,6 +450,9 @@ static int qjack_client_init(QJackClient *c)
jack_get_client_name(c->client));
}

/* Allocate working buffer for process callback */
c->process_buffers = g_new(float *, c->nchannels);

jack_set_process_callback(c->client, qjack_process , c);
jack_set_port_registration_callback(c->client, qjack_port_registration, c);
jack_set_xrun_callback(c->client, qjack_xrun, c);
Expand Down Expand Up @@ -579,6 +584,7 @@ static void qjack_client_fini_locked(QJackClient *c)

qjack_buffer_free(&c->fifo);
g_free(c->port);
g_free(c->process_buffers);

c->state = QJACK_STATE_DISCONNECTED;
/* fallthrough */
Expand Down

0 comments on commit 07ffc4b

Please sign in to comment.