PR #23444 opened by michaelni URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/23444 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/23444.patch
swresample: pad resample destination instead of overrunning it Fixes the overflow exercised by fate-swr-resample-realloc. Alternative to #23442 / #23053 Found-by: Ivan Grigorev <[email protected]> Signed-off-by: Michael Niedermayer <[email protected]> >From 15976a0266ad0c832bbeace6e6203a64e005e565 Mon Sep 17 00:00:00 2001 From: Ivan Grigorev <[email protected]> Date: Wed, 10 Jun 2026 17:42:43 +0200 Subject: [PATCH 1/2] swresample/tests: add resample realloc regression test Add a regression test exercising the swr_convert(N) -> swr_convert(2N) edge case: the second call reuses the internal preout buffer at full capacity, with no trailing slack from swri_realloc_audio()'s amortized doubling. internal_sample_fmt is forced to S16P to reach the int16 SIMD resample path, where ff_resample_common_int16_sse2 overruns its destination by 2 bytes on the last iteration. Without a resampler fix this test fails under valgrind/ASAN with a heap-buffer-overflow (Invalid write of size 4, 2 bytes past the end). Signed-off-by: Ivan Grigorev <[email protected]> --- libswresample/Makefile | 3 +- .../tests/swresample_resample_realloc.c | 120 ++++++++++++++++++ tests/fate/libswresample.mak | 4 + tests/ref/fate/swr-resample-realloc | 0 4 files changed, 126 insertions(+), 1 deletion(-) create mode 100644 libswresample/tests/swresample_resample_realloc.c create mode 100644 tests/ref/fate/swr-resample-realloc diff --git a/libswresample/Makefile b/libswresample/Makefile index 8b9a0fe6f5..12fbfc35c1 100644 --- a/libswresample/Makefile +++ b/libswresample/Makefile @@ -24,4 +24,5 @@ SHLIBOBJS += log2_tab.o # Windows resource file SHLIBOBJS-$(HAVE_GNU_WINDRES) += swresampleres.o -TESTPROGS = swresample +TESTPROGS = swresample \ + swresample_resample_realloc \ diff --git a/libswresample/tests/swresample_resample_realloc.c b/libswresample/tests/swresample_resample_realloc.c new file mode 100644 index 0000000000..a72194a7f8 --- /dev/null +++ b/libswresample/tests/swresample_resample_realloc.c @@ -0,0 +1,120 @@ +/* + * Exercise the swr_convert(N) -> swr_convert(2N) edge case where the + * second call reuses the internal preout buffer at full capacity, with + * no trailing slack from swri_realloc_audio()'s amortized doubling. + * Forces internal_sample_fmt=S16P to reach the int16 SIMD resample path. + * + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include <stdint.h> +#include <stdio.h> +#include <stdlib.h> + +#include "libavutil/channel_layout.h" +#include "libavutil/mem.h" +#include "libavutil/opt.h" +#include "libavutil/samplefmt.h" +#include "libswresample/swresample.h" + +int main(void) +{ + const int IN_RATE = 48000; + const int OUT_RATE = 16000; + /* First call asks for N out frames, second call asks for 2N. */ + const int N1_OUT = 160; + const int N2_OUT = 320; + const int N1_IN = N1_OUT * IN_RATE / OUT_RATE; /* 480 */ + const int N2_IN = N2_OUT * IN_RATE / OUT_RATE; /* 960 */ + + SwrContext *swr = swr_alloc(); + AVChannelLayout mono = AV_CHANNEL_LAYOUT_MONO; + int ret = 0; + + if (!swr) { + fprintf(stderr, "swr_alloc failed\n"); + return 1; + } + + av_opt_set_chlayout (swr, "in_chlayout", &mono, 0); + av_opt_set_chlayout (swr, "out_chlayout", &mono, 0); + av_opt_set_int (swr, "in_sample_rate", IN_RATE, 0); + av_opt_set_int (swr, "out_sample_rate", OUT_RATE, 0); + av_opt_set_sample_fmt (swr, "in_sample_fmt", AV_SAMPLE_FMT_S16, 0); + av_opt_set_sample_fmt (swr, "out_sample_fmt", AV_SAMPLE_FMT_S16, 0); + /* Force the int16 SIMD resample path. */ + av_opt_set_sample_fmt (swr, "internal_sample_fmt", AV_SAMPLE_FMT_S16P, 0); + + if ((ret = swr_init(swr)) < 0) { + fprintf(stderr, "swr_init failed: %d\n", ret); + ret = 1; + goto end; + } + + { + int16_t *input = av_calloc(N2_IN, sizeof(int16_t)); + int16_t *out = av_calloc(N2_OUT, sizeof(int16_t)); + const uint8_t *in_planes[1]; + uint8_t *out_planes[1]; + int i, n; + + if (!input || !out) { + fprintf(stderr, "alloc failed\n"); + av_free(input); + av_free(out); + ret = 1; + goto end; + } + + /* Non-zero samples so the SIMD inner loop produces real data. */ + for (i = 0; i < N2_IN; ++i) + input[i] = (int16_t)((i * 7) & 0x3fff); + + /* Call #1: out_count = N. swri_realloc_audio() doubles count and + * grows s->preout to capacity 2N (e.g. 640 bytes for N=160). */ + in_planes[0] = (const uint8_t *)input; + out_planes[0] = (uint8_t *)out; + n = swr_convert(swr, out_planes, N1_OUT, in_planes, N1_IN); + if (n < 0) { + fprintf(stderr, "swr_convert call#1 failed: %d\n", n); + av_free(input); + av_free(out); + ret = 1; + goto end; + } + + /* Call #2: out_count = 2N. a->count == 2N, so swri_realloc_audio() + * skips realloc and reuses the existing buffer at full capacity. */ + in_planes[0] = (const uint8_t *)input; + out_planes[0] = (uint8_t *)out; + n = swr_convert(swr, out_planes, N2_OUT, in_planes, N2_IN); + if (n < 0) { + fprintf(stderr, "swr_convert call#2 failed: %d\n", n); + av_free(input); + av_free(out); + ret = 1; + goto end; + } + + av_free(input); + av_free(out); + } + +end: + swr_free(&swr); + return ret; +} diff --git a/tests/fate/libswresample.mak b/tests/fate/libswresample.mak index 5317d4148d..fbf9cf064c 100644 --- a/tests/fate/libswresample.mak +++ b/tests/fate/libswresample.mak @@ -1107,3 +1107,7 @@ fate-swr-custom-rematrix: REF = 2a14a44deb4ae26e3b474ddbfbc048f8 FATE_SWR += $(FATE_SWR_CUSTOM_REMATRIX-yes) FATE_FFMPEG += $(FATE_SWR) fate-swr: $(FATE_SWR) + +FATE_SWR += fate-swr-resample-realloc +fate-swr-resample-realloc: libswresample/tests/swresample_resample_realloc$(EXESUF) +fate-swr-resample-realloc: CMD = run libswresample/tests/swresample_resample_realloc$(EXESUF) diff --git a/tests/ref/fate/swr-resample-realloc b/tests/ref/fate/swr-resample-realloc new file mode 100644 index 0000000000..e69de29bb2 -- 2.52.0 >From dd5d86be8e3df6535e783be21a17e6565a8ec0d7 Mon Sep 17 00:00:00 2001 From: Michael Niedermayer <[email protected]> Date: Wed, 10 Jun 2026 17:57:30 +0200 Subject: [PATCH 2/2] swresample: pad resample destination instead of overrunning it Fixes the overflow exercised by fate-swr-resample-realloc. Alternative to #23442 / #23053 Found-by: Ivan Grigorev <[email protected]> Signed-off-by: Michael Niedermayer <[email protected]> --- libswresample/resample.h | 1 + libswresample/swresample.c | 12 +++++++++--- libswresample/x86/resample_init.c | 3 +++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/libswresample/resample.h b/libswresample/resample.h index 1731dad3cf..7056a95775 100644 --- a/libswresample/resample.h +++ b/libswresample/resample.h @@ -49,6 +49,7 @@ typedef struct ResampleContext { int felem_size; int filter_shift; int phase_count_compensation; /* desired phase_count when compensation is enabled */ + int dst_padding; /* extra destination samples resample may write */ struct { void (*resample_one)(void *dst, const void *src, diff --git a/libswresample/swresample.c b/libswresample/swresample.c index d777efd802..28a06165e2 100644 --- a/libswresample/swresample.c +++ b/libswresample/swresample.c @@ -21,6 +21,7 @@ #include "libavutil/mem.h" #include "libavutil/opt.h" #include "swresample_internal.h" +#include "resample.h" #include "audioconvert.h" #include "libavutil/avassert.h" #include "libavutil/channel_layout.h" @@ -603,18 +604,22 @@ static int swr_convert_internal(struct SwrContext *s, AudioData *out, int out_co // in_max= out_count*(int64_t)s->in_sample_rate / s->out_sample_rate + resample_filter_taps; // in_count= FFMIN(in_count, in_in + 2 - s->hist_buffer_count); + int resample_dst_slack = 0; + if (s->resample && s->engine == SWR_ENGINE_SWR) + resample_dst_slack = s->resample->dst_padding; + if((ret=swri_realloc_audio(&s->postin, in_count))<0) return ret; if(s->resample_first){ av_assert0(s->midbuf.ch_count == s->used_ch_layout.nb_channels); - if((ret=swri_realloc_audio(&s->midbuf, out_count))<0) + if((ret=swri_realloc_audio(&s->midbuf, out_count + resample_dst_slack))<0) return ret; }else{ av_assert0(s->midbuf.ch_count == s->out.ch_count); if((ret=swri_realloc_audio(&s->midbuf, in_count))<0) return ret; } - if((ret=swri_realloc_audio(&s->preout, out_count))<0) + if((ret=swri_realloc_audio(&s->preout, out_count + resample_dst_slack))<0) return ret; postin= &s->postin; @@ -634,7 +639,8 @@ static int swr_convert_internal(struct SwrContext *s, AudioData *out, int out_co preout= midbuf; if(s->int_sample_fmt == s->out_sample_fmt && s->out.planar - && !(s->out_sample_fmt==AV_SAMPLE_FMT_S32P && (s->dither.output_sample_bits&31))){ + && !(s->out_sample_fmt==AV_SAMPLE_FMT_S32P && (s->dither.output_sample_bits&31)) + && !resample_dst_slack){ if(preout==in){ out_count= FFMIN(out_count, in_count); //TODO check at the end if this is needed or redundant av_assert0(s->in.planar); //we only support planar internally so it has to be, we support copying non planar though diff --git a/libswresample/x86/resample_init.c b/libswresample/x86/resample_init.c index 5e12f2bddf..f8fe359cba 100644 --- a/libswresample/x86/resample_init.c +++ b/libswresample/x86/resample_init.c @@ -51,13 +51,16 @@ av_cold void swri_resample_dsp_x86_init(ResampleContext *c) switch(c->format){ case AV_SAMPLE_FMT_S16P: + /* The int16 SIMD store writes 4 bytes per 2-byte output sample */ if (EXTERNAL_SSE2(mm_flags)) { c->dsp.resample_linear = ff_resample_linear_int16_sse2; c->dsp.resample_common = ff_resample_common_int16_sse2; + c->dst_padding = 1; } if (EXTERNAL_XOP(mm_flags)) { c->dsp.resample_linear = ff_resample_linear_int16_xop; c->dsp.resample_common = ff_resample_common_int16_xop; + c->dst_padding = 1; } break; case AV_SAMPLE_FMT_FLTP: -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
