https://issues.dlang.org/show_bug.cgi?id=14097
Issue ID: 14097
Summary: root/async.c: free after use
Product: D
Version: D2
Hardware: x86
OS: Windows
Status: NEW
Severity: critical
Priority: P1
Component: DMD
Assignee: nob...@puremagic.com
Reporter: ket...@ketmar.no-ip.org
Created attachment 1473
--> https://issues.dlang.org/attachment.cgi?id=1473&action=edit
fix "use after free" in async.c
async file reader (windows version at least) has a nasty bug: it uses
`AsyncRead` structure after `free()`. this is what makes dmd.exe crash under my
wine.
i found that by inserting `printf`s everywhere. then i saw this:
..\dmd\src\dmd.exe -c -o- -Isrc -Iimport -Hfimport\core\sync\barrier.di
src\core\sync\barrier.d
AsyncRead::create: aw = 00242754; max=1
*** addFile(file = 00262510)
filesdim = 0, filesmax = 1
addFile done: (f = 00242760; file = 00262510; result=0; event=0000003C)
*** ::start aw->filesdim = 00242754 1
*** AsyncRead::read 000 i=0 (f=00242760; file = 00262510; event = 0000003C)
*** aw->filesdim = 00242754 1
++ aw->filesdim = 00242754 1 i=0
000: i=0; f=00242760; file=2499856; event=0000003C
001: i=0; f=00242760; file=2499856; event=0000003C
AsyncRead::read 001 i=0 (f=00242760; file = 00262510; event = 0000003C; res=0)
AsyncRead::read 002 i=0 (f=00242760; file = 00262510; event = 0000003C; res=0)
AsyncRead::dispose; aw = 00242754
002: i=0; f=00242760; file=2499856; event=0000003C
startthread done: aw->filesdim = 00242754 1
the line "002: i=0; f=00242760; file=2499856; event=0000003C" is from
`startthread`, it printed right after `SetEvent(f->event);` call. as you can
see, `AsyncRead::dispose` was already called, yet `startthread` is still using
`aw`.
seems that this is highly depends on timing, 'cause SOMETIMES (very rarely) dmd
works ok in my wine. but most of the time it crashes. yet it works fine on real
windows.
nevertheless, just commenting out `free(aw);` in `AsyncRead::dispose` fixes it
all. as dmd.exe leaks memory as crazy anyway, there is no harm in leaking a
little more, and it was the easiest fix.
posix version of this seems to be protected from the bug, as posix
`AsyncRead::dispose` correctly waits while all operations on all files are
complete. more than that, posix `startthread` correctly caches `aw->filesdim`,
so it will not use freed memory in loop condition check.
so i attached a patch that waits for completion of all file operations before
freeing `aw`. this seems to fix the issue, and this is what posix version do.
--
