Closed Bug 881742 Opened 12 years ago Closed 10 years ago

crash in _VEC_memcpy | webrtc::Plane::Copy(int, int, unsigned char const*)

Categories

(Core :: WebRTC, defect, P4)

22 Branch
x86
Windows 7
defect

Tracking

()

VERIFIED FIXED
mozilla41
Tracking Status
firefox22 --- affected
firefox41 --- verified

People

(Reporter: jsmith, Assigned: cruceru.adrian)

Details

(Keywords: crash)

Crash Data

Attachments

(2 files, 1 obsolete file)

This bug was filed from the Socorro interface and is report bp-c0304d1f-e6af-4833-9bbb-c8c3c2130606 . ============================================================= Frame Module Signature Source 0 msvcr100.dll _VEC_memcpy 1 xul.dll webrtc::Plane::Copy media/webrtc/trunk/webrtc/common_video/plane.cc:67 2 xul.dll webrtc::I420VideoFrame::CreateFrame media/webrtc/trunk/webrtc/common_video/i420_video_frame.cc:55 3 xul.dll webrtc::VP8DecoderImpl::ReturnFrame media/webrtc/trunk/webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc:884 4 xul.dll webrtc::VP8DecoderImpl::Decode media/webrtc/trunk/webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc:786 5 xul.dll webrtc::VCMGenericDecoder::Decode media/webrtc/trunk/webrtc/modules/video_coding/main/source/generic_decoder.cc:173 6 xul.dll webrtc::VideoCodingModuleImpl::Decode media/webrtc/trunk/webrtc/modules/video_coding/main/source/video_coding_impl.cc:1052 7 xul.dll webrtc::VideoCodingModuleImpl::Decode media/webrtc/trunk/webrtc/modules/video_coding/main/source/video_coding_impl.cc:914 8 xul.dll webrtc::ViEChannel::ChannelDecodeProcess media/webrtc/trunk/webrtc/video_engine/vie_channel.cc:2215 9 xul.dll webrtc::ThreadWindows::Run media/webrtc/trunk/webrtc/system_wrappers/source/thread_win.cc:170 10 xul.dll webrtc::ThreadWindows::StartThread media/webrtc/trunk/webrtc/system_wrappers/source/thread_win.cc:66 11 msvcr100.dll _callthreadstartex f:\dd\vctools\crt_bld\self_x86\crt\src\threadex.c:314 12 msvcr100.dll _threadstartex f:\dd\vctools\crt_bld\self_x86\crt\src\threadex.c:292 13 kernel32.dll BaseThreadInitThunk 14 ntdll.dll __RtlUserThreadStart 15 ntdll.dll _RtlUserThreadStart
OS: Windows NT → Windows 7
Version: Trunk → 22 Branch
Whiteboard: [WebRTC],[blocking-webrtc-]
Attached patch nullwrite.patchSplinter Review
Crash Reason EXCEPTION_ACCESS_VIOLATION_WRITE Crash Address 0x0 Line: memcpy(buffer_.get(), buffer, size); We crash with a write to address 0 so that means that buffer_.get() is NULL. Since we try and re-allocate to proper size before memcpy, issue is with the reallocation. Either size is too big or alignment is invalid, either way, fix is simple, just check that alocation succeeded, if not, return error. Root cause may be malformed video being decoded.
Flags: needinfo?
Note: would be interesting to actually find a scenario / test case as this might just delay crash to a function later on.
Ok: as mentioned, this is an upstream bug in how AlignedMalloc() is used in MaybeResize(). AlignedMalloc is fallible, and will cause it to return NULL on failure, but MaybeResize() doesn't appear to check it. Please a) file a bug upstream at webrtc.org, and pose the bug number/link here, b) if you can, propose a patch and post it here. You could propose it to the upstream as a patch (and mark it for reviews!) or do so here and we can propose it upstream later. Also verify that the current trunk of webrtc.org hasn't fixed it (I suspect it hasn't been fixed). Thanks for digging through the backlog!
Flags: needinfo?
Attached patch nullwritefix.patch (obsolete) — Splinter Review
Request review, please add any other persons that need to review this.
Attachment #830702 - Flags: review?(rjesup)
Comment on attachment 830702 [details] [diff] [review] nullwritefix.patch Review of attachment 830702 [details] [diff] [review]: ----------------------------------------------------------------- ::: media/webrtc/trunk/webrtc/common_video/plane.cc @@ +47,5 @@ > AlignedMalloc<uint8_t>(new_size, kBufferAlignment)); > + > + if (NULL == new_buffer.get()) > + return -1; > + remove the trailing space. Also, match the norm for the code, which would be: if (!new_buffer.get())
Attachment #830702 - Flags: review?(rjesup) → review+
Updated patch after review
Attachment #830702 - Attachment is obsolete: true
Attachment #830737 - Flags: review?(rjesup)
Note: - Checked upstream and issue is not fixed ( http://code.google.com/p/webrtc/source/browse/trunk/webrtc/common_video/plane.cc ) Will raise the bug there with patch in a couple of hours.
Attachment #830737 - Flags: review?(rjesup) → review+
Adrian: the process now is to mark it "checkin-needed"; someone should pick it up and merge it to the tree in relatively short order. Thanks!
Thanks for the review and guidance, don't have permission to add "checkin-needed" yet - if anyone has rights, feel free to add it
Still a bug in upstream. Hadn't noticed this hadn't landed yet, will land momentarily. Note that it's rare-to-impossible (*maybe* due to insanely-large-video decoded) unless you're on the edge of OOM already.
backlog: --- → webRTC+
Rank: 45
Priority: -- → P4
Whiteboard: [WebRTC],[blocking-webrtc-]
Assignee: nobody → cruceru.adrian
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
No longer blocks: webrtc_upstream_bugs
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: