e.g. 0 com.apple.WebCore 0x00000003223a4f81 WTF::RefPtr<JSC::ArrayBuffer, WTF::RawPtrTraits<JSC::ArrayBuffer>, WTF::DefaultRefDerefTraits<JSC::ArrayBuffer> >::operator!() const + 0 (RefPtr.h:85) [inlined] 1 com.apple.WebCore 0x00000003223a4f81 JSC::ArrayBufferView::isDetached() const + 0 (ArrayBufferView.h:47) [inlined] 2 com.apple.WebCore 0x00000003223a4f81 JSC::ArrayBufferView::isShared() const + 0 (ArrayBufferView.h:66) [inlined] 3 com.apple.WebCore 0x00000003223a4f81 JSC::JSArrayBufferView::unsharedImpl() + 14 (JSArrayBufferViewInlines.h:79) [inlined] 4 com.apple.WebCore 0x00000003223a4f81 JSC::JSArrayBufferView::toWrapped(JSC::VM&, JSC::JSValue) + 129 (JSArrayBufferViewInlines.h:127) 5 com.apple.WebCore 0x000000032244159f WTF::RefPtr<JSC::ArrayBufferView, WTF::RawPtrTraits<JSC::ArrayBufferView>, WTF::DefaultRefDerefTraits<JSC::ArrayBufferView> > WebCore::Detail::BufferSourceConverter<WebCore::IDLArrayBufferView, (WebCore::Detail::BufferSourceConverterAllowSharedMode)1>::convert<WebCore::jsCryptoPrototypeFunction_getRandomValuesBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSCrypto*)::'lambda'(JSC::JSGlobalObject&, JSC::ThrowScope&)>(JSC::JSGlobalObject&, JSC::JSValue, WebCore::jsCryptoPrototypeFunction_getRandomValuesBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSCrypto*)::'lambda'(JSC::JSGlobalObject&, JSC::ThrowScope&)&&) + 20 (JSDOMConvertBufferSource.h:127) [inlined] 6 com.apple.WebCore 0x000000032244159f WTF::RefPtr<JSC::ArrayBufferView, WTF::RawPtrTraits<JSC::ArrayBufferView>, WTF::DefaultRefDerefTraits<JSC::ArrayBufferView> > WebCore::Converter<WebCore::IDLArrayBufferView>::convert<WebCore::jsCryptoPrototypeFunction_getRandomValuesBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSCrypto*)::'lambda'(JSC::JSGlobalObject&, JSC::ThrowScope&)>(JSC::JSGlobalObject&, JSC::JSValue, WebCore::jsCryptoPrototypeFunction_getRandomValuesBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSCrypto*)::'lambda'(JSC::JSGlobalObject&, JSC::ThrowScope&)&&) + 20 (JSDOMConvertBufferSource.h:391) [inlined] 7 com.apple.WebCore 0x000000032244159f WebCore::Converter<WebCore::IDLArrayBufferView>::ReturnType WebCore::convert<WebCore::IDLArrayBufferView, WebCore::jsCryptoPrototypeFunction_getRandomValuesBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSCrypto*)::'lambda'(JSC::JSGlobalObject&, JSC::ThrowScope&)>(JSC::JSGlobalObject&, JSC::JSValue, WebCore::jsCryptoPrototypeFunction_getRandomValuesBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSCrypto*)::'lambda'(JSC::JSGlobalObject&, JSC::ThrowScope&)&&) + 20 (JSDOMConvertBase.h:76) [inlined] 8 com.apple.WebCore 0x000000032244159f WebCore::jsCryptoPrototypeFunction_getRandomValuesBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSCrypto*) + 42 (JSCrypto.cpp:219) [inlined] 9 com.apple.WebCore 0x000000032244159f long long WebCore::IDLOperation<WebCore::JSCrypto>::call<&(WebCore::jsCryptoPrototypeFunction_getRandomValuesBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSCrypto*)), (WebCore::CastedThisErrorBehavior)0>(JSC::JSGlobalObject&, JSC::CallFrame&, char const*) + 187 (JSDOMOperation.h:53) [inlined] 10 com.apple.WebCore 0x000000032244159f WebCore::jsCryptoPrototypeFunction_getRandomValues(JSC::JSGlobalObject*, JSC::CallFrame*) + 207 (JSCrypto.cpp:228) 11 ??? 0x00002a9fd14011d8 0 + 46865898803672 12 com.apple.JavaScriptCore 0x0000000328367a5d llint_entry + 108286 (LowLevelInterpreter.asm:1093) <rdar://problem/74098645>
<rdar://problem/74109720>
Created attachment 419613 [details] Test
Created attachment 421195 [details] Patch
The patch fixes the crash. If people agree this is not a security problem (looks like a null pointer crash to me) I will add the testcase.
Comment on attachment 421195 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=421195&action=review > Source/JavaScriptCore/runtime/JSArrayBufferViewInlines.h:79 > - RELEASE_ASSERT(!result->isShared()); > + RELEASE_ASSERT(!result || !result->isShared()); Why is it okay to return nullptr here? Why are we getting nullptr from possiblySharedImpl?
Created attachment 421308 [details] Patch
Created attachment 421314 [details] Patch
Created attachment 421318 [details] Patch
Comment on attachment 421195 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=421195&action=review >> Source/JavaScriptCore/runtime/JSArrayBufferViewInlines.h:79 >> + RELEASE_ASSERT(!result || !result->isShared()); > > Why is it okay to return nullptr here? > Why are we getting nullptr from possiblySharedImpl? possiblySharedImpl is not able to allocate in OOM situation so nullptr is returned. I think it is okay to return nullptr, this will result in an Exception being thrown, which seems appropriate for me in this situation.
Comment on attachment 421318 [details] Patch Yes, this is correct.
Committed r273373: <https://commits.webkit.org/r273373> All reviewed patches have been landed. Closing bug and clearing flags on attachment 421318 [details].
It looks like the new test added in https://commits.webkit.org/r273373 is frequently failing on the bots https://results.webkit.org/?suite=layout-tests&test=crypto%2Fcrypto-random-values-oom.html --- /Volumes/Data/slave/catalina-debug-tests-wk1/build/layout-test-results/crypto/crypto-random-values-oom-expected.txt +++ /Volumes/Data/slave/catalina-debug-tests-wk1/build/layout-test-results/crypto/crypto-random-values-oom-actual.txt @@ -1,3 +1,4 @@ +CONSOLE MESSAGE: Unhandled Promise Rejection: RangeError: Out of memory Test crypto.getRandomValues behavior in oom situation. On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
Reverted r273373 for reason: The test added with this change is frequently failing Committed r273418 (234524@main): <https://commits.webkit.org/234524@main>
This revert doesn’t make logical sense. It make the assertions stronger.
Oh, I see, reverting the *test* is valuable. Reverting the code change, not valuable, but it ends up carried along.
Created attachment 421683 [details] Patch
Comment on attachment 421683 [details] Patch The test was unstable because of the console output, this new patch suppresses it.
Comment on attachment 421683 [details] Patch Looks like the test is timing out on Windows. Perhaps we wanna skip it there.
Created attachment 421741 [details] Patch
Committed r273624: <https://commits.webkit.org/r273624> All reviewed patches have been landed. Closing bug and clearing flags on attachment 421741 [details].