Null-dereference READ in blink::PersistentBase<blink::LocalFrame, |
||||||||||
Issue descriptionDetailed report: https://clusterfuzz.com/testcase?key=5791621626200064 Fuzzer: libFuzzer_v8_serialized_script_value_fuzzer Job Type: mac_libfuzzer_chrome_asan Platform Id: mac Crash Type: Null-dereference READ Crash Address: 0x000000000010 Crash State: blink::PersistentBase<blink::LocalFrame, blink::DummyPageHolder::GetFrame blink::LLVMFuzzerTestOneInput Sanitizer: address (ASAN) Regressed: https://clusterfuzz.com/revisions?job=mac_libfuzzer_chrome_asan&range=493100:493156 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5791621626200064 Issue filed automatically. See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.
,
Aug 21 2017
It looks like g_page_holder is null in this case. That indicates a failure during setup that I cannot explain. Jeremy, can you take a look? I don't think this is related to my change.
,
Aug 22 2017
I suspect the libfuzzer roll is responsible. Perhaps LLVMFuzzerInitialize is no longer being called? https://chromium.googlesource.com/chromium/src/+/a01f1b580b1dce96e37b737304148d290d6ac006
,
Aug 23 2017
Hm, that's interesting. And it happens only on Mac. Kostya, could c#3 be a truth? :)
,
Aug 23 2017
There were a few changes done by the Apple folks that may potentially lead to this on Mac, but still unlikely. We have tests that validate that LLVMFuzzerInitialize works as expected. Sadly, I have no Mac expertise and can't help debug this.
,
Aug 23 2017
Thanks for the prompt response! Ok, I'll get a mac and figure out what's going on.
,
Aug 31 2017
It looks like jbroman@ is right, but only partially. LLVMFuzzerInitialize is indeed not being called, but it is not being called even with an old libFuzzer revisions. This particular crash doesn't reproduce with an old libFuzzer, because it used not to call LLVMFuzzerTestOneInput with an empty input (size of 0), and now it does call LLVMFuzzerTestOneInput anyway.
,
Sep 5 2017
On Linux, LLVMFuzzerInitialize is being called properly. Then, LLVMFuzzerTestOneInput even with empty input does not crash.
,
Sep 6 2017
Reproduced with a smaller example:
#include <stddef.h>
#include <stdint.h>
#include <stdio.h>
extern "C" int LLVMFuzzerInitialize(int* argc, char*** argv) {
printf("Hello from LLVMFuzzerInitialize, argc: %i\n", *argc);
return *argc;
}
extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
printf("Hello from LLVMFuzzerTestOneInput, size: %zu\n", size);
if (size) {
return data[0];
}
return size;
}
$ out/asan/mac_test_fuzzer /tmp/1
HELLO: EF->LLVMFuzzerInitialize: 0x0
INFO: Seed: 4290147427
INFO: Loaded 1 modules (3 guards): 3 [0x10e8384b0, 0x10e8384bc),
out/asan/mac_test_fuzzer: Running 1 inputs 1 time(s) each.
Running: /tmp/1
Hello from LLVMFuzzerTestOneInput, size: 10
Hello from LLVMFuzzerTestOneInput, size: 10
Executed /tmp/1 in 0 ms
***
*** NOTE: fuzzing was not performed, you have only
*** executed the target code on a fixed set of inputs.
***
If I enable warnings in FuzzerExtFunctions.def, I see the following output:
$ out/asan/mac_test_fuzzer /tmp/1
WARNING: Failed to find function "LLVMFuzzerInitialize". Reason dlsym(RTLD_DEFAULT, LLVMFuzzerInitialize): symbol not found.
WARNING: Failed to find function "LLVMFuzzerCustomMutator". Reason dlsym(RTLD_DEFAULT, LLVMFuzzerCustomMutator): symbol not found.
WARNING: Failed to find function "LLVMFuzzerCustomCrossOver". Reason dlsym(RTLD_DEFAULT, LLVMFuzzerCustomCrossOver): symbol not found.
<...>
Which means that libFuzzer itself is working fine, i.e. it is looking for the symbols. That leads me to a conclusion that there an issue with how we build stuff on Mac.
,
Sep 6 2017
$ nm out/asan/obj/testing/libfuzzer/fuzzers/mac_test_fuzzer/mac_test_fuzzer.o
0000000000000000 T _LLVMFuzzerInitialize
0000000000000090 T _LLVMFuzzerTestOneInput
U ___asan_init
U ___asan_report_load1
U ___asan_report_load4
U ___asan_version_mismatch_check_v8
U ___sanitizer_cov_trace_pc_guard
U ___sanitizer_cov_trace_pc_guard_init
0000000000000140 t _asan.module_ctor
U _printf
0000000000000120 t _sancov.module_ctor
00000000000001a8 s l___sancov_gen_
00000000000001ac s l___sancov_gen_.2
U section$end$__DATA$__sancov_guards
U section$start$__DATA$__sancov_guards
_LLVMFuzzerInitialize is there, but it has zero offset (I guess)? Wonder if it could be an issue...
,
Sep 6 2017
No, that zero offset is not an issue, just verified :)
,
Sep 6 2017
I've found that 2yo reply from Kostya regarding weak function declarations not working on Mac: http://clang-developers.42468.n3.nabble.com/Build-issues-when-trying-to-use-LibFuzzer-on-Mac-OS-X-tp4045832p4046420.html
,
Sep 6 2017
Well, there is also a nice-written comment in libFuzzer code (https://chromium.googlesource.com/chromium/llvm-project/compiler-rt/lib/fuzzer.git/+/master/FuzzerExtFunctionsWeak.cpp#9): // Implementation for Linux. This relies on the linker's support for weak // symbols. We don't use this approach on Apple platforms because it requires // clients of LibFuzzer to pass ``-U _<symbol_name>`` to the linker to allow // weak symbols to be undefined. That is a complication we don't want to expose // to clients right now.
,
Sep 6 2017
It looks like I have a fix locally. It consists of two changes: 1) add "-Wl,-U,_LLVMFuzzerCustomCrossOver", to https://cs.chromium.org/chromium/src/testing/libfuzzer/BUILD.gn?l=31 2) enable https://cs.chromium.org/chromium/src/third_party/libFuzzer/src/FuzzerExtFunctionsWeak.cpp?l=16 for LIBFUZZER_APPLE and disable https://cs.chromium.org/chromium/src/third_party/libFuzzer/src/FuzzerExtFunctionsDlsym.cpp?l=15 It works for my small fuzz target, now testing for v8_serialized_script_value_fuzzer. I'm completely unsure about the second part of my fix. Kostya (kcc@), may I ask you to take a look and comment whether it's feasible to do so?
,
Sep 6 2017
The fix works for v8_serialized_script_value_fuzzer too. I'm thinking about doing the following: 1) https://1cs.chromium.org/chromium/src/third_party/libFuzzer/src/FuzzerExtFunctionsWeak.cpp?l=16: add "|| UNDEFINED_WEAK_SYMBOLS_ALLOWED" 2) https://cs.chromium.org/chromium/src/third_party/libFuzzer/src/FuzzerExtFunctionsDlsym.cpp?l=15: add "&& !UNDEFINED_WEAK_SYMBOLS_ALLOWED" And then define UNDEFINED_WEAK_SYMBOLS_ALLOWED in Chromium GN config. Thus, we won't break nor complicate things for other libFuzzer customers using Mac, but can make it work for our use case.
,
Sep 6 2017
As per suggestion from Kostya, started a discussion at llvm-dev@ mailing list: http://lists.llvm.org/pipermail/llvm-dev/2017-September/117185.html
,
Sep 12 2017
,
Oct 1 2017
Automatically applying components based on information from OWNERS files. If this seems incorrect, please apply the Test-Predator-Wrong-Components label.
,
Oct 24 2017
For more information, please see https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md. The link referenced in the description is no longer valid. (bulk edit)
,
Nov 7 2017
,
Dec 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/004f348aba415381cf5b66801e0e0f26c41baae6 commit 004f348aba415381cf5b66801e0e0f26c41baae6 Author: Max Moroz <mmoroz@chromium.org> Date: Tue Dec 19 18:18:11 2017 [fuzzer] Add attributes to LLVMFuzzerInitialize definition. That prevents the linker from dead-stripping the function, as it is not called directly, it is resolved in the runtime via dlsym(). Bug: chromium:754124 , chromium:787723 Change-Id: I46a02ef01349f59b7ed944ce1483b7277e234a19 Reviewed-on: https://chromium-review.googlesource.com/833995 Commit-Queue: Max Moroz <mmoroz@chromium.org> Reviewed-by: Andreas Haas <ahaas@chromium.org> Reviewed-by: Mathias Bynens <mathias@chromium.org> Cr-Commit-Position: refs/heads/master@{#50212} [modify] https://crrev.com/004f348aba415381cf5b66801e0e0f26c41baae6/test/fuzzer/fuzzer-support.cc
,
Dec 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/3ffbef33bcedf4fdc31d02df3290454b2d5f5812 commit 3ffbef33bcedf4fdc31d02df3290454b2d5f5812 Author: Clemens Hammacher <clemensh@chromium.org> Date: Tue Dec 19 18:57:01 2017 Revert "[fuzzer] Add attributes to LLVMFuzzerInitialize definition." This reverts commit 004f348aba415381cf5b66801e0e0f26c41baae6. Reason for revert: Breaks msvc compile: https://build.chromium.org/p/client.v8/builders/V8%20Win64%20-%20msvc/builds/672 Original change's description: > [fuzzer] Add attributes to LLVMFuzzerInitialize definition. > > That prevents the linker from dead-stripping the function, as it is not called > directly, it is resolved in the runtime via dlsym(). > > Bug: chromium:754124 , chromium:787723 > Change-Id: I46a02ef01349f59b7ed944ce1483b7277e234a19 > Reviewed-on: https://chromium-review.googlesource.com/833995 > Commit-Queue: Max Moroz <mmoroz@chromium.org> > Reviewed-by: Andreas Haas <ahaas@chromium.org> > Reviewed-by: Mathias Bynens <mathias@chromium.org> > Cr-Commit-Position: refs/heads/master@{#50212} TBR=ahaas@chromium.org,mmoroz@chromium.org,mathias@chromium.org Change-Id: Iba35b55ee4d11aca0dfb9cffde7a6a51e0c8e46c No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: chromium:754124 , chromium:787723 Reviewed-on: https://chromium-review.googlesource.com/834548 Reviewed-by: Clemens Hammacher <clemensh@chromium.org> Commit-Queue: Clemens Hammacher <clemensh@chromium.org> Cr-Commit-Position: refs/heads/master@{#50213} [modify] https://crrev.com/3ffbef33bcedf4fdc31d02df3290454b2d5f5812/test/fuzzer/fuzzer-support.cc
,
Dec 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/f9eb31bb8eae3218ce5e17f58dfd920b2648790e commit f9eb31bb8eae3218ce5e17f58dfd920b2648790e Author: Max Moroz <mmoroz@chromium.org> Date: Tue Dec 19 19:44:20 2017 [fuzzer] Declare LLVMFuzzerInitialize with attributes only if V8_OS_MACOSX. R=ahaas@chromium.org, clemensh@chromium.org, mathias@chromium.org Bug: chromium:754124 , chromium:787723 Change-Id: I7eafee50a47ca0ad56a5458f1f232e3ed07c1cca Reviewed-on: https://chromium-review.googlesource.com/834457 Reviewed-by: Clemens Hammacher <clemensh@chromium.org> Commit-Queue: Max Moroz <mmoroz@chromium.org> Cr-Commit-Position: refs/heads/master@{#50217} [modify] https://crrev.com/f9eb31bb8eae3218ce5e17f58dfd920b2648790e/test/fuzzer/fuzzer-support.cc
,
Dec 22 2017
,
Jan 4 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7e2d52131d743540c0c361845b6979f7d20146b1 commit 7e2d52131d743540c0c361845b6979f7d20146b1 Author: Max Moroz <mmoroz@chromium.org> Date: Thu Jan 04 22:00:52 2018 Fix v8_serialized_script_value_fuzzer on macOS. R=jbroman@chromium.org Bug: 754124 Change-Id: Ia7dd043982d404afa316be81fb030c9caed9e7c0 Reviewed-on: https://chromium-review.googlesource.com/849476 Reviewed-by: Jeremy Roman <jbroman@chromium.org> Commit-Queue: Max Moroz <mmoroz@chromium.org> Cr-Commit-Position: refs/heads/master@{#527110} [modify] https://crrev.com/7e2d52131d743540c0c361845b6979f7d20146b1/third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp
,
Jan 7 2018
ClusterFuzz has detected this issue as fixed in range 526949:527421. Detailed report: https://clusterfuzz.com/testcase?key=5791621626200064 Fuzzer: libFuzzer_v8_serialized_script_value_fuzzer Job Type: mac_libfuzzer_chrome_asan Platform Id: mac Crash Type: Null-dereference READ Crash Address: 0x000000000010 Crash State: blink::PersistentBase<blink::LocalFrame, blink::DummyPageHolder::GetFrame blink::LLVMFuzzerTestOneInput Sanitizer: address (ASAN) Regressed: https://clusterfuzz.com/revisions?job=mac_libfuzzer_chrome_asan&range=493100:493156 Fixed: https://clusterfuzz.com/revisions?job=mac_libfuzzer_chrome_asan&range=526949:527421 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5791621626200064 See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md for more information. If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
,
Jan 7 2018
ClusterFuzz testcase 5791621626200064 is verified as fixed, so closing issue as verified. If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue. |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by msrchandra@chromium.org
, Aug 10 2017Labels: M-62 Test-Predator-Wrong
Owner: reillyg@chromium.org
Status: Assigned (was: Untriaged)