After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 758726 - avviddec: slice offset handling of real video leads to memory mishandling.
avviddec: slice offset handling of real video leads to memory mishandling.
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-libav
git master
Other Linux
: Normal normal
: 1.6.3
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-11-27 02:40 UTC by Vineeth
Modified: 2016-01-18 13:44 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
remove realvideo slice_offset handling (3.02 KB, patch)
2015-11-27 02:42 UTC, Vineeth
committed Details | Review

Description Vineeth 2015-11-27 02:40:35 UTC
In case of realvideos, slice offset is being handled in avviddec.
In case slice count is valid, then the offset from is being copied to slice offset and the data pointer is incremented based on this.
But by doing this
when the data is being passed on to
avcodec_decode_video2 --> av_packet_split_side_data
it expects the original data/size to be passed to it.
this change results in invalid memory read

=32228== Thread 6 multiqueue0:src_:
==32228== Invalid read of size 4
==32228==    at 0xDA77ACC: av_packet_split_side_data (avpacket.c:405)
==32228==    by 0xDE48565: avcodec_decode_video2 (utils.c:2432)
==32228==    by 0xD90B1AF: gst_ffmpegviddec_frame (gstavviddec.c:1354)
==32228==    by 0xD90CE74: gst_ffmpegviddec_handle_frame (gstavviddec.c:1644)
==32228==    by 0x41327BB: gst_video_decoder_decode_frame (gstvideodecoder.c:3406)
==32228==    by 0x4136BF8: gst_video_decoder_chain_forward (gstvideodecoder.c:2190)
==32228==    by 0x4138E3D: gst_video_decoder_chain (gstvideodecoder.c:2492)
==32228==    by 0x41DEAC7: gst_pad_push_data (gstpad.c:4108)
==32228==    by 0x41E7676: gst_pad_push (gstpad.c:4479)
==32228==    by 0x671C9EA: gst_multi_queue_loop (gstmultiqueue.c:1238)
==32228==    by 0x4215C88: gst_task_func (gsttask.c:331)
==32228==    by 0x4216E2E: default_func (gsttaskpool.c:68)
==32228==  Address 0x571fb28 is 5,008 bytes inside a block of size 5,011 alloc'd
==32228==    at 0x402E324: realloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==32228==    by 0x4358CB8: g_realloc (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==32228==    by 0xD90D12B: gst_ffmpegviddec_handle_frame (gstavviddec.c:1613)
==32228==    by 0x41327BB: gst_video_decoder_decode_frame (gstvideodecoder.c:3406)
==32228==    by 0x4136BF8: gst_video_decoder_chain_forward (gstvideodecoder.c:2190)
==32228==    by 0x4138E3D: gst_video_decoder_chain (gstvideodecoder.c:2492)
==32228==    by 0x41DEAC7: gst_pad_push_data (gstpad.c:4108)
==32228==    by 0x41E7676: gst_pad_push (gstpad.c:4479)
==32228==    by 0x671C9EA: gst_multi_queue_loop (gstmultiqueue.c:1238)
==32228==    by 0x4215C88: gst_task_func (gsttask.c:331)
==32228==    by 0x4216E2E: default_func (gsttaskpool.c:68)
==32228==    by 0x437A404: ??? (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==32228== 
==32228== Invalid read of size 1
==32228==    at 0x4031053: memcpy (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==32228==    by 0xDA780EF: av_packet_ref (string3.h:51)
==32228==    by 0xDD88538: ff_thread_decode_frame (pthread_frame.c:360)
==32228==    by 0xDE485B1: avcodec_decode_video2 (utils.c:2442)
==32228==    by 0xD90B1AF: gst_ffmpegviddec_frame (gstavviddec.c:1354)
==32228==    by 0xD90CE74: gst_ffmpegviddec_handle_frame (gstavviddec.c:1644)
==32228==    by 0x41327BB: gst_video_decoder_decode_frame (gstvideodecoder.c:3406)
==32228==    by 0x4136BF8: gst_video_decoder_chain_forward (gstvideodecoder.c:2190)
==32228==    by 0x4138E3D: gst_video_decoder_chain (gstvideodecoder.c:2492)
==32228==    by 0x41DEAC7: gst_pad_push_data (gstpad.c:4108)
==32228==    by 0x41E7676: gst_pad_push (gstpad.c:4479)
==32228==    by 0x671C9EA: gst_multi_queue_loop (gstmultiqueue.c:1238)
==32228==  Address 0x571fb2b is 0 bytes after a block of size 5,011 alloc'd
==32228==    at 0x402E324: realloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==32228==    by 0x4358CB8: g_realloc (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)
==32228==    by 0xD90D12B: gst_ffmpegviddec_handle_frame (gstavviddec.c:1613)
==32228==    by 0x41327BB: gst_video_decoder_decode_frame (gstvideodecoder.c:3406)
==32228==    by 0x4136BF8: gst_video_decoder_chain_forward (gstvideodecoder.c:2190)
==32228==    by 0x4138E3D: gst_video_decoder_chain (gstvideodecoder.c:2492)
==32228==    by 0x41DEAC7: gst_pad_push_data (gstpad.c:4108)
==32228==    by 0x41E7676: gst_pad_push (gstpad.c:4479)
==32228==    by 0x671C9EA: gst_multi_queue_loop (gstmultiqueue.c:1238)
==32228==    by 0x4215C88: gst_task_func (gsttask.c:331)
==32228==    by 0x4216E2E: default_func (gsttaskpool.c:68)
==32228==    by 0x437A404: ??? (in /lib/i386-linux-gnu/libglib-2.0.so.0.4002.0)



Ideally this might need to be handled in ffmpeg by avpacket.c to check for slice_offset related changes for realvideo.


But do we really need to handle slice offset in avviddec?
I could see that real decoders in ffmpeg (rv34 and rv10) are already handling the same. So even if we don't handle it here, it will be taken care of..

I propose to remove the changes.
Please suggest if it can be removed..
Comment 1 Vineeth 2015-11-27 02:42:44 UTC
Created attachment 316360 [details] [review]
remove realvideo slice_offset handling

Proposal to remove slice offset handling from avviddec.
Comment 2 Vineeth 2015-12-22 06:25:19 UTC
ping :)
Comment 3 Sebastian Dröge (slomo) 2015-12-22 08:50:51 UTC
Comment on attachment 316360 [details] [review]
remove realvideo slice_offset handling

Makes sense if all 4 variants are handling that internally. Do you have a sample file? :)
Comment 4 Vineeth 2015-12-22 23:13:08 UTC
Guess any sample rm file can be checked with this.

I used
http://samples.mplayerhq.hu/real/VC-RV30/141a.rm
Comment 5 Sebastian Dröge (slomo) 2015-12-23 12:18:58 UTC
commit ae27b9c50320ea4302d49dab1d220dee5a02df28
Author: Vineeth TM <vineeth.tm@samsung.com>
Date:   Fri Nov 27 11:16:07 2015 +0900

    avviddec: remove realvideo slice_offset handling
    
    Handling slice_offset in avviddec is resulting in invalid memory read.
    Since rv decoders anyways handle slice_offset, removing the same to fix
    memory mishandlings
    
    https://bugzilla.gnome.org/show_bug.cgi?id=758726