Opened 10 years ago

Closed 10 years ago

#3387 closed defect (fixed)

Out of bound memory accesses with png encoder (and possibly crashes)

Reported by: gjdfgh Owned by:
Priority: important Component: avcodec
Version: git-master Keywords: png
Cc: Blocked By:
Blocking: Reproduced by developer: yes
Analyzed by developer: no

Description

Summary of the bug:
How to reproduce:

% ffmpeg -i in.mkv -pred 5 -compression_level 7 out%03d.png

This results in out of bound accesses as reported by valgrind:

==6850== Invalid read of size 8
==6850==    at 0x86E352D: diff_bytes_mmx (dsputilenc_mmx.c:667)
==6850==    by 0x8570D4C: png_filter_row.isra.0 (pngenc.c:126)
==6850==    by 0x8570DFB: png_choose_filter (pngenc.c:170)
==6850==    by 0x8571306: encode_frame (pngenc.c:393)
==6850==    by 0x86159C3: avcodec_encode_video2 (utils.c:1890)
==6850==    by 0x8778CDA: worker (frame_thread_encoder.c:93)
==6850==    by 0x470DCF0: start_thread (pthread_create.c:311)
==6850==    by 0x4811C3D: clone (clone.S:131)
==6850==  Address 0xc62205d is 3 bytes before a block of size 2,959,903 alloc'd
==6850==    at 0x402AF50: memalign (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==6850==    by 0x402B07E: posix_memalign (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==6850==    by 0x8879EF7: av_malloc (mem.c:94)
==6850==    by 0x886B469: av_buffer_allocz (buffer.c:70)
==6850==    by 0x886BB40: av_buffer_pool_get (buffer.c:305)
==6850==    by 0x861389B: avcodec_default_get_buffer2 (utils.c:677)
==6850==    by 0x8614694: ff_get_buffer (utils.c:973)
==6850==    by 0x877935A: ff_thread_video_encode_frame (frame_thread_encoder.c:250)
==6850==    by 0x8615AE1: avcodec_encode_video2 (utils.c:1873)
==6850==    by 0x80D02D4: reap_filters (ffmpeg.c:997)
==6850==    by 0x80B70B3: main (ffmpeg.c:3375)
==6850== 

I suspect this is also the cause of mysterious sporadic crashes on OSX when encoding png reported by some of my users.

Tested with git 89c5de6.

Patches should be submitted to the ffmpeg-devel mailing list and not this bug tracker.

Change History (2)

comment:1 by Carl Eugen Hoyos, 10 years ago

Keywords: png added
Reproduced by developer: set
Status: newopen

Clément sent a patch:
http://thread.gmane.org/gmane.comp.video.ffmpeg.devel/174892

$ valgrind ./ffmpeg_g -i tests/lena.pnm -pred 3 -vcodec png -f null -
==9870== Memcheck, a memory error detector
==9870== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al.
==9870== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
==9870== Command: ./ffmpeg_g -i tests/lena.pnm -pred 3 -vcodec png -f null -
==9870==
ffmpeg version N-60801-g6e63867 Copyright (c) 2000-2014 the FFmpeg developers
  built on Feb 21 2014 15:42:30 with gcc 4.7 (SUSE Linux)
  configuration: --enable-gpl
  libavutil      52. 65.100 / 52. 65.100
  libavcodec     55. 52.102 / 55. 52.102
  libavformat    55. 33.100 / 55. 33.100
  libavdevice    55. 10.100 / 55. 10.100
  libavfilter     4.  1.103 /  4.  1.103
  libswscale      2.  5.101 /  2.  5.101
  libswresample   0. 17.104 /  0. 17.104
  libpostproc    52.  3.100 / 52.  3.100
Input #0, image2, from 'tests/lena.pnm':
  Duration: 00:00:00.04, start: 0.000000, bitrate: N/A
    Stream #0:0: Video: ppm, rgb24, 256x256, 25 tbr, 25 tbn, 25 tbc
Output #0, null, to 'pipe:':
  Metadata:
    encoder         : Lavf55.33.100
    Stream #0:0: Video: png, rgb24, 256x256, q=2-31, 200 kb/s, 90k tbn, 25 tbc
Stream mapping:
  Stream #0:0 -> #0:0 (ppm -> png)
Press [q] to stop, [?] for help
==9870== Thread 18:
==9870== Invalid read of size 8
==9870==    at 0xB498A6: diff_bytes_mmx (dsputilenc_mmx.c:667)
==9870==    by 0x99AB40: png_filter_row.isra.0 (pngenc.c:126)
==9870==    by 0x99ABD5: png_choose_filter (pngenc.c:170)
==9870==    by 0x99B09F: encode_frame (pngenc.c:393)
==9870==    by 0xA51273: avcodec_encode_video2 (utils.c:1890)
==9870==    by 0xC10868: worker (frame_thread_encoder.c:93)
==9870==    by 0x5D1AE0D: start_thread (in /lib64/libpthread-2.15.so)
==9870==  Address 0x762dbfd is 3 bytes before a block of size 248,863 alloc'd
==9870==    at 0x4C290FE: memalign (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==9870==    by 0x4C291A7: posix_memalign (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==9870==    by 0xD16099: av_malloc (mem.c:94)
==9870==    by 0xD0991D: av_buffer_allocz (buffer.c:70)
==9870==    by 0xD09EFB: av_buffer_pool_get (buffer.c:305)
==9870==    by 0xA4E113: video_get_buffer (utils.c:677)
==9870==    by 0xA4FCC6: get_buffer_internal (utils.c:972)
==9870==    by 0xA50225: ff_get_buffer (utils.c:984)
==9870==    by 0xC10E73: ff_thread_video_encode_frame (frame_thread_encoder.c:254)
==9870==    by 0x47CDF7: reap_filters (ffmpeg.c:997)
==9870==    by 0x466DA7: main (ffmpeg.c:3399)
==9870==
frame=    1 fps=0.0 q=0.0 Lsize=N/A time=00:00:00.04 bitrate=N/A
video:125kB audio:0kB subtitle:0 data:0 global headers:0kB muxing overhead -100.017128%
==9870==
==9870== HEAP SUMMARY:
==9870==     in use at exit: 80 bytes in 2 blocks
==9870==   total heap usage: 2,252 allocs, 2,250 frees, 2,850,769 bytes allocated
==9870==
==9870== LEAK SUMMARY:
==9870==    definitely lost: 0 bytes in 0 blocks
==9870==    indirectly lost: 0 bytes in 0 blocks
==9870==      possibly lost: 0 bytes in 0 blocks
==9870==    still reachable: 80 bytes in 2 blocks
==9870==         suppressed: 0 bytes in 0 blocks
==9870== Rerun with --leak-check=full to see details of leaked memory
==9870==
==9870== For counts of detected and suppressed errors, rerun with: -v
==9870== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 2 from 2)

comment:2 by Clément Bœsch, 10 years ago

Resolution: fixed
Status: openclosed
Note: See TracTickets for help on using tickets.