api: discourage UaF on teardown

While it does make sense to make readers unblock by setting the read
event on teardown, this is something that consumers of the library
should do _before_ calling EndSession, not something that makes sense
for the library to do itself. The reason is that, in the hypothetical
case in which this makes sense, immediately after unblocking the reader
via SetEvent, the function goes on to free all of the memory that that
reader might want to use. So, rather, the proper shutdown flow is from
the application side, and looks like:

    Closing = true;
    SetEvent(WintunGetReadWaitEvent());
    WaitForReadersToReturn();
    WintunEndSession();

Alternatively, rather than using WaitForSingleObject on the read event,
consumers can WaitForMultipleObjects and include a shutdown event, which
is what the example code does.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
This commit is contained in:
Jason A. Donenfeld 2021-05-07 12:02:00 +02:00
parent d2db3b9977
commit d9555bea1b

View File

@ -154,7 +154,6 @@ out:
void WINAPI void WINAPI
WintunEndSession(_In_ TUN_SESSION *Session) WintunEndSession(_In_ TUN_SESSION *Session)
{ {
SetEvent(Session->Descriptor.Send.TailMoved); // Wake the reader if it's sleeping.
DeleteCriticalSection(&Session->Send.Lock); DeleteCriticalSection(&Session->Send.Lock);
DeleteCriticalSection(&Session->Receive.Lock); DeleteCriticalSection(&Session->Receive.Lock);
CloseHandle(Session->Handle); CloseHandle(Session->Handle);