Bug 480042 - False-positive "uninitialised value" with GStreamer and ffmpeg
Summary: False-positive "uninitialised value" with GStreamer and ffmpeg
Status: RESOLVED DUPLICATE of bug 132232
Alias: None
Product: valgrind
Classification: Developer tools
Component: memcheck (show other bugs)
Version: 3.18.1
Platform: Mint (Ubuntu based) Linux
: NOR minor
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-01-19 11:25 UTC by Loïc Le Page
Modified: 2024-01-28 16:26 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Loïc Le Page 2024-01-19 11:25:53 UTC
# SUMMARY

The ffmpeg h264 software decoder used with a high profile in a GStreamer pipeline produces false-positive "uninitialised value" errors when run under Valgrind.

I tested it with Valgrind 3.18.1 and 3.22.0 and GStreamer from versions 1.20.3 to the git main branch and ffmpeg from 4.4.2 to 6.1.

# STEPS TO REPRODUCE

1. install GStreamer on your system (https://gstreamer.freedesktop.org/documentation/installing/on-linux.html)
2. open a terminal and call:
```
valgrind gst-launch-1.0 videotestsrc num-buffers=1 ! video/x-raw,width=256,height=66 ! x264enc ! h264parse ! video/x-h264,profile=high ! avdec_h264 max-threads=1 thread-type=frame direct-rendering=true ! filesink location=raw.bin
```

# OBSERVED RESULT

We have some errors for reading uninitialised memory.
```
==116012== Thread 2 videotestsrc0:sr:
==116012== Syscall param writev(vector[...]) points to uninitialised byte(s)
==116012==    at 0x4CCCA8D: __writev (writev.c:26)
==116012==    by 0x4CCCA8D: writev (writev.c:24)
==116012==    by 0xC52473B: gst_writev (gstelements_private.c:157)
==116012==    by 0xC524BE5: gst_writev_iovecs (gstelements_private.c:254)
==116012==    by 0xC525FA1: gst_writev_buffer_list (gstelements_private.c:554)
==116012==    by 0xC533860: gst_file_sink_render_list_internal (gstfilesink.c:775)
==116012==    by 0xC533B3A: gst_file_sink_flush_buffer (gstfilesink.c:837)
==116012==    by 0xC533211: gst_file_sink_event (gstfilesink.c:696)
==116012==    by 0x8BA8612: gst_base_sink_event (gstbasesink.c:3680)
==116012==    by 0x491ED3D: gst_pad_send_event_unchecked (gstpad.c:5963)
==116012==    by 0x491D55A: gst_pad_push_event_unchecked (gstpad.c:5596)
==116012==    by 0x4916BE7: push_sticky (gstpad.c:4080)
==116012==    by 0x490BD29: events_foreach (gstpad.c:613)
==116012==  Address 0x756f81f is 2,255 bytes inside a block of size 25,511 alloc'd
==116012==    at 0x484C899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==116012==    by 0x4A76738: g_malloc (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7200.4)
==116012==    by 0x48A919C: _sysmem_new_block (gstallocator.c:434)
==116012==    by 0x48A9561: default_alloc (gstallocator.c:533)
==116012==    by 0x48A8DCE: gst_allocator_alloc (gstallocator.c:336)
==116012==    by 0x48B8648: gst_buffer_new_allocate (gstbuffer.c:902)
==116012==    by 0x8AAF685: video_buffer_pool_alloc (gstvideopool.c:251)
==116012==    by 0x48BE2A8: do_alloc_buffer (gstbufferpool.c:286)
==116012==    by 0x48C00AF: default_acquire_buffer (gstbufferpool.c:1140)
==116012==    by 0x48C06D2: gst_buffer_pool_acquire_buffer (gstbufferpool.c:1299)
==116012==    by 0x8A9C66E: gst_video_decoder_allocate_output_frame_with_params (gstvideodecoder.c:4705)
==116012==    by 0x8A9C308: gst_video_decoder_allocate_output_frame (gstvideodecoder.c:4642)
==116012==
```

# EXPECTED RESULT

No errors (see below for more details).

# SOFTWARE/OS VERSIONS

Linux Mint 21.3 (based on Ubuntu 22.04.3)

# ADDITIONAL INFORMATION

I did different tests that show that the memory block is effectively fully initialised (see: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/5882#note_2243694)

To sum up what happens:
- during the decoding of an H264 frame ffmpeg (through libavcodec) requires the allocation of a block of memory from GStreamer
- GStreamer allocates this block of memory with `malloc` and gives back to ffmpeg pointers for each image plane
- ffmpeg fills the pixels colors in each plane using this function: https://gitlab.freedesktop.org/gstreamer/meson-ports/ffmpeg/-/blob/meson-4.4/libavcodec/h264_mb_template.c?ref_type=heads#L41
- the decoded image is passed back to GStreamer that is going to write the packed image planes to a file. This is where the Valgrind error is triggered.

In this example, the image color format is YUV420 and because of memory alignments there is a lot of padding not only between the color planes but also at the end of each image line. This padding is never initialised, but I can also guaranty that it is not read either because if I flag the actual pixels areas (excluding any padding) with VALGRIND_MAKE_MEM_DEFINED right after getting the decoded image, the issue doesn't occur anymore.

In order to check that it was not a real issue in ffmpeg or GStreamer, I've also initialized the pixels areas (excluding any paddings) with 0xFF right before passing it to ffmpeg for the decoding. The resulting decoded image (as presented in the raw.bin file) doesn't have any single byte equal to 0xFF which proves that all pixels have effectively been written by ffmpeg.

The logical conclusion is that, somehow, Valgrind has not detected that memory has been effectively written during the image decoding by ffmpeg. Indeed, if I build ffmpeg disabling the ASM optimizations, the issue doesn't occur anymore. 

Therefore, it seems that some of the ASM instructions used by ffmpeg during the image decoding actually modify the memory but this is not detected by Valgrind. It seems that everything occurs during the call of this function: https://gitlab.freedesktop.org/gstreamer/meson-ports/ffmpeg/-/blob/meson-4.4/libavcodec/h264_mb_template.c?ref_type=heads#L41. Unfortunately, I haven't been able to isolate better which instruction is causing the issue. I did some small tests with direct copies using SSE instructions and in all cases it was perfectly detected by Valgrind.

The original issue in GStreamer with all the details is here: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/3203. I've proposed a patch to fix it in GStreamer: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/5882 but I finally closed it because it is just hiding the issue, not fixing its cause.
Comment 1 Paul Floyd 2024-01-19 16:23:55 UTC
There isn't much that we can do about reports like this.

The problem is with with packing holes and padding of structs in memory. There is a bugzilla item that suggested making a different warning for padding (which is less likely to be an error).

Memcheck can't distinguish between padding holes and aggregate elements that are genuinely uninitialized.
Comment 2 Tom Hughes 2024-01-19 16:50:58 UTC
Arguably it's a genuine bug anyway - you're leaking unknown data into that file which could include anything which has previously been store in a malloced block in your program and might include sensitive data of various sorts.
Comment 3 Paul Floyd 2024-01-28 16:26:31 UTC
*** This bug has been marked as a duplicate of bug 132232 ***