-
-
Notifications
You must be signed in to change notification settings - Fork 435
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Possible atomicity violation in reseeding register_fork_handler #911
Possible atomicity violation in reseeding register_fork_handler #911
Comments
Agreed, this doesn't look safe. Since the fix should be straightforward, would you like to make a PR? (Update the CHANGELOG too.) |
Using Using std::sync::Once should fix this. |
@bjorn3 agreed. Unfortunately
|
Option 2 seems best, because it is not a breaking change. |
What about using |
My preference is still option 1. @BurtonQin did you encounter this bug in action or find it through analysis? I'm wondering how easy it is to hit in practice (and how much to care about a quick fix and backports). |
Through analysis.
|
Then I suggest we implement (2) now and (1) for Rand 0.8. |
rand/src/rngs/adapter/reseeding.rs
Lines 303 to 309 in 885655d
In function register_fork_handler:
It seems to me that atomic FORK_HANDLER_REGISTERED is utilized to guarantee that libc::pthread_at_fork is only called once.
However, if the function is called by two threads, a possible atomicity violation may happen:
The fix is simple: use one compare_and_swap() to replace the two separate atomic load and store.
The text was updated successfully, but these errors were encountered: