- Revision
- 285402
- Author
- [email protected]
- Date
- 2021-11-08 08:19:29 -0800 (Mon, 08 Nov 2021)
Log Message
Infinite loop in SourceBufferParserWebM::SegmentReader::Read
https://bugs.webkit.org/show_bug.cgi?id=232740
<rdar://83573873>
Reviewed by Eric Carlson.
Source/WebCore:
No test because the reproduction steps are still unknown.
Customer reports indicate an infinite loop condition on a background queue inside
SourceBufferParserWebM::SegmentReader::Read(), and those reports indicate that the
MTPluginByteSourceRead() call is failing and returning an error. When an error occurs, the
SourceBufferParser::Segment::read() method drops the error and simply reports that zero
bytes were read. And consequently in the SegmentReader::Read() method, that method will
loop forever attempting to satisfy the request for a certain number of bytes to be read.
The cause of the MTPluginByteSourceRead() call failing is not well known at this point.
It could be the case of a faulty server reporting an incorrect content-length, or it could
be a result of a WebContent process crash. Regardless, that error should be propagated
up the call stack so that the caller can correctly break out of its while loop.
Change the return type the Segment::read() method from `size_t` to an Expected object
containing either a `size_t` or a ReaderError. Check the return value from within
SegmentReader::Read() and bail if an error was returned. ReadInto() will need to be
modified similarly.
* platform/graphics/cocoa/SourceBufferParser.cpp:
(WebCore::SourceBufferParser::Segment::read const):
(WebCore::SourceBufferParser::Segment::takeSharedBuffer):
* platform/graphics/cocoa/SourceBufferParser.h:
* platform/graphics/cocoa/SourceBufferParserWebM.cpp:
(WebCore::segmentReadErrorToWebmStatus):
* platform/graphics/cocoa/SourceBufferParserWebM.h:
Source/WebCore/PAL:
* pal/spi/cocoa/MediaToolboxSPI.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (285401 => 285402)
--- trunk/Source/WebCore/ChangeLog 2021-11-08 15:57:47 UTC (rev 285401)
+++ trunk/Source/WebCore/ChangeLog 2021-11-08 16:19:29 UTC (rev 285402)
@@ -1,3 +1,38 @@
+2021-11-08 Jer Noble <[email protected]>
+
+ Infinite loop in SourceBufferParserWebM::SegmentReader::Read
+ https://bugs.webkit.org/show_bug.cgi?id=232740
+ <rdar://83573873>
+
+ Reviewed by Eric Carlson.
+
+ No test because the reproduction steps are still unknown.
+
+ Customer reports indicate an infinite loop condition on a background queue inside
+ SourceBufferParserWebM::SegmentReader::Read(), and those reports indicate that the
+ MTPluginByteSourceRead() call is failing and returning an error. When an error occurs, the
+ SourceBufferParser::Segment::read() method drops the error and simply reports that zero
+ bytes were read. And consequently in the SegmentReader::Read() method, that method will
+ loop forever attempting to satisfy the request for a certain number of bytes to be read.
+
+ The cause of the MTPluginByteSourceRead() call failing is not well known at this point.
+ It could be the case of a faulty server reporting an incorrect content-length, or it could
+ be a result of a WebContent process crash. Regardless, that error should be propagated
+ up the call stack so that the caller can correctly break out of its while loop.
+
+ Change the return type the Segment::read() method from `size_t` to an Expected object
+ containing either a `size_t` or a ReaderError. Check the return value from within
+ SegmentReader::Read() and bail if an error was returned. ReadInto() will need to be
+ modified similarly.
+
+ * platform/graphics/cocoa/SourceBufferParser.cpp:
+ (WebCore::SourceBufferParser::Segment::read const):
+ (WebCore::SourceBufferParser::Segment::takeSharedBuffer):
+ * platform/graphics/cocoa/SourceBufferParser.h:
+ * platform/graphics/cocoa/SourceBufferParserWebM.cpp:
+ (WebCore::segmentReadErrorToWebmStatus):
+ * platform/graphics/cocoa/SourceBufferParserWebM.h:
+
2021-11-08 Youenn Fablet <[email protected]>
newAudioChunkPushed callback should take the total number of audio samples as parameter
Modified: trunk/Source/WebCore/PAL/ChangeLog (285401 => 285402)
--- trunk/Source/WebCore/PAL/ChangeLog 2021-11-08 15:57:47 UTC (rev 285401)
+++ trunk/Source/WebCore/PAL/ChangeLog 2021-11-08 16:19:29 UTC (rev 285402)
@@ -1,3 +1,13 @@
+2021-11-08 Jer Noble <[email protected]>
+
+ Infinite loop in SourceBufferParserWebM::SegmentReader::Read
+ https://bugs.webkit.org/show_bug.cgi?id=232740
+ <rdar://83573873>
+
+ Reviewed by Eric Carlson.
+
+ * pal/spi/cocoa/MediaToolboxSPI.h:
+
2021-11-07 Myles C. Maxfield <[email protected]>
[WebGPU] Add internal WebGPU API interface
Modified: trunk/Source/WebCore/PAL/pal/spi/cocoa/MediaToolboxSPI.h (285401 => 285402)
--- trunk/Source/WebCore/PAL/pal/spi/cocoa/MediaToolboxSPI.h 2021-11-08 15:57:47 UTC (rev 285401)
+++ trunk/Source/WebCore/PAL/pal/spi/cocoa/MediaToolboxSPI.h 2021-11-08 16:19:29 UTC (rev 285402)
@@ -44,6 +44,7 @@
kMTPluginFormatReaderError_ParsingFailure = -16503,
kMTPluginSampleCursorError_NoSamples = -16507,
kMTPluginSampleCursorError_LocationNotAvailable = -16508,
+ kMTPluginByteSourceError_EndOfStream = -16511,
};
enum {
Modified: trunk/Source/WebCore/platform/graphics/cocoa/SourceBufferParser.cpp (285401 => 285402)
--- trunk/Source/WebCore/platform/graphics/cocoa/SourceBufferParser.cpp 2021-11-08 15:57:47 UTC (rev 285401)
+++ trunk/Source/WebCore/platform/graphics/cocoa/SourceBufferParser.cpp 2021-11-08 16:19:29 UTC (rev 285402)
@@ -32,9 +32,11 @@
#include "SharedBuffer.h"
#include "SourceBufferParserAVFObjC.h"
#include "SourceBufferParserWebM.h"
-#include <pal/cocoa/MediaToolboxSoftLink.h>
+#include <pal/spi/cocoa/MediaToolboxSPI.h>
#include <wtf/text/WTFString.h>
+#include <pal/cocoa/MediaToolboxSoftLink.h>
+
namespace WebCore {
MediaPlayerEnums::SupportsType SourceBufferParser::isContentTypeSupported(const ContentType& type)
@@ -106,21 +108,24 @@
);
}
-size_t SourceBufferParser::Segment::read(size_t position, size_t sizeToRead, uint8_t* destination) const
+auto SourceBufferParser::Segment::read(size_t position, size_t sizeToRead, uint8_t* destination) const -> ReadResult
{
size_t segmentSize = size();
sizeToRead = std::min(sizeToRead, segmentSize - std::min(position, segmentSize));
return WTF::switchOn(m_segment,
#if HAVE(MT_PLUGIN_FORMAT_READER)
- [&](const RetainPtr<MTPluginByteSourceRef>& byteSource) -> size_t
+ [&](const RetainPtr<MTPluginByteSourceRef>& byteSource) -> ReadResult
{
size_t sizeRead = 0;
- if (MTPluginByteSourceRead(byteSource.get(), sizeToRead, CheckedInt64(position), destination, &sizeRead) != noErr)
- return 0;
- return sizeRead;
+ auto status = MTPluginByteSourceRead(byteSource.get(), sizeToRead, CheckedInt64(position), destination, &sizeRead);
+ if (status == noErr)
+ return sizeRead;
+ if (status == kMTPluginByteSourceError_EndOfStream)
+ return Unexpected<ReadError> { ReadError::EndOfFile };
+ return Unexpected<ReadError> { ReadError::FatalError };
},
#endif
- [&](const Ref<SharedBuffer>& buffer)
+ [&](const Ref<SharedBuffer>& buffer) -> ReadResult
{
buffer->copyTo(destination, position, sizeToRead);
return sizeToRead;
@@ -135,7 +140,10 @@
[&](RetainPtr<MTPluginByteSourceRef>&)
{
Vector<uint8_t> vector(size());
- vector.shrink(read(0, vector.size(), vector.data()));
+ auto readResult = read(0, vector.size(), vector.data());
+ if (!readResult.has_value())
+ return SharedBuffer::create();
+ vector.shrink(readResult.value());
return SharedBuffer::create(WTFMove(vector));
},
#endif
Modified: trunk/Source/WebCore/platform/graphics/cocoa/SourceBufferParser.h (285401 => 285402)
--- trunk/Source/WebCore/platform/graphics/cocoa/SourceBufferParser.h 2021-11-08 15:57:47 UTC (rev 285401)
+++ trunk/Source/WebCore/platform/graphics/cocoa/SourceBufferParser.h 2021-11-08 16:19:29 UTC (rev 285402)
@@ -33,6 +33,7 @@
#include <pal/spi/cocoa/MediaToolboxSPI.h>
#include <variant>
#include <wtf/CompletionHandler.h>
+#include <wtf/Expected.h>
#include <wtf/RefCounted.h>
#include <wtf/ThreadSafeRefCounted.h>
@@ -75,8 +76,12 @@
RefPtr<SharedBuffer> getSharedBuffer() const;
size_t size() const;
- size_t read(size_t position, size_t, uint8_t* destination) const;
+ enum class ReadError { EndOfFile, FatalError };
+ using ReadResult = Expected<size_t, ReadError>;
+
+ ReadResult read(size_t position, size_t, uint8_t* destination) const;
+
private:
std::variant<
#if HAVE(MT_PLUGIN_FORMAT_READER)
Modified: trunk/Source/WebCore/platform/graphics/cocoa/SourceBufferParserWebM.cpp (285401 => 285402)
--- trunk/Source/WebCore/platform/graphics/cocoa/SourceBufferParserWebM.cpp 2021-11-08 15:57:47 UTC (rev 285401)
+++ trunk/Source/WebCore/platform/graphics/cocoa/SourceBufferParserWebM.cpp 2021-11-08 16:19:29 UTC (rev 285402)
@@ -273,6 +273,14 @@
using namespace webm;
+static Status segmentReadErrorToWebmStatus(SourceBufferParser::Segment::ReadError error)
+{
+ switch (error) {
+ case SourceBufferParser::Segment::ReadError::EndOfFile: return Status(Status::kEndOfFile);
+ case SourceBufferParser::Segment::ReadError::FatalError: return Status(Status::Code(SourceBufferParserWebM::ErrorCode::ReaderFailed));
+ }
+}
+
class SourceBufferParserWebM::SegmentReader final : public webm::Reader {
WTF_MAKE_FAST_ALLOCATED;
public:
@@ -301,7 +309,10 @@
continue;
}
- uint64_t lastRead = currentSegment.read(m_positionWithinSegment, numToRead, outputBuffer);
+ auto readResult = currentSegment.read(m_positionWithinSegment, numToRead, outputBuffer);
+ if (!readResult.has_value())
+ return segmentReadErrorToWebmStatus(readResult.error());
+ auto lastRead = readResult.value();
m_position += lastRead;
*numActuallyRead += lastRead;
m_positionWithinSegment += lastRead;
@@ -391,7 +402,10 @@
err = PAL::CMBlockBufferGetDataPointer(rawBlockBuffer, 0, &segmentSizeAtPosition, nullptr, (char**)&blockBufferData);
if (err != kCMBlockBufferNoErr)
return Status(Status::kNotEnoughMemory);
- lastRead = currentSegment.read(m_positionWithinSegment, numToRead, blockBufferData);
+ auto readResult = currentSegment.read(m_positionWithinSegment, numToRead, blockBufferData);
+ if (!readResult.has_value())
+ return segmentReadErrorToWebmStatus(readResult.error());
+ lastRead = readResult.value();
destinationOffset = 0;
} else {
ASSERT(sharedBuffer->hasOneSegment(), "Can only deal with sharedBuffer containing a single DataSegment");
Modified: trunk/Source/WebCore/platform/graphics/cocoa/SourceBufferParserWebM.h (285401 => 285402)
--- trunk/Source/WebCore/platform/graphics/cocoa/SourceBufferParserWebM.h 2021-11-08 15:57:47 UTC (rev 285401)
+++ trunk/Source/WebCore/platform/graphics/cocoa/SourceBufferParserWebM.h 2021-11-08 16:19:29 UTC (rev 285402)
@@ -101,6 +101,7 @@
UnsupportedAudioCodec,
ContentEncrypted,
VariableFrameDuration,
+ ReaderFailed,
};
enum class State : uint8_t {
