[android] mono_dl_open_file: use g_file_test only on absolute paths (#16392)

monojenkins · lambdageek · commit 6434153d8b7d · 2019-08-22T13:36:13.000-04:00
The intention of calling `g_file_test (file, G_FILE_TEST_EXISTS)` where file is the name of a shared library we want to open is to speed up probing for non-existent libraries. See #12074 The problem is that if file is just a simple "libdl.so" then `dlopen (file)` doesn't just look for it in the current working directory, it will probe some other paths too. (For example on desktop linux you'd also look in all the directories in LD_LIBRARY_PATH). So the g_file_test() call is not a robust way to avoid calling dlopen if the filename is relative. But it actually broke more things: dotnet/android#3388 When probing for "libdl.so" on Android mono_lookup_pinvoke_call will first try prepending some paths that it knows about and we end up calling `dlopen ("/system/lib/libdl.so")` which will fail because Bionic has security restrictions on what code can dlopen something from /system/lib with an absolute path. Eventually mono_lookup_pinvoke_call will go back to trying the bare "libdl.so" which hits `g_file_test` and returns NULL. The new code only does the file test if we pass it an absolute path, which gives Bionic's dlopen a chance to deal with relative paths however it needs to.
diff --git a/mono/utils/mono-dl-posix.c b/mono/utils/mono-dl-posix.c
@@ -69,7 +69,16 @@ mono_dl_open_file (const char *file, int flags)
 	/* Bionic doesn't support NULL filenames */
 	if (!file)
 		return NULL;
-	if (!g_file_test (file, G_FILE_TEST_EXISTS))
+	/* The intention of calling `g_file_test (file, G_FILE_TEST_EXISTS)` is
+	 * to speed up probing for non-existent libraries.  The problem is that
+	 * if file is just a simple "libdl.so" then `dlopen (file)` doesn't just
+	 * look for it in the current working directory, it will probe some
+	 * other paths too.  (For example on desktop linux you'd also look in
+	 * all the directories in LD_LIBRARY_PATH).  So the g_file_test() call
+	 * is not a robust way to avoid calling dlopen if the filename is
+	 * relative.
+	 */
+	if (g_path_is_absolute (file) && !g_file_test (file, G_FILE_TEST_EXISTS))
 		return NULL;
 #endif
 #if defined(_AIX)
