1405940 - Crash - WebRtc - Null Pointer dereference in sigslot::lock_block

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, defect, P2)

Description

[Looben Yang]

Attachment: NullPtr_sigslot_lock_block_Repro.html

Reproduction test case: NullPtr_sigslot_lock_block_Repro.htm
		
Steps to reproduce: 
	1. Open PoC NullPtr_sigslot_lock_block_Repro.htm in Firefox browser.
	2. Firefox crashes by dereferenceing NULL pointer in sigslot::lock_block<sigslot::single_threaded>::{ctor}.

Firefox version: 58.0a1 (2017-10-04) (32-bit)
OS: Windows 10

Stack trace:

	(22e4.1904): Access violation - code c0000005 (!!! second chance !!!)
	eax=00000014 ebx=00000010 ecx=00000014 edx=05f44618 esi=00000014 edi=221fcc78
	eip=10aab053 esp=07ffecd0 ebp=07ffece4 iopl=0         nv up ei pl nz na pe nc
	cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00010206
	xul!sigslot::lock_block<sigslot::single_threaded>::{ctor}+0x2 [inlined in xul!sigslot::signal2<mozilla::TransportLayer *,enum mozilla::TransportLayer::State,sigslot::single_threaded>::connect<mozilla::PeerConnectionMedia>+0x17]:
	10aab053 8b06            mov     eax,dword ptr [esi]  ds:002b:00000014=????????
	5:216> !analyze -v
	*******************************************************************************
	*                                                                             *
	*                        Exception Analysis                                   *
	*                                                                             *
	*******************************************************************************


	FAULTING_IP: 
	xul!sigslot::signal2<mozilla::TransportLayer *,enum mozilla::TransportLayer::State,sigslot::single_threaded>::connect<mozilla::PeerConnectionMedia>+17 [z:\build\build\src\media\mtransport\sigslot.h @ 2320]
	10aab053 8b06            mov     eax,dword ptr [esi]

	EXCEPTION_RECORD:  (.exr -1)
	ExceptionAddress: 10aab053 (xul!sigslot::lock_block<sigslot::single_threaded>::{ctor}+0x00000002)
	   ExceptionCode: c0000005 (Access violation)
	  ExceptionFlags: 00000000
	NumberParameters: 2
	   Parameter[0]: 00000000
	   Parameter[1]: 00000014
	Attempt to read from address 00000014

	FAULTING_THREAD:  00001904

	DEFAULT_BUCKET_ID:  NULL_CLASS_PTR_READ

	PROCESS_NAME:  firefox.exe

	ERROR_CODE: (NTSTATUS) 0xc0000005 - The instruction at 0x%p referenced memory at 0x%p. The memory could not be %s.

	EXCEPTION_CODE: (NTSTATUS) 0xc0000005 - The instruction at 0x%p referenced memory at 0x%p. The memory could not be %s.

	EXCEPTION_PARAMETER1:  00000000

	EXCEPTION_PARAMETER2:  00000014

	READ_ADDRESS:  00000014 

	FOLLOWUP_IP: 
	xul!sigslot::signal2<mozilla::TransportLayer *,enum mozilla::TransportLayer::State,sigslot::single_threaded>::connect<mozilla::PeerConnectionMedia>+17 [z:\build\build\src\media\mtransport\sigslot.h @ 2320]
	10aab053 8b06            mov     eax,dword ptr [esi]

	BUGCHECK_STR:  NULL_CLASS_PTR_READ

	NTGLOBALFLAG:  400

	APPLICATION_VERIFIER_FLAGS:  0

	APP:  firefox.exe

	ANALYSIS_VERSION: 10.0.10240.9 x86fre

	LAST_CONTROL_TRANSFER:  from 10ae28aa to 10aab053

	STACK_TEXT:  
	07ffece4 10ae28aa 221fcc00 10ae26db 00000000 xul!sigslot::signal2<mozilla::TransportLayer *,enum mozilla::TransportLayer::State,sigslot::single_threaded>::connect<mozilla::PeerConnectionMedia>+0x17
	07ffed00 10ae2f32 221fcc00 22602a70 00000000 xul!mozilla::TransportLayerIce::PostSetup+0x3e
	07ffedf4 10a98103 29cc0a60 00000000 00000001 xul!mozilla::TransportLayerIce::SetParameters+0x1a7
	07ffee34 10a9bab1 22602a70 23230160 00000000 xul!mozilla::AddNewIceStreamForRestart_s+0x6d
	07ffee54 0fe7d307 25ba7160 00000001 00000001 xul!mozilla::runnable_args_func<void (__cdecl*)(RefPtr<mozilla::PeerConnectionMedia>,RefPtr<mozilla::TransportFlow>,unsigned int,bool),mozilla::PeerConnectionMedia *,RefPtr<mozilla::TransportFlow>,unsigned int,bool>::Run+0x2c
	07fff3c4 0fd70e15 05f19700 00000001 07fff3df xul!nsThread::ProcessNextEvent+0x298
	07fff3e0 0fd704c3 05f44610 05f19700 0646e0bc xul!NS_ProcessNextEvent+0x1a
	07fff6ac 0fe7d307 05f44610 05f1a5e0 05f19700 xul!mozilla::net::nsSocketTransportService::Run+0x2a2
	07fffc18 0fd70e15 05f19700 05f19700 07fffc33 xul!nsThread::ProcessNextEvent+0x298
	07fffc34 0fd70d89 05f1a5e0 05f1a5e0 51ec2610 xul!NS_ProcessNextEvent+0x1a
	07fffc58 0ffeea46 05f1a5e0 cefd3247 05f19700 xul!mozilla::ipc::MessagePumpForNonMainThreads::Run+0x95
	07fffc90 0ffeea04 05f1a5e0 00000001 51ec2600 xul!MessageLoop::RunHandler+0x20
	07fffcb0 0ffedd73 76686870 0114e7d0 0114e880 xul!MessageLoop::Run+0x19
	07fffcd4 515fb259 0055dfe0 0690f698 515f0cac xul!nsThread::ThreadFunc+0xb7
	07fffcf0 515f0cb9 0114e7d0 07fffd38 757ee87f nss3!_PR_NativeRunThread+0xcc
	07fffcfc 757ee87f 0114e7d0 a8b96693 757ee840 nss3!pr_root+0xd
	07fffd38 76688744 0690f698 00000000 ab4c1143 ucrtbase!_set_app_type+0x4f
	07fffd4c 51ecb856 0690f698 757ee840 76688720 KERNEL32!BaseThreadInitThunk+0x24
	07fffd60 7706582d 0690f698 aa2cb4d7 00000000 mozglue!patched_BaseThreadInitThunk+0x27
	07fffda8 770657fd ffffffff 77086389 00000000 ntdll!__RtlUserThreadStart+0x2f
	07fffdb8 00000000 757ee840 0690f698 00000000 ntdll!_RtlUserThreadStart+0x1b


	FAULTING_SOURCE_LINE:  z:\build\build\src\media\mtransport\sigslot.h

	FAULTING_SOURCE_FILE:  z:\build\build\src\media\mtransport\sigslot.h

	FAULTING_SOURCE_LINE_NUMBER:  2320

	FAULTING_SOURCE_CODE:  
	No source found for 'z:\build\build\src\media\mtransport\sigslot.h'


	SYMBOL_STACK_INDEX:  0

	SYMBOL_NAME:  xul!sigslot::signal2<mozilla::TransportLayer *,enum mozilla::TransportLayer::State,sigslot::single_threaded>::connect<mozilla::PeerConnectionMedia>+17

	FOLLOWUP_NAME:  MachineOwner

	MODULE_NAME: xul

	IMAGE_NAME:  xul.dll

	DEBUG_FLR_IMAGE_TIMESTAMP:  59d57146

	STACK_COMMAND:  ~216s ; kb

	BUCKET_ID:  NULL_CLASS_PTR_READ_xul!sigslot::signal2_mozilla::TransportLayer_*,enum_mozilla::TransportLayer::State,sigslot::single_threaded_::connect_mozilla::PeerConnectionMedia_+17

	PRIMARY_PROBLEM_CLASS:  NULL_CLASS_PTR_READ_xul!sigslot::signal2_mozilla::TransportLayer_*,enum_mozilla::TransportLayer::State,sigslot::single_threaded_::connect_mozilla::PeerConnectionMedia_+17

	FAILURE_PROBLEM_CLASS:  NULL_CLASS_PTR_READ

	FAILURE_EXCEPTION_CODE:  c0000005

	FAILURE_IMAGE_NAME:  xul.dll

	FAILURE_FUNCTION_NAME:  sigslot::signal2_mozilla::TransportLayer_*,enum_mozilla::TransportLayer::State,sigslot::single_threaded_::connect_mozilla::PeerConnectionMedia_

	FAILURE_SYMBOL_NAME:  xul.dll!sigslot::signal2_mozilla::TransportLayer_*,enum_mozilla::TransportLayer::State,sigslot::single_threaded_::connect_mozilla::PeerConnectionMedia_

	FAILURE_BUCKET_ID:  NULL_CLASS_PTR_READ_c0000005_xul.dll!sigslot::signal2_mozilla::TransportLayer_*,enum_mozilla::TransportLayer::State,sigslot::single_threaded_::connect_mozilla::PeerConnectionMedia_

	ANALYSIS_SOURCE:  UM

	FAILURE_ID_HASH_STRING:  um:null_class_ptr_read_c0000005_xul.dll!sigslot::signal2_mozilla::transportlayer_*,enum_mozilla::transportlayer::state,sigslot::single_threaded_::connect_mozilla::peerconnectionmedia_

	FAILURE_ID_HASH:  {2f47a0ca-22ea-c1e0-1940-4ec853c0fa14}

	Followup:     MachineOwner
	---------

Comment 1

[Andreas Pehrson [:pehrsons]]

Nils, could you take a look at this?

Comment 2

[Alex Chronopoulos [:achronop]]

https://crash-stats.mozilla.com/report/index/9a0e3eae-c1be-4e9f-9b21-d5e610171006

Comment 3

[Nils Ohlmeier [:drno]]

Michael I think this might be either related to the bugs you are working on, or maybe this is even giving you the root cause for the crashes. Can you take a look a this please?

Comment 4

[Michael Froman [:mjf]]

Looking at it.

Comment 5

[Michael Froman [:mjf]]

Attachment: Bug 1405940 - Fix Null Pointer dereference in sigslot::lock_block

Caused by several issues:
1) We were allowing an answer with modified ufrag/pass to
   begin an ICE restart even if the offer didn't indicate
   it was restarting.
2) This should no longer happen, but in cases where restart logic
   was started inappropriately, TransportLayerIce::SetParameters
   could get a null stream, and we check for that now.

Review commit: https://reviewboard.mozilla.org/r/190844/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/190844/

Comment 6

[Byron Campen [:bwc]]

Comment on attachment 8919910 [details]
Bug 1405940 - Fix Null Pointer dereference in sigslot::lock_block

https://reviewboard.mozilla.org/r/190844/#review196056

Comment 7

[Byron Campen [:bwc]]

Comment on attachment 8919910 [details]
Bug 1405940 - Fix Null Pointer dereference in sigslot::lock_block

https://reviewboard.mozilla.org/r/190844/#review196360

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/03bb347a9e4d
Fix Null Pointer dereference in sigslot::lock_block r=bwc

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/03bb347a9e4d

Comment 10

[Ryan VanderMeulen [:RyanVM]]

INFO: Last good revision: 6cf0089510fad8deb866136f5b92bbced9498447 (2016-08-10)
INFO: First bad revision: 0502bd9e025edde29777ba1de4280f9b52af4663 (2016-08-11)
INFO: Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6cf0089510fad8deb866136f5b92bbced9498447&tochange=0502bd9e025edde29777ba1de4280f9b52af4663

Could we have landed a test for this?

Comment 11

[Nils Ohlmeier [:drno]]

That sounds like a good idea. Lets at least put a jsep unit test.

Comment 12

[Michael Froman [:mjf]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #10)
> INFO: Last good revision: 6cf0089510fad8deb866136f5b92bbced9498447
> (2016-08-10)
> INFO: First bad revision: 0502bd9e025edde29777ba1de4280f9b52af4663
> (2016-08-11)
> INFO: Pushlog:
> https://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=6cf0089510fad8deb866136f5b92bbced9498447&tochange=0502
> bd9e025edde29777ba1de4280f9b52af4663
> 
> Could we have landed a test for this?

Given that the crash log graph shows this happening as early as May 2017, I'm not sure that pushlog is a good indicator of what caused this.

The test html file attachment that reproduces this needed to run for a non-determinant time to cause the crash.  I'm not sure if a one-shot jsep unit test will be a reliable way to detect this issue.  I'll take a look, and if so, I'll open another bug for the test addition.

Comment 13

[Nils Ohlmeier [:drno]]

(In reply to Michael Froman [:mjf] from comment #12)
> (In reply to Ryan VanderMeulen [:RyanVM] from comment #10)
> > Could we have landed a test for this?
> 
> Given that the crash log graph shows this happening as early as May 2017,
> I'm not sure that pushlog is a good indicator of what caused this.
> 
> The test html file attachment that reproduces this needed to run for a
> non-determinant time to cause the crash.  I'm not sure if a one-shot jsep
> unit test will be a reliable way to detect this issue.  I'll take a look,
> and if so, I'll open another bug for the test addition.

I meant at least a test for not allowing rollbacks via SDP answers.

Comment 14

[Michael Froman [:mjf]]

(In reply to Nils Ohlmeier [:drno] from comment #13)
> (In reply to Michael Froman [:mjf] from comment #12)
> > (In reply to Ryan VanderMeulen [:RyanVM] from comment #10)
> > > Could we have landed a test for this?
> > 
> > Given that the crash log graph shows this happening as early as May 2017,
> > I'm not sure that pushlog is a good indicator of what caused this.
> > 
> > The test html file attachment that reproduces this needed to run for a
> > non-determinant time to cause the crash.  I'm not sure if a one-shot jsep
> > unit test will be a reliable way to detect this issue.  I'll take a look,
> > and if so, I'll open another bug for the test addition.
> 
> I meant at least a test for not allowing rollbacks via SDP answers.

The problem that I saw in the test file for this bug wasn't rollbacks via SDP answers, it was inappropriate restarts by the answerer (ICE creds changing on the answer when they didn't on the offer).  I covered the tests in Bug 1413709 (both jsep session unit tests and mochitests).  Let me know if you think I solved the wrong problem, and I'll come back to the tests.