[Bug 10225] New: SafeDisc 2. x triggers async device ioctl status code propagation bug in wine server

http://bugs.winehq.org/show_bug.cgi?id=10225

Summary: SafeDisc 2.x triggers async device ioctl status code
                    propagation bug in wine server
           Product: Wine
           Version: CVS/GIT
          Platform: PC
        OS/Version: Linux
            Status: UNCONFIRMED
          Severity: major
          Priority: P2
         Component: wine-kernel
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]

Created an attachment (id=8843)
 --> (http://bugs.winehq.org/attachment.cgi?id=8843)
relevant trace which shows ioctl status code propagation bug

Hello,

while testing my proof-of-concept patch which makes SafeDisc v2 (yuck!) finally
work, I stumbled across a nasty ioctl status code propagation (completion) bug
in wine server.
Wasted several hours on it .. I was suspecting SD issue whole time, but its a
wine server bug :(

Attached is relevant trace (WINEDEBUG=+seh,+tid,+relay,+ntoskrnl,+server wine
./BfVietnam.exe)

Before the bug is triggered, the SafeDisc security driver makes several device
ioctl requests which return 0xC0000001 (unsuccessful) on purpose, e.g. reading
debug Drx registers with index that does not exist.
This is intended to fool reversers :-)

The next operation - checking IDT entries should succeed - but does not (as
seen from calling client).
The kernel driver completes the operation successfully and returns
irp.IoStatus.u.Status == 0 with xxxx bytes in output buffer.
I verified it several times by debugging the kernel driver.

Upon driver ioctl call return, get_next_device_request() is triggered again in
ntoskrnl event loop which sets the return data (and status code) ->
set_ioctl_result().
This queues an APC, waking up the client (alertable state).

The client process device ioctl completes with 0 (FALSE) which is wrong.
Further debugging showed that ioctl_completion APC (dlls/ntdll/file.c) returns
the errornous status code in get_ioctl_result() when woken up by server.
This error is then propagated in server_ioctl_file() to result in wrong
NtDeviceIoControlFile() return code.

What puzzled me is that "ioctl->status" is correctly set by set_ioctl_result()
(=0) but get_ioctl_result() - called within client context - yields the
previous error code (0xC0000001 of failed request).

This is either a problem of global vs. thread local error status (global_error
vs. current->error/thread) or the ioctl call data is the wrong one.

The following patch works for me, letting SD 2.x continue - though i'm not sure
if it's right:

--- snip ---

diff --git a/server/device.c b/server/device.c
index 46b2796..8fe8f7c 100644
--- a/server/device.c
+++ b/server/device.c
@@ -528,7 +528,7 @@ DECL_HANDLER(get_ioctl_result)
                 ioctl->out_data = NULL;
             }
         }
-        set_error( ioctl->status );
+
         list_remove( &ioctl->dev_entry );
         release_object( ioctl );  /* no longer on the device queue */
     }

--- snip ---

Regards