Possible null dereference under SourceBuffer::sourceBufferPrivateDidReceiveSample()

[Chris Dumez]

Possible null dereference under SourceBuffer::sourceBufferPrivateDidReceiveSample():
Thread[0] EXC_BAD_ACCESS (SIGSEGV) (KERN_INVALID_ADDRESS at 0x0000000000000000)
[  0] 0x00007fffa678ee35 WebCore`WebCore::SourceBuffer::sourceBufferPrivateDidReceiveSample(WebCore::SourceBufferPrivate*, WTF::PassRefPtr<WebCore::MediaSample>) + 2085 at SourceBuffer.cpp:1464:21
       1460	                // spliced audio frame.
       1461	                // FIXME: Add support for sample splicing.
       1462	
       1463	                // If track buffer contains video coded frames:
    -> 1464	                if (trackBuffer.description->isVideo()) {
       1465	                    // 1.14.2.1 Let overlapped frame presentation timestamp equal the presentation timestamp
       1466	                    // of overlapped frame.
       1467	                    MediaTime overlappedFramePresentationTimestamp = overlappedFrame->presentationTime();
       1468	
    

     0x00007fffa678ee26:    testq %rbx, %rbx
     0x00007fffa678ee29:       je 0xe39e2e             ; <+2078> [inlined] WTF::RefPtr<WebCore::MediaDescription>::operator->() const at SourceBuffer.cpp:1464
     0x00007fffa678ee2b:     incl 0x8(%rbx)
     0x00007fffa678ee2e:     movq 0xb0(%r14), %rdi
 ->  0x00007fffa678ee35:     movq (%rdi), %rax
     0x00007fffa678ee38:    callq *0x18(%rax)
     0x00007fffa678ee3b:    testb %al, %al
     0x00007fffa678ee3d:       je 0xe39ef9             ; <+2281> at SourceBuffer.cpp:1477
     0x00007fffa678ee43:     movq %r12, -0x300(%rbp)

[  1] 0x00007fffa6795c79 WebCore`WebCore::SourceBufferPrivateAVFObjC::processCodedFrame(int, opaqueCMSampleBuffer*, WTF::String const&) + 217 at SourceBufferPrivateAVFObjC.mm:699:9
       695 	
       696 	    if (m_client) {
       697 	        RefPtr<MediaSample> mediaSample = MediaSampleAVFObjC::create(sampleBuffer, trackID);
       698 	        LOG(MediaSourceSamples, "SourceBufferPrivateAVFObjC::processCodedFrame(%p) - sample(%s)", this, toString(*mediaSample).utf8().data());
    -> 699 	        m_client->sourceBufferPrivateDidReceiveSample(this, WTFMove(mediaSample));
       700 	    }
       701 	
       702 	    return true;
       703 	}

[Chris Dumez]

Created attachment 283333 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/27282945>

[Chris Dumez]

Jer, it looks like you wrote this code, what do you think?

[Alexey Proskuryakov]

Comment on attachment 283333 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=283333&action=review

> Source/WebCore/ChangeLog:8
> +        Add a null check for trackBuffer.description before dereferencing as it seems

Test?

[Chris Dumez]

(In reply to comment #4)
> Comment on attachment 283333 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=283333&action=review
> 
> > Source/WebCore/ChangeLog:8
> > +        Add a null check for trackBuffer.description before dereferencing as it seems
> 
> Test?

I'll talk to Jer as I have no idea how to exercise this code path. We have seen this crash in the wild but I have not been able to reproduce.

[Brent Fulgham]

(In reply to comment #5)
> > Test?
> 
> I'll talk to Jer as I have no idea how to exercise this code path. We have
> seen this crash in the wild but I have not been able to reproduce.

It's been a month -- any update?

[WebKit Commit Bot]

Comment on attachment 283333 [details]
Patch

Clearing flags on attachment: 283333

Committed r214693: <http://trac.webkit.org/changeset/214693>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.