From: Ryan Lucia Date: Tue, 3 Sep 2019 15:41:27 +0000 (-0400) Subject: Remove MAX_PATH usage in w32process (mono/mono#16553) X-Git-Tag: submit/tizen/20210909.063632~10331^2~5^2~612 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=95ce153202be70e1e69f1791d67f0248e64a36c9;p=platform%2Fupstream%2Fdotnet%2Fruntime.git 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 --- 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 --- 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 --- 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 */