Skip to content

Commit

Permalink
8268129: LibraryLookup::ofDefault leaks symbols from loaded libraries
Browse files Browse the repository at this point in the history
Reviewed-by: jvernee, psandoz
  • Loading branch information
mcimadamore committed Jun 4, 2021
1 parent 40c9e25 commit 59a539f
Show file tree
Hide file tree
Showing 47 changed files with 730 additions and 725 deletions.
53 changes: 53 additions & 0 deletions make/modules/jdk.incubator.foreign/Lib.gmk
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
#
# Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
#
# This code is free software; you can redistribute it and/or modify it
# under the terms of the GNU General Public License version 2 only, as
# published by the Free Software Foundation. Oracle designates this
# particular file as subject to the "Classpath" exception as provided
# by Oracle in the LICENSE file that accompanied this code.
#
# This code is distributed in the hope that it will be useful, but WITHOUT
# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
# version 2 for more details (a copy is included in the LICENSE file that
# accompanied this code).
#
# You should have received a copy of the GNU General Public License version
# 2 along with this work; if not, write to the Free Software Foundation,
# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
#
# Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
# or visit www.oracle.com if you need additional information or have any
# questions.
#

include LibCommon.gmk

ifeq ($(call isTargetOs, linux), true)

$(eval $(call SetupJdkLibrary, BUILD_SYSLOOKUPLIB, \
NAME := syslookup, \
CFLAGS := $(CFLAGS_JDKLIB), \
CXXFLAGS := $(CXXFLAGS_JDKLIB), \
LDFLAGS := $(LDFLAGS_JDKLIB) -Wl$(COMMA)--no-as-needed, \
LIBS := $(LIBCXX) -lc -lm -ldl, \
))

else ifeq ($(call isTargetOs, windows), false)

$(eval $(call SetupJdkLibrary, BUILD_SYSLOOKUPLIB, \
NAME := syslookup, \
CFLAGS := $(CFLAGS_JDKLIB), \
CXXFLAGS := $(CXXFLAGS_JDKLIB), \
LDFLAGS := $(LDFLAGS_JDKLIB), \
LIBS := $(LIBCXX), \
))


endif

TARGETS += $(BUILD_SYSLOOKUPLIB)

