search

microsoft / krabsetw

KrabsETW provides a modern C++ wrapper and a .NET wrapper around the low-level ETW trace consumption functions.
Other
581 stars 149 forks source link

Possible race condition #200

Open daladim opened 1 year ago

daladim commented 1 year ago

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

daladim commented 1 year ago

For race 1 : note that CloseTrace could return ERROR_CTX_CLOSE_PENDING, which means that it will continue invoking the callback.

I'm not sure what pUserTrace points at in krabsetw, but it was definitely a big issue in ferrisetw.