preload: fix sigmask block and restore race #253
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
To avoid clobbering a threads signal mask with that of another thread,
take locks before storing a thread's signal mask in case another thread
has already stored its own signal mask in the trap_path_sig_restore
variable, but has not yet restored it.
Without this change, the following could happen:
If umockdev-run is used to run a process which spawns multiple threads, and
each of those threads writes to a file, and has specific signal mask
requirements, the following may happen
- Thread A:
- store thread A's sigmask in trap_path_sig_restore and block all
signals for thread A
- acquire trap_path_lock
- Thread B:
- store thread B's sigmask in trap_path_sig_restore and block all
signals for thread B. Thread A's original signal mask is lost.
- wait on acquiring trap_path_lock
- Thread A:
- "restore" thread A's sigmask using trap_path_sig_restore, which
now contains thread B's sigmask, thus clobbering thread A's
original sigmask with that of thread B.
|
Hello @barathrm ! Oh dear, you clearly win my "bug report and analysis of the year" trophy! 🏆 Thanks so much for tracking this down! The fix is straightforward, as usual in these cases (where there's a ton of work in debugging and understanding the problem). Unfortunately meson is making a boo-boo in Debian testing. That has nothing to do with your fix, it just broke independently. I'll investigate this, and then rebase/queue your PR after that fix. Cheers! |
martinpitt
approved these changes
Jul 22, 2024
There was a problem hiding this comment.
I reported the meson bug here: mesonbuild/meson#13461
Not sure how to work around this, I don't want to block on this, so I'll ignore the Debian-testing failure. It's tested on all the other OSes.
Thanks again!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related to and discussed more at #252
To avoid clobbering a threads signal mask with that of another thread, take locks before storing a thread's signal mask in case another thread has already stored its own signal mask in the trap_path_sig_restore variable, but has not yet restored it.
Without this change, the following could happen:
If umockdev-run is used to run a process which spawns multiple threads, and each of those threads writes to a file, and has specific signal mask requirements, the following may happen
Thread A:
Thread B:
Thread A: