[Bug 38764] New: Improper device request/IRP handling causes heap corruption in wineserver

https://bugs.winehq.org/show_bug.cgi?id=38764

Bug ID: 38764
           Summary: Improper device request/IRP handling causes heap
                    corruption in wineserver
           Product: Wine
           Version: 1.7.45
          Hardware: x86-64
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: wineserver
          Assignee: [e-mail]
          Reporter: [e-mail]
      Distribution: ---

Hello folks,

while testing Erich's patch in bug 21448 I encountered heap corruption in
wineserver.

This shouldn't happen regardless of any client side mishap :|

--- snip ---
$ gdb -q --args wineserver -f 
Reading symbols from wineserver...done.

(gdb) r
Starting program: /home/focht/projects/wine/wine.repo/install/bin/wineserver -f
Missing separate debuginfos, use: dnf debuginfo-install
glibc-2.21-5.fc22.x86_64
*** Error in `/home/focht/projects/wine/wine.repo/install/bin/wineserver':
corrupted double-linked list: 0x0000000000aeb190 ***
======= Backtrace: =========
/lib64/libc.so.6[0x3f72a77e9d]
/lib64/libc.so.6[0x3f72a7e813]
/lib64/libc.so.6[0x3f72a802bd]
/lib64/libc.so.6(__libc_malloc+0x6e)[0x3f72a83b5e]
/home/focht/projects/wine/wine.repo/install/bin/wineserver[0x43ff8c]
/home/focht/projects/wine/wine.repo/install/bin/wineserver[0x4484a1]
/home/focht/projects/wine/wine.repo/install/bin/wineserver[0x414e58]
/home/focht/projects/wine/wine.repo/install/bin/wineserver[0x415232]
/home/focht/projects/wine/wine.repo/install/bin/wineserver[0x415844]
/home/focht/projects/wine/wine.repo/install/bin/wineserver[0x420ccd]
/lib64/libc.so.6(__libc_start_main+0xf0)[0x3f72a20790]
/home/focht/projects/wine/wine.repo/install/bin/wineserver[0x402cd9]
======= Memory map: ========
00400000-00498000 r-xp 00000000 00:22 8371574                           
/home/focht/projects/wine/wine.repo/install/bin/wineserver
00697000-00698000 r--p 00097000 00:22 8371574                           
/home/focht/projects/wine/wine.repo/install/bin/wineserver
00698000-00699000 rw-p 00098000 00:22 8371574                           
/home/focht/projects/wine/wine.repo/install/bin/wineserver
00699000-00bbc000 rw-p 00000000 00:00 0                                  [heap]
...
Program received signal SIGABRT, Aborted.
0x0000003f72a34a98 in raise () from /lib64/libc.so.6
Missing separate debuginfos, use: dnf debuginfo-install
libgcc-5.1.1-1.fc22.x86_64
(gdb) bt
#0  0x0000003f72a34a98 in raise () from /lib64/libc.so.6
#1  0x0000003f72a3672a in abort () from /lib64/libc.so.6
#2  0x0000003f72a77ea2 in __libc_message () from /lib64/libc.so.6
#3  0x0000003f72a7e813 in malloc_consolidate () from /lib64/libc.so.6
#4  0x0000003f72a802bd in _int_malloc () from /lib64/libc.so.6
#5  0x0000003f72a83b5e in malloc () from /lib64/libc.so.6
#6  0x000000000043ff8c in read_request (thread=0xae4910) at
/home/focht/projects/wine/wine.repo/src/server/request.c:284
#7  0x00000000004484a1 in thread_poll_event (fd=0xae4b40, event=1) at
/home/focht/projects/wine/wine.repo/src/server/thread.c:267
#8  0x0000000000414e58 in fd_poll_event (fd=0xae4b40, event=1) at
/home/focht/projects/wine/wine.repo/src/server/fd.c:446
#9  0x0000000000415232 in main_loop_epoll () at
/home/focht/projects/wine/wine.repo/src/server/fd.c:541
#10 0x0000000000415844 in main_loop () at
/home/focht/projects/wine/wine.repo/src/server/fd.c:886
#11 0x0000000000420ccd in main (argc=2, argv=0x7fffffffdae8) at
/home/focht/projects/wine/wine.repo/src/server/main.c:148
--- snip ---

Running with Valgrind:

--- snip ---
$ valgrind --leak-check=yes wineserver -f
==27752== Memcheck, a memory error detector
==27752== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==27752== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
==27752== Command: wineserver -f
...
==27752== Invalid write of size 8
==27752==    at 0x41054B: list_remove (list.h:100)
==27752==    by 0x410BC5: set_irp_result (device.c:306)
==27752==    by 0x411CBB: req_get_next_device_request (device.c:690)
==27752==    by 0x43FDD6: call_req_handler (request.c:247)
==27752==    by 0x43FF6B: read_request (request.c:281)
==27752==    by 0x4484A0: thread_poll_event (thread.c:267)
==27752==    by 0x414E57: fd_poll_event (fd.c:446)
==27752==    by 0x415231: main_loop_epoll (fd.c:541)
==27752==    by 0x415843: main_loop (fd.c:886)
==27752==    by 0x420CCC: main (main.c:148)
==27752==  Address 0x5e88368 is 104 bytes inside a block of size 112 free'd
==27752==    at 0x4A07D29: free (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==27752==    by 0x427235: release_object (object.c:320)
==27752==    by 0x410B28: set_irp_result (device.c:291)
==27752==    by 0x411CBB: req_get_next_device_request (device.c:690)
==27752==    by 0x43FDD6: call_req_handler (request.c:247)
==27752==    by 0x43FF6B: read_request (request.c:281)
==27752==    by 0x4484A0: thread_poll_event (thread.c:267)
==27752==    by 0x414E57: fd_poll_event (fd.c:446)
==27752==    by 0x415231: main_loop_epoll (fd.c:541)
==27752==    by 0x415843: main_loop (fd.c:886)
==27752==    by 0x420CCC: main (main.c:148)
==27752== 
==27752== Invalid write of size 8
==27752==    at 0x41055E: list_remove (list.h:101)
==27752==    by 0x410BC5: set_irp_result (device.c:306)
==27752==    by 0x411CBB: req_get_next_device_request (device.c:690)
==27752==    by 0x43FDD6: call_req_handler (request.c:247)
==27752==    by 0x43FF6B: read_request (request.c:281)
==27752==    by 0x4484A0: thread_poll_event (thread.c:267)
==27752==    by 0x414E57: fd_poll_event (fd.c:446)
==27752==    by 0x415231: main_loop_epoll (fd.c:541)
==27752==    by 0x415843: main_loop (fd.c:886)
==27752==    by 0x420CCC: main (main.c:148)
==27752==  Address 0x5e88360 is 96 bytes inside a block of size 112 free'd
==27752==    at 0x4A07D29: free (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==27752==    by 0x427235: release_object (object.c:320)
==27752==    by 0x410B28: set_irp_result (device.c:291)
==27752==    by 0x411CBB: req_get_next_device_request (device.c:690)
==27752==    by 0x43FDD6: call_req_handler (request.c:247)
==27752==    by 0x43FF6B: read_request (request.c:281)
==27752==    by 0x4484A0: thread_poll_event (thread.c:267)
==27752==    by 0x414E57: fd_poll_event (fd.c:446)
==27752==    by 0x415231: main_loop_epoll (fd.c:541)
==27752==    by 0x415843: main_loop (fd.c:886)
==27752==    by 0x420CCC: main (main.c:148)
...
--- snip ---

I'm attaching full log later.

$ wine --version
wine-1.7.45-62-g46bdb6e

Regards

[Bug 38764] Improper device request/IRP handling causes heap corruption in wineserver

https://bugs.winehq.org/show_bug.cgi?id=38764

--- Comment #1 from Anastasius Focht  ---
Created attachment 51698
  --> https://bugs.winehq.org/attachment.cgi?id=51698
valgrind runs on wineserver showing heap corruption

Hello folks,

attached are two valgrind runs with heap corruption.

It doesn't happen every time .. I had a "success" rate 1 out of 10 times.

Regards

[Bug 38764] Improper device request/IRP handling causes heap corruption in wineserver

https://bugs.winehq.org/show_bug.cgi?id=38764

Sebastian Lackner  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #2 from Sebastian Lackner  ---
Created attachment 51699
  --> https://bugs.winehq.org/attachment.cgi?id=51699
Proposed patch

The problem occurs because the set_irp_result function assumes, that irp->file
has a refcount greater than 1, which is not always the case.

The call to 'release_object( file );' can destroy the associated file, but
later 'list_remove( &irp->dev_entry );' is executed which assumes that the file
still exists.

After ensuring that the patch doesn't have any unintentional side effects I'll
send it to wine-patches.

[Bug 38764] Improper device request/IRP handling causes heap corruption in wineserver

https://bugs.winehq.org/show_bug.cgi?id=38764

Sebastian Lackner  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch

[Bug 38764] Improper device request/IRP handling causes heap corruption in wineserver

https://bugs.winehq.org/show_bug.cgi?id=38764

--- Comment #3 from Sebastian Lackner  ---
Should be fixed with
http://source.winehq.org/git/wine.git/commit/ece08c10a162f429262a56b76a9a167...,
please retest.

[Bug 38764] Improper device request/IRP handling causes heap corruption in wineserver

https://bugs.winehq.org/show_bug.cgi?id=38764

Anastasius Focht  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
      Fixed by SHA1|                            |ece08c10a162f429262a56b76a9
                   |                            |a1675cf48baea
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #4 from Anastasius Focht  ---
Hello folks,

this is fixed by commit
https://source.winehq.org/git/wine.git/commitdiff/ece08c10a162f429262a56b76a...

Thanks     Sebastian

(tested with 20 times start of "Deus Ex: Invisible War" with
https://raw.githubusercontent.com/wine-compholio/wine-staging/master/patches...
applied)

Regards

[Bug 38764] Improper device request/IRP handling causes heap corruption in wineserver

https://bugs.winehq.org/show_bug.cgi?id=38764

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #5 from Alexandre Julliard  ---
Closing bugs fixed in 1.7.46.