[FFmpeg-trac] #6626(undetermined:new): ff_vdpau_common_init writes past the end of av_alloc_vdpaucontext memory

[FFmpeg]

#6626: ff_vdpau_common_init writes past the end of av_alloc_vdpaucontext memory
-------------------------------------+-------------------------------------
             Reporter:  aaronp24     |                     Type:  defect
               Status:  new          |                 Priority:  normal
            Component:               |                  Version:
  undetermined                       |  unspecified
             Keywords:               |               Blocked By:
             Blocking:               |  Reproduced by developer:  0
Analyzed by developer:  0            |
-------------------------------------+-------------------------------------
 '''Summary of the bug:'''
 libavcodec writes past the end of data it allocated, when mplayer calls
 `ff_vdpau_common_init()` without calling `av_vdpau_bind_context()`

 '''How to reproduce:'''
 {{{
 % valgrind mplayer -vc ffh264vdpau <any H.264 video>
 ffmpeg version 1:3.3.3-2 from Arch Linux
 }}}

 '''Details:'''
 I'm looking into a bug where `mplayer -vc ffh264vdpau` causes libavcodec
 to write past the end of an allocation and corrupt data in the vdpau
 driver. According to Valgrind,

 {{{
 VO: [vdpau] 1280x536 => 1280x536 H.264 VDPAU acceleration
 ==19159== Invalid write of size 1
 ==19159==    at 0xAF0326F: ff_vdpau_common_init (vdpau.c:145)
 ==19159==    by 0xAEA9B89: setup_hwaccel (utils.c:1100)
 ==19159==    by 0xAEA9B89: ff_get_format (utils.c:1165)
 ==19159==    by 0xAB4C365: get_pixel_format.isra.6 (h264_slice.c:864)
 ==19159==    by 0xAB4D1D2: h264_init_ps (h264_slice.c:1102)
 ==19159==    by 0xAB4D1D2: h264_field_start.isra.7 (h264_slice.c:1385)
 ==19159==    by 0xAB520B7: ff_h264_queue_decode_slice (h264_slice.c:2091)
 ==19159==    by 0xAB55BBE: decode_nal_units (h264dec.c:674)
 ==19159==    by 0xAB55BBE: h264_decode_frame (h264dec.c:1016)
 ==19159==    by 0xAEA58AD: avcodec_decode_video2 (utils.c:2275)
 ==19159==    by 0x334BB2: ??? (in /usr/bin/mplayer)
 ==19159==    by 0x262000: ??? (in /usr/bin/mplayer)
 ==19159==    by 0x1B2A57: ??? (in /usr/bin/mplayer)
 ==19159==    by 0x1B4A8D: ??? (in /usr/bin/mplayer)
 ==19159==    by 0x1B8A06: ??? (in /usr/bin/mplayer)
 ==19159==  Address 0x2701c8c8 is 16 bytes after a block of size 760
 alloc'd
 ==19159==    at 0x4C2E2A6: memalign (in /usr/lib/valgrind
 /vgpreload_memcheck-amd64-linux.so)
 ==19159==    by 0x4C2E3E2: posix_memalign (in /usr/lib/valgrind
 /vgpreload_memcheck-amd64-linux.so)
 ==19159==    by 0xBF640FB: av_malloc (mem.c:87)
 ==19159==    by 0xBF642C9: av_mallocz (mem.c:224)
 ==19159==    by 0x332D7C: ??? (in /usr/bin/mplayer)
 ==19159==    by 0x333CEA: ??? (in /usr/bin/mplayer)
 ==19159==    by 0x333F1A: ??? (in /usr/bin/mplayer)
 ==19159==    by 0x335656: ??? (in /usr/bin/mplayer)
 ==19159==    by 0xAEA9AF1: ff_get_format (utils.c:1140)
 ==19159==    by 0xAB4C365: get_pixel_format.isra.6 (h264_slice.c:864)
 ==19159==    by 0xAB4D1D2: h264_init_ps (h264_slice.c:1102)
 ==19159==    by 0xAB4D1D2: h264_field_start.isra.7 (h264_slice.c:1385)
 ==19159==    by 0xAB520B7: ff_h264_queue_decode_slice (h264_slice.c:2091)
 }}}

 I tracked down the problem to mplayer doing this:

 {{{
 avctx->hwaccel_context = vdpc = av_alloc_vdpaucontext();
 }}}

 where `av_alloc_vdpaucontext()` does this:

 {{{
 return av_mallocz(sizeof(AVVDPAUContext));
 }}}

 However, `ff_vdpau_common_init()` casts the `hwaccel_context` pointer to a
 `VDPAUHWContext*`, which is a larger structure that contains an
 `AVVDPAUContext` as its first element. When this function tries to write
 the `reset` field, it writes past the end of the data allocated by
 `av_alloc_vdpaucontext()`.

 {{{
 hwctx->reset            = 0;
 }}}

 It looks like there is code in `av_vdpau_bind_context()` to reallocate
 this structure to the larger size:

 {{{
 VDPAUHWContext *hwctx;
 if (av_reallocp(&avctx->hwaccel_context, sizeof(*hwctx)))
     return AVERROR(ENOMEM);
 }}}

 However, mplayer doesn't call this function, so the realloc never happens.

 According to the git history, there used to be a call to
 `av_vdpau_bind_context()` from `vdpau_alloc()` (called from
 `vdpau_init()`) but Mark Thompson removed that code in commit
 e462ace84b92e54d2a5fa651d6469aefe0f1efbf.

 What is the proper fix here? Should mplayer be calling
 `av_vdpau_bind_context()`, or should `av_alloc_vdpaucontext()` be
 allocating enough space up front?

--
Ticket URL: <https://trac.ffmpeg.org/ticket/6626>
FFmpeg <https://ffmpeg.org>
FFmpeg issue tracker