[FFmpeg-devel] [FFmpeg-cvslog] avfilter/video: don't zero allocated buffers if memory poisoning is used

[James Almer]

On 8/13/2024 4:22 AM, Martin Storsjö wrote:
> On Tue, 13 Aug 2024, James Almer wrote:
> 
>> ffmpeg | branch: master | James Almer <jamrial at gmail.com> | Sat Aug 10 
>> 21:31:16 2024 -0300| [41307ff3e9384c51d646bff7e3dcf0d554098a8f] | 
>> committer: James Almer
>>
>> avfilter/video: don't zero allocated buffers if memory poisoning is used
>>
>> Same as in avcodec/get_buffer.c
>> Should help in debugging use of uninitialized memory.
>>
>> Signed-off-by: James Almer <jamrial at gmail.com>
>>
>>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/? 
>>> a=commit;h=41307ff3e9384c51d646bff7e3dcf0d554098a8f
>> ---
>>
>> libavfilter/video.c | 12 ++++++++----
>> 1 file changed, 8 insertions(+), 4 deletions(-)
> 
> This change broke a bunch of fate tests - in particular fate-vsynth3- 
> rpza and most fate-filter-pixfmts-*.
> 
> The issue doesn't show up in normal builds of ffmpeg, unless building 
> with --enable-memory-poisoning. And in such a build, tools like valgrind 
> doesn't detect the issue right away, as the memory poisoning causes the 
> buffers to be deterministically initialized to a nonzero value (but the 
> fate test produces the wrong output).

in fate-vsynth3-rpza i'm seeing the following when comparing 
vsynth3-rpza.mov before and after:

>  000000f0  4c 20 54 20 60 25 64 25  60 02 60 23 68 44 68 65  |L T `%d%`.`#hDhe|
>  00000100  68 04 6c 25 60 86 64 86  5c 45 5c 67 5c 87 5c c8  |h.l%`.d.\E\g\.\.|
>  00000110  5c 66 64 87 5c c8 64 c8  68 85 68 c7 68 e7 69 08  |\fd.\.d.h.h.h.i.|
> -00000120  60 c8 68 c8 5d 2a 65 4a  5c e8 5d 0a 00 00 00 00  |`.h.]*eJ\.].....|
> -00000130  61 2a 65 2a 00 00 00 00  69 28 69 6a 00 00 00 00  |a*e*....i(ij....|
> -00000140  61 6b 65 8c 00 00 00 00  6d 16 6d 38 6d 58 6d 99  |ake.....m.m8mXm.|
> +00000120  60 c8 68 c8 5d 2a 65 4a  5c e8 5d 0a 2a 2a 2a 2a  |`.h.]*eJ\.].****|
> +00000130  61 2a 65 2a 2a 2a 2a 2a  69 28 69 6a 2a 2a 2a 2a  |a*e*****i(ij****|
> +00000140  61 6b 65 8c 2a 2a 2a 2a  6d 16 6d 38 6d 58 6d 99  |ake.****m.m8mXm.|
>  00000150  6d 37 75 57 6d 99 75 99  75 56 75 98 75 b8 75 d9  |m7uWm.u.uVu.u.u.|
>  00000160  75 97 7d 98 75 d9 7d f9  6d b9 71 db 6d fb 6e 3c  |u.}.u.}.m.q.m.n<|
>  00000170  71 da 75 fa 6e 3b 76 3c  75 f9 76 3b 76 5b 7a 7c  |q.u.n;v<u.v;v[z||

This repeated like a hundred times. You can see 0x2a (FF_MEMORY_POISON) 
replacing a bunch of zeroes. So i guess the encoder is either 
propagating packets with more bytes than needed, or it forgot to zero 
them itself.

I tried using a non zeroed, non poisoned buffer to see where asan/ubsan 
complained, but despite the output being non deterministic (Albeit only 
changing in one every several runs), neither asan or ubsan complained 
about it.

Valgrind however did, and gave me this:

> ==119305== Syscall param write(buf) points to uninitialised byte(s)
> ==119305==    at 0x4A778BF: __libc_write (write.c:26)
> ==119305==    by 0x4A778BF: write (write.c:24)
> ==119305==    by 0x932785: file_write (file.c:157)
> ==119305==    by 0x729635: retry_transfer_wrapper (avio.c:517)
> ==119305==    by 0x72983E: ffurl_write2 (avio.c:574)
> ==119305==    by 0x72A8AF: writeout (aviobuf.c:142)
> ==119305==    by 0x72AA38: flush_buffer (aviobuf.c:167)
> ==119305==    by 0x72B16A: avio_seek (aviobuf.c:302)
> ==119305==    by 0x8248C7: mov_write_trailer (movenc.c:8391)
> ==119305==    by 0x84E372: av_write_trailer (mux.c:1319)
> ==119305==    by 0x23B17D: of_write_trailer (ffmpeg_mux.c:753)
> ==119305==    by 0x260DA3: transcode (ffmpeg.c:888)
> ==119305==    by 0x2610DB: main (ffmpeg.c:990)
> ==119305==  Address 0x4ba99c8 is 264 bytes inside a block of size 262,144 alloc'd
> ==119305==    at 0x484DE30: memalign (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==119305==    by 0x484DF92: posix_memalign (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==119305==    by 0x19BB47D: av_malloc (mem.c:107)
> ==119305==    by 0x729214: ffio_fdopen (avio.c:429)
> ==119305==    by 0x7294C6: ffio_open_whitelist (avio.c:483)
> ==119305==    by 0x729544: avio_open2 (avio.c:494)
> ==119305==    by 0x246946: of_open (ffmpeg_mux_init.c:3260)
> ==119305==    by 0x24AEF1: open_files (ffmpeg_opt.c:1274)
> ==119305==    by 0x24B106: ffmpeg_parse_options (ffmpeg_opt.c:1321)
> ==119305==    by 0x26100B: main (ffmpeg.c:972)
> ==119305==  Uninitialised value was created by a heap allocation
> ==119305==    at 0x484DE30: memalign (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==119305==    by 0x484DF92: posix_memalign (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==119305==    by 0x19BB47D: av_malloc (mem.c:107)
> ==119305==    by 0x199CF79: av_buffer_alloc (buffer.c:82)
> ==119305==    by 0x199DA3B: pool_alloc_buffer (buffer.c:369)
> ==119305==    by 0x199DBC0: av_buffer_pool_get (buffer.c:407)
> ==119305==    by 0x2C26EB: ff_frame_pool_get (framepool.c:218)
> ==119305==    by 0x5E7653: ff_default_get_video_buffer2 (video.c:100)
> ==119305==    by 0x5E76E5: ff_default_get_video_buffer (video.c:113)
> ==119305==    by 0x5E7753: ff_get_video_buffer (video.c:126)
> ==119305==    by 0x5E744D: ff_null_get_video_buffer (video.c:46)
> ==119305==    by 0x5E7734: ff_get_video_buffer (video.c:123)

I didn't check the filter-pixfmts tests, but all of them fail with vuyx 
and xv36le, so it may be swscale using uninitialized bytes for those two.