ikvmnet / ikvm

A Java Virtual Machine and Bytecode-to-IL Converter for .NET
Other
1.22k stars 111 forks source link

`sun.misc.Unsafe` cannot write to unmanaged memory #346

Closed TheLastRar closed 1 year ago

TheLastRar commented 1 year ago

IKVM version : 8.5.1-image-netcoreapp3.1-win7-x64

Spotted when investigating lwjgl3

Given the following java code

public class unsafememory {

    public static sun.misc.Unsafe getUnsafe() throws Exception {
        try {
            return sun.misc.Unsafe.getUnsafe();
        }
        catch (SecurityException tryReflectionInstead) {
            Class<sun.misc.Unsafe> k = sun.misc.Unsafe.class;
            for (java.lang.reflect.Field f : k.getDeclaredFields()) {
                f.setAccessible(true);
                Object x = f.get(null);
                if (k.isInstance(x)) {
                    return k.cast(x);
                }
            }
            throw new NoSuchFieldError("the Unsafe");
        }
    }

    public static void main(String[] args) throws Exception {
        sun.misc.Unsafe unsafe = getUnsafe();

        //Read/Write Memory
        long memAddr = unsafe.allocateMemory(8);
        System.out.println(String.format("Got Memory @ 0x%016X", memAddr)); 

        int to_unmanaged = 1234;
        System.out.println(String.format("Put int %d", to_unmanaged)); 

        unsafe.putInt(null, memAddr, to_unmanaged);

        int from_unmanaged = unsafe.getInt(null, memAddr);
        System.out.println(String.format("Got int %d", from_unmanaged)); 

        unsafe.freeMemory(memAddr);
    }
}

JVM produces this

Got Memory @ 0x0000022AFD1DDB20
Put int 1234
Got int 1234

IKVM produces this

Got Memory @ 0x000001A64CE575B0
Put int 1234
Exception in thread "Thread-0" java.lang.InternalError: cli.System.NullReferenceException
        at IKVM.Java.Externs.sun.misc.Unsafe.PutField(Unsafe.cs:176)
        at IKVM.Java.Externs.sun.misc.Unsafe.putInt(Unsafe.cs:447)
        at sun.misc.Unsafe.putInt(Native Method)
        at lwgl2test.unsafememory.main(unsafememory.java:34)
        at java.lang.reflect.Method.invoke(Method.java:486)
        at IKVM.Runtime.Accessors.Java.Lang.Reflect.MethodAccessor.InvokeInvoke(MethodAccessor.cs:35)
Caused by: cli.System.NullReferenceException
wasabii commented 1 year ago

Geeze another? Ok.

TheLastRar commented 1 year ago

Apologies for the amount of issues I've been posting

wasabii commented 1 year ago

Naw. It's okay. This stuff needs to get worked out.

I'm super annoyed about Java sosftware using Unsafe, really.

wasabii commented 1 year ago

Hmm. Is this one supposed to actually work? There are two versions of putInt. One that accepts an address, and the other that accepts an offset.

wasabii commented 1 year ago

Yeah I found it. null behaves as if putInt(address, val) was called.

wasabii commented 1 year ago

This can't be right. Need to think more.

wasabii commented 1 year ago

Yeah. Okay. @TheLastRar This isn't supposed to work. The version of putInt/etc, that accepts an object as the first parameter, should only be used with a value obtained from objectFieldOffset or staticFieldOffset: it functions only on fields. The version of putInt with only two arguments is supposed to be used for absolute non heap addresses.

This is code you came up with. Was it a test case of something else?

wasabii commented 1 year ago

