From 8cf12b45b3e162a8e376a7a51ec1efa9c10551d6 Mon Sep 17 00:00:00 2001 From: Andreas Pape Date: Fri, 27 Nov 2020 11:59:57 +0100 Subject: [PATCH] pcm: ioplug: Limit transfer size to buffer boundary Commit 1714332719fc91507ca24dd3567e50d7094b3001 introduced 2nd transfer() call to transfer all remaining available frames. Unfortunately this valuable fix introduced two regressions: a) If the prior calculated avail value exceeds the buffer size a too large size value is passed to the underlaying plugin and results in memory corruption if not blocked by attached plugin internally. Avail values > buffer size can happen if e.g. xrun detection is disabled, as avail is calculated by pure difference between hw and app position. This patch limits 2nd transfer call to fit into area. b) the 2nd transfer callback is executed with adapted offset+size but without updating pcm->appl_ptr. An underlaying plugin may rely on consistent pointer positions. To maintain consistency the appl_ptr is temporarily forwarded during the 2nd transfer call and rewinded afterwards. Signed-off-by: Andreas Pape --- src/pcm/pcm_ioplug.c | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/src/pcm/pcm_ioplug.c b/src/pcm/pcm_ioplug.c index a437ca32..b148ff1a 100644 --- a/src/pcm/pcm_ioplug.c +++ b/src/pcm/pcm_ioplug.c @@ -724,22 +724,45 @@ static snd_pcm_sframes_t snd_pcm_ioplug_avail_update(snd_pcm_t *pcm) pcm->access != SND_PCM_ACCESS_RW_INTERLEAVED && pcm->access != SND_PCM_ACCESS_RW_NONINTERLEAVED) { if (io->data->callback->transfer) { + int err; const snd_pcm_channel_area_t *areas; snd_pcm_uframes_t offset, size = UINT_MAX; snd_pcm_sframes_t result; + /* avail may be > buffer_size. Copy up to buffer size. */ + snd_pcm_uframes_t avail_part = avail % pcm->buffer_size; + if (avail && !avail_part) /*avail = (n*buffer_size) results in buffer_size */ + avail_part = pcm->buffer_size; - __snd_pcm_mmap_begin(pcm, &areas, &offset, &size); + size = avail_part; + err = __snd_pcm_mmap_begin(pcm, &areas, &offset, &size); + if (err < 0) + return err; result = io->data->callback->transfer(io->data, areas, offset, size); if (result < 0) return result; + avail_part -= size; /* If the available data doesn't fit in the contiguous area at the end of the mmap we must transfer the remaining data to the beginning of the mmap. */ - if (size < avail) { + if (avail_part > 0) { + /* temporarily move app ptr forward. + This is mandatory for this 2nd call to mmap_begin() and is + in addition needed to always find consistent state in underlaying + plugin during transfer callback which may access app_ptr as 'read only' */ + snd_pcm_uframes_t fwd = size; + size = avail_part; + snd_pcm_ioplug_forward(pcm, fwd); + err = __snd_pcm_mmap_begin(pcm, &areas, &offset, &size); + if (err < 0) { + snd_pcm_ioplug_rewind(pcm, fwd); + return err; + } result = io->data->callback->transfer(io->data, areas, - 0, avail - size); + offset, size); + snd_pcm_ioplug_rewind(pcm, fwd); + if (result < 0) return result; }