From 0f839c7b61d91faffc83c8d7bbdffe1726f8e1c8 Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Sat, 4 Jun 2022 11:47:48 +0200 Subject: [PATCH] modules: clamp input offset and size So that we don't cause memory errors with invalid input. --- src/modules/module-echo-cancel.c | 29 +++++++++++++------ src/modules/module-example-sink.c | 8 +++-- src/modules/module-filter-chain.c | 18 ++++++++---- src/modules/module-loopback.c | 22 ++++++++------ src/modules/module-pipe-tunnel.c | 11 +++---- .../modules/module-combine-sink.c | 21 +++++++++----- .../module-protocol-pulse/pulse-server.c | 23 +++++++++------ src/modules/module-protocol-simple.c | 7 ++--- src/modules/module-pulse-tunnel.c | 10 ++++--- src/modules/module-raop-sink.c | 8 +++-- 10 files changed, 96 insertions(+), 61 deletions(-) diff --git a/src/modules/module-echo-cancel.c b/src/modules/module-echo-cancel.c index 0cc880f6d..8d1205a2a 100644 --- a/src/modules/module-echo-cancel.c +++ b/src/modules/module-echo-cancel.c @@ -339,16 +339,20 @@ static void capture_process(void *data) struct impl *impl = data; struct pw_buffer *buf; struct spa_data *d; - uint32_t i, index, size; + uint32_t i, index, offs, size; int32_t avail; if ((buf = pw_stream_dequeue_buffer(impl->capture)) == NULL) { pw_log_debug("out of capture buffers: %m"); return; } + d = &buf->buffer->datas[0]; + + offs = SPA_MIN(d->chunk->offset, d->maxsize); + size = SPA_MIN(d->chunk->size, d->maxsize - offs); avail = spa_ringbuffer_get_write_index(&impl->rec_ring, &index); - size = buf->buffer->datas[0].chunk->size; + if (avail + size > impl->rec_ringsize) { uint32_t rindex, drop; @@ -375,10 +379,12 @@ static void capture_process(void *data) /* captured samples, with echo from sink */ d = &buf->buffer->datas[i]; + offs = SPA_MIN(d->chunk->offset, d->maxsize); + size = SPA_MIN(d->chunk->size, d->maxsize - offs); + spa_ringbuffer_write_data(&impl->rec_ring, impl->rec_buffer[i], impl->rec_ringsize, index % impl->rec_ringsize, - SPA_PTROFF(d->data, d->chunk->offset, void), - d->chunk->size); + SPA_PTROFF(d->data, offs, void), size); } spa_ringbuffer_write_update(&impl->rec_ring, index + size); @@ -526,16 +532,20 @@ static void sink_process(void *data) struct impl *impl = data; struct pw_buffer *buf; struct spa_data *d; - uint32_t i, index, size; + uint32_t i, index, offs, size; int32_t avail; if ((buf = pw_stream_dequeue_buffer(impl->sink)) == NULL) { pw_log_debug("out of sink buffers: %m"); return; } + d = &buf->buffer->datas[0]; + + offs = SPA_MIN(d->chunk->offset, d->maxsize); + size = SPA_MIN(d->chunk->size, d->maxsize - offs); avail = spa_ringbuffer_get_write_index(&impl->play_ring, &index); - size = buf->buffer->datas[0].chunk->size; + if (avail + size > impl->play_ringsize) { uint32_t rindex, drop; @@ -562,12 +572,13 @@ static void sink_process(void *data) /* echo from sink */ d = &buf->buffer->datas[i]; + offs = SPA_MIN(d->chunk->offset, d->maxsize); + size = SPA_MIN(d->chunk->size, d->maxsize - offs); + spa_ringbuffer_write_data(&impl->play_ring, impl->play_buffer[i], impl->play_ringsize, index % impl->play_ringsize, - SPA_PTROFF(d->data, d->chunk->offset, void), - d->chunk->size); + SPA_PTROFF(d->data, offs, void), size); } - spa_ringbuffer_write_update(&impl->play_ring, index + size); if (avail + size >= impl->aec_blocksize) { diff --git a/src/modules/module-example-sink.c b/src/modules/module-example-sink.c index ebdc2671e..f61ebbc7d 100644 --- a/src/modules/module-example-sink.c +++ b/src/modules/module-example-sink.c @@ -187,7 +187,7 @@ static void playback_stream_process(void *d) struct pw_buffer *buf; struct spa_data *bd; void *data; - uint32_t size; + uint32_t offs, size; if ((buf = pw_stream_dequeue_buffer(impl->stream)) == NULL) { pw_log_debug("out of buffers: %m"); @@ -195,8 +195,10 @@ static void playback_stream_process(void *d) } bd = &buf->buffer->datas[0]; - data = SPA_PTROFF(bd->data, bd->chunk->offset, void); - size = bd->chunk->size; + + offs = SPA_MIN(bd->chunk->offset, bd->maxsize); + size = SPA_MIN(bd->chunk->size, bd->maxsize - offs); + data = SPA_PTROFF(bd->data, offs, void); /* write buffer contents here */ pw_log_info("got buffer of size %d and data %p", size, data); diff --git a/src/modules/module-filter-chain.c b/src/modules/module-filter-chain.c index 1a77d9ce2..3e537b25d 100644 --- a/src/modules/module-filter-chain.c +++ b/src/modules/module-filter-chain.c @@ -589,7 +589,7 @@ static void capture_process(void *d) struct impl *impl = d; struct pw_buffer *in, *out; struct graph *graph = &impl->graph; - uint32_t i, size = 0, n_hndl = graph->n_hndl; + uint32_t i, outsize = 0, n_hndl = graph->n_hndl; int32_t stride = 0; if ((in = pw_stream_dequeue_buffer(impl->capture)) == NULL) @@ -604,10 +604,16 @@ static void capture_process(void *d) for (i = 0; i < in->buffer->n_datas; i++) { struct spa_data *ds = &in->buffer->datas[i]; struct graph_port *port = &graph->input[i]; + uint32_t offs, size; + + offs = SPA_MIN(ds->chunk->offset, ds->maxsize); + size = SPA_MIN(ds->chunk->size, ds->maxsize - offs); + if (port->desc) port->desc->connect_port(port->hndl, port->port, - SPA_MEMBER(ds->data, ds->chunk->offset, void)); - size = SPA_MAX(size, ds->chunk->size); + SPA_PTROFF(ds->data, offs, void)); + + outsize = SPA_MAX(outsize, size); stride = SPA_MAX(stride, ds->chunk->stride); } for (i = 0; i < out->buffer->n_datas; i++) { @@ -616,14 +622,14 @@ static void capture_process(void *d) if (port->desc) port->desc->connect_port(port->hndl, port->port, dd->data); else - memset(dd->data, 0, size); + memset(dd->data, 0, outsize); dd->chunk->offset = 0; - dd->chunk->size = size; + dd->chunk->size = outsize; dd->chunk->stride = stride; } for (i = 0; i < n_hndl; i++) { struct graph_hndl *hndl = &graph->hndl[i]; - hndl->desc->run(hndl->hndl, size / sizeof(float)); + hndl->desc->run(hndl->hndl, outsize / sizeof(float)); } done: diff --git a/src/modules/module-loopback.c b/src/modules/module-loopback.c index e0dac99ae..543ee3d81 100644 --- a/src/modules/module-loopback.c +++ b/src/modules/module-loopback.c @@ -188,28 +188,32 @@ static void capture_process(void *d) pw_log_debug("out of playback buffers: %m"); if (in != NULL && out != NULL) { - uint32_t size = 0; - int32_t stride = 0; for (i = 0; i < out->buffer->n_datas; i++) { struct spa_data *ds, *dd; + uint32_t outsize = 0; + int32_t stride = 0; dd = &out->buffer->datas[i]; if (i < in->buffer->n_datas) { + uint32_t offs, size; + ds = &in->buffer->datas[i]; - memcpy(dd->data, - SPA_PTROFF(ds->data, ds->chunk->offset, void), - ds->chunk->size); + offs = SPA_MIN(ds->chunk->offset, ds->maxsize); + size = SPA_MIN(ds->chunk->size, ds->maxsize - offs); + stride = SPA_MAX(stride, stride); - size = SPA_MAX(size, ds->chunk->size); - stride = SPA_MAX(stride, ds->chunk->stride); + memcpy(dd->data, + SPA_PTROFF(ds->data, offs, void), size); + + outsize = SPA_MAX(outsize, size); } else { - memset(dd->data, 0, size); + memset(dd->data, 0, outsize); } dd->chunk->offset = 0; - dd->chunk->size = size; + dd->chunk->size = outsize; dd->chunk->stride = stride; } } diff --git a/src/modules/module-pipe-tunnel.c b/src/modules/module-pipe-tunnel.c index 16f1bbbfe..02d8d1b55 100644 --- a/src/modules/module-pipe-tunnel.c +++ b/src/modules/module-pipe-tunnel.c @@ -211,7 +211,7 @@ static void playback_stream_process(void *data) { struct impl *impl = data; struct pw_buffer *buf; - uint32_t i, size, offset; + uint32_t i, size, offs; ssize_t written; if ((buf = pw_stream_dequeue_buffer(impl->stream)) == NULL) { @@ -222,11 +222,12 @@ static void playback_stream_process(void *data) for (i = 0; i < buf->buffer->n_datas; i++) { struct spa_data *d; d = &buf->buffer->datas[i]; - size = d->chunk->size; - offset = d->chunk->offset; + + offs = SPA_MIN(d->chunk->offset, d->maxsize); + size = SPA_MIN(d->chunk->size, d->maxsize - offs); while (size > 0) { - written = write(impl->fd, SPA_MEMBER(d->data, offset, void), size); + written = write(impl->fd, SPA_MEMBER(d->data, offs, void), size); if (written < 0) { if (errno == EINTR) { /* retry if interrupted */ @@ -238,7 +239,7 @@ static void playback_stream_process(void *data) pw_log_warn("Failed to write to pipe sink"); } } - offset += written; + offs += written; size -= written; } } diff --git a/src/modules/module-protocol-pulse/modules/module-combine-sink.c b/src/modules/module-protocol-pulse/modules/module-combine-sink.c index 8a51f1c4d..5d5ff77e5 100644 --- a/src/modules/module-protocol-pulse/modules/module-combine-sink.c +++ b/src/modules/module-protocol-pulse/modules/module-combine-sink.c @@ -124,8 +124,7 @@ static void capture_process(void *d) for (i = 0; i < MAX_SINKS; i++) { struct pw_buffer *out; - uint32_t j, size = 0; - int32_t stride = 0; + uint32_t j; if (data->streams[i].stream == NULL || data->streams[i].cleanup) continue; @@ -142,23 +141,29 @@ static void capture_process(void *d) for (j = 0; j < out->buffer->n_datas; j++) { struct spa_data *ds, *dd; + uint32_t outsize = 0; + int32_t stride = 0; dd = &out->buffer->datas[j]; if (j < in->buffer->n_datas) { + uint32_t offs, size; + ds = &in->buffer->datas[j]; - memcpy(dd->data, - SPA_PTROFF(ds->data, ds->chunk->offset, void), - ds->chunk->size); + offs = SPA_MIN(ds->chunk->offset, ds->maxsize); + size = SPA_MIN(ds->chunk->size, ds->maxsize - offs); - size = SPA_MAX(size, ds->chunk->size); + memcpy(dd->data, + SPA_PTROFF(ds->data, offs, void), size); + + outsize = SPA_MAX(outsize, size); stride = SPA_MAX(stride, ds->chunk->stride); } else { - memset(dd->data, 0, size); + memset(dd->data, 0, outsize); } dd->chunk->offset = 0; - dd->chunk->size = size; + dd->chunk->size = outsize; dd->chunk->stride = stride; } diff --git a/src/modules/module-protocol-pulse/pulse-server.c b/src/modules/module-protocol-pulse/pulse-server.c index 5dabbe4fd..8194e2791 100644 --- a/src/modules/module-protocol-pulse/pulse-server.c +++ b/src/modules/module-protocol-pulse/pulse-server.c @@ -1353,7 +1353,8 @@ static void stream_process(void *data) void *p; struct pw_buffer *buffer; struct spa_buffer *buf; - uint32_t size, minreq = 0, index; + struct spa_data *d; + uint32_t offs, size, minreq = 0, index; struct process_data pd; bool do_flush = false; @@ -1366,7 +1367,8 @@ static void stream_process(void *data) return; buf = buffer->buffer; - if ((p = buf->datas[0].data) == NULL) + d = &buf->datas[0]; + if ((p = d->data) == NULL) return; spa_zero(pd); @@ -1383,7 +1385,7 @@ static void stream_process(void *data) if (avail < (int32_t)minreq || stream->corked) { /* underrun, produce a silence buffer */ - size = SPA_MIN(buf->datas[0].maxsize, minreq); + size = SPA_MIN(d->maxsize, minreq); memset(p, 0, size); if (stream->draining && !stream->corked) { @@ -1419,7 +1421,7 @@ static void stream_process(void *data) pd.read_inc = skip; avail = stream->attr.maxlength; } - size = SPA_MIN(buf->datas[0].maxsize, (uint32_t)avail); + size = SPA_MIN(d->maxsize, (uint32_t)avail); size = SPA_MIN(size, minreq); spa_ringbuffer_read_data(&stream->ring, @@ -1434,13 +1436,16 @@ static void stream_process(void *data) pd.playing_for = size; pd.underrun = false; } - buf->datas[0].chunk->offset = 0; - buf->datas[0].chunk->stride = stream->frame_size; - buf->datas[0].chunk->size = size; + d->chunk->offset = 0; + d->chunk->stride = stream->frame_size; + d->chunk->size = size; buffer->size = size / stream->frame_size; } else { int32_t filled = spa_ringbuffer_get_write_index(&stream->ring, &index); - size = buf->datas[0].chunk->size; + + offs = SPA_MIN(d->chunk->offset, d->maxsize); + size = SPA_MIN(d->chunk->size, d->maxsize - offs); + if (filled < 0) { /* underrun, can't really happen because we never read more * than what's available on the other side */ @@ -1458,7 +1463,7 @@ static void stream_process(void *data) spa_ringbuffer_write_data(&stream->ring, stream->buffer, MAXLENGTH, index % MAXLENGTH, - SPA_PTROFF(p, buf->datas[0].chunk->offset, void), + SPA_PTROFF(p, offs, void), SPA_MIN(size, MAXLENGTH)); index += size; diff --git a/src/modules/module-protocol-simple.c b/src/modules/module-protocol-simple.c index 309b2729c..16c834d16 100644 --- a/src/modules/module-protocol-simple.c +++ b/src/modules/module-protocol-simple.c @@ -310,11 +310,8 @@ static void capture_process(void *data) } d = &buf->buffer->datas[0]; - size = d->chunk->size; - offset = d->chunk->offset; - - if (size + offset > d->maxsize) - size = d->maxsize - SPA_MIN(offset, d->maxsize); + offset = SPA_MIN(d->chunk->offset, d->maxsize); + size = SPA_MIN(d->chunk->size, d->maxsize - offset); while (size > 0) { res = send(client->source->fd, diff --git a/src/modules/module-pulse-tunnel.c b/src/modules/module-pulse-tunnel.c index ff0ea440e..a8a641e9c 100644 --- a/src/modules/module-pulse-tunnel.c +++ b/src/modules/module-pulse-tunnel.c @@ -246,7 +246,7 @@ static void playback_stream_process(void *d) struct pw_buffer *buf; struct spa_data *bd; int32_t filled; - uint32_t write_index, size; + uint32_t write_index, offs, size; if ((buf = pw_stream_dequeue_buffer(impl->stream)) == NULL) { pw_log_debug("out of buffers: %m"); @@ -254,7 +254,9 @@ static void playback_stream_process(void *d) } bd = &buf->buffer->datas[0]; - size = SPA_MIN(bd->chunk->size, RINGBUFFER_SIZE); + offs = SPA_MIN(bd->chunk->offset, bd->maxsize); + size = SPA_MIN(bd->chunk->size, bd->maxsize - offs); + size = SPA_MIN(size, RINGBUFFER_SIZE); filled = spa_ringbuffer_get_write_index(&impl->ring, &write_index); @@ -281,8 +283,8 @@ static void playback_stream_process(void *d) } spa_ringbuffer_write_data(&impl->ring, impl->buffer, RINGBUFFER_SIZE, - write_index & RINGBUFFER_MASK, - SPA_PTROFF(bd->data, bd->chunk->offset, void), + write_index & RINGBUFFER_MASK, + SPA_PTROFF(bd->data, offs, void), size); write_index += size; spa_ringbuffer_write_update(&impl->ring, write_index); diff --git a/src/modules/module-raop-sink.c b/src/modules/module-raop-sink.c index 822cd1bf3..654718ddd 100644 --- a/src/modules/module-raop-sink.c +++ b/src/modules/module-raop-sink.c @@ -430,7 +430,7 @@ static void playback_stream_process(void *d) struct pw_buffer *buf; struct spa_data *bd; uint8_t *data; - uint32_t size; + uint32_t offs, size; if ((buf = pw_stream_dequeue_buffer(impl->stream)) == NULL) { pw_log_debug("out of buffers: %m"); @@ -438,8 +438,10 @@ static void playback_stream_process(void *d) } bd = &buf->buffer->datas[0]; - data = SPA_PTROFF(bd->data, bd->chunk->offset, uint8_t); - size = bd->chunk->size; + + offs = SPA_MIN(bd->chunk->offset, bd->maxsize); + size = SPA_MIN(bd->chunk->size, bd->maxsize - offs); + data = SPA_PTROFF(bd->data, offs, uint8_t); while (size > 0 && impl->block_size > 0) { uint32_t avail, to_fill;