Remove MAX_PATH usage in w32process (mono/mono#16553)

* Remove MAX_PATH usage in w32process

* Fixes

* Shrink buffer temporarily

* More fixes

* Reset buffer size to 260

* Fix length check and add notes

* Actually free things

* Add tracing and zero len at start

* Typo

* Fix another leak

* Simplify logic

Commit migrated from https://github.com/mono/mono/commit/5377f9655d24c3ff19545398f89df462c77589b3

diff --git a/src/mono/mono/metadata/w32process-internals.h b/src/mono/mono/metadata/w32process-internals.h
index 347cfd7..b5fc4b6 100644 (file)
--- a/src/mono/mono/metadata/w32process-internals.h
+++ b/src/mono/mono/metadata/w32process-internals.h
@@ -45,11 +45,11 @@ mono_w32process_get_pid (gpointer handle);
 gboolean
 mono_w32process_try_get_modules (gpointer process, gpointer *modules, guint32 size, guint32 *needed);
 
-guint32
-mono_w32process_module_get_name (gpointer process, gpointer module, gunichar2 *basename, guint32 size);
+gunichar2 *
+mono_w32process_module_get_name (gpointer process, gpointer module, guint32 *len);
 
-guint32
-mono_w32process_module_get_filename (gpointer process, gpointer module, gunichar2 *basename, guint32 size);
+gunichar2 *
+mono_w32process_module_get_filename (gpointer process, gpointer module, guint32 *len);
 
 gboolean
 mono_w32process_module_get_information (gpointer process, gpointer module, MODULEINFO *modinfo, guint32 size);

diff --git a/src/mono/mono/metadata/w32process-unix.c b/src/mono/mono/metadata/w32process-unix.c
index fd22131..e9da730 100644 (file)
--- a/src/mono/mono/metadata/w32process-unix.c
+++ b/src/mono/mono/metadata/w32process-unix.c
@@ -911,82 +911,64 @@ mono_w32process_try_get_modules (gpointer handle, gpointer *modules, guint32 siz
        return TRUE;
 }
 
-guint32
-mono_w32process_module_get_filename (gpointer handle, gpointer module, gunichar2 *basename, guint32 size)
+gunichar2 *
+mono_w32process_module_get_filename (gpointer handle, gpointer module, guint32 *len)
 {
-       gint pid, len;
-       gsize bytes;
+       gint pid;
+       gsize bytes = 0;
        gchar *path;
        gunichar2 *proc_path;
 
-       size *= sizeof (gunichar2); /* adjust for unicode characters */
+       mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER_PROCESS, "%s: Getting module file name, process handle %p module %p " G_GUINT32_FORMAT,
+                   __func__, handle, module);
 
-       if (basename == NULL || size == 0)
-               return 0;
+       *len = 0;
 
        pid = mono_w32process_get_pid (handle);
+       if (pid == 0)
+               return NULL;
 
        path = mono_w32process_get_path (pid);
        if (path == NULL)
-               return 0;
+               return NULL;
 
        proc_path = mono_unicode_from_external (path, &bytes);
-       g_free (path);
-
-       if (proc_path == NULL)
-               return 0;
 
-       len = (bytes / 2);
+       *len = bytes / sizeof (gunichar2);
 
-       /* Add the terminator */
-       bytes += 2;
-
-       if (size < bytes) {
-               mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER_PROCESS, "%s: Size %" G_GUINT32_FORMAT " smaller than needed (%zd); truncating", __func__, size, bytes);
-               memcpy (basename, proc_path, size);
-       } else {
-               mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER_PROCESS, "%s: Size %" G_GUINT32_FORMAT " larger than needed (%zd)", __func__, size, bytes);
-               memcpy (basename, proc_path, bytes);
-       }
-
-       g_free (proc_path);
-
-       return len;
+       g_free (path);
+       return proc_path;
 }
 
