From patchwork Thu Apr 5 12:38:03 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Hendrik Schreiber X-Patchwork-Id: 8337 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.2.1.70 with SMTP id c67csp5995152jad; Thu, 5 Apr 2018 05:38:13 -0700 (PDT) X-Google-Smtp-Source: AIpwx49F0QY4m2Id6TYsrR8PmqK1LhlqVfu7NiUeMfpruaiJ2ksrVnYHARwJza/0Vun17cyUwBrk X-Received: by 10.28.211.147 with SMTP id k141mr9426167wmg.15.1522931893817; Thu, 05 Apr 2018 05:38:13 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1522931893; cv=none; d=google.com; s=arc-20160816; b=Li3gPe3huFOrGnDy3GkJt234xxr0tyrP5Q/ivmwBDzxDJba7U7cUzzVZ4f6DbDponz tRUjpK7BjwTWC/nulvuf/VwDORXdChQMCJYBK1yVwkBjsedmbZdhp2b48OjC8RIALWLv lKVZQY2fBcNzTKkh5aBYifileHIBQr13I6729MvoCU1zSM1KygWZeZE4oxlEEnLwm2Hy V4bdae9SE28qHQWTRDp1OpiqRHj/IRQO41ik7KPa1OhnGduU0Tpho90h+PxqN2Et3DOp CYVNy4OQhmu98NmtJ5wgy6jCOQheyvrsQXsbrlyrsK5IJ0gJZOGhvD9bwpk85B3Fi31I QW9Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:subject:to:date :message-id:mime-version:from:dkim-signature:delivered-to :arc-authentication-results; bh=j+LhjIIv8/r6+iRZ4E1el9+nzkx9jxEaigp7uZUw2Vo=; b=JXiJDaR4gbdEmBesGyAqJatYlZRKoAonOrOWATDukooBxYKEshNa2ddDZjSS3dOAer SMsN9wiLeqde197eJ8IXge9YHWkHDQ20y3cqdxWM6vBCBNR88ltjJ7bT/7PtAxnZphN3 fW6q5b1iUXRgRd+2xlw6DYHL4WF520+chAOeBJ954kCbDZKQqV4a2qkV3QVBKDla3UCw U+lWPQNjIlPe/mu+i5Iuj24VTuSGnHa9WfLMuigQAfVXzSvm8KyFitQUR81SwXTTvMDD GcmAOHcYHMymthrrYOr80Gyn3NC8uRQ98pkcvhYk7I/kkl3KG8BfU0L3YEwB4GqjPkQe qr8Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@tagtraum.com header.s=strato-dkim-0002 header.b=lrzd6rwr; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id f16si3652589wre.462.2018.04.05.05.38.12; Thu, 05 Apr 2018 05:38:13 -0700 (PDT) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@tagtraum.com header.s=strato-dkim-0002 header.b=lrzd6rwr; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 0EC44689228; Thu, 5 Apr 2018 15:37:50 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mo4-p00-ob.smtp.rzone.de (mo4-p00-ob.smtp.rzone.de [81.169.146.161]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 4D9F9680053 for ; Thu, 5 Apr 2018 15:37:43 +0300 (EEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1522931884; s=strato-dkim-0002; d=tagtraum.com; h=To:Date:Message-Id:Subject:Content-Type:From:X-RZG-CLASS-ID: X-RZG-AUTH:From:Subject:Sender; bh=3sQ4pLZn8aWVBtwirav8Xu+JjKWm0XXH3fsRGd/u8/0=; b=lrzd6rwrTv5xznL5/0hv4gZRO8pcJ7gBe7eALOzcJmt2Fy2V6t3U+5nE4yUsvcUA7m RtdonSY+67ZofoJzanvM4h4iHCiUv51ubu2DqgTCh1quoYzM5sf8nDksrRq9eMMCBmtG tnMRGe5z9XUA7XkLRXhxeGTNHS5zUPEPmUAnvjnBPovsZVDqqj3R/i+ElrWHE7gVkpPd gC9spqWsMrFFG5S1pLKtNOqCZTv88rMcUzYqKxMdj7epZUQIqwIeRQGRxezcp4vb4zsf K6tIO6Oy9rs5u1LjxCVNQblpZk+U0+K8k/QKU9RqaUhkxlaftTkr1Qwsx2Gv+9jS3JzK jcNA== X-RZG-AUTH: :JH8kYUGvb+3Ii/qlUn9LjMD+ERhSJBv88/WPfL6XNd42u1S2Y4iU5H4Zyw/rPJZukobE8gdid73DvkTD X-RZG-CLASS-ID: mo00 Received: from mankell.fritz.box (xdsl-78-34-197-43.netcologne.de [78.34.197.43]) by smtp.strato.de (RZmta 43.1 DYNA|AUTH) with ESMTPSA id k01530u35Cc3Hvw (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (curve secp521r1 with 521 ECDH bits, eq. 15360 bits RSA)) (Client did not present a certificate) for ; Thu, 5 Apr 2018 14:38:03 +0200 (CEST) From: Hendrik Schreiber Mime-Version: 1.0 (Mac OS X Mail 11.3 \(3445.6.18\)) Message-Id: <096C6A08-1B2C-4025-B96A-A8B2B5CD6C28@tagtraum.com> Date: Thu, 5 Apr 2018 14:38:03 +0200 To: ffmpeg-devel@ffmpeg.org X-Mailer: Apple Mail (2.3445.6.18) Subject: [FFmpeg-devel] Patch for seg fault in swr_convert_internal() -> sum2_float during dithering X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" 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 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 --- libswresample/swresample.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libswresample/swresample.c b/libswresample/swresample.c index f076b6c8cf..5bd39caac4 100644 --- a/libswresample/swresample.c +++ b/libswresample/swresample.c @@ -678,7 +678,7 @@ static int swr_convert_internal(struct SwrContext *s, AudioData *out, int out_co s->mix_2_1_simd(conv_src->ch[ch], preout->ch[ch], s->dither.noise.ch[ch] + s->dither.noise.bps * s->dither.noise_pos, s->native_simd_one, 0, 0, len1); if(out_count != len1) for(ch=0; chch_count; ch++) - s->mix_2_1_f(conv_src->ch[ch] + off, preout->ch[ch] + off, s->dither.noise.ch[ch] + s->dither.noise.bps * s->dither.noise_pos + off + len1, s->native_one, 0, 0, out_count - len1); + s->mix_2_1_f(conv_src->ch[ch] + off, preout->ch[ch] + off, s->dither.noise.ch[ch] + s->dither.noise.bps * s->dither.noise_pos + off, s->native_one, 0, 0, out_count - len1); } else { for(ch=0; chch_count; ch++) s->mix_2_1_f(conv_src->ch[ch], preout->ch[ch], s->dither.noise.ch[ch] + s->dither.noise.bps * s->dither.noise_pos, s->native_one, 0, 0, out_count);