Skip to content

BSTRByReference constructor loses reference to allocated memory #1292

@dbwiddis

Description

@dbwiddis
  1. Version of JNA and related jars: 5.6.0
  2. Version and vendor of the java virtual machine: Reproduced on Oracle JDK 1.8.0_221, OpenJDK 11 and OpenJDK 15.0.1
  3. Operating system: Reproduced on Windows 7, macOS 11 and Linux (Ubuntu 18)
  4. System architecture: 32 bit Windows (Intel64), 64 bit macOS (Intel64), 64 bit Linux (ARM)
  5. Complete description of the problem:

The process of instantiating a BSTRByReference from an existing BSTR stores the peer value of the BSTR's pointer, but does not keep a reference to the actual BSTR used to create it:

public void setValue(BSTR value) {
    this.getPointer().setPointer(0, value.getPointer());
}

The pointed-to value is tied to the Memory backing the BSTR, and that native memory allocation can be lost when the BSTR is garbage collected.

  1. Steps to reproduce:
  • This test case fails randomly: testPlatformToStrings(com.sun.jna.platform.ByReferencePlatformToStringTest, line 88)
  • To reproduce the failure more reliably, add in some forced GC and time delay.
public static void main(String[] args) throws InterruptedException {
    BSTRByReference foo = new BSTRByReference(new BSTR("foo"));
    System.gc();
    System.gc();
    Thread.sleep(1000);
    System.out.println(foo.getValue().toString());
}

The toString can access unallocated memory, causing a crash, or memory allocated (and written to) by another JVM thread, producing undefined behavior. The above has resulted in each of the following:

  • OutOfMemoryError if the memory read for BSTR size prompts creating too large an array
  • NegativeArraySizeException if the memory read for BSTR size is negative
  • Invalid Memory Access Error if JNA catches the problem
  • Corruption of the text string, e.g., fo成
  • SIGSEGV (# C [libsystem_platform.dylib+0x10a9] _platform_memmove$VARIANT$Haswell+0x29)
  • JVM Crash

In addition, the getValue() method itself causes Invalid memory access if the pointer value is not populated, so should have some null checking involved.

BSTRByReference b = new BSTRByReference();
System.out.println(b.getValue());

Thoughts:

  • At a minimum, we should clearly document the expectation the user keeps track of the BSTR allocation in the BSTRByReference javadocs (and that it's normally done using the Windows API).
  • I'm not sure it's appropriate for users to be instantiating their own Memory-backed BSTR's in the first place. A BSTR should be created/allocated with SysAllocString().
  • If we do keep user-instantiatable BSTRs we should probably alter BSTRByReference to maintain a reference to the BSTR object used to create it.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions