Modified: trunk/Source/WebCore/ChangeLog (236805 => 236806)
--- trunk/Source/WebCore/ChangeLog 2018-10-03 18:38:11 UTC (rev 236805)
+++ trunk/Source/WebCore/ChangeLog 2018-10-03 19:28:31 UTC (rev 236806)
@@ -1,3 +1,22 @@
+2018-10-03 Jer Noble <jer.no...@apple.com>
+
+ CRASH in CVPixelBufferGetBytePointerCallback()
+ https://bugs.webkit.org/show_bug.cgi?id=190092
+
+ Reviewed by Eric Carlson.
+
+ Speculative fix for crash that occurs when callers of CVPixelBufferGetBytePointerCallback() attempt
+ to read the last byte of a CVPixelBuffer (as a pre-flight check) and crash due to a memory access
+ error. It's speculated that mismatching CVPixelBufferLockBytePointer / CVPixelBufferUnlockBytePointer
+ calls could result in an incorrect state inside the CVPixelBuffer. Add log count checks, locking, and
+ release logging to try to pinpoint if mismatch lock counts are occurring in this code path.
+
+ * platform/graphics/cv/PixelBufferConformerCV.cpp:
+ (WebCore::CVPixelBufferGetBytePointerCallback):
+ (WebCore::CVPixelBufferReleaseBytePointerCallback):
+ (WebCore::CVPixelBufferReleaseInfoCallback):
+ (WebCore::PixelBufferConformerCV::createImageFromPixelBuffer):
+
2018-10-03 Chris Dumez <cdu...@apple.com>
Regression(r236779): Crash when changing the input element type from inside an 'input' event listener
Modified: trunk/Source/WebCore/platform/graphics/cv/PixelBufferConformerCV.cpp (236805 => 236806)
--- trunk/Source/WebCore/platform/graphics/cv/PixelBufferConformerCV.cpp 2018-10-03 18:38:11 UTC (rev 236805)
+++ trunk/Source/WebCore/platform/graphics/cv/PixelBufferConformerCV.cpp 2018-10-03 19:28:31 UTC (rev 236806)
@@ -29,6 +29,7 @@
#if HAVE(CORE_VIDEO)
#include "GraphicsContextCG.h"
+#include "Logging.h"
#include <wtf/SoftLinking.h>
#include "CoreVideoSoftLink.h"
@@ -55,23 +56,87 @@
#endif
}
-static const void* CVPixelBufferGetBytePointerCallback(void* info)
+struct CVPixelBufferInfo {
+ RetainPtr<CVPixelBufferRef> pixelBuffer;
+ int lockCount { 0 };
+};
+
+static const void* CVPixelBufferGetBytePointerCallback(void* refcon)
{
- CVPixelBufferRef pixelBuffer = static_cast<CVPixelBufferRef>(info);
- CVPixelBufferLockBaseAddress(pixelBuffer, kCVPixelBufferLock_ReadOnly);
- return CVPixelBufferGetBaseAddress(pixelBuffer);
+ ASSERT(refcon);
+ if (!refcon) {
+ RELEASE_LOG_ERROR(Media, "CVPixelBufferGetBytePointerCallback() called with NULL refcon");
+ RELEASE_LOG_STACKTRACE(Media);
+ return nullptr;
+ }
+ auto info = static_cast<CVPixelBufferInfo*>(refcon);
+
+ CVReturn result = CVPixelBufferLockBaseAddress(info->pixelBuffer.get(), kCVPixelBufferLock_ReadOnly);
+
+ ASSERT(result == kCVReturnSuccess);
+ if (result != kCVReturnSuccess) {
+ RELEASE_LOG_ERROR(Media, "CVPixelBufferLockBaseAddress() returned error code %d", result);
+ RELEASE_LOG_STACKTRACE(Media);
+ return nullptr;
+ }
+
+ ++info->lockCount;
+ void* address = CVPixelBufferGetBaseAddress(info->pixelBuffer.get());
+ RELEASE_LOG_INFO(Media, "CVPixelBufferGetBytePointerCallback() returning bytePointer: %p, size: %zu", address, CVPixelBufferGetDataSize(info->pixelBuffer.get()));
+ return address;
}
-static void CVPixelBufferReleaseBytePointerCallback(void* info, const void*)
+static void CVPixelBufferReleaseBytePointerCallback(void* refcon, const void*)
{
- CVPixelBufferRef pixelBuffer = static_cast<CVPixelBufferRef>(info);
- CVPixelBufferUnlockBaseAddress(pixelBuffer, kCVPixelBufferLock_ReadOnly);
+ ASSERT(refcon);
+ if (!refcon) {
+ RELEASE_LOG_ERROR(Media, "CVPixelBufferReleaseBytePointerCallback() called with NULL refcon");
+ RELEASE_LOG_STACKTRACE(Media);
+ return;
+ }
+ auto info = static_cast<CVPixelBufferInfo*>(refcon);
+
+ CVReturn result = CVPixelBufferUnlockBaseAddress(info->pixelBuffer.get(), kCVPixelBufferLock_ReadOnly);
+ ASSERT(result == kCVReturnSuccess);
+ if (result != kCVReturnSuccess) {
+ RELEASE_LOG_ERROR(Media, "CVPixelBufferLockBaseAddress() returned error code %d", result);
+ RELEASE_LOG_STACKTRACE(Media);
+ return;
+ }
+
+ ASSERT(info->lockCount);
+ if (!info->lockCount) {
+ RELEASE_LOG_ERROR(Media, "CVPixelBufferReleaseBytePointerCallback() called without matching CVPixelBufferGetBytePointerCallback()");
+ RELEASE_LOG_STACKTRACE(Media);
+ }
+ --info->lockCount;
}
-static void CVPixelBufferReleaseInfoCallback(void* info)
+static void CVPixelBufferReleaseInfoCallback(void* refcon)
{
- CVPixelBufferRef pixelBuffer = static_cast<CVPixelBufferRef>(info);
- CFRelease(pixelBuffer);
+ ASSERT(refcon);
+ if (!refcon) {
+ RELEASE_LOG_ERROR(Media, "CVPixelBufferReleaseInfoCallback() called with NULL refcon");
+ RELEASE_LOG_STACKTRACE(Media);
+ return;
+ }
+ auto info = static_cast<CVPixelBufferInfo*>(refcon);
+
+ ASSERT(!info->lockCount);
+ if (info->lockCount) {
+ RELEASE_LOG_ERROR(Media, "CVPixelBufferReleaseInfoCallback() called with a non-zero lockCount: %d", info->lockCount);
+ RELEASE_LOG_STACKTRACE(Media);
+
+ CVReturn result = CVPixelBufferUnlockBaseAddress(info->pixelBuffer.get(), kCVPixelBufferLock_ReadOnly);
+ ASSERT(result == kCVReturnSuccess);
+ if (result != kCVReturnSuccess) {
+ RELEASE_LOG_ERROR(Media, "CVPixelBufferLockBaseAddress() returned error code %d", result);
+ RELEASE_LOG_STACKTRACE(Media);
+ }
+ }
+
+ info->pixelBuffer = nullptr;
+ delete info;
}
RetainPtr<CVPixelBufferRef> PixelBufferConformerCV::convert(CVPixelBufferRef rawBuffer)
@@ -112,9 +177,12 @@
size_t bytesPerRow = CVPixelBufferGetBytesPerRow(buffer.get());
size_t byteLength = CVPixelBufferGetDataSize(buffer.get());
- CFRetain(buffer.get()); // Balanced by CVPixelBufferReleaseInfoCallback in providerCallbacks.
+ CVPixelBufferInfo* info = new CVPixelBufferInfo();
+ info->pixelBuffer = WTFMove(buffer);
+ info->lockCount = 0;
+
CGDataProviderDirectCallbacks providerCallbacks = { 0, CVPixelBufferGetBytePointerCallback, CVPixelBufferReleaseBytePointerCallback, 0, CVPixelBufferReleaseInfoCallback };
- RetainPtr<CGDataProviderRef> provider = adoptCF(CGDataProviderCreateDirect(buffer.get(), byteLength, &providerCallbacks));
+ RetainPtr<CGDataProviderRef> provider = adoptCF(CGDataProviderCreateDirect(info, byteLength, &providerCallbacks));
return adoptCF(CGImageCreate(width, height, 8, 32, bytesPerRow, sRGBColorSpaceRef(), bitmapInfo, provider.get(), nullptr, false, kCGRenderingIntentDefault));
}
