On Wed, 4 Jan 2023 17:05:54 GMT, Thomas Stuefe <stu...@openjdk.org> wrote:
> Does Asan assume 8-byte-alignment? I'm hesitant to set this alignment in > stone since Metaspace could be allocated, at least now, with smaller > alignments. We don't do this now, but I'd like to keep the option open. But > see my proposal below, if you follow that, alignment should be no problem. IIRC ASan doesn't require 8 byte alignment, but it helps with accuracy, as ASan will need to round to the correct alignment. So it may end up over unpoisoning and under poisoning. > > How is 32-bit handled? ASan works for both 32-bit and 64-bit. https://github.com/google/sanitizers/wiki/AddressSanitizerAlgorithm > Can you please explain the poisoning and unpoisening a bit? How is the > use-after-free instrumentation done (I assume build time instrumentation?) > and how fast is it, e.g. how fine granular can you go - can you have millions > of micro-ranges, is that still feasible and reasonable? Should we poison > before munmap? https://github.com/google/sanitizers/wiki/AddressSanitizerAlgorithm `use-after-free` is implemented at runtime and is enabled by default for ASan. Detecting both `use-after-free` and `use-after-poison` is done via instrumentation at compile time. The cost to detect either should be the same. Poisoning/unpoisoning is implemented very similarly. Poisoning/unpoisoning is a simple calculation to map the address range to the shadow range and then a simple `memset` of the shadow range, effectively. > Metaspace-wise, I think tracking _blocks_ is not the best approach. > Especially _dealloced_ blocks, since there, you need to keep the header > unpoisened, but the header is where most overwriters would happen. And block > deallocation is a special rare case anyway. > > Metaspace allocation works like this: > > * VSN allocates memory via mmap and hands out _chunks_ to arenas. Chunks are > 1k ... 4M in size. > * An arena then owns a series of chunks and hands out parts of these chunks > as individual _blocks_ to the user. Blocks are usually 12 bytes.. ~1K, with a > heavy emphasis on tiny sizes. > * Arena is owned by a class loader. Typically individual blocks are not > deallocated. Instead, if the arena is deleted, it releases all _chunks_ to a > pool (ChunkManager). Optionally these chunks are also temporarily > uncommitted. These chunks can be reused by Arenas later, being committed > again had they been uncommitted before. > * Block deallocation is a rare case that is only used if Blocks are released > before the arena is released. This happens e.g., if a class is redefined and > its old bytecode is let go, but class and classloader and hence arena live on. > > I think Chunks are a better point for tracking: > > * way less ranges to track since granularity is coarser. Less runtime costs? > * Alignment is no problem since chunk start addresses are aligned to chunk > size (it is a buddy allocator), so at least 1k aligned. > * Chunks, in contrast to blocks, are maintained in a way that does not put > metadata into them. Metadata is kept separate. So you can poison freed chunks > at your heart's content with no header to tiptoe around. > * You lose a bit of accuracy, but honestly, not that much. The difference > between "memory in classloader-owned chunk" and "memory handed out to user" > is typically small. > > So I think it would be fine to unpoisen a whole chunk when given to an arena > instead of unpoisening every individual allocation from the arena. And poison > chunks when the Arena dies and puts the chunks back into the pool. So, if you > instrument `ChunkManager::get_chunk()` (poisen) and > `ChunkManager::return_chunk()` (unpoisen) in addition to your existing > pre-poisoning in VSN, it would be sufficient. You could remove the code in > binlist and blocktree, then. > > Cheers, Thomas Hm. Makes sense and I like this suggestion! Let me investigate implementing your suggestion to switch to chunks instead of blocks. ------------- PR: https://git.openjdk.org/jdk/pull/11702