nhyatt/putty-source
1
0
Fork 0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-01-10 09:58:01 +00:00
Code Issues Projects Releases Wiki Activity

Fix a dangerous cross-thread memory access.

Browse Source 
When a winhandl.c input thread returns EOF to the main thread, the
latter might immediately delete the input thread's context. I
carefully wrote in a comment that in that case we had to not touch ctx
ever again after signalling to the main thread - but the test for
whether that was true, which also touched ctx, itself came _after_ the
SetEvent which sent that signal. Ahem.

Spotted by Minefield, which it looks as if I haven't run for a while.

(cherry picked from commit 9fec2e7738)
This commit is contained in:
Simon Tatham 2015-04-07 22:17:08 +01:00
parent 02893bcba0
commit 2856422eab
1 changed files with 12 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
windows/winhandl.c
View File

@ -115,7 +115,7 @@ static DWORD WINAPI handle_input_threadfunc(void *param)
struct handle_input *ctx = (struct handle_input *) param; struct handle_input *ctx = (struct handle_input *) param;
OVERLAPPED ovl, *povl; OVERLAPPED ovl, *povl;
HANDLE oev; HANDLE oev;
int readret, readlen; int readret, readlen, finished;
if (ctx->flags & HANDLE_FLAG_OVERLAPPED) { if (ctx->flags & HANDLE_FLAG_OVERLAPPED) {
povl = &ovl; povl = &ovl;
@ -165,18 +165,20 @@ static DWORD WINAPI handle_input_threadfunc(void *param)
(ctx->flags & HANDLE_FLAG_IGNOREEOF)) (ctx->flags & HANDLE_FLAG_IGNOREEOF))
continue; continue;
/*
* If we just set ctx->len to 0, that means the read operation
* has returned end-of-file. Telling that to the main thread
* will cause it to set its 'defunct' flag and dispose of the
* handle structure at the next opportunity, in which case we
* mustn't touch ctx at all after the SetEvent. (Hence we do
* even _this_ check before the SetEvent.)
*/
finished = (ctx->len == 0);
SetEvent(ctx->ev_to_main); SetEvent(ctx->ev_to_main);
if (!ctx->len) { if (finished)
/*
* The read operation has returned end-of-file. Telling
* that to the main thread will cause it to set its
* 'defunct' flag and dispose of the handle structure at
* the next opportunity, so we must not touch ctx at all
* after this.
*/
break; break;
}
WaitForSingleObject(ctx->ev_from_main, INFINITE); WaitForSingleObject(ctx->ev_from_main, INFINITE);
if (ctx->done) { if (ctx->done) {