[Bug 10293] New: sequentially running games/ apps with different SafeDisc versions fails
http://bugs.winehq.org/show_bug.cgi?id=10293
Summary: sequentially running games/apps with different SafeDisc versions fails Product: Wine Version: CVS/GIT Platform: PC OS/Version: Linux Status: UNCONFIRMED Severity: enhancement Priority: P2 Component: wine-kernel AssignedTo: wine-bugs@winehq.org ReportedBy: focht@gmx.net
Created an attachment (id=8945) --> (http://bugs.winehq.org/attachment.cgi?id=8945) Patch which fixes various isses regarding kmode driver unload/reload
Hello,
currently wine's kernel mode driver cleanup/unloading facility has several issues when it comes to applications/games that unload and reload different driver versions on the fly.
Example: run multiple SafeDisc 2.x, 3.x and 4.x games sequentially. Symptoms: game either hangs/crashes/exits silently
Usually the SD security driver is placed in "c:\windows\system32\drivers\secdrv.sys" If a SafeDisc protected program encounters a driver version that is incompatible it does the following:
- stop the current security service (should unload the driver) - unpack own security driver from resources to temp storage and move the binary to "c:\windows\system32\drivers\secdrv.sys", overwriting the previous - restart the service (loads the new driver)
This currently fails for various reasons:
(1)
The driver binary is loaded using LoadLibrary() -> the matching FreeLibrary() is missing on service stop. This leads to sharing violation when the driver binary going to be replaced (CopyFile fails).
(2)
No driver unload routine called. Very problematic. Various objects (symlinks) created by driver entry are not freed. When the driver is reloaded the init routine usually fails with "object exists".
(3)
Driver state objects (driver_obj, driver_extension) have undefined state due to static storage. Raises all sorts of problems because the driver entry routine does not init all fields.
---------
Attached is a patch which fixes these problems, allowing to sequentially run programs with different SafeDisc Versions.
Regards
http://bugs.winehq.org/show_bug.cgi?id=10293
--- Comment #1 from Alexandre Julliard julliard@winehq.org 2007-11-04 03:09:59 --- winedevice is supposed to exit when the service is stopped, which will take care of the cleanup.
http://bugs.winehq.org/show_bug.cgi?id=10293
--- Comment #2 from Anastasius Focht focht@gmx.net 2007-11-04 05:16:50 --- Created an attachment (id=8953) --> (http://bugs.winehq.org/attachment.cgi?id=8953) trace with WINEDEBUG=+tid,+seh,+loaddll,+ntoskrnl,+winedevice,+advapi,+relay
Hello,
--- quote --- winedevice is supposed to exit when the service is stopped, which will take care of the cleanup. --- quote ---
Hmm ... isn't winedevice supposed to be a "system process", which has special treatment in wine server? When a process is treated as "system process", it attaches an additional system-wide process event to wait the service request handler on (beside the ServiceMain threads). That means the pipe listening thread, handling service requests is kept alive until last user process exists (in this case the game).
So let me explain the situation again, maybe there was an misunderstanding...
A previous game start left an "incompatible" secdrv.sys in place - no processes running anymore. When the next game starts, it automagically starts the service, loading the wrong driver version. The process detects this and stops the service, replaces the binary, restarts service. While carrying out the service actions, the user process (game) is alive the whole time. Why should winedevice stop? It's a system process - by design...
So actually the service pipe listener is waiting - until another ServiceStart request arrives which results in ServiceMain thread created again in same winedevice.exe process.
Well, let me say few words about the "process exit" = "cure for all cleanup problems"... Although the wine server per process object directories are purged on process exit, I think it's good programming practice to have a clean shutdown sequence in winedevice too, allowing the kernel driver's Unload called. Just relying that process destroy cleans up all process objects/handle tables - including the leaked ones - to "fix" missing cleanups... uhm well. Having at least the driver unload called and unloading the binary _before_ user process exit sequence is a good thing (tm) to spot other bugs.
---
Attached is a trace log which shows the flow (with my patch applied so one can watch the whole show).
game, calling SCM api: 0x11
winedevice.exe:
main thread 0x18 pipe listener thread: 0x19 secdrv ServiceMain thread: 0x1A (terminates after SCM stop request)
secdrv ServiceMain: 0x1B
Regards
http://bugs.winehq.org/show_bug.cgi?id=10293
Luke Bratch l_bratch@yahoo.co.uk changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |l_bratch@yahoo.co.uk
http://bugs.winehq.org/show_bug.cgi?id=10293
--- Comment #3 from Alexandre Julliard julliard@winehq.org 2007-11-05 06:44:05 --- (In reply to comment #2)
Hmm ... isn't winedevice supposed to be a "system process", which has special treatment in wine server?
The system process thing is to make sure the process terminates when all user processes are done, but it doesn't prevent it from terminating earlier if it wants to.
So let me explain the situation again, maybe there was an misunderstanding...
I don't think there's any misunderstanding, I'm just pointing out that the right fix is to make winedevice exit and restart a new instance for the new driver, instead of trying to restart the winedevice instance from inside itself.
Well, let me say few words about the "process exit" = "cure for all cleanup problems"... Although the wine server per process object directories are purged on process exit, I think it's good programming practice to have a clean shutdown sequence in winedevice too, allowing the kernel driver's Unload called. Just relying that process destroy cleans up all process objects/handle tables - including the leaked ones - to "fix" missing cleanups... uhm well.
Calling the Unload function can certainly be done, though it's not clear that it's actually necessary in any of the drivers I've seen. But things like closing handles or freeing libraries just before exiting the process is a waste of time.
http://bugs.winehq.org/show_bug.cgi?id=10293
Benedikt Morbach benedikt.morbach@googlemail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |benedikt.morbach@googlemail. | |com
http://bugs.winehq.org/show_bug.cgi?id=10293
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |obfuscation
http://bugs.winehq.org/show_bug.cgi?id=10293
Andrey Turkin andrey.turkin@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |andrey.turkin@gmail.com
--- Comment #4 from Andrey Turkin andrey.turkin@gmail.com 2008-09-29 15:34:48 --- Is this still an issue? StartServiceCtrlDispatcher guts changed substantially during last year.
http://bugs.winehq.org/show_bug.cgi?id=10293
Anastasius Focht focht@gmx.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |RESOLVED Resolution| |FIXED Version|CVS/GIT |0.9.48.
--- Comment #5 from Anastasius Focht focht@gmx.net 2009-01-17 05:53:15 --- Hello,
corrected version tag and marking this fixed. Though kernel driver cleanup procedure still remains questionable.
The commit to make the kernel drivers service hosting process (winedevice) properly exit was most likely c673b2284d03c17154ccc977203cbc6d05a95e69
Regards
http://bugs.winehq.org/show_bug.cgi?id=10293
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #6 from Alexandre Julliard julliard@winehq.org 2009-01-30 11:03:44 --- Closing bugs fixed in 1.1.14.
http://bugs.winehq.org/show_bug.cgi?id=10293
Anastasius Focht focht@gmx.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |c673b2284d03c17154ccc977203 | |cbc6d05a95e69
--- Comment #7 from Anastasius Focht focht@gmx.net 2011-10-12 04:18:30 CDT --- Hello,
filling/correcting fields ...
Regards
-
wine-bugs＠winehq.org