mbox series

[FFmpeg-devel,0/3] Fix invalid frees, segfaults and memory leaks in avcodec/fft wrappers

Message ID 20231112151144.2307049-1-sramacher@debian.org
Headers show
Series Fix invalid frees, segfaults and memory leaks in avcodec/fft wrappers | expand

Message

Sebastian Ramacher Nov. 12, 2023, 3:11 p.m. UTC
The wrappers in avcodec/fft which were introduced in 6.1 may lead to invalid frees, segfaults and
memory leaks. Consider the following example program:

#include <libavcodec/avfft.h>

int main() {
  FFTContext* fft = av_fft_init(11, 0);
  av_fft_end(fft);

  FFTContext* mdct = av_mdct_init(11, 0, 1.0);
  av_mdct_end(mdct);

  mdct = av_mdct_init(11, 1, 1.0);
  av_mdct_end(mdct);

  RDFTContext* rdft = av_rdft_init(11, DFT_R2C);
  av_rdft_end(rdft);

  DCTContext* dct = av_dct_init(11, DCT_II);
  av_dct_end(dct);
}

When executed under valgrind, one obtains:

==2300086== Memcheck, a memory error detector
==2300086== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==2300086== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright info
==2300086== Command: ./a.out
==2300086== 
==2300086== Conditional jump or move depends on uninitialised value(s)
==2300086==    at 0x5FB6CBE: av_tx_uninit (in /usr/lib/x86_64-linux-gnu/libavutil.so.58.29.100)
==2300086==    by 0x490B3AA: av_fft_end (in /usr/lib/x86_64-linux-gnu/libavcodec.so.60.31.102)
==2300086==    by 0x1090D7: main (test.c:5)
==2300086==  Uninitialised value was created by a heap allocation
==2300086==    at 0x4845990: memalign (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==2300086==    by 0x4845AED: posix_memalign (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==2300086==    by 0x5FFAC14: av_malloc (in /usr/lib/x86_64-linux-gnu/libavutil.so.58.29.100)
==2300086==    by 0x4A4B4A5: av_fft_init (in /usr/lib/x86_64-linux-gnu/libavcodec.so.60.31.102)
==2300086==    by 0x1090CF: main (test.c:4)
==2300086== 
==2300086== Conditional jump or move depends on uninitialised value(s)
==2300086==    at 0x4843131: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==2300086==    by 0x490B4A5: av_dct_end (in /usr/lib/x86_64-linux-gnu/libavcodec.so.60.31.102)
==2300086==    by 0x10913A: main (test.c:17)
==2300086==  Uninitialised value was created by a heap allocation
==2300086==    at 0x4845990: memalign (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==2300086==    by 0x4845AED: posix_memalign (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==2300086==    by 0x5FFAC14: av_malloc (in /usr/lib/x86_64-linux-gnu/libavutil.so.58.29.100)
==2300086==    by 0x4A4B96F: av_dct_init (in /usr/lib/x86_64-linux-gnu/libavcodec.so.60.31.102)
==2300086==    by 0x109132: main (test.c:16)
==2300086== 
==2300086== 
==2300086== HEAP SUMMARY:
==2300086==     in use at exit: 66,528 bytes in 270 blocks
==2300086==   total heap usage: 1,353 allocs, 1,083 frees, 386,566 bytes allocated
==2300086== 
==2300086== 8,064 (640 direct, 7,424 indirect) bytes in 1 blocks are definitely lost in loss record 247 of 249
==2300086==    at 0x4845990: memalign (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==2300086==    by 0x4845AED: posix_memalign (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==2300086==    by 0x5FFAC14: av_malloc (in /usr/lib/x86_64-linux-gnu/libavutil.so.58.29.100)
==2300086==    by 0x5FFAF80: av_mallocz (in /usr/lib/x86_64-linux-gnu/libavutil.so.58.29.100)
==2300086==    by 0x5FB732D: ??? (in /usr/lib/x86_64-linux-gnu/libavutil.so.58.29.100)
==2300086==    by 0x5FB7616: av_tx_init (in /usr/lib/x86_64-linux-gnu/libavutil.so.58.29.100)
==2300086==    by 0x4A4B678: av_mdct_init (in /usr/lib/x86_64-linux-gnu/libavcodec.so.60.31.102)
==2300086==    by 0x10910A: main (test.c:10)
==2300086== 
==2300086== 8,192 bytes in 1 blocks are possibly lost in loss record 248 of 249
==2300086==    at 0x4845990: memalign (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==2300086==    by 0x4845AED: posix_memalign (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==2300086==    by 0x5FFAC14: av_malloc (in /usr/lib/x86_64-linux-gnu/libavutil.so.58.29.100)
==2300086==    by 0x6030F60: ??? (in /usr/lib/x86_64-linux-gnu/libavutil.so.58.29.100)
==2300086==    by 0x5FBC968: ??? (in /usr/lib/x86_64-linux-gnu/libavutil.so.58.29.100)
==2300086==    by 0x5FB73C8: ??? (in /usr/lib/x86_64-linux-gnu/libavutil.so.58.29.100)
==2300086==    by 0x5FB73C8: ??? (in /usr/lib/x86_64-linux-gnu/libavutil.so.58.29.100)
==2300086==    by 0x5FB7616: av_tx_init (in /usr/lib/x86_64-linux-gnu/libavutil.so.58.29.100)
==2300086==    by 0x4A4B678: av_mdct_init (in /usr/lib/x86_64-linux-gnu/libavcodec.so.60.31.102)
==2300086==    by 0x10910A: main (test.c:10)
==2300086== 
==2300086== LEAK SUMMARY:
==2300086==    definitely lost: 640 bytes in 1 blocks
==2300086==    indirectly lost: 7,424 bytes in 4 blocks
==2300086==      possibly lost: 8,192 bytes in 1 blocks
==2300086==    still reachable: 48,256 bytes in 243 blocks
==2300086==         suppressed: 0 bytes in 0 blocks
==2300086== Reachable blocks (those to which a pointer was found) are not shown.
==2300086== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==2300086== 
==2300086== For lists of detected and suppressed errors, rerun with: -s
==2300086== ERROR SUMMARY: 4 errors from 4 contexts (suppressed: 0 from 0)

This patch series fixes the above issues. The initial issue in av_fft_end was discuvered via the
test suite of r-cran-av.

Sebastian Ramacher (3):
  avcodec/fft: Do not uninit never initialized ctx2
  avcodec/fft: Set potentially unused wrapper variables to avoid invalid
    free/uninit
  avcoded/fft: Fix memory leak if ctx2 is used

 libavcodec/avfft.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)