[2020-02] Avoid following invalid pointers in mono_w32process_get_modules on Darwin. (#20297)mono-6.12.0.93

* Avoid following invalid pointers in mono_w32process_get_modules on Darwin.

`_dyld_get_image_name` and `_dyld_get_image_header` can return pointers
to data that will be freed or unmapped by concurrent calls to `dlclose`.
Avoid this by using `_dyld_register_func_for_add_image` and
`_dyld_register_func_for_remove_image` to install callbacks that
maintain a runtime-internal set of loaded dynamic libraries.

Some notes:

- dyld doesn't provide a public "app store friendly" API (like
`dl_iterate_phdr`) to traverse a snapshot of all loaded dynamic
libraries in a process. There are two alternative private APIs that do
this: one uses `_dyld_process_info_create` and friends and is present in
macOS 10.12 and above, and the other depends on
`_dyld_get_all_image_infos`, which is documented by Apple (... in a WWDC
video) to "go away" at some point in dyld3.

- `dladdr` linearly scans the dyld loaded image array (and each image's
segment set) for a matching address, so `image_added` will get slower as
more dynamic libraries are loaded in the process. But dyld3's
implementation of dlopen already scales linearly in time with the number
of loaded libraries. The use of `dladdr` can be avoided by using
`_dyld_register_for_image_loads` or
`_dyld_register_for_bulk_image_loads`, but those are both private APIs.

- The callbacks registered with `_dyld_register_func_for_add_image` and
`_dyld_register_func_for_remove_image` can never be unregistered, so
this is yet another thing that makes it impossible to safely unload an
embedded mono.

- `MonoW32ProcessModule::inode` only seems to be used to avoid dupicates
in other implementations of w32process; sorting the returned module list
in `mono_w32process_get_modules` wouldn't be necessary if the
`mono_dyld_image_info`s were also kept in an intrusive circular doubly
linked list.

- It looks like `mono_w32process_get_modules` in `w32process-unix-bsd.c`
has a similar race condition.

Also see: https://github.com/mono/mono/issues/20107

* Formatting.

* Make `image_added` and `image_removed` non-blocking.

It is theoretically possible for `mono_w32process_get_modules` to cause
a deadlock if either `image_added` or `image_removed` is waiting for the
image table mutex to be released and if `g_new0`, `g_strdup`,
`g_hash_table_iter_next`, or any of the symbols they depend on are
lazily bound and haven't already been resolved.

Only one thread may iterate over the image table and only one thread may
add/remove entries to/from the image table.

The writer thread attemps to lock a mutex to block readers from
iterating over the table. If the writer successfully acquires this lock,
then no readers are concurrently iterating over the image table and it
is safe to eagerly free stale data.

If the writer doesn't successfully acquire this lock, then the reader
thread is running concurrently with the writer; if this happens, the
writer pushes stale data into a limbo list. Readers opportunistically
clear this list.

Use `mono_mutex_t` instead of `MonoCoopMutex`. Hand-roll the necessary
GC state transitions inside `mono_w32process_get_modules`.

Add `mono_conc_hashtable_foreach_snapshot` to mono-conc-hashtable.{h,c}.

`MonoConcurrentHashTable` depends on the hazard pointer table
(`hazard_table` inside hazard-pointer.c), which is initialized lazily on
the first call to `mono_thread_small_id_alloc`. Perhaps this should be
explicitly initialized early during `mono_init_internal`.

* Revert "Make `image_added` and `image_removed` non-blocking."

This reverts commit a22a4440df5f0f13da29b1b90e80a3420223c2b9.

* Use mono_mutex_t instead of MonoCoopMutex

* Explicitly load symbols not used by `image_added` before `mono_w32process_get_modules` runs

Co-authored-by: Imran Hameed <imhameed@microsoft.com>

0 files changed, 0 insertions, 0 deletions