[FFmpeg-cvslog] avfilter/af_stereowiden: fix read/write past the end of buffer
Alexey Tourbin
git at videolan.org
Sat Jul 30 23:16:06 EEST 2016
ffmpeg | branch: master | Alexey Tourbin <alexey.tourbin at gmail.com> | Tue Jul 12 08:07:54 2016 +0300| [906ee4114117fd50a3c7bf4f07fa26dae5922c26] | committer: Paul B Mahol
avfilter/af_stereowiden: fix read/write past the end of buffer
The stereowiden filter uses a buffer, s->buffer[], and a pointer
within the buffer, s->write, to implement inter-channel delays.
The loop which applies the delayed samples turns out to be faulty.
109 for (n = 0; n < in->nb_samples; n++, src += 2, dst += 2) {
110 const float left = src[0], right = src[1];
111 float *read = s->write + 2;
112
113 if (read > s->buffer + s->length)
114 read = s->buffer;
115
116 dst[0] = drymix * left - crossfeed * right - feedback * read[1];
117 dst[1] = drymix * right - crossfeed * left - feedback * read[0];
118
119 s->write[0] = left;
120 s->write[1] = right;
121
122 if (s->write == s->buffer + s->length)
123 s->write = s->buffer;
124 else
125 s->write += 2;
126 }
For one, the buffer gets written past its end in lines 119-120, before
the bound check is done in lines 122-123. This can be easily confirmed
by valgrind.
==3544== Invalid read of size 4
==3544== at 0x593B41: filter_frame (af_stereowiden.c:116)
==3544== Address 0xb1b03c4 is 4 bytes after a block of size 7,680 alloc'd
==3544==
==3544== Invalid read of size 4
==3544== at 0x593B66: filter_frame (af_stereowiden.c:117)
==3544== Address 0xb1b03c0 is 0 bytes after a block of size 7,680 alloc'd
==3544==
==3544== Invalid write of size 4
==3544== at 0x593B79: filter_frame (af_stereowiden.c:119)
==3544== Address 0xb1b03c0 is 0 bytes after a block of size 7,680 alloc'd
==3544==
==3544== Invalid write of size 4
==3544== at 0x593B7D: filter_frame (af_stereowiden.c:120)
==3544== Address 0xb1b03c4 is 4 bytes after a block of size 7,680 alloc'd
Also, using two separate pointers, s->write and read = s->write + 2,
does not seem to be well thought out. To apply the delay of s->buffer[],
it is enough to read the delayed samples at the current position within
the buffer, and then to store new samples at the same current position.
Thus the application of delayed samples can probably be best described
with a single pointer s->cur.
I also introduce a minor change to ensure that the size of s->buffer[]
is always a multiple of 2. Since the delay parameter is a float, it is
otherwise possible to trick the code into allocating off-by-one buffer.
> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=906ee4114117fd50a3c7bf4f07fa26dae5922c26
---
libavfilter/af_stereowiden.c | 27 +++++++++++----------------
1 file changed, 11 insertions(+), 16 deletions(-)
diff --git a/libavfilter/af_stereowiden.c b/libavfilter/af_stereowiden.c
index 154a8b1..24146ff 100644
--- a/libavfilter/af_stereowiden.c
+++ b/libavfilter/af_stereowiden.c
@@ -34,7 +34,7 @@ typedef struct StereoWidenContext {
float drymix;
float *buffer;
- float *write;
+ float *cur;
int length;
} StereoWidenContext;
@@ -72,11 +72,12 @@ static int config_input(AVFilterLink *inlink)
AVFilterContext *ctx = inlink->dst;
StereoWidenContext *s = ctx->priv;
- s->length = 2 * s->delay * inlink->sample_rate / 1000;
+ s->length = s->delay * inlink->sample_rate / 1000;
+ s->length *= 2;
s->buffer = av_calloc(s->length, sizeof(*s->buffer));
if (!s->buffer)
return AVERROR(ENOMEM);
- s->write = s->buffer;
+ s->cur = s->buffer;
return 0;
}
@@ -106,23 +107,17 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
}
dst = (float *)out->data[0];
- for (n = 0; n < in->nb_samples; n++, src += 2, dst += 2) {
+ for (n = 0; n < in->nb_samples; n++, src += 2, dst += 2, s->cur += 2) {
const float left = src[0], right = src[1];
- float *read = s->write + 2;
- if (read > s->buffer + s->length)
- read = s->buffer;
+ if (s->cur == s->buffer + s->length)
+ s->cur = s->buffer;
- dst[0] = drymix * left - crossfeed * right - feedback * read[1];
- dst[1] = drymix * right - crossfeed * left - feedback * read[0];
+ dst[0] = drymix * left - crossfeed * right - feedback * s->cur[1];
+ dst[1] = drymix * right - crossfeed * left - feedback * s->cur[0];
- s->write[0] = left;
- s->write[1] = right;
-
- if (s->write == s->buffer + s->length)
- s->write = s->buffer;
- else
- s->write += 2;
+ s->cur[0] = left;
+ s->cur[1] = right;
}
if (out != in)
More information about the ffmpeg-cvslog
mailing list