[FFmpeg-devel] Patch for seg fault in swr_convert_internal() -> sum2_float during dithering

[Michael Niedermayer]

On Thu, Apr 05, 2018 at 02:38:03PM +0200, Hendrik Schreiber wrote:
> Hey there,
> 
> I have recently switched to using FFmpeg for conversions of 24bit stereo WAV to 16bit stereo WAV (with dithering).
> 
> For some very large files, I occasionally encountered a segmentation fault in _sum2_float. Unfortunately, I was not able to reproduce the issue in a small test setting, but only in a quite large environment.
> 
> Debugging showed that the issue was caused in function swr_convert_internal() in swresample.c, specifically in line 681
> 
> s->mix_2_1_f(
> conv_src->ch[ch] + off, // out array
> preout->ch[ch] + off,   // in1 array
> s->dither.noise.ch[ch] + s->dither.noise.bps * s->dither.noise_pos + off + len1, // in2 array
> s->native_one,          // coefficients
> 0,                      // coefficient index 1
> 0,                      // coefficient index 2
> out_count - len1        // length
> );
> 
> (https://github.com/FFmpeg/FFmpeg/blob/53688b62ca96ad9a3b0e7d201caca61c79a68648/libswresample/swresample.c#L681)
> 
> where dithering is applied. Here, s->mix_2_1_f() is the same as sum2_float(). The in2 array pointer is too large.
> 
> I was able to log values before one of the crashs and found:
> 
> out_count=682
> len1=672
> (out_count - len1)=10
> off=2688
> s->dither.noise.bps  =4
> s->dither.noise_pos  =130262
> s->dither.noise.count=131072
> s->dither.noise.bps * s->dither.noise_pos + off + len1 = 524408 // in2 start address offset greater than buffer size!!!
> 
> The buffer count has a total size of s->dither.noise.count * s->dither.noise.bps = 524288
> Therefore the start address for the in2 array (3rd argument for mix_2_1_f(...)) is already outside of the buffer.
> 
> I was not able to find a reason for the “+len1” in “s->dither.noise.bps * s->dither.noise_pos + off + len1”. It looks out of place to me. Without it the buffer overrun does not occur.
> 
> “make fate” worked like a charm.
> 
> -hendrik
> 
> tagtraum industries incorporated
> 724 Nash Drive
> Raleigh, NC 27608
> USA
> +1 (919) 809-7797
> http://www.tagtraum.com/
> http://www.beatunes.com/
> 

> From ef79e9286c4b8b694715e978f48abe1601956c0e Mon Sep 17 00:00:00 2001
> From: Hendrik Schreiber <hs at tagtraum.com>
> Date: Thu, 5 Apr 2018 13:58:37 +0200
> Subject: [PATCH] Fix for seg fault in swr_convert_internal() -> sum2_float
>  during dithering. Removed +len1 in call to s->mix_2_1_f() as I found no
>  logical explanation for it. After removal, problem was gone.
> 
> Signed-off-by: Hendrik Schreiber <hs at tagtraum.com>
> ---
>  libswresample/swresample.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

will apply

thanks

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Democracy is the form of government in which you can choose your dictator
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180407/176d4286/attachment.sig>