-guint32
-mono_w32process_module_get_name (gpointer handle, gpointer module, gunichar2 *basename, guint32 size)
+gunichar2 *
+mono_w32process_module_get_name (gpointer handle, gpointer module, guint32 *len)
 {
        MonoW32Handle *handle_data;
        MonoW32HandleProcess *process_handle;
        pid_t pid;
        gunichar2 *procname;
        char *procname_ext = NULL;
-       glong len;
-       gsize bytes;
+       gsize bytes = 0;
        GSList *mods = NULL, *mods_iter;
        MonoW32ProcessModule *found_module;
        char *pname = NULL;
 
-       mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER_PROCESS, "%s: Getting module base name, process handle %p module %p basename %p size %" G_GUINT32_FORMAT,
-                  __func__, handle, module, basename, size);
-
-       size = size * sizeof (gunichar2); /* adjust for unicode characters */
+       mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER_PROCESS, "%s: Getting module base name, process handle %p module %p " G_GUINT32_FORMAT,
+                  __func__, handle, module);
 
-       if (basename == NULL || size == 0)
-               return 0;
+       *len = 0;
 
        if (!mono_w32handle_lookup_and_ref (handle, &handle_data)) {
                mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER_PROCESS, "%s: unknown handle %p", __func__, handle);
                mono_w32error_set_last (ERROR_INVALID_HANDLE);
-               return 0;
+               return NULL;
        }
 
        if (handle_data->type != MONO_W32TYPE_PROCESS) {
                mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER_PROCESS, "%s: unknown process handle %p", __func__, handle);
                mono_w32error_set_last (ERROR_INVALID_HANDLE);
                mono_w32handle_unref (handle_data);
-               return 0;
+               return NULL;
        }
 
        process_handle = (MonoW32HandleProcess*) handle_data->specific;
@@ -1041,35 +1023,19 @@ mono_w32process_module_get_name (gpointer handle, gpointer module, gunichar2 *ba
                        /* bugger */
                        g_free (procname_ext);
                        mono_w32handle_unref (handle_data);
-                       return 0;
+                       return NULL;
                }
 
-               len = (bytes / 2);
-
-               /* Add the terminator */
-               bytes += 2;
+               *len = bytes / sizeof (gunichar2);
 
-               if (size < bytes) {
-                       mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER_PROCESS, "%s: Size %" G_GUINT32_FORMAT " smaller than needed (%zd); truncating", __func__, size, bytes);
-
-                       memcpy (basename, procname, size);
-               } else {
-                       mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER_PROCESS, "%s: Size %" G_GUINT32_FORMAT " larger than needed (%zd)",
-                                  __func__, size, bytes);
-
-                       memcpy (basename, procname, bytes);
-               }
-
-               g_free (procname);
                g_free (procname_ext);
-
                mono_w32handle_unref (handle_data);
-               return len;
+               return procname;
        }
 
        mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_IO_LAYER_PROCESS, "%s: Can't find procname_ext %p", __func__, handle);
        mono_w32handle_unref (handle_data);
-       return 0;
+       return NULL;
 }
 
 gboolean

diff --git a/src/mono/mono/metadata/w32process.c b/src/mono/mono/metadata/w32process.c
index 83a1fb4..ea7fb03 100644 (file)
--- a/src/mono/mono/metadata/w32process.c
+++ b/src/mono/mono/metadata/w32process.c
@@ -33,16 +33,48 @@ mono_w32process_try_get_modules (gpointer process, HMODULE *modules, guint32 siz
        return EnumProcessModules (process, modules, size, needed);
 }
 
