Closed
Bug 1109540
Opened 9 years ago
Closed 9 years ago
Conditional jump or move depends on uninitialised value(s) at mozilla::GStreamerReader::NotifyDataArrived
Categories
(Core :: Audio/Video, defect)
Tracking
()
RESOLVED
FIXED
mozilla37
Tracking | Status | |
---|---|---|
firefox35 | --- | wontfix |
firefox36 | --- | fixed |
firefox37 | --- | fixed |
firefox-esr31 | --- | unaffected |
People
(Reporter: mitchwharper, Assigned: eflores)
Details
(Keywords: csectype-uninitialized, valgrind, Whiteboard: [adv-main36-])
Attachments
(1 file)
870 bytes,
patch
|
ajones
:
review+
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta-
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:34.0) Gecko/20100101 Firefox/34.0 Build ID: 20141126041045 Actual results: ==6262== Conditional jump or move depends on uninitialised value(s) ==6262== at 0x9259CA7: mozilla::GStreamerReader::NotifyDataArrived(char const*, unsigned int, long) (GStreamerReader.cpp:1240) ==6262== by 0x91D21BC: mozilla::MediaDecoderStateMachine::NotifyDataArrived(char const*, unsigned int, long) (MediaDecoderStateMachine.cpp:1478) ==6262== by 0x91C5BA5: mozilla::MediaDecoder::NotifyDataArrived(char const*, unsigned int, long) (MediaDecoder.cpp:1525) ==6262== by 0x91D7987: mozilla::ChannelMediaResource::CopySegmentToCache(nsIInputStream*, void*, char const*, unsigned int, unsigned int, unsigned int*) (MediaResource.cpp:507) ==6262== by 0x7D9F09F: nsPipeInputStream::ReadSegments(tag_nsresult (*)(nsIInputStream*, void*, char const*, unsigned int, unsigned int, unsigned int*), void*, unsigned int, unsigned int*) (nsPipe3.cpp:822) ==6262== by 0x91D8AC7: mozilla::ChannelMediaResource::OnDataAvailable(nsIRequest*, nsIInputStream*, unsigned int) (MediaResource.cpp:545) ==6262== by 0x91D8B1E: mozilla::ChannelMediaResource::Listener::OnDataAvailable(nsIRequest*, nsISupports*, nsIInputStream*, unsigned long, unsigned int) (MediaResource.cpp:141) ==6262== by 0x91604F5: mozilla::dom::HTMLMediaElement::MediaLoadListener::OnDataAvailable(nsIRequest*, nsISupports*, nsIInputStream*, unsigned long, unsigned int) (HTMLMediaElement.cpp:390) ==6262== by 0x7E192F7: nsBaseChannel::OnDataAvailable(nsIRequest*, nsISupports*, nsIInputStream*, unsigned long, unsigned int) (nsBaseChannel.cpp:781) ==6262== by 0x7E26D6A: nsInputStreamPump::OnStateTransfer() (nsInputStreamPump.cpp:609) ==6262== by 0x7E27230: nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) (nsInputStreamPump.cpp:436) ==6262== by 0x7DA1313: nsInputStreamReadyEvent::Run() (nsStreamUtils.cpp:88) ==6262== Uninitialised value was created by a heap allocation ==6262== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==6262== by 0x40334E6: moz_xmalloc (mozalloc.cpp:52) ==6262== by 0x9258510: mozilla::GStreamerDecoder::CreateStateMachine() (mozalloc.h:201) ==6262== by 0x91C6619: mozilla::MediaDecoder::Load(nsIStreamListener**, mozilla::MediaDecoder*) (MediaDecoder.cpp:550) ==6262== by 0x9164DE4: mozilla::dom::HTMLMediaElement::FinishDecoderSetup(mozilla::MediaDecoder*, mozilla::MediaResource*, nsIStreamListener**, mozilla::MediaDecoder*) (HTMLMediaElement.cpp:2680) ==6262== by 0x916504D: mozilla::dom::HTMLMediaElement::InitializeDecoderForChannel(nsIChannel*, nsIStreamListener**) (HTMLMediaElement.cpp:2630) ==6262== by 0x9165FB8: mozilla::dom::HTMLMediaElement::MediaLoadListener::OnStartRequest(nsIRequest*, nsISupports*) (HTMLMediaElement.cpp:349) ==6262== by 0x7E1945F: nsBaseChannel::OnStartRequest(nsIRequest*, nsISupports*) (nsBaseChannel.cpp:737) ==6262== by 0x7E26B86: nsInputStreamPump::OnStateStart() (nsInputStreamPump.cpp:531) ==6262== by 0x7E27181: nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) (nsInputStreamPump.cpp:433) ==6262== by 0x7DA1313: nsInputStreamReadyEvent::Run() (nsStreamUtils.cpp:88) ==6262== by 0x7DB34B3: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:823)
Reporter | ||
Updated•9 years ago
|
OS: Windows 8 → Linux
Comment 1•9 years ago
|
||
Bobby, I hear you're doing media code now - can you have a look?
Component: Untriaged → Video/Audio
Flags: needinfo?(bobbyholley)
Product: Firefox → Core
Comment 2•9 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #1) > Bobby, I hear you're doing media code now - can you have a look? No, sorry. I don't know anything about the gstreamer code. Over to Anthony to find an owner. BTW - Please don't take my security background and current media work as a reason to flag me for fixing arbitrary media security bugs. I'm here to help ship MSE, and don't plan to own this code.
Flags: needinfo?(bobbyholley) → needinfo?(ajones)
Comment 3•9 years ago
|
||
(In reply to Bobby Holley (:bholley) from comment #2) > (In reply to :Gijs Kruitbosch from comment #1) > > Bobby, I hear you're doing media code now - can you have a look? > > No, sorry. I don't know anything about the gstreamer code. Over to Anthony > to find an owner. > > BTW - Please don't take my security background and current media work as a > reason to flag me for fixing arbitrary media security bugs. I'm here to help > ship MSE, and don't plan to own this code. Sorry! I'll keep it in mind. For reference, it looks like this and all the other bugs filed by Mitch today came out of the investigation in bug 1108455.
Updated•9 years ago
|
Keywords: csectype-uninitialized
Reporter | ||
Comment 4•9 years ago
|
||
Valgrind command: `G_SLICE=always-malloc valgrind --tool=memcheck --vex-iropt-register-updates=allregs-at-mem-access --smc-check=all-non-file ./firefox` Steps taken: 1. Start the browser 2. Open a new tab 3. Visit https://www.webrtc-experiment.com/RTCMultiConnection/MultiRTC/ in two separate tabs 4. Input the same room ID for both instances 5. Enable video and audio on the second tab, and allow access 6. Share my microphone and camera 7. Switch to other tab 8. Enable video and audio on first tab 9. Share camera and microphone 10. Preview camera from second user (this is where the first jump on uninitialized memory occured) 11. Preview microphone from second user 12. Switch tabs 13. Preview camera and mic from first user 14. Exit browser
I don't understand where gstreamer is related to webrtc.
Flags: needinfo?(ajones) → needinfo?(mreavy)
Reporter | ||
Comment 6•9 years ago
|
||
It may not be directly related to webrtc, only steps 1-3 are required for this to occur (it happens every time the page is opened). When you load the page, it says it establishes a data connection with the server. This all happens before getUserMedia() is ever called.
Updated•9 years ago
|
Flags: needinfo?(ajones)
Updated•9 years ago
|
Flags: needinfo?(ajones)
Ralph - can you look into this?
Flags: needinfo?(giles)
Assignee | ||
Comment 9•9 years ago
|
||
Attachment #8536910 -
Flags: review?(ajones)
Updated•9 years ago
|
Attachment #8536910 -
Flags: review?(ajones) → review+
Comment 10•9 years ago
|
||
This seems pretty benign -- either we'll call UpdateEstimatedDuration() when we didn't need to, or we don't call it when we should have. The WebAudio version of that is an empty function even, not sure which of the three variants would be called here. Anthony, do you know?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(ajones)
Comment 11•9 years ago
|
||
I'm happy to look at it, but it looks like edwin already has a fix.
Flags: needinfo?(giles)
Ralph - looks like edwin has it under control.
I say we land the patch on nightly and move on.
Flags: needinfo?(ajones) → needinfo?(giles)
Keywords: checkin-needed
Comment 15•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/eb6e806e0bbd
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox37:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Comment 16•9 years ago
|
||
Comment on attachment 8536910 [details] [diff] [review] 1109540.patch Approval Request Comment [Feature/regressing bug #]: 422540 [User impact if declined]: Users exposed to security issue. This isn't an MSE bug, but we'd like to keep the media code in sync to simplify backporting other changes. [Describe test coverage new/current, TBPL]: Landed on m-c. [Risks and why]: Low; this just avoid using uninitialized data when starting playback. [String/UUID change made/needed]: None.
Attachment #8536910 -
Flags: approval-mozilla-aurora?
Updated•9 years ago
|
status-firefox36:
--- → affected
Updated•9 years ago
|
Attachment #8536910 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 19•9 years ago
|
||
Comment on attachment 8536910 [details] [diff] [review] 1109540.patch (see approval request in comment 16) Goes all the way down to release, though not really exploitable AFAICT. Should be fine without it. May as well uplift to beta though.
Flags: needinfo?(edwin)
Attachment #8536910 -
Flags: approval-mozilla-beta?
Comment 20•9 years ago
|
||
Comment on attachment 8536910 [details] [diff] [review] 1109540.patch We released 35 and beta is now 36.
Attachment #8536910 -
Flags: approval-mozilla-beta? → approval-mozilla-beta-
Updated•9 years ago
|
status-firefox35:
--- → wontfix
status-firefox-esr31:
--- → unaffected
Updated•9 years ago
|
Whiteboard: [adv-main36-]
Updated•9 years ago
|
Group: core-security → core-security-release
Updated•8 years ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•