microsoft / microsoft/mimalloc

main thread random context doesn't reinitialize correctly with non-weak source

Open
#1,133 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C
Stars
13.4k
Forks
1.2k
Avg merge
4d 45m
Merged PRs (30d)
13

Description

TLDR is that `_mi_auto_process_init` attempts to reseed the main thread's random ctx (which would cause BCrypt to get loaded there) but there's a bug preventing that from working.

Starting point:

https://github.com/microsoft/mimalloc/blob/09a27098aa6e9286518bd9c74e6ffa7199c3f04e/src/init.c#L580

`_mi_auto_process_init` calls `mi_heap_main_init` which on `_WIN32 && !MI_SHARED_LIB` builds calls `_mi_random_init_weak` for bcrypt-related reasons.

Then, we call `_mi_random_reinit_if_weak` at the end of `_mi_auto_process_init`

https://github.com/microsoft/mimalloc/blob/09a27098aa6e9286518bd9c74e6ffa7199c3f04e/src/init.c#L600-L601

BUT, that will never actually do anything.

Why? The `mi_random_ctx_t` `weak` bool will _always_ be false after any call to `mi_random_init_ex`.

Why? At the end of `mi_random_init_ex`, for either the weak or normal path, we call `chacha_init`.

And what does that guy do? Memsets the entire `mi_random_ctx_t` to zero, wiping out the `weak` bool.

https://github.com/microsoft/mimalloc/blob/09a27098aa6e9286518bd9c74e6ffa7199c3f04e/src/random.c#L97-L102

We discovered this when we saw a thread was doing the mimalloc BCrypt-related initialization (e.g. LoadLibrary) during the initialization of a thread, which involved calling `LoadLibrary` a couple times and holding an internal BCrypt critical section while the thread initialization has the loader lock held (because the root `calloc` in that new thread is inside of `__vcrt_getptd_noexit`). So it seems, given the above mentioned code, that it's not intended that we'd be leaving the main thread's initialization without initializing the BCrypt stuff?

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in src/init.c at _mi_auto_process_init and trace the calls to mi_heap_main_init and _mi_random_reinit_if_weak, then inspect src/random.c around mi_random_init_ex and chacha_init. Check the Windows non-shared-library initialization path and verify that the main-thread random context is correctly reinitialized without triggering the problematic BCrypt work during thread initialization.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
operating-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.