microsoft / microsoft/krabsetw
Possible race condition
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 801
- Forks
- 167
- Avg merge
- 3d 15m
- Merged PRs (30d)
- 2
Description
Hello, I'm currently fixing two possible races in the Rust counterpart of krabsetw, and these may also exist in krabsetw.
Race 1
It looks there is no specific synchronization mechanism in trace_callback_thunk().
Which means a thread could be closing the trace session and destroying the trace, just at the same time the last callback was triggered. And the *pUserTrace dereferenced in another thread could be dangling here
Race 2
I'm not sure whether this is an issue in krabsetw. What if a thread closes the trace session and destroys the trace while a callback is still in progress (e.g. a callback is stuck in a blocking function call)?
I'm not sure whether this is possible for the callbacks to access data with pointers pointing to a trace. But as an example, the Rust counterpart would store the state of the closure (used as a callback) into the trace, and thus, dropping it while the callback (closure) is still running would destroy its state (which is obviously a bad idea, because the callback is still running).
Disclaimer: I'm not sure exactly how the O365::Security::ETW::NativePtr<T> smart pointer work, and I haven't really read it (yet?). It is used here, maaaaaybe this could make everything work in krabsetw. But I'm not totally sure, and I prefer creating an issue here to discuss this with you
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reading trace_callback_thunk() in krabs/krabs/etw.hpp and the NativePtr usage in Microsoft.O365.Security.Native.ETW/UserTrace.hpp. Compare the callback and trace lifetime behavior with the linked Rust counterpart. Done means determining whether either race is possible and documenting or implementing a safe lifetime and synchronization approach.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- operating-systems
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100