microsoft / microsoft/krabsetw

Possible race condition

Open
#200 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

needs-repro
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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.