Skip to content

Commit

Permalink
[GR-40769] Check for native access when creating a Pointer
Browse files Browse the repository at this point in the history
PullRequest: truffleruby/3474
  • Loading branch information
eregon committed Sep 27, 2022
2 parents 2607892 + f94339e commit 788cb4c
Show file tree
Hide file tree
Showing 19 changed files with 187 additions and 110 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ Changes:
* Refactored sharing of array objects between threads using new `SharedArrayStorage` (@aardvark179).
* Marking of native structures wrapped in objects is now done on C call exit to reduce memory overhead (@aardvark179).

Security:

* The native access permission is now properly checked before any native pointer (e.g. `Truffle::FFI::Pointer`) is created (@eregon).

# 22.2.0

New features:
Expand Down
36 changes: 19 additions & 17 deletions src/main/java/org/truffleruby/cext/CExtNodes.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.jcodings.Encoding;
import org.jcodings.IntHolder;
import org.truffleruby.Layouts;
import org.truffleruby.RubyContext;
import org.truffleruby.RubyLanguage;
import org.truffleruby.builtins.CoreMethod;
import org.truffleruby.builtins.CoreMethodArrayArgumentsNode;
Expand Down Expand Up @@ -149,9 +150,9 @@ public class CExtNodes {
public static final int RUBY_TAG_THROW = 0x7;
public static final int RUBY_TAG_FATAL = 0x8;

public static Pointer newNativeStringPointer(int capacity, RubyLanguage language) {
public static Pointer newNativeStringPointer(RubyLanguage language, RubyContext context, int capacity) {
// We need up to 4 \0 bytes for UTF-32. Always use 4 for speed rather than checking the encoding min length.
Pointer pointer = Pointer.mallocAutoRelease(capacity + 4, language);
Pointer pointer = Pointer.mallocAutoRelease(language, context, capacity + 4);
pointer.writeInt(capacity, 0);
return pointer;
}
Expand Down Expand Up @@ -765,7 +766,7 @@ public abstract static class RbStrNewNulNode extends CoreMethodArrayArgumentsNod
@Specialization
protected RubyString rbStrNewNul(int byteLength,
@Cached MutableTruffleString.FromNativePointerNode fromNativePointerNode) {
final Pointer pointer = Pointer.callocAutoRelease(byteLength + 1, getLanguage());
final Pointer pointer = Pointer.callocAutoRelease(getLanguage(), getContext(), byteLength + 1);
var nativeTString = fromNativePointerNode.execute(pointer, 0, byteLength, Encodings.BINARY.tencoding,
false);
return createMutableString(nativeTString, Encodings.BINARY);
Expand Down Expand Up @@ -842,8 +843,8 @@ protected RubyString rbStrResize(RubyString string, int newByteLength,
string.clearCodeRange();
return string;
} else {
var newNativeTString = TrStrCapaResizeNode.resize(pointer, newByteLength, newByteLength, tencoding,
fromNativePointerNode, getLanguage());
var newNativeTString = TrStrCapaResizeNode.resize(getLanguage(), getContext(), pointer, newByteLength,
newByteLength, tencoding, fromNativePointerNode);
string.setTString(newNativeTString);

// Like MRI's rb_str_resize()
Expand All @@ -869,18 +870,18 @@ protected RubyString trStrCapaResize(RubyString string, int newCapacity,
return string;
} else {
int byteLength = string.tstring.byteLength(tencoding);
var newNativeTString = resize(pointer, newCapacity, byteLength, tencoding, fromNativePointerNode,
getLanguage());
var newNativeTString = resize(getLanguage(), getContext(), pointer, newCapacity, byteLength, tencoding,
fromNativePointerNode);
string.setTString(newNativeTString);

return string;
}
}

static MutableTruffleString resize(Pointer pointer, int newCapacity, int newByteLength,
TruffleString.Encoding tencoding, MutableTruffleString.FromNativePointerNode fromNativePointerNode,
RubyLanguage language) {
final Pointer newPointer = newNativeStringPointer(newCapacity, language);
static MutableTruffleString resize(RubyLanguage language, RubyContext context, Pointer pointer, int newCapacity,
int newByteLength, TruffleString.Encoding tencoding,
MutableTruffleString.FromNativePointerNode fromNativePointerNode) {
final Pointer newPointer = newNativeStringPointer(language, context, newCapacity);
newPointer.writeBytes(0, pointer, 0, Math.min(pointer.getSize(), newCapacity));

return fromNativePointerNode.execute(newPointer, 0, newByteLength, tencoding, false);
Expand Down Expand Up @@ -1285,8 +1286,8 @@ protected Pointer toNative(RubyString string,
pointer = (Pointer) getInternalNativePointerNode.execute(tstring, tencoding);
} else {
int byteLength = tstring.byteLength(tencoding);
pointer = allocateAndCopyToNative(tstring, tencoding, byteLength, copyToNativeMemoryNode,
getLanguage());
pointer = allocateAndCopyToNative(getLanguage(), getContext(), tstring, tencoding, byteLength,
copyToNativeMemoryNode);

var nativeTString = fromNativePointerNode.execute(pointer, 0, byteLength, tencoding, false);
string.setTString(nativeTString);
Expand All @@ -1300,9 +1301,10 @@ protected Pointer toNativeImmutable(ImmutableRubyString string) {
return string.getNativeString(getLanguage());
}

public static Pointer allocateAndCopyToNative(AbstractTruffleString tstring, TruffleString.Encoding tencoding,
int capacity, TruffleString.CopyToNativeMemoryNode copyToNativeMemoryNode, RubyLanguage language) {
final Pointer pointer = newNativeStringPointer(capacity, language);
public static Pointer allocateAndCopyToNative(RubyLanguage language, RubyContext context,
AbstractTruffleString tstring, TruffleString.Encoding tencoding, int capacity,
TruffleString.CopyToNativeMemoryNode copyToNativeMemoryNode) {
final Pointer pointer = newNativeStringPointer(language, context, capacity);
copyToNativeMemoryNode.execute(tstring, 0, pointer, 0, capacity, tencoding);
return pointer;
}
Expand Down Expand Up @@ -2026,7 +2028,7 @@ public abstract static class DataHolderCreate extends PrimitiveArrayArgumentsNod

@Specialization
protected DataHolder create(Object address) {
return new DataHolder(address, Pointer.NULL);
return new DataHolder(address, Pointer.getNullPointer(getContext()));
}
}

Expand Down
8 changes: 4 additions & 4 deletions src/main/java/org/truffleruby/cext/DataHolder.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,17 @@ public final class DataHolder implements TruffleObject {
private Object pointer;
private Object marker;

public DataHolder(Object address, Object marker) {
this.pointer = address;
public DataHolder(Object pointer, Object marker) {
this.pointer = pointer;
this.marker = marker;
}

public Object getPointer() {
return pointer;
}

public void setPointer(Object address) {
this.pointer = address;
public void setPointer(Object pointer) {
this.pointer = pointer;
}

public Object getMarker() {
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/org/truffleruby/core/VMPrimitiveNodes.java
Original file line number Diff line number Diff line change
Expand Up @@ -643,9 +643,9 @@ protected long argvLength() {
}

int argc = getContext().nativeArgc;
Pointer argv = new Pointer(getContext().nativeArgv, argc * Pointer.SIZE);
Pointer first = argv.readPointer(0);
Pointer last = argv.readPointer((argc - 1) * Pointer.SIZE);
Pointer argv = new Pointer(getContext(), getContext().nativeArgv, argc * Pointer.SIZE);
Pointer first = argv.readPointer(getContext(), 0);
Pointer last = argv.readPointer(getContext(), (argc - 1) * Pointer.SIZE);
long lastByte = last.getAddress() + last.findNullByte(getContext(), InteropLibrary.getUncached(), 0);
nativeArgvLength = lastByte - first.getAddress();

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/truffleruby/core/array/ArrayNodes.java
Original file line number Diff line number Diff line change
Expand Up @@ -2325,7 +2325,7 @@ protected RubyArray storeToNative(RubyArray array,
@Cached IsSharedNode isSharedNode,
@Cached ConditionProfile sharedProfile) {
final int size = arraySizeProfile.profile(array.size);
Pointer pointer = Pointer.mallocAutoRelease(size * Pointer.SIZE, getLanguage());
Pointer pointer = Pointer.mallocAutoRelease(getLanguage(), getContext(), size * Pointer.SIZE);
Object newStore = new NativeArrayStorage(pointer, size);
stores.copyContents(store, 0, newStore, 0, size);
array.setStore(newStore);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
import com.oracle.truffle.api.profiles.ConditionProfile;
import com.oracle.truffle.api.profiles.LoopConditionProfile;
import org.truffleruby.RubyContext;
import org.truffleruby.cext.UnwrapNode;
import org.truffleruby.cext.UnwrapNodeGen.UnwrapNativeNodeGen;
import org.truffleruby.cext.ValueWrapper;
Expand Down Expand Up @@ -122,9 +123,10 @@ protected int capacity() {
}

@ExportMessage
protected NativeArrayStorage expand(int newCapacity) {
protected NativeArrayStorage expand(int newCapacity,
@CachedLibrary("this") ArrayStoreLibrary node) {
final int capacity = this.length;
Pointer newPointer = Pointer.malloc(capacity);
Pointer newPointer = Pointer.malloc(RubyContext.get(node), capacity);
newPointer.writeBytes(0, pointer, 0, capacity);
newPointer.writeBytes(capacity, newCapacity - capacity, (byte) 0);
/* We copy the contents of the marked objects to ensure the references will be kept alive even if the old store
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ private void initializeLocaleEncoding(TruffleNFIPlatform nfi, NativeConfiguratio
} catch (InteropException e) {
throw CompilerDirectives.shouldNotReachHere(e);
}
final byte[] bytes = new Pointer(address).readZeroTerminatedByteArray(
final byte[] bytes = new Pointer(context, address).readZeroTerminatedByteArray(
context,
InteropLibrary.getUncached(),
0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ protected MissingValue decode(Nil nil) {
protected RubyString read(VirtualFrame frame, long address,
@CachedLibrary(limit = "getRubyLibraryCacheLimit()") RubyLibrary rubyLibrary,
@CachedLibrary(limit = "1") InteropLibrary interop) {
final Pointer pointer = new Pointer(address);
final Pointer pointer = new Pointer(getContext(), address);
checkAssociated(
(Pointer[]) frame.getObject(FormatFrameDescriptor.SOURCE_ASSOCIATED_SLOT),
pointer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,9 @@ private synchronized Pointer createNativeString(RubyLanguage language) {
if (nativeString == null) {
var tencoding = getEncodingUncached().tencoding;
int byteLength = tstring.byteLength(tencoding);
nativeString = CExtNodes.StringToNativeNode.allocateAndCopyToNative(tstring, tencoding, byteLength,
TruffleString.CopyToNativeMemoryNode.getUncached(), language);
nativeString = CExtNodes.StringToNativeNode.allocateAndCopyToNative(language,
RubyLanguage.getCurrentContext(), tstring, tencoding, byteLength,
TruffleString.CopyToNativeMemoryNode.getUncached());
}
return nativeString;
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/truffleruby/core/string/StringNodes.java
Original file line number Diff line number Diff line change
Expand Up @@ -1589,7 +1589,7 @@ protected Object initializeCopyNative(RubyString self, RubyString from,
var tencoding = encoding.tencoding;
final Pointer fromPointer = (Pointer) getInternalNativePointerNode.execute(tstring, tencoding);

final Pointer newPointer = Pointer.mallocAutoRelease(fromPointer.getSize(), getLanguage());
final Pointer newPointer = Pointer.mallocAutoRelease(getLanguage(), getContext(), fromPointer.getSize());
newPointer.writeBytes(0, fromPointer, 0, fromPointer.getSize());

// TODO (eregon, 2022): should we have the copy be native too, or rather take the opportunity of having to copy to be managed?
Expand Down
10 changes: 6 additions & 4 deletions src/main/java/org/truffleruby/core/support/IONodes.java
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
import java.util.Arrays;

import com.oracle.truffle.api.strings.TruffleString;
import org.truffleruby.RubyContext;
import org.truffleruby.builtins.CoreMethod;
import org.truffleruby.builtins.CoreMethodArrayArgumentsNode;
import org.truffleruby.builtins.CoreModule;
Expand Down Expand Up @@ -515,13 +516,14 @@ protected RubyPointer getThreadBuffer(long size,
final RubyPointer instance = new RubyPointer(
coreLibrary().truffleFFIPointerClass,
getLanguage().truffleFFIPointerShape,
getBuffer(thread, size, sizeProfile));
getBuffer(getContext(), thread, size, sizeProfile));
AllocationTracing.trace(instance, this);
return instance;
}

public static Pointer getBuffer(RubyThread rubyThread, long size, ConditionProfile sizeProfile) {
return rubyThread.ioBuffer.allocate(rubyThread, size, sizeProfile);
public static Pointer getBuffer(RubyContext context, RubyThread rubyThread, long size,
ConditionProfile sizeProfile) {
return rubyThread.getIoBuffer(context).allocate(context, rubyThread, size, sizeProfile);
}
}

Expand All @@ -532,7 +534,7 @@ public abstract static class IOThreadBufferFreeNode extends PrimitiveArrayArgume
protected Object getThreadBuffer(RubyPointer pointer,
@Cached ConditionProfile freeProfile) {
RubyThread thread = getLanguage().getCurrentThread();
thread.ioBuffer.free(thread, pointer.pointer, freeProfile);
thread.getIoBuffer(getContext()).free(thread, pointer.pointer, freeProfile);
return nil;
}
}
Expand Down
8 changes: 7 additions & 1 deletion src/main/java/org/truffleruby/core/thread/RubyThread.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.truffleruby.core.support.PRNGRandomizerNodes;
import org.truffleruby.core.support.RubyPRNGRandomizer;
import org.truffleruby.core.tracepoint.TracePointState;
import org.truffleruby.extra.ffi.Pointer;
import org.truffleruby.language.Nil;
import org.truffleruby.language.RubyDynamicObject;
import org.truffleruby.language.objects.ObjectGraph;
Expand Down Expand Up @@ -62,7 +63,7 @@ public final class RubyThread extends RubyDynamicObject implements ObjectGraphNo
volatile Object value = null;
public final AtomicBoolean wakeUp = new AtomicBoolean(false);
volatile int priority = Thread.NORM_PRIORITY;
public ThreadLocalBuffer ioBuffer = ThreadLocalBuffer.NULL_BUFFER;
ThreadLocalBuffer ioBuffer;
Object threadGroup;
public String sourceLocation;
Object name = Nil.INSTANCE;
Expand Down Expand Up @@ -132,6 +133,11 @@ public void setCurrentFiber(RubyFiber fiber) {
currentFiber = fiber;
}

public ThreadLocalBuffer getIoBuffer(RubyContext context) {
Pointer.checkNativeAccess(context);
return ioBuffer;
}

@Override
public void getAdjacentObjects(Set<Object> reachable) {
ObjectGraph.addProperty(reachable, threadLocalVariables);
Expand Down
20 changes: 10 additions & 10 deletions src/main/java/org/truffleruby/core/thread/ThreadLocalBuffer.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,19 @@
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
import com.oracle.truffle.api.profiles.ConditionProfile;

import org.truffleruby.RubyContext;
import org.truffleruby.extra.ffi.Pointer;

public final class ThreadLocalBuffer {

public static final ThreadLocalBuffer NULL_BUFFER = new ThreadLocalBuffer(new Pointer(0, 0), null);
private static final long ALIGNMENT = 8L;
private static final long ALIGNMENT_MASK = ALIGNMENT - 1;

public final Pointer start;
long remaining;
private final ThreadLocalBuffer parent;

private ThreadLocalBuffer(Pointer start, ThreadLocalBuffer parent) {
public ThreadLocalBuffer(Pointer start, ThreadLocalBuffer parent) {
this.start = start;
this.remaining = start.getSize();
this.parent = parent;
Expand Down Expand Up @@ -61,14 +61,14 @@ public void free(RubyThread thread, Pointer ptr, ConditionProfile freeProfile) {

public void freeAll(RubyThread thread) {
ThreadLocalBuffer current = this;
thread.ioBuffer = NULL_BUFFER;
thread.ioBuffer = null;
while (current != null) {
current.freeMemory();
current = current.parent;
}
}

public Pointer allocate(RubyThread thread, long size, ConditionProfile allocationProfile) {
public Pointer allocate(RubyContext context, RubyThread thread, long size, ConditionProfile allocationProfile) {
/* If there is space in the thread's existing buffer then we will return a pointer to that and reduce the
* remaining space count. Otherwise we will either allocate a new buffer, or (if no space is currently being
* used in the existing buffer) replace it with a larger one. */
Expand All @@ -77,13 +77,13 @@ public Pointer allocate(RubyThread thread, long size, ConditionProfile allocatio
* or reallocating a buffer that we technically have a pointer to. */
final long allocationSize = alignUp(size);
if (allocationProfile.profile(remaining >= allocationSize)) {
final Pointer pointer = new Pointer(cursor(), allocationSize);
final Pointer pointer = new Pointer(context, cursor(), allocationSize);
remaining -= allocationSize;
assert invariants();
return pointer;
} else {
final ThreadLocalBuffer newBuffer = allocateNewBlock(thread, allocationSize);
final Pointer pointer = new Pointer(newBuffer.start.getAddress(), allocationSize);
final ThreadLocalBuffer newBuffer = allocateNewBlock(context, thread, allocationSize);
final Pointer pointer = new Pointer(context, newBuffer.start.getAddress(), allocationSize);
newBuffer.remaining -= allocationSize;
assert newBuffer.invariants();
return pointer;
Expand All @@ -95,17 +95,17 @@ private static long alignUp(long size) {
}

@TruffleBoundary
private ThreadLocalBuffer allocateNewBlock(RubyThread thread, long size) {
private ThreadLocalBuffer allocateNewBlock(RubyContext context, RubyThread thread, long size) {
// Allocate a new buffer. Chain it if we aren't the default thread buffer, otherwise make a new default buffer.
final long blockSize = Math.max(size, 1024);
final ThreadLocalBuffer newBuffer;
if (this.parent == null && this.isEmpty()) {
// Free the old block
freeMemory();
// Create new bigger block
newBuffer = new ThreadLocalBuffer(Pointer.malloc(blockSize), null);
newBuffer = new ThreadLocalBuffer(Pointer.malloc(context, blockSize), null);
} else {
newBuffer = new ThreadLocalBuffer(Pointer.malloc(blockSize), this);
newBuffer = new ThreadLocalBuffer(Pointer.malloc(context, blockSize), this);
}
thread.ioBuffer = newBuffer;
return newBuffer;
Expand Down
6 changes: 5 additions & 1 deletion src/main/java/org/truffleruby/core/thread/ThreadManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import org.truffleruby.core.klass.RubyClass;
import org.truffleruby.core.string.StringUtils;
import org.truffleruby.core.support.PRNGRandomizerNodes;
import org.truffleruby.extra.ffi.Pointer;
import org.truffleruby.interop.InteropNodes;
import org.truffleruby.interop.TranslateInteropExceptionNode;
import org.truffleruby.language.Nil;
Expand Down Expand Up @@ -375,6 +376,7 @@ public void startForeignThread(RubyThread rubyThread, Thread javaThread) {

public void start(RubyThread thread, Thread javaThread) {
thread.thread = javaThread;
thread.ioBuffer = context.getEnv().isNativeAccessAllowed() ? Pointer.getNullBuffer(context) : null;
registerThread(thread);

final RubyFiber rootFiber = thread.getRootFiber();
Expand All @@ -397,7 +399,9 @@ private void cleanupKillOtherFibers(RubyThread thread) {
public void cleanupThreadState(RubyThread thread, Thread javaThread) {
context.fiberManager.cleanup(thread.getRootFiber(), javaThread);

thread.ioBuffer.freeAll(thread);
if (thread.ioBuffer != null) {
thread.ioBuffer.freeAll(thread);
}

unregisterThread(thread);
thread.thread = null;
Expand Down
Loading

0 comments on commit 788cb4c

Please sign in to comment.