n4r1b / ferrisetw

Basically a KrabsETW rip-off written in Rust
Other
64 stars 23 forks source link

Possible race conditions #45

Closed daladim closed 1 year ago

daladim commented 2 years ago

Race 1

There is no specific synchronization mechanism in trace_callback_thunk(). Which means a thread could be closing the trace session and destroying the TraceData, just at the same time the last callback was triggered. And the event_record.user_context dereferenced in another thread, supposed to point to a TraceData could be dangling here

Race 2

If a thread closes the trace session and destroys the TraceData, the associated Provider, hence the associated callbacks, which are closures, are dropped. That's a problem in case a callback is still in progress (e.g. a callback is stuck in a blocking function call), because the closure may contain state (e.g. in a move || closure).

Notes

krabsetw may have the same issues. See the issue in krabsetw

daladim commented 2 years ago

I'm on my way to fix it.

But this (more or less) requires some changes to make the unsafes less unsafe. E.g.

daladim commented 2 years ago

Note that race 2 is rather likely. If CloseTrace returns an ERROR_CTX_CLOSE_PENDING, this means the callback will be invoked again, after we may be tempted to drop the closure.

daladim commented 1 year ago

Both races have been fixed, and fixes are included in ferrisetw 1.0