[Bug 15551] New: SafeDisc regression

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

Summary: SafeDisc regression
           Product: Wine
           Version: 1.1.5
          Platform: Other
        OS/Version: other
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: ntoskrnl
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]

Created an attachment (id=16537)
 --> (http://bugs.winehq.org/attachment.cgi?id=16537)
backtrace

commit 2e917e3a3c82e3f3cb7d9a5980c523418de12908
Author: Alexander Morozov 
Date:   Fri Oct 3 17:51:26 2008 +0400

ntoskrnl.exe: Implement some functions.

causes a regression with secdrv.sys breaking at least Battlefield 1942 causing
a crash on start up.

doing a git-revert 2e917e3a3c82e3f3cb7d9a5980c523418de12908 makes it work
again.

gcc 4.2.3, Ubuntu 8.04

[Bug 15551] SafeDisc regression

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

Jeff Zaroyko  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |regression

--- Comment #1 from Jeff Zaroyko   2008-10-08 06:28:08 ---
Author of the patch does not appear to be registered in bugzilla - can't add
cc.

[Bug 15551] SafeDisc regression

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

Jeff Zaroyko  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |obfuscation

[Bug 15551] SafeDisc regression

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

Alexander Morozov  changed:

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

--- Comment #2 from Alexander Morozov   2008-10-08 07:25:34 ---
Please, show log with WINEDEBUG=+ntoskrnl
Also you can try

diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c
index 9af2f16..a3fcca2 100644
--- a/dlls/ntoskrnl.exe/ntoskrnl.c
+++ b/dlls/ntoskrnl.exe/ntoskrnl.c
@@ -145,7 +145,6 @@ static NTSTATUS process_ioctl( DEVICE_OBJECT *device, 
ULONG code, void *in_buff,

/* so we can spot things that we should initialize */
     memset( &irp, 0x55, sizeof(irp) );
-    memset( &irpsp, 0x66, sizeof(irpsp) );
     memset( &mdl, 0x77, sizeof(mdl) );

irp.RequestorMode = UserMode;

or

diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c
index 9af2f16..fc43820 100644
--- a/dlls/ntoskrnl.exe/ntoskrnl.c
+++ b/dlls/ntoskrnl.exe/ntoskrnl.c
@@ -160,6 +160,7 @@ static NTSTATUS process_ioctl( DEVICE_OBJECT *device, 
ULONG code, void *in_buff,
     irpsp.Parameters.DeviceIoControl.IoControlCode = code;
     irpsp.Parameters.DeviceIoControl.Type3InputBuffer = in_buff;
     irpsp.DeviceObject = device;
+    irpsp.CompletionRoutine = NULL;

mdl.Next = NULL;
     mdl.Size = 0;

[Bug 15551] SafeDisc regression

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

--- Comment #3 from Jeff Zaroyko   2008-10-08 17:24:27 ---
Created an attachment (id=16545)
 --> (http://bugs.winehq.org/attachment.cgi?id=16545)
+ntoskrnl log for original crash

[Bug 15551] SafeDisc regression

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

--- Comment #4 from Jeff Zaroyko   2008-10-08 17:27:39 ---
Created an attachment (id=16546)
 --> (http://bugs.winehq.org/attachment.cgi?id=16546)
+ntoskrnl log with first patch applied

[Bug 15551] SafeDisc regression

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

--- Comment #5 from Jeff Zaroyko   2008-10-08 17:28:28 ---
Created an attachment (id=16547)
 --> (http://bugs.winehq.org/attachment.cgi?id=16547)
+ntoskrnl log with just second patch applied

[Bug 15551] SafeDisc regression

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

Jim Cameron  changed:

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

--- Comment #6 from Jim Cameron   2008-10-08 17:36:47 ---
Confirmed, also breaks Myst IV.

[Bug 15551] SafeDisc regression

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

--- Comment #7 from Vitaliy Margolen   2008-10-08 20:22:08 ---
Created an attachment (id=16550)
 --> (http://bugs.winehq.org/attachment.cgi?id=16550)
A possible fix

This fixes it for me, but of course it's just a hack that identifies what all
needs to be initialized. I'm not sure what is the meaning of the UserIosb. But
something tells me that shouldn't be just NULL.

[Bug 15551] SafeDisc regression

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

Vitaliy Margolen  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |major
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
         OS/Version|other                       |Linux
           Platform|Other                       |PC-x86-64

--- Comment #8 from Vitaliy Margolen   2008-10-08 20:22:42 ---
Confirming.

[Bug 15551] SafeDisc regression

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

Austin English  changed:

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

[Bug 15551] SafeDisc regression

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

--- Comment #9 from Alexander Morozov   2008-10-09 03:48:58 ---
Created an attachment (id=16554)
 --> (http://bugs.winehq.org/attachment.cgi?id=16554)
Possible fix

On Win 2003 UserIosb is not NULL. But I do not understand what is saved in
UserIosb subfields in IRP which is sent to IOCTL handler (may be these values
are random).

Also I think that using irp.UserIosb = NULL is also acceptable decision.

[Bug 15551] SafeDisc regression

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

--- Comment #10 from Jim Cameron   2008-10-09 15:42:30 ---
Alexander's patch works for me.

[Bug 15551] SafeDisc regression

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

--- Comment #11 from Jeff Zaroyko   2008-10-09 17:00:14 ---
(In reply to comment #10)
> Alexander's patch works for me.

Same here.

[Bug 15551] SafeDisc regression

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

--- Comment #12 from Vitaliy Margolen   2008-10-10 08:29:45 ---
(In reply to comment #9)
According to this article:
http://www.tech-archive.net/Archive/Development/microsoft.public.development...

UserIosb does not make much sense in Wine. So I suggest removing the code that
uses it all together from IofCompleteRequest. Besides process_ioctl() does not
use UserIosb anyway but the irp->IoStatus.u.Status.

[Bug 15551] SafeDisc regression

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

--- Comment #13 from Alexander Morozov   2008-10-10 08:51:57 ---
UserIosb is also used in IoBuildDeviceIoControlRequest. IofCompleteRequest
copies IoStatus to UserIosb and so driver which called
IoBuildDeviceIoControlRequest can get information about IRP processing.

[Bug 15551] SafeDisc regression

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

--- Comment #14 from Vitaliy Margolen   2008-10-10 11:22:32 ---
(In reply to comment #13)
Which again questionable. Where all that code came from?

[Bug 15551] SafeDisc regression

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

Austin English  changed:

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

--- Comment #15 from Austin English   2008-10-11 17:37:25 ---
*** Bug 12653 has been marked as a duplicate of this bug. ***

[Bug 15551] SafeDisc regression

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

Vitaliy Margolen  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |12653

[Bug 15551] SafeDisc regression

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

Vitaliy Margolen  changed:

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

--- Comment #16 from Vitaliy Margolen   2008-10-12 14:53:03 ---
*** Bug 15597 has been marked as a duplicate of this bug. ***

[Bug 15551] SafeDisc regression

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

--- Comment #17 from Alexander Morozov   2008-10-13 02:43:04 ---
> (In reply to comment #13)
> Which again questionable. Where all that code came from?
> From commit 2e917e3a3c82e3f3cb7d9a5980c523418de12908. See descripton of this

bug.

How was this code created? It was created with using tests (see attachment).

In "[7/9] ntoskrnl.exe: Improve process_ioctl." (see wine-patches)
IoAllocateIrp was used for creating IRP and UserIosb was NULL, so the problem
described in this bug was not. But this patch was not applied.

[Bug 15551] SafeDisc regression

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

--- Comment #18 from Alexander Morozov   2008-10-13 02:43:59 ---
Created an attachment (id=16616)
 --> (http://bugs.winehq.org/attachment.cgi?id=16616)
ntoskrnl.exe tests

[Bug 15551] SafeDisc regression

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

--- Comment #19 from Vitaliy Margolen   2008-10-13 09:09:02 ---
I'm more asking what prompted you to use UserIosb if you don't know what's it
for?

Anyway, I think the correct fix is to set it to NULL. Because the APC that it
should be used in (ioctl_completion) works different in Wine and gets it's
result from the server.

[Bug 15551] SafeDisc regression

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

--- Comment #20 from Alexander Morozov   2008-10-13 09:24:47 ---
> Anyway, I think the correct fix is to set it to NULL. Because the APC that it
> should be used in (ioctl_completion) works different in Wine and gets it's
> result from the server.

Well, let`s make so.

[Bug 15551] SafeDisc regression

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

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |testcase

[Bug 15551] SafeDisc regression

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

Austin English  changed:

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

--- Comment #21 from Austin English   2008-10-14 21:42:32 ---
*** Bug 15621 has been marked as a duplicate of this bug. ***

[Bug 15551] SafeDisc regression

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

Jeff Zaroyko  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #22 from Jeff Zaroyko   2008-10-16 06:12:37 ---
patch by Alexander is now in git

[Bug 15551] SafeDisc regression

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

Alexandre Julliard  changed:

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

--- Comment #23 from Alexandre Julliard   2008-10-24 11:14:05 ---
Closing bugs fixed in 1.1.7.