################################################################################
2 changes: 1 addition & 1 deletion src/java.base/share/classes/java/lang/ClassLoader.java
Original file line number Diff line number Diff line change
Expand Up @@ -2432,7 +2432,7 @@ static NativeLibrary loadLibrary(Class<?> fromClass, String name) {
/*
* Invoked in the VM class linking code.
*/
private static long findNative(ClassLoader loader, String entryName) {
static long findNative(ClassLoader loader, String entryName) {
if (loader == null) {
return BootLoader.getNativeLibraries().find(entryName);
} else {
Expand Down
5 changes: 5 additions & 0 deletions src/java.base/share/classes/java/lang/System.java
Original file line number Diff line number Diff line change
Expand Up @@ -2393,6 +2393,11 @@ public String join(String prefix, String suffix, String delimiter, String[] elem
public Object classData(Class<?> c) {
return c.getClassData();
}

@Override
public long findNative(ClassLoader loader, String entry) {
return ClassLoader.findNative(loader, entry);
}
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -393,4 +393,6 @@ public interface JavaLangAccess {
* @see java.lang.invoke.MethodHandles.Lookup#defineHiddenClass(byte[], boolean, MethodHandles.Lookup.ClassOption...)
*/
Object classData(Class<?> c);

long findNative(ClassLoader loader, String entry);
}
Original file line number Diff line number Diff line change
Expand Up @@ -385,20 +385,6 @@ boolean open() {
}
}

public static final NativeLibrary defaultLibrary = new NativeLibraryImpl(Object.class, "<default>", true, true) {

@Override
boolean open() {
throw new UnsupportedOperationException("Cannot load default library");
}

@Override
public long find(String name) {
return NativeLibraries.findEntryInProcess(name);
}

};

/*
* The run() method will be invoked when this class loader becomes
* phantom reachable to unload the native library.
Expand Down Expand Up @@ -479,5 +465,4 @@ private static Class<?> getFromClass() {
private static native void unload(String name, boolean isBuiltin, boolean isJNI, long handle);
private static native String findBuiltinLib(String name);
private static native long findEntry0(NativeLibraryImpl lib, String name);
private static native long findEntryInProcess(String name);
}
23 changes: 0 additions & 23 deletions src/java.base/share/native/libjava/NativeLibraries.c
Original file line number Diff line number Diff line change
Expand Up @@ -246,29 +246,6 @@ Java_jdk_internal_loader_NativeLibraries_findEntry0
return res;
}

/*
* Class: jdk_internal_loader_NativeLibraries
* Method: findEntryInProcess
* Signature: (Ljava/lang/String;)J
*/
JNIEXPORT jlong JNICALL
Java_jdk_internal_loader_NativeLibraries_findEntryInProcess
(JNIEnv *env, jclass cls, jstring name)
{
const char *cname;
jlong res;

if (!initIDs(env))
return jlong_zero;

cname = (*env)->GetStringUTFChars(env, name, 0);
if (cname == 0)
return jlong_zero;
res = ptr_to_jlong(findEntryInProcess(cname));
(*env)->ReleaseStringUTFChars(env, name, cname);
return res;
}

/*
* Class: jdk_internal_loader_NativeLibraries
* Method: findBuiltinLib
Expand Down
2 changes: 0 additions & 2 deletions src/java.base/share/native/libjava/jni_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -333,8 +333,6 @@ JNIEXPORT void InitializeEncoding(JNIEnv *env, const char *name);

void* getProcessHandle();

void* findEntryInProcess(const char* name);

void buildJniFunctionName(const char *sym, const char *cname,
char *jniEntryName);

Expand Down
4 changes: 0 additions & 4 deletions src/java.base/unix/native/libjava/jni_util_md.c
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,6 @@ void* getProcessHandle() {
return procHandle;
}

void* findEntryInProcess(const char* name) {
return JVM_FindLibraryEntry(RTLD_DEFAULT, name);
}

void buildJniFunctionName(const char *sym, const char *cname,
char *jniEntryName) {
strcpy(jniEntryName, sym);
Expand Down
26 changes: 0 additions & 26 deletions src/java.base/windows/native/libjava/jni_util_md.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
#include <stdlib.h>
#include <string.h>
#include <windows.h>
#include <psapi.h>
#include <locale.h>

#include "jni.h"
Expand All @@ -36,31 +35,6 @@ void* getProcessHandle() {
return (void*)GetModuleHandle(NULL);
}

/*
* Windows doesn't have an RTLD_DEFAULT equivalent, so in stead we have to
* iterate over all the modules loaded by the process to implement the
* default library behaviour.
*/
void* findEntryInProcess(const char* name) {
HANDLE hProcess = GetCurrentProcess();

HMODULE hMods[1024];
DWORD cbNeeded; // array size in bytes

// first come, first served
if (EnumProcessModules(hProcess, hMods, sizeof(hMods), &cbNeeded)) {
for (size_t i = 0; i < (cbNeeded / sizeof(HMODULE)); i++) {
HMODULE mod = hMods[i];
FARPROC proc = GetProcAddress(mod, name);
if(proc != NULL) {
return proc;
}
}
}

return NULL;
}

/*
* Windows symbols can be simple like JNI_OnLoad or __stdcall format
* like _JNI_OnLoad@8. We need to handle both.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import jdk.internal.foreign.AbstractCLinker;
import jdk.internal.foreign.NativeMemorySegmentImpl;
import jdk.internal.foreign.PlatformLayouts;
import jdk.internal.foreign.SystemLookup;
import jdk.internal.foreign.abi.SharedUtils;
import jdk.internal.foreign.abi.aarch64.AArch64VaList;
import jdk.internal.foreign.abi.x64.sysv.SysVVaList;
Expand Down Expand Up @@ -114,7 +115,7 @@ public sealed interface CLinker permits AbstractCLinker {
* Returns the C linker for the current platform.
* <p>
* This method is <a href="package-summary.html#restricted"><em>restricted</em></a>.
* Restricted method are unsafe, and, if used incorrectly, their use might crash
* Restricted methods are unsafe, and, if used incorrectly, their use might crash
* the JVM or, worse, silently result in memory corruption. Thus, clients should refrain from depending on
* restricted methods, and use safe and supported functionalities, where possible.
*
Expand All @@ -129,6 +130,25 @@ static CLinker getInstance() {
return SharedUtils.getSystemLinker();
}

/**
* Obtains a system lookup which is suitable to find symbols in the standard C libraries. The set of symbols
* available for lookup is unspecified, as it depends on the platform and on the operating system.
* <p>
* This method is <a href="package-summary.html#restricted"><em>restricted</em></a>.
* Restricted methods are unsafe, and, if used incorrectly, their use might crash
* the JVM or, worse, silently result in memory corruption. Thus, clients should refrain from depending on
* restricted methods, and use safe and supported functionalities, where possible.
* @return a system-specific library lookup which is suitable to find symbols in the standard C libraries.
* @throws IllegalCallerException if access to this method occurs from a module {@code M} and the command line option
* {@code --enable-native-access} is either absent, or does not mention the module name {@code M}, or
* {@code ALL-UNNAMED} in case {@code M} is an unnamed module.
*/
@CallerSensitive
static SymbolLookup systemLookup() {
Reflection.ensureNativeAccess(Reflection.getCallerClass());
return SystemLookup.getInstance();
}

/**
* Obtains a foreign method handle, with the given type and featuring the given function descriptor,
* which can be used to call a target foreign function at the given address.
Expand All @@ -137,13 +157,14 @@ static CLinker getInstance() {
* an additional prefix parameter, of type {@link SegmentAllocator}, which will be used by the linker runtime
* to allocate structs returned by-value.
*
* @see LibraryLookup#lookup(String)
*
* @param symbol downcall symbol.
* @param type the method type.
* @param function the function descriptor.
* @return the downcall method handle.
* @throws IllegalArgumentException in the case of a method type and function descriptor mismatch.
* @throws IllegalArgumentException in the case of a method type and function descriptor mismatch, or if the symbol
* is {@link MemoryAddress#NULL}
*
* @see SymbolLookup
*/
MethodHandle downcallHandle(Addressable symbol, MethodType type, FunctionDescriptor function);

Expand All @@ -154,14 +175,15 @@ static CLinker getInstance() {
* If the provided method type's return type is {@code MemorySegment}, then the provided allocator will be used by
* the linker runtime to allocate structs returned by-value.
*
* @see LibraryLookup#lookup(String)
*
* @param symbol downcall symbol.
* @param allocator the segment allocator.
* @param type the method type.
* @param function the function descriptor.
* @return the downcall method handle.
* @throws IllegalArgumentException in the case of a method type and function descriptor mismatch.
* @throws IllegalArgumentException in the case of a method type and function descriptor mismatch, or if the symbol
* is {@link MemoryAddress#NULL}
*
* @see SymbolLookup
*/
MethodHandle downcallHandle(Addressable symbol, SegmentAllocator allocator, MethodType type, FunctionDescriptor function);

Expand All @@ -174,13 +196,16 @@ static CLinker getInstance() {
* If the provided method type's return type is {@code MemorySegment}, then the resulting method handle features an
* additional prefix parameter (inserted immediately after the address parameter), of type {@link SegmentAllocator}),
* which will be used by the linker runtime to allocate structs returned by-value.
*
* @see LibraryLookup#lookup(String)
* <p>
* The returned method handle will throw an {@link IllegalArgumentException} if the target address passed to it is
* {@link MemoryAddress#NULL}, or a {@link NullPointerException} if the target address is {@code null}.
*
* @param type the method type.
* @param function the function descriptor.
* @return the downcall method handle.
* @throws IllegalArgumentException in the case of a method type and function descriptor mismatch.
*
* @see SymbolLookup
*/
MethodHandle downcallHandle(MethodType type, FunctionDescriptor function);

Expand Down Expand Up @@ -339,7 +364,7 @@ static MemorySegment toCString(String str, Charset charset, ResourceScope scope)
* over the decoding process is required.
* <p>
* This method is <a href="package-summary.html#restricted"><em>restricted</em></a>.
* Restricted method are unsafe, and, if used incorrectly, their use might crash
* Restricted methods are unsafe, and, if used incorrectly, their use might crash
* the JVM or, worse, silently result in memory corruption. Thus, clients should refrain from depending on
* restricted methods, and use safe and supported functionalities, where possible.
*
Expand All @@ -366,7 +391,7 @@ static String toJavaString(MemoryAddress addr) {
* over the decoding process is required.
* <p>
* This method is <a href="package-summary.html#restricted"><em>restricted</em></a>.
* Restricted method are unsafe, and, if used incorrectly, their use might crash
* Restricted methods are unsafe, and, if used incorrectly, their use might crash
* the JVM or, worse, silently result in memory corruption. Thus, clients should refrain from depending on
* restricted methods, and use safe and supported functionalities, where possible.
*
Expand Down Expand Up @@ -440,7 +465,7 @@ private static MemorySegment toCString(byte[] bytes, SegmentAllocator allocator)
* Allocates memory of given size using malloc.
* <p>
* This method is <a href="package-summary.html#restricted"><em>restricted</em></a>.
* Restricted method are unsafe, and, if used incorrectly, their use might crash
* Restricted methods are unsafe, and, if used incorrectly, their use might crash
* the JVM or, worse, silently result in memory corruption. Thus, clients should refrain from depending on
* restricted methods, and use safe and supported functionalities, where possible.
*
Expand All @@ -466,7 +491,7 @@ static MemoryAddress allocateMemory(long size) {
* Frees the memory pointed by the given memory address.
* <p>
* This method is <a href="package-summary.html#restricted"><em>restricted</em></a>.
* Restricted method are unsafe, and, if used incorrectly, their use might crash
* Restricted methods are unsafe, and, if used incorrectly, their use might crash
* the JVM or, worse, silently result in memory corruption. Thus, clients should refrain from depending on
* restricted methods, and use safe and supported functionalities, where possible.
*
Expand Down Expand Up @@ -622,7 +647,7 @@ sealed interface VaList extends Addressable permits WinVaList, SysVVaList, AArch
* backed by the {@linkplain ResourceScope#globalScope() global} resource scope.
* <p>
* This method is <a href="package-summary.html#restricted"><em>restricted</em></a>.
* Restricted method are unsafe, and, if used incorrectly, their use might crash
* Restricted methods are unsafe, and, if used incorrectly, their use might crash
* the JVM or, worse, silently result in memory corruption. Thus, clients should refrain from depending on
* restricted methods, and use safe and supported functionalities, where possible.
*
Expand All @@ -643,7 +668,7 @@ static VaList ofAddress(MemoryAddress address) {
* with given resource scope.
* <p>
* This method is <a href="package-summary.html#restricted"><em>restricted</em></a>.
* Restricted method are unsafe, and, if used incorrectly, their use might crash
* Restricted methods are unsafe, and, if used incorrectly, their use might crash
* the JVM or, worse, silently result in memory corruption. Thus, clients should refrain from depending on
* restricted methods, and use safe and supported functionalities, where possible.
*
Expand Down
Loading

1 comment on commit 59a539f

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.