RFR: 8358597: [asan] Buffer overflow in ArchiveBuilder::make_shallow_copy with Symbols

[Johan Sjölen]

On Fri, 26 Sep 2025 02:32:12 GMT, Ioi Lam <iklam at openjdk.org> wrote:

> The bug: when Symbols are copied into the dynamic CDS archive, extra padding bytes may be copied, which triggers "buffer overflow" errors in asan.
> 
> The fix is to copy the exact number of bytes for Symbols.
> 
> Since `ArchiveBuilder::make_shallow_copy()` deals with different alignments and sizes, I renamed the variables and added comments/asserts to make the code more readable.

Marked as reviewed by jsjolen (Reviewer).

Hi Ioi,

This change looks like it solves the bug. I was thinking about this, it seems that we're already doing some special-casing when looking at `MSO::ClassType`. Perhaps it would be good to have all special-cases at the top and have them 'hug' together? Below is a variant of your fix that does that. AFAIU it's "well-known" that AOTCache zeroes out all memory allocated, so we don't need to mention that in the comment.

```c++
  address src = src_info->source_addr();
  int bytes_needed = src_info->size_in_bytes();
  char* dest;
  char* oldtop;
  char* newtop;

  oldtop = dump_region->top();

  if (src_info->msotype() == MetaspaceObj::ClassType) {
    // Allocate space for a pointer directly in front of the future InstanceKlass, so
    // we can do a quick lookup from InstanceKlass* -> RunTimeClassInfo*
    // without building another hashtable. See RunTimeClassInfo::get_for()
    // in systemDictionaryShared.cpp.
    Klass* klass = (Klass*)src;
    if (klass->is_instance_klass()) {
      SystemDictionaryShared::validate_before_archiving(InstanceKlass::cast(klass));
      bytes_needed += sizeof(address);
    }
    // Allocate space for the future InstanceKlass with proper alignment
    const size_t alignment =
#ifdef _LP64
      UseCompressedClassPointers ?
        nth_bit(ArchiveBuilder::precomputed_narrow_klass_shift()) :
        SharedSpaceObjectAlignment;
#else
      SharedSpaceObjectAlignment;
#endif
  } else if (src_info->msotype() == MetaspaceObj::SymbolType) {
    // Symbols may be allocated by using AllocateHeap, so their sizes
    // may be less than size_in_bytes() indicates.
    bytes_needed = ((Symbol*)src)->byte_size();
  } else {
    // All other cases need no special-handling.
  }
  dest = dump_region->allocate(bytes_needed);
  newtop = dump_region->top();

  memcpy(dest, src, bytes_needed);

-------------

PR Review: https://git.openjdk.org/jdk/pull/27508#pullrequestreview-3271981634
PR Comment: https://git.openjdk.org/jdk/pull/27508#issuecomment-3338391828