-static guint32
-mono_w32process_module_get_name (gpointer process, gpointer module, gunichar2 *basename, guint32 size)
-{
-       return GetModuleBaseNameW (process, (HMODULE)module, basename, size);
+static gunichar2 *
+mono_w32process_module_get_name (gpointer process, gpointer module, guint32 *len)
+{
+       gunichar2 *basename = NULL;
+       guint32 size = 260; // reasonable length to start with given historical behavior
+
+       basename = g_new0 (gunichar2, size);
+       *len = GetModuleBaseNameW (process, (HMODULE)module, basename, size);
+       // GetModuleBaseNameW will set the null byte but include it in the returned length
+       while (*len >= size) {
+               if (*len == 0) {
+                       g_free (basename);
+                       return NULL;
+               }
+               size *= 2; // double the buffer and try again
+               g_free (basename);
+               basename = g_new0 (gunichar2, size);
+               *len = GetModuleBaseNameW (process, (HMODULE)module, basename, size);
+       }
+       return basename;
 }
 
-static guint32
-mono_w32process_module_get_filename (gpointer process, gpointer module, gunichar2 *basename, guint32 size)
+static gunichar2 *
+mono_w32process_module_get_filename (gpointer process, gpointer module, guint32 *len)
 {
-       return GetModuleFileNameExW (process, (HMODULE)module, basename, size);
+       gunichar2 *basename = NULL;
+       guint32 size = 260; // reasonable length to start with given historical behavior
+
+       basename = g_new0 (gunichar2, size);
+       *len = GetModuleFileNameExW (process, (HMODULE)module, basename, size);
+       // GetModuleFileNameExW will set the null byte but _not_ include it in the returned length
+       while (*len >= (size - 1)) {
+               if (*len == 0) {
+                       g_free (basename);
+                       return NULL;
+               }
+               size *= 2; // double the buffer and try again
+               g_free (basename);
+               basename = g_new0 (gunichar2, size);
+               *len = GetModuleFileNameExW (process, (HMODULE)module, basename, size);
+       }
+       return basename;
 }
 
 static gboolean
@@ -529,8 +561,10 @@ ves_icall_System_Diagnostics_Process_GetModules_internal (MonoObjectHandle this_
        MonoArrayHandle temp_arr = NULL_HANDLE_ARRAY;
        MonoArrayHandle arr = NULL_HANDLE_ARRAY;
        HMODULE mods [1024];
-       gunichar2 filename [MAX_PATH]; // FIXME (MAX_PATH)
-       gunichar2 modname [MAX_PATH]; // FIXME (MAX_PATH)
+       gunichar2 *filename = NULL;
+       gunichar2 *modname = NULL;
+       guint32 filename_len = 0;
+       guint32 modname_len = 0;
        DWORD needed = 0;
        guint32 count = 0;
        guint32 module_count = 0;
@@ -561,13 +595,17 @@ ves_icall_System_Diagnostics_Process_GetModules_internal (MonoObjectHandle this_
        return_val_if_nok (error, NULL_HANDLE_ARRAY);
 
        for (i = 0; i < module_count; i++) {
-               if (mono_w32process_module_get_name (process, mods [i], modname, MAX_PATH)
-                        && mono_w32process_module_get_filename (process, mods [i], filename, MAX_PATH))
-               {
+               modname = mono_w32process_module_get_name (process, mods [i], &modname_len);
+               filename = mono_w32process_module_get_filename (process, mods [i], &filename_len);
+               if (modname && filename) {
                        process_add_module (module, filever, str, process, mods [i], filename, modname, get_process_module_class (), error);
-                       return_val_if_nok (error, NULL_HANDLE_ARRAY);
-                       MONO_HANDLE_ARRAY_SETREF (temp_arr, num_added++, module);
+                       if (is_ok (error))
+                               MONO_HANDLE_ARRAY_SETREF (temp_arr, num_added++, module);
                }
+               g_free (modname);
+               g_free (filename);
+               if (!is_ok (error))
+                       return NULL_HANDLE_ARRAY;
        }
 
        if (assemblies) {
@@ -598,16 +636,21 @@ ves_icall_System_Diagnostics_Process_GetModules_internal (MonoObjectHandle this_
 MonoStringHandle
 ves_icall_System_Diagnostics_Process_ProcessName_internal (HANDLE process, MonoError *error)
 {
-       gunichar2 name [MAX_PATH]; // FIXME (MAX_PATH)
+       gunichar2 *name = NULL;
        HMODULE mod = 0;
        DWORD needed = 0;
+       guint32 len = 0;
 
        if (!mono_w32process_try_get_modules (process, &mod, sizeof (mod), &needed))
                return NULL_HANDLE_STRING;
 
-       const guint32 len = mono_w32process_module_get_name (process, mod, name, MAX_PATH);
+       name = mono_w32process_module_get_name (process, mod, &len);
+       if (!name)
+               return NULL_HANDLE_STRING;
 
-       return len ? mono_string_new_utf16_handle (mono_domain_get (), name, len, error) : NULL_HANDLE_STRING;
+       MonoStringHandle res = mono_string_new_utf16_handle (mono_domain_get (), name, len, error);
+       g_free (name);
+       return res;
 }
 #endif /* ENABLE_NETCORE */