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)
Tracking
()
VERIFIED
FIXED
mozilla41
backlog | webrtc/webaudio+ |
People
(Reporter: jsmith, Assigned: cruceru.adrian)
Details
(Keywords: crash)
Crash Data
Attachments
(2 files, 1 obsolete file)
663 bytes,
patch
|
Details | Diff | Splinter Review | |
600 bytes,
patch
|
jesup
:
review+
|
Details | Diff | Splinter Review |
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
Updated•12 years ago
|
Updated•12 years ago
|
Whiteboard: [WebRTC],[blocking-webrtc-]
Assignee | ||
Comment 1•11 years ago
|
||
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?
Assignee | ||
Comment 2•11 years ago
|
||
Note: would be interesting to actually find a scenario / test case as this might just delay crash to a function later on.
Comment 3•11 years ago
|
||
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?
Assignee | ||
Comment 4•11 years ago
|
||
Request review, please add any other persons that need to review this.
Attachment #830702 -
Flags: review?(rjesup)
Comment 5•11 years ago
|
||
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+
Assignee | ||
Comment 6•11 years ago
|
||
Updated patch after review
Attachment #830702 -
Attachment is obsolete: true
Attachment #830737 -
Flags: review?(rjesup)
Assignee | ||
Comment 7•11 years ago
|
||
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.
Assignee | ||
Comment 8•11 years ago
|
||
Updated•11 years ago
|
Attachment #830737 -
Flags: review?(rjesup) → review+
Comment 9•11 years ago
|
||
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!
Assignee | ||
Comment 10•11 years ago
|
||
Thanks for the review and guidance, don't have permission to add "checkin-needed" yet - if anyone has rights, feel free to add it
Comment 11•10 years ago
|
||
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-]
Comment 12•10 years ago
|
||
Updated•10 years ago
|
Blocks: webrtc_upstream_bugs
Comment 13•10 years ago
|
||
Assignee: nobody → cruceru.adrian
Status: NEW → RESOLVED
Closed: 10 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Comment 14•9 years ago
|
||
Can't see any instances of this crash in Socorro [1] for the past 4 weeks, in builds after this fix landed.
[1] - https://crash-stats.mozilla.com/report/list?range_unit=days&range_value=28&signature=_VEC_memcpy+%7C+webrtc%3A%3APlane%3A%3ACopy%28int%2C+int%2C+unsigned+char+const%2A%29#tab-sigsummary
Status: RESOLVED → VERIFIED
Updated•6 years ago
|
No longer blocks: webrtc_upstream_bugs
You need to log in
before you can comment on or make changes to this bug.
Description
•