use-after-free in C2
Tobias Hartmann
tobias.hartmann at oracle.com
Fri Feb 17 09:08:56 UTC 2023
Good catch, Justin! Thanks for investigating and reporting. Let's discuss in the bug comments.
Best regards,
Tobias
On 16.02.23 16:34, Justin King wrote:
> Created JDK-8302670 <https://bugs.openjdk.org/browse/JDK-8302670> for record keeping and
> facilitating discussion.
>
> On Thu, Feb 16, 2023 at 7:27 AM Justin King <jcking at google.com <mailto:jcking at google.com>> wrote:
>
> Yeah, I think that is it. Anybody have any idea what the intention is supposed to be with
> PhaseIterGVN and it's copying by value of Unique_Node_List and Node_Stack? Both of its
> constructors do this, one happens to copy Compile::for_igvn() and ultimately modify it. Is it
> supposed to be taking ownership, are any changes by PhaseIterGVN to either supposed to be
> visible to the caller, or is it meant to be like copy-on-write where changes are not visible to
> the caller?
>
> On Thu, Feb 16, 2023 at 7:18 AM Justin King <jcking at google.com <mailto:jcking at google.com>> wrote:
>
> Okay, continuing to look for the remaining surprises. I think I found it, but I need to
> verify. It looks like PhaseIterGVN is copying Compile::for_igvn() by value and modifying it
> either intentionally or unintentionally, triggering Arealloc. This results in
> Compile::for_igvn() now pointing to free'd memory. I'll see if I can verify.
>
> On Wed, Feb 15, 2023 at 9:05 AM Justin King <jcking at google.com <mailto:jcking at google.com>>
> wrote:
>
> I sent https://github.com/openjdk/jdk/pull/12577
> <https://github.com/openjdk/jdk/pull/12577>
> and https://github.com/openjdk/jdk/pull/12578
> <https://github.com/openjdk/jdk/pull/12578> with their corresponding created bugs. I'll
> continue looking for the others once those are in.
>
> On Tue, Feb 14, 2023 at 4:02 PM Justin King <jcking at google.com
> <mailto:jcking at google.com>> wrote:
>
> Looks like there is definitely a lingering use-after-free like bug, maybe multiple.
> One is related to the copying around of Unique_Node_List and the copy then modifying
> the list, causing a Arealloc, with the original (which is then used again) seeing
> the free'd block. I'll see about pinpointing the issues at a later date.
>
> On Tue, Feb 14, 2023 at 9:27 AM Vladimir Kozlov <vladimir.kozlov at oracle.com
> <mailto:vladimir.kozlov at oracle.com>> wrote:
>
> Hi Justin,
>
> Please file a bug for this issue. We need to keep records.
>
> Thanks,
> Vladimir K
>
> On 2/10/23 12:42 PM, Justin King wrote:
> > Hm. That size is still too big, I don't think the node is 224 bytes. Let me
> double check again.
> >
> > On Fri, Feb 10, 2023 at 12:06 PM Justin King <jcking at google.com
> <mailto:jcking at google.com> <mailto:jcking at google.com
> <mailto:jcking at google.com>>> wrote:
> >
> > Looks to be the temporary node created by clone_map is leaking. I think
> there is a missing call to undo some of the
> > work done by clone_map.
> >
> > ==3591618==ERROR: AddressSanitizer: use-after-poison on address
> 0x6110000e20b8 at pc 0x7f7d8cac1875 bp
> > 0x7f7d1028be20 sp 0x7f7d1028be18
> > READ of size 4 at 0x6110000e20b8 thread T13
> > #0 0x7f7d8cac1874 in
> Unique_Node_List::remove_useless_nodes(VectorSet&)
> src/hotspot/share/opto/node.cpp:2967
> > #1 0x7f7d8cc90b03 in
> PhaseRemoveUseless::PhaseRemoveUseless(PhaseGVN*, Unique_Node_List*,
> Phase::PhaseNumber)
> > src/hotspot/share/opto/phaseX.cpp:423
> > #2 0x7f7d8b03853b in Compile::Compile(ciEnv*, ciMethod*, int,
> Options, DirectiveSet*)
> > src/hotspot/share/opto/compile.cpp:797
> > #3 0x7f7d8ace8ece in C2Compiler::compile_method(ciEnv*, ciMethod*,
> int, bool, DirectiveSet*)
> > src/hotspot/share/opto/c2compiler.cpp:113
> > #4 0x7f7d8b0507f8 in
> CompileBroker::invoke_compiler_on_method(CompileTask*)
> > src/hotspot/share/compiler/compileBroker.cpp:2237
> > #5 0x7f7d8b053e57 in CompileBroker::compiler_thread_loop()
> src/hotspot/share/compiler/compileBroker.cpp:1916
> > #6 0x7f7d8bc1f3a8 in JavaThread::thread_main_inner()
> src/hotspot/share/runtime/javaThread.cpp:710
> > #7 0x7f7d8bc1f99f in JavaThread::thread_main_inner()
> src/hotspot/share/runtime/javaThread.cpp:689
> > #8 0x7f7d8bc1f99f in JavaThread::run()
> src/hotspot/share/runtime/javaThread.cpp:695
> > #9 0x7f7d8d535b35 in Thread::call_run()
> src/hotspot/share/runtime/thread.cpp:224
> > #10 0x7f7d8cb7de8f in thread_native_entry
> src/hotspot/os/linux/os_linux.cpp:737
> > #11 0x7f7d8faa7fd3 in start_thread nptl/pthread_create.c:442
> > #12 0x7f7d8fb2866b in clone3
> ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81
> >
> > 0x6110000e20b8 is located 56 bytes inside of 224-byte region
> [0x6110000e2080,0x6110000e2160)
> > allocated by thread T13 here:
> > #0 0x7f7d8fcb89cf in __interceptor_malloc
> ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
> > #1 0x7f7d8cb65b9a in os::malloc(unsigned long, MEMFLAGS,
> NativeCallStack const&)
> > src/hotspot/share/runtime/os.cpp:672
> > #2 0x7f7d8a68c47e in Chunk::operator new(unsigned long,
> AllocFailStrategy::AllocFailEnum, unsigned long)
> > src/hotspot/share/memory/arena.cpp:190
> > #3 0x7f7d8a68c47e in Arena::grow(unsigned long,
> AllocFailStrategy::AllocFailEnum)
> > src/hotspot/share/memory/arena.cpp:345
> > #4 0x7f7d8cac1c44 in Arena::internal_amalloc(unsigned long,
> AllocFailStrategy::AllocFailEnum)
> > src/hotspot/share/memory/arena.hpp:113
> > #5 0x7f7d8cac1c44 in Arena::AmallocWords(unsigned long,
> AllocFailStrategy::AllocFailEnum)
> > src/hotspot/share/memory/arena.hpp:140
> > #6 0x7f7d8cac1c44 in Node::clone() const
> src/hotspot/share/opto/node.cpp:495
> > #7 0x7f7d8b980c38 in GraphKit::clone_map()
> src/hotspot/share/opto/graphKit.cpp:727
> > #8 0x7f7d8c538b44 in
> LibraryCallKit::inline_unsafe_load_store(BasicType, LibraryCallKit::LoadStoreKind,
> > LibraryCallKit::AccessKind) src/hotspot/share/opto/library_call.cpp:2599
> > #9 0x7f7d8c5aad3e in LibraryCallKit::try_to_inline(int)
> src/hotspot/share/opto/library_call.cpp:416
> > #10 0x7f7d8c5ae34b in LibraryIntrinsic::generate(JVMState*)
> src/hotspot/share/opto/library_call.cpp:116
> > #11 0x7f7d8b4430e9 in Parse::do_call()
> src/hotspot/share/opto/doCall.cpp:662
> > #12 0x7f7d8cc4c1ef in Parse::do_one_bytecode()
> src/hotspot/share/opto/parse2.cpp:2704
> > #13 0x7f7d8cc1f3f6 in Parse::do_one_block()
> src/hotspot/share/opto/parse1.cpp:1554
> > #14 0x7f7d8cc207ce in Parse::do_all_blocks()
> src/hotspot/share/opto/parse1.cpp:706
> > #15 0x7f7d8cc2a214 in Parse::Parse(JVMState*, ciMethod*, float)
> src/hotspot/share/opto/parse1.cpp:613
> > #16 0x7f7d8acec235 in ParseGenerator::generate(JVMState*)
> src/hotspot/share/opto/callGenerator.cpp:99
> > #17 0x7f7d8b03738c in Compile::Compile(ciEnv*, ciMethod*, int,
> Options, DirectiveSet*)
> > src/hotspot/share/opto/compile.cpp:763
> > #18 0x7f7d8ace8ece in C2Compiler::compile_method(ciEnv*, ciMethod*,
> int, bool, DirectiveSet*)
> > src/hotspot/share/opto/c2compiler.cpp:113
> > #19 0x7f7d8b0507f8 in
> CompileBroker::invoke_compiler_on_method(CompileTask*)
> > src/hotspot/share/compiler/compileBroker.cpp:2237
> > #20 0x7f7d8b053e57 in CompileBroker::compiler_thread_loop()
> src/hotspot/share/compiler/compileBroker.cpp:1916
> > #21 0x7f7d8bc1f3a8 in JavaThread::thread_main_inner()
> src/hotspot/share/runtime/javaThread.cpp:710
> > #22 0x7f7d8bc1f99f in JavaThread::thread_main_inner()
> src/hotspot/share/runtime/javaThread.cpp:689
> > #23 0x7f7d8bc1f99f in JavaThread::run()
> src/hotspot/share/runtime/javaThread.cpp:695
> > #24 0x7f7d8d535b35 in Thread::call_run()
> src/hotspot/share/runtime/thread.cpp:224
> > #25 0x7f7d8cb7de8f in thread_native_entry
> src/hotspot/os/linux/os_linux.cpp:737
> > #26 0x7f7d8faa7fd3 in start_thread nptl/pthread_create.c:442
> >
> >
> >
> > On Thu, Feb 9, 2023 at 2:31 PM Justin King <jcking at google.com
> <mailto:jcking at google.com> <mailto:jcking at google.com
> <mailto:jcking at google.com>>> wrote:
> >
> > Hm. It unfortunately does not show where it was poisoned, as the Arena
> uses large chunks with multiple separate
> > allocations per chunk. ASan only keeps track of malloc/free. But if I
> change the Arena implementation when
> > building under ASan to just use a right-sized chunk for each request,
> it should be able to show us. I'll try
> > that and bump this once I get something more definitive.
> >
> > On Thu, Feb 9, 2023 at 1:41 PM <dean.long at oracle.com
> <mailto:dean.long at oracle.com> <mailto:dean.long at oracle.com
> <mailto:dean.long at oracle.com>>> wrote:
> >
> > Can ASan show where the memory was freed? We've had crashes in
> the past (like JDK-8270028) that could be
> > related to memory corruption or how ResourceArea recycles memory.
> The allocation below seems to be using an
> > Arena without a ResourceArea, but if some other code used the same
> arena wrapped in a ResourceArea, then it
> > seems like that could lead to potential problems.
> >
> > dl
> >
> > On 2/9/23 8:59 AM, Justin King wrote:
> >> Hi,
> >>
> >> I was looking at instrumenting Arena again for ASan. The WIP patch is
> >> 047d4aa9a091cf5a84b9308454862e39666ca253
> >>
> <https://github.com/jcking/jdk/commit/047d4aa9a091cf5a84b9308454862e39666ca253
> <https://github.com/jcking/jdk/commit/047d4aa9a091cf5a84b9308454862e39666ca253>>. I ran back into the
> >> suspicious logic in C2
> <https://bugs.openjdk.org/browse/JDK-8298984
> <https://bugs.openjdk.org/browse/JDK-8298984>> where nodes are used after calling
> >> Arena::Afree. The first issue is present in Node::destruct, which
> I fixed by moving the call to
> >> Arena::Afree to the bottom of the function (addressed in patch).
> The second issue came up after in
> >> Compile::Compile, the stack trace is below. It looks like there
> are residual freed nodes being operated
> >> on? Maybe we are failing to unregister a temporary node from the
> node list? Maybe related to clone_map
> >> which returns SafePointNode?
> >>
> >> ==3146540==ERROR: AddressSanitizer: use-after-poison on address
> 0x62d00996b370 at pc 0x7f9f93048335 bp
> >> 0x7f9ed29fae20 sp 0x7f9ed29fae18
> >> READ of size 4 at 0x62d00996b370 thread T13
> >> #0 0x7f9f93048334 in
> Unique_Node_List::remove_useless_nodes(VectorSet&)
> >> src/hotspot/share/opto/node.cpp:2967
> >> #1 0x7f9f932124b3 in
> PhaseRemoveUseless::PhaseRemoveUseless(PhaseGVN*, Unique_Node_List*,
> >> Phase::PhaseNumber) src/hotspot/share/opto/phaseX.cpp:423
> >> #2 0x7f9f91621beb in Compile::Compile(ciEnv*, ciMethod*, int,
> Options, DirectiveSet*)
> >> src/hotspot/share/opto/compile.cpp:797
> >> #3 0x7f9f912e37fe in C2Compiler::compile_method(ciEnv*,
> ciMethod*, int, bool, DirectiveSet*)
> >> src/hotspot/share/opto/c2compiler.cpp:113
> >> #4 0x7f9f91638e07 in
> CompileBroker::invoke_compiler_on_method(CompileTask*)
> >> src/hotspot/share/compiler/compileBroker.cpp:2237
> >> #5 0x7f9f9163bfd7 in CompileBroker::compiler_thread_loop()
> >> src/hotspot/share/compiler/compileBroker.cpp:1916
> >> #6 0x7f9f921e3eec in JavaThread::thread_main_inner()
> src/hotspot/share/runtime/javaThread.cpp:710
> >> #7 0x7f9f921e434f in JavaThread::thread_main_inner()
> src/hotspot/share/runtime/javaThread.cpp:689
> >> #8 0x7f9f921e434f in JavaThread::run()
> src/hotspot/share/runtime/javaThread.cpp:695
> >> #9 0x7f9f93aa3f55 in Thread::call_run()
> src/hotspot/share/runtime/thread.cpp:224
> >> #10 0x7f9f9310144f in thread_native_entry
> src/hotspot/os/linux/os_linux.cpp:737
> >> #11 0x7f9f960a7fd3 in start_thread nptl/pthread_create.c:442
> >> #12 0x7f9f9612866b in clone3
> ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81
> >>
> >> 0x62d00996b370 is located 20336 bytes inside of 32744-byte region
> [0x62d009966400,0x62d00996e3e8)
> >> allocated by thread T13 here:
> >> #0 0x7f9f962b89cf in __interceptor_malloc
> ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
> >> #1 0x7f9f930e915a in os::malloc(unsigned long, MEMFLAGS,
> NativeCallStack const&)
> >> src/hotspot/share/runtime/os.cpp:672
> >> #2 0x7f9f90c8a08a in Chunk::operator new(unsigned long,
> AllocFailStrategy::AllocFailEnum, unsigned
> >> long) src/hotspot/share/memory/arena.cpp:190
> >> #3 0x7f9f90c8a08a in Arena::grow(unsigned long,
> AllocFailStrategy::AllocFailEnum)
> >> src/hotspot/share/memory/arena.cpp:325
> >> #4 0x7f9f932109f5 in Arena::internal_amalloc(unsigned long,
> AllocFailStrategy::AllocFailEnum)
> >> src/hotspot/share/memory/arena.hpp:113
> >> #5 0x7f9f932109f5 in Arena::Amalloc(unsigned long,
> AllocFailStrategy::AllocFailEnum)
> >> src/hotspot/share/memory/arena.hpp:133
> >> #6 0x7f9f932109f5 in NodeHash::NodeHash(Arena*, unsigned int)
> src/hotspot/share/opto/phaseX.cpp:68
> >> #7 0x7f9f932293c7 in PhaseValues::PhaseValues(Arena*,
> unsigned int) src/hotspot/share/opto/phaseX.cpp:697
> >> #8 0x7f9f9161f678 in PhaseGVN::PhaseGVN(Arena*, unsigned int)
> src/hotspot/share/opto/phaseX.hpp:415
> >> #9 0x7f9f9161f678 in Compile::Compile(ciEnv*, ciMethod*, int,
> Options, DirectiveSet*)
> >> src/hotspot/share/opto/compile.cpp:714
> >> #10 0x7f9f912e37fe in C2Compiler::compile_method(ciEnv*,
> ciMethod*, int, bool, DirectiveSet*)
> >> src/hotspot/share/opto/c2compiler.cpp:113
> >> #11 0x7f9f91638e07 in
> CompileBroker::invoke_compiler_on_method(CompileTask*)
> >> src/hotspot/share/compiler/compileBroker.cpp:2237
> >> #12 0x7f9f9163bfd7 in CompileBroker::compiler_thread_loop()
> >> src/hotspot/share/compiler/compileBroker.cpp:1916
> >> #13 0x7f9f921e3eec in JavaThread::thread_main_inner()
> src/hotspot/share/runtime/javaThread.cpp:710
> >> #14 0x7f9f921e434f in JavaThread::thread_main_inner()
> src/hotspot/share/runtime/javaThread.cpp:689
> >> #15 0x7f9f921e434f in JavaThread::run()
> src/hotspot/share/runtime/javaThread.cpp:695
> >> #16 0x7f9f93aa3f55 in Thread::call_run()
> src/hotspot/share/runtime/thread.cpp:224
> >> #17 0x7f9f9310144f in thread_native_entry
> src/hotspot/os/linux/os_linux.cpp:737
> >> #18 0x7f9f960a7fd3 in start_thread nptl/pthread_create.c:442
> >>
> >> Thread T13 created by T1 here:
> >> #0 0x7f9f96249726 in __interceptor_pthread_create
> >> ../../../../src/libsanitizer/asan/asan_interceptors.cpp:207
> >> #1 0x7f9f93102d88 in os::create_thread(Thread*,
> os::ThreadType, unsigned long)
> >> src/hotspot/os/linux/os_linux.cpp:888
> >> #2 0x7f9f91693d93 in
> CompilerThread::CompilerThread(CompileQueue*, CompilerCounters*)
> >> src/hotspot/share/compiler/compilerThread.cpp:34
> >> #3 0x7f9f91625c7c in
> CompileBroker::make_thread(CompileBroker::ThreadType, _jobject*, CompileQueue*,
> >> AbstractCompiler*, JavaThread*)
> src/hotspot/share/compiler/compileBroker.cpp:842
> >> #4 0x7f9f91628f71 in CompileBroker::init_compiler_threads()
> >> src/hotspot/share/compiler/compileBroker.cpp:943
> >> #5 0x7f9f9162a464 in
> CompileBroker::compilation_init_phase1(JavaThread*)
> >> src/hotspot/share/compiler/compileBroker.cpp:654
> >> #6 0x7f9f93adc3a4 in Threads::create_vm(JavaVMInitArgs*,
> bool*) src/hotspot/share/runtime/threads.cpp:701
> >> #7 0x7f9f92465b51 in JNI_CreateJavaVM_inner
> src/hotspot/share/prims/jni.cpp:3588
> >> #8 0x7f9f92465b51 in JNI_CreateJavaVM
> src/hotspot/share/prims/jni.cpp:3674
> >> #9 0x7f9f968d2e25 in InitializeJVM
> src/java.base/share/native/libjli/java.c:1459
> >> #10 0x7f9f968d2e25 in JavaMain
> src/java.base/share/native/libjli/java.c:413
> >> #11 0x7f9f968db708 in ThreadJavaMain
> src/java.base/unix/native/libjli/java_md.c:650
> >> #12 0x7f9f960a7fd3 in start_thread nptl/pthread_create.c:442
> >>
> >> Thread T1 created by T0 here:
> >> #0 0x7f9f96249726 in __interceptor_pthread_create
> >> ../../../../src/libsanitizer/asan/asan_interceptors.cpp:207
> >> #1 0x7f9f968dd3a1 in CallJavaMainInNewThread
> src/java.base/unix/native/libjli/java_md.c:691
> >> #2 0x7f9f968d822d in ContinueInNewThread
> src/java.base/share/native/libjli/java.c:2280
> >> #3 0x7f9f968d96ae in JLI_Launch
> src/java.base/share/native/libjli/java.c:340
> >> #4 0x5594a81c337c in main
> src/java.base/share/native/launcher/main.c:166
> >> #5 0x7f9f96046189 in __libc_start_call_main
> ../sysdeps/nptl/libc_start_call_main.h:58
> >>
> >> --
> >>
> >> Google Logo
> >> Justin King
> >> Software Engineer
> >> jcking at google.com <mailto:jcking at google.com>
> <mailto:jcking at google.com <mailto:jcking at google.com>>
> >>
> >>
> >>
> >>
> >
> >
> > --
> >
> > Google Logo
> > Justin King
> > Software Engineer
> > jcking at google.com <mailto:jcking at google.com> <mailto:jcking at google.com
> <mailto:jcking at google.com>>
> >
> >
> >
> >
> >
> > --
> >
> > Google Logo
> > Justin King
> > Software Engineer
> > jcking at google.com <mailto:jcking at google.com> <mailto:jcking at google.com
> <mailto:jcking at google.com>>
> >
> >
> >
> >
> >
> > --
> >
> > Google Logo
> > Justin King
> > Software Engineer
> > jcking at google.com <mailto:jcking at google.com> <mailto:jcking at google.com
> <mailto:jcking at google.com>>
> >
> >
> >
>
>
>
> --
>
> Google Logo
> Justin King
> Software Engineer
> jcking at google.com <mailto:jcking at google.com>
>
>
>
>
>
> --
>
> Google Logo
> Justin King
> Software Engineer
> jcking at google.com <mailto:jcking at google.com>
>
>
>
>
>
> --
>
> Google Logo
> Justin King
> Software Engineer
> jcking at google.com <mailto:jcking at google.com>
>
>
>
>
>
> --
>
> Google Logo
> Justin King
> Software Engineer
> jcking at google.com <mailto:jcking at google.com>
>
>
>
>
>
> --
>
> Google Logo
> Justin King
> Software Engineer
> jcking at google.com <mailto:jcking at google.com>
>
>
>
More information about the hotspot-compiler-dev
mailing list