[mythtv-commits] Ticket #13428: Valgrind error in avformatdecoder.cpp

[MythTV]

#13428: Valgrind error in avformatdecoder.cpp
----------------------------------+-------------------------
     Reporter:  Klaas de Waal     |      Owner:  (none)
         Type:  Patch - Bug Fix   |     Status:  new
     Priority:  minor             |  Milestone:  31.0
    Component:  MythTV - General  |    Version:  Master Head
     Severity:  low               |   Keywords:  valgrind
Ticket locked:  0                 |
----------------------------------+-------------------------
 Running mythfrontend with valgrind gives the following error message:

 {{{
 ==7659== Conditional jump or move depends on uninitialised value(s)
 ==7659==    at 0x49627BC: h261_probe (h261dec.c:35)
 ==7659==    by 0x4959CF4: av_probe_input_format3 (format.c:171)
 ==7659==    by 0x4959F81: av_probe_input_format2 (format.c:225)
 ==7659==    by 0x69424DF: AvFormatDecoder::CanHandle(char*, QString
 const&, int) (avformatdecoder.cpp:961)
 ==7659==    by 0x68B0CDB: MythPlayer::CreateDecoder(char*, int)
 (mythplayer.cpp:923)
 ==7659==    by 0x68B16C8: MythPlayer::OpenFile(unsigned int)
 (mythplayer.cpp:989)
 ...
 ==7659==  Uninitialised value was created by a heap allocation
 ==7659==    at 0x4839593: operator new[](unsigned long)
 (vg_replace_malloc.c:433)
 ==7659==    by 0x68B0F73: MythPlayer::OpenFile(unsigned int)
 (mythplayer.cpp:958)
 ...
 }}}

 What happens is that mythplayer.cpp allocates 256kB of buffer memory and
 fill this with at least 2kB (testreadsize) bytes (line 958).
 This is passed on to AvFormatDecoder::CanHandle.
 There it makes sure that there is at least AVPROBE_PADDING_SIZE (32 bytes)
 of unused memory available in the buffer.
 This is memory which has not initialized.
 According to valgrind this memory is then used in h261_probe and
 vc1_probe.

 There are two places in avformatdecoder.cpp where this happens; in
 CanHandle and in OpenFile.
 The solution is to clear the 32-byte padding memory in both places.

 A patch that implements this solution is attached.

 Note: it would also be possible to clear the complete buffer immediately
 after allocation. However, this means clearing 256kB instead of only 32
 bytes and thus comes with a (minor) performance penalty.

-- 
Ticket URL: <https://code.mythtv.org/trac/ticket/13428>
MythTV <http://www.mythtv.org>
MythTV Media Center