Bug 242612

Summary: [AArch32] SHOULD NEVER BE REACHED in void* JSC::LocalAllocator::allocateSlowCase(JSC::Heap&, JSC::GCDeferralContext*, JSC::AllocationFailureMode)
Product: WebKit Reporter: Mikhail R. Gadelha <mikhail>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED WONTFIX    
Severity: Normal CC: bfulgham, mark.lam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Testcase
none
Original testcase found by the fuzzer none

Mikhail R. Gadelha
Reported 2022-07-11 15:35:54 PDT
Created attachment 460800 [details] Testcase Found by Igalia Fuzzing Campaign. The attached test case fails with the following message: SHOULD NEVER BE REACHED WebKit//Source/JavaScriptCore/heap/LocalAllocator.cpp(148) : void* JSC::LocalAllocator::allocateSlowCase(JSC::Heap&, JSC::GCDeferralContext*, JSC::AllocationFailureMode)
Attachments
Testcase (161 bytes, text/javascript)
2022-07-11 15:35 PDT, Mikhail R. Gadelha 		no flags
Details
Original testcase found by the fuzzer (94.62 KB, text/javascript)
2022-07-12 14:21 PDT, Mikhail R. Gadelha 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2022-07-11 15:36:06 PDT
<rdar://problem/96847395>
Mark Lam
Comment 2 2022-07-12 13:34:25 PDT
Mikhail, I'm not able to reproduce this on a Release or Debug build of https://commits.webkit.org/252364@main running on M1. What code version did you build? What type of build? What port? What machine did you test on? Thanks.
Mark Lam
Comment 3 2022-07-12 13:46:03 PDT
Assuming ToT code, LocalAllocator.cpp:148 is: ``` MarkedBlock::Handle* block = m_directory->tryAllocateBlock(heap); if (!block) { if (failureMode == AllocationFailureMode::Assert) RELEASE_ASSERT_NOT_REACHED(); // <---- line 148 else return nullptr; } ```
Mikhail R. Gadelha
Comment 4 2022-07-12 14:19:53 PDT
Hey Mark, I just double-checked in an ARMv8l linux machine and I was able to reproduce it using WebKit from commit e94f950ce43b. Maybe it's a linux only crash?
Mikhail R. Gadelha
Comment 5 2022-07-12 14:20:32 PDT
I'll also upload the original test case found by the fuzzer.
Mikhail R. Gadelha
Comment 6 2022-07-12 14:21:13 PDT
Created attachment 460825 [details] Original testcase found by the fuzzer
Mikhail R. Gadelha
Comment 7 2022-07-12 14:41:27 PDT
I couldn't reproduce it on our ARM64 linux box anymore... I'm not sure how our ARM64 fuzzer found it...
Mark Lam
Comment 8 2022-07-12 14:46:32 PDT
From the site of the crash, we know that this is an OOM issue.
Mikhail R. Gadelha
Comment 9 2022-07-12 14:50:12 PDT
I see, probably it was triggered because our fuzzer runs 32 tests in parallel and we set no memory limit to them. Maybe there was a memory-hungry test running in parallel. So, is it a bug? Or is it just the error message that needs to be improved?
Mark Lam
Comment 10 2022-07-12 14:52:38 PDT
(In reply to Mikhail R. Gadelha from comment #9) > I see, probably it was triggered because our fuzzer runs 32 tests in > parallel and we set no memory limit to them. Maybe there was a memory-hungry > test running in parallel. > > So, is it a bug? Or is it just the error message that needs to be improved? It's not a bug. One option is to make it not crash on failure to allocate. That would mean having the caller not pass in AllocationFailureMode::Assert. Since we don't have a crash stack trace, and can't reproduce the issue, I'm not sure we can do anything about this. This is also not a security issue. It's an ordering shut down due to resource exhaustion. At best, it's a fuzzer blocker.
Mark Lam
Comment 11 2022-07-12 14:53:58 PDT
(In reply to Mark Lam from comment #10) > One option is to make it not crash on failure to allocate. By this, I mean make the caller check for the allocator returning nullptr, and throwing an OOME. This may or may not be easy to do depending on who the callers are.
Mark Lam
Comment 12 2022-07-12 14:55:33 PDT
If this is not blocking the fuzzer, we can resolve this as WONTFIX.
Mikhail R. Gadelha
Comment 13 2022-07-12 15:02:57 PDT
Closing it as WONTFIX is fine. I'll add a rule to filter out these cases, we already do it for other OOM tests.
Note You need to log in before you can comment on or make changes to this bug.