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.

--

Reply via email to