From the Unsafe.java file:


     * More specifically, fetches a field or array element within the given
     * object <code>o</code> at the given offset, or (if <code>o</code> is
     * null) from the memory address whose numerical value is the given
     * offset.
     * <p>
     * The results are undefined unless one of the following cases is true:
     * <ul>
     * <li>The offset was obtained from {@link #objectFieldOffset} on
     * the {@link java.lang.reflect.Field} of some Java field and the object
     * referred to by <code>o</code> is of a class compatible with that
     * field's class.
     *
     * <li>The offset and object reference <code>o</code> (either null or
     * non-null) were both obtained via {@link #staticFieldOffset}
     * and {@link #staticFieldBase} (respectively) from the
     * reflective {@link Field} representation of some Java field.
     *
     * <li>The object referred to by <code>o</code> is an array, and the offset
     * is an integer of the form <code>B+N*S</code>, where <code>N</code> is
     * a valid index into the array, and <code>B</code> and <code>S</code> are
     * the values obtained by {@link #arrayBaseOffset} and {@link
     * #arrayIndexScale} (respectively) from the array's class.  The value
     * referred to is the <code>N</code><em>th</em> element of the array.
TheLastRar commented 1 year ago

The test case was something I built to try and replicate code I found in LWJGL3's MemoryUtil class

Specificity these lines; https://github.com/LWJGL/lwjgl3/blob/3.3.1/modules/lwjgl/core/src/main/java/org/lwjgl/system/MemoryUtil.java#L1962 https://github.com/LWJGL/lwjgl3/blob/3.3.1/modules/lwjgl/core/src/main/java/org/lwjgl/system/MemoryUtil.java#L1909

The lwjgl3 library is a rendering library used by Minecraft 1.13+, although the exact version differs from the one I've linked

wasabii commented 1 year ago

I think this is an accident of OpenJDK then. Works there. But they document that operation as undefined.

I don't think I can fix this one without getting into CLR internals.

TheLastRar commented 1 year ago

What specificity requires getting into CLR internals and isn't solved by https://github.com/ikvmnet/ikvm/commit/8db17b22a5ae58ba94483185da640c38f643d274?

wasabii commented 1 year ago

null being passed to putInt, or getInt, means we're trying to set the value of a static field. The offset passed must be an offset retrieved from staticFieldOffset. IKVM uses GCHandles that are meaningless (cookies), but point to the FieldWrapper.

There is no way to distinguish that offset passed is referring to an absolute memory position, vs a static field cookie. LWJGL is attempting to pass an absolute memory position.

A specific method exists for usage like this: putInt(long offset, int value). This method only has two arguments, and is documented as setting the memory at that location.

This works for OpenJDK because the offset returned for staticFieldOffset just so happens to be the absolute address of that static field. In IKVM we're using cookies.

To emulate the way OpenJDK is doing it, we'd need to return an absolute position to the static field from staticFieldOffset. It's possible we could just do this, however, its really relying on the nuances of the runtime at that point, which I was trying to avoid.

LWJGL is using this method incorrectly. They should be using the version of the method with two arguments instead of three. That should be opened with them as a bug.

I'd have to look into the details of the various supported runtimes to figure out if we can do it the same way OpenJDK is doing it. For Framework, Core and Mono. That is, can we get the absolute position of the static field. And, are there any runtime limitations that prevent that from working (maybe the GC compacts EEClass, no idea).


From: TheLastRar @.> Sent: Saturday, June 3, 2023 10:19 AM To: ikvmnet/ikvm @.> Cc: Jerome Haltom @.>; State change @.> Subject: Re: [ikvmnet/ikvm] sun.misc.Unsafe cannot write to unmanaged memory (Issue #346)

What specificity requires getting into CLR internals and isn't solved by 8db17b2https://github.com/ikvmnet/ikvm/commit/8db17b22a5ae58ba94483185da640c38f643d274?

— Reply to this email directly, view it on GitHubhttps://github.com/ikvmnet/ikvm/issues/346#issuecomment-1575016654, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAEGGSWN3GELPSBQ6R6KDX3XJNIYVANCNFSM6AAAAAAYYRMXRM. You are receiving this because you modified the open/close state.Message ID: @.***>

TheLastRar commented 1 year ago

null being passed to putInt, or getInt, means we're trying to set the value of a static field. The offset passed must be an offset retrieved from staticFieldOffset.

I was under the impression that to use staticFieldOffset, you needed to use it with the object returned from staticFieldBase, that is;

Object field_base = unsafe.staticFieldBase(field);
long field_offset = unsafe.staticFieldOffset(field);

int ret = unsafe.getInt(field_base, field_offset);

Using unsafe.getInt(null, field_offset) instead gave me an access violation on JVM, is there code or a test case where that is expected to work?

wasabii commented 1 year ago

Maybe you're right and I misread those conditions. Staring at it longer.


From: TheLastRar @.> Sent: Saturday, June 3, 2023 11:01 AM To: ikvmnet/ikvm @.> Cc: Jerome Haltom @.>; State change @.> Subject: Re: [ikvmnet/ikvm] sun.misc.Unsafe cannot write to unmanaged memory (Issue #346)

null being passed to putInt, or getInt, means we're trying to set the value of a static field. The offset passed must be an offset retrieved from staticFieldOffset.

I was under the impression that to use staticFieldOffset, you needed to use it with the object returned from staticFieldBase, that is;

Object field_base = unsafe.staticFieldBase(field); long field_offset = unsafe.staticFieldOffset(field);

int ret = unsafe.getInt(field_base, field_offset);

Using unsafe.getInt(null, field_offset) instead gave me an access violation on JVM, is there code or a test case where that is expected to work?

— Reply to this email directly, view it on GitHubhttps://github.com/ikvmnet/ikvm/issues/346#issuecomment-1575039962, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAEGGSWMZVGCMVQQ3T4LHW3XJNNUXANCNFSM6AAAAAAYYRMXRM. You are receiving this because you modified the open/close state.Message ID: @.***>

TheLastRar commented 1 year ago

Looking at Unsafe.java myself, the description continues further from what was included in your comment https://github.com/ikvmnet/ikvm/issues/346#issuecomment-1574475969

     *
     * </ul>
     * <p>
     * If one of the above cases is true, the call references a specific Java
     * variable (field or array element).  However, the results are undefined
     * if that variable is not in fact of the type returned by this method.
     * <p>
     * This method refers to a variable by means of two parameters, and so
     * it provides (in effect) a <em>double-register</em> addressing mode
     * for Java variables.  When the object reference is null, this method
     * uses its offset as an absolute address.  This is similar in operation
     * to methods such as {@link #getInt(long)}, which provide (in effect) a
     * <em>single-register</em> addressing mode for non-Java variables.
     * However, because Java variables may have a different layout in memory
     * from non-Java variables, programmers should not assume that these
     * two addressing modes are ever equivalent.  Also, programmers should
     * remember that offsets from the double-register addressing mode cannot
     * be portably confused with longs used in the single-register addressing
     * mode.

Which defines the behaviour where the object reference is null (about half way down my snipplet)

wasabii commented 1 year ago

You were right. Got it. Tests fixed too.