[Bug 16946] New: Calling CloseServiceHandle() multiple times on same client side service handle corrupts heap

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

Summary: Calling CloseServiceHandle() multiple times on same
                    client side service handle corrupts heap
           Product: Wine
           Version: 1.1.12
          Platform: Other
               URL: http://www.nbc.com/Video/NBCDirectInstaller.exe
        OS/Version: other
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: advapi32
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]

Hello,

I've seen this some time ago but forgot about it until encountering it again
recently while investigating bug 16626 (this is follow up bug).

Prerequisite:

--- snip ---
$ rm -rf ~/.wine
$ winetricks -q wmp10 dotnet20 mdac28
$ wine NBCDirectInstaller.exe 
--- snip ---

The installer uses managed (.NET) wrapper for SCM to fetch a list of current
services using EnumServicesStatus().
First call to EnumServicesStatus() to query for buffer size and second time to
get the real list.
Although M$ .NET Framework error handling is somewhat questionable (it doesn't
even check for failure on first call), it's actually Wine's management of
client side service handles that causes the breakage.

--- snip ---
...
0009:Call advapi32.OpenSCManagerW(00000000,00000000,00000004) ret=03f72620
0009:trace:service:OpenSCManagerW ((null),(null),0x00000004)
0009:trace:service:sc_handle_alloc sc_handle type=0 -> 0x1b1fc0 
...
0013:trace:service:svcctl_OpenSCManagerW ((null), (null), 4) 
...
0009:trace:service:OpenSCManagerW returning 0x1b1fc0 (access : 0x00000004)
0009:Ret  advapi32.OpenSCManagerW() retval=001b1fc0 ret=03f72620 
...
0009:Call
advapi32.EnumServicesStatusW(001b1fc0,00000030,00000003,00000000,00000000,0032f32c,0032f328,0032f324)
ret=03f726df
0009:fixme:service:EnumServicesStatusW 0x1b1fc0 type=30 state=3 (nil) 0
0x32f32c 0x32f328 0x32f324
0009:Ret  advapi32.EnumServicesStatusW() retval=00000000 ret=03f726df
0009:Call KERNEL32.GetLastError() ret=03f726e5
0009:Ret  KERNEL32.GetLastError() retval=00000005 ret=03f726e5
0009:Call KERNEL32.LocalAlloc(00000000,00000000) ret=04248243
0009:Ret  KERNEL32.LocalAlloc() retval=03ec9988 ret=04248243
0009:Call
advapi32.EnumServicesStatusW(001b1fc0,00000030,00000003,03ec9988,00000000,0032f32c,0032f328,0032f324)
ret=03f726df
0009:fixme:service:EnumServicesStatusW 0x1b1fc0 type=30 state=3 0x3ec9988 0
0x32f32c 0x32f328 0x32f324
0009:Ret  advapi32.EnumServicesStatusW() retval=00000000 ret=03f726df
0009:Call KERNEL32.GetLastError() ret=03f726e5
0009:Ret  KERNEL32.GetLastError() retval=00000005 ret=03f726e5 
...
0009:Call advapi32.CloseServiceHandle(001b1fc0) ret=007aa722
0009:trace:service:CloseServiceHandle 0x1b1fc0 
...
0009:trace:service:sc_handle_destroy_manager destroying SC Manager 0x1b1fc0
0009:Ret  advapi32.CloseServiceHandle() retval=00000001 ret=007aa722 
...
--- snip ---

Releasing the same client handle again:

--- snip ---
0009:Call advapi32.CloseServiceHandle(001b1fc0) ret=007aa722
0009:trace:service:CloseServiceHandle 0x1b1fc0
0009:Call rpcrt4.NdrClientInitializeNew(0032ef8c,0032f06c,60376640,00000000)
ret=60366f60
0009:Ret  rpcrt4.NdrClientInitializeNew() retval=0032f06c ret=60366f60
0009:Call rpcrt4.NdrGetBuffer(0032f06c,00000018,00000000) ret=60366fbe
0009:err:rpc:I_RpcGetBuffer no binding
0009:Call KERNEL32.RaiseException(000006a6,00000000,00000000,00000000)
ret=60b21399
0009:trace:seh:raise_exception code=6a6 flags=0 addr=0x7b844f6c ip=0x7b844fef
tid=0009
0009:trace:seh:raise_exception  eax=7b82cca9 ebx=7b8c3940 ecx=00000000
edx=00000010 esi=0032ee90 edi=0032ee10
0009:trace:seh:raise_exception  ebp=0032edf8 esp=0032ed94 cs=0023 ds=002b
es=002b fs=0063 gs=006b flags=00000246
0009:trace:seh:call_stack_handlers calling handler at 0x60366cec code=6a6
flags=0
0009:trace:seh:call_stack_handlers handler at 0x60366cec returned 1
0009:trace:seh:call_stack_handlers calling handler at 0x6036266f code=6a6
flags=0
0009:Call rpcrt4.I_RpcExceptionFilter(000006a6) ret=60360fdd
0009:Ret  rpcrt4.I_RpcExceptionFilter() retval=00000001 ret=60360fdd
0009:trace:seh:__regs_RtlUnwind code=6a6 flags=2
0009:trace:seh:__regs_RtlUnwind calling handler at 0x7bc3b2ec code=6a6 flags=2
0009:trace:seh:__regs_RtlUnwind handler at 0x7bc3b2ec returned 1
0009:trace:seh:__regs_RtlUnwind calling handler at 0x60366cec code=6a6 flags=2
0009:Call rpcrt4.NdrFreeBuffer(0032f06c) ret=60366e7f
0009:Ret  rpcrt4.NdrFreeBuffer() retval=00000000 ret=60366e7f
0009:trace:seh:__regs_RtlUnwind handler at 0x60366cec returned 1
0009:Ret  advapi32.CloseServiceHandle() retval=00000000 ret=007aa722 
--- snip ---

Even if RPC code correctly detects invalid SCM handles, the latter
sc_handle_free() will eventually corrupt heap which will later lead to
breakage.
This needs to be avoided at all costs.

Second allocation returns garbage sub block due to corrupted heap lists,
causing exception.

--- snip ---
...
0009:Call ntdll.RtlAllocateHeap(00110000,00000000,0000001c) ret=79e78360
0009:Ret  ntdll.RtlAllocateHeap() retval=001b1fc0 ret=79e78360
0009:Call KERNEL32.GetLastError() ret=79e71862
0009:Ret  KERNEL32.GetLastError() retval=000006a6 ret=79e71862
0009:Call ntdll.RtlTryEnterCriticalSection(0013cedc) ret=79e74811
0009:Ret  ntdll.RtlTryEnterCriticalSection() retval=00000001 ret=79e74811
0009:Call KERNEL32.GetLastError() ret=79e783fb
0009:Ret  KERNEL32.GetLastError() retval=000006a6 ret=79e783fb
0009:Call ntdll.RtlAllocateHeap(00110000,00000000,0000003c) ret=79e78360
0009:Ret  ntdll.RtlAllocateHeap() retval=03ec9988 ret=79e78360
0009:Call KERNEL32.GetLastError() ret=79e71862
0009:Ret  KERNEL32.GetLastError() retval=000006a6 ret=79e71862
0009:Call KERNEL32.InitializeCriticalSection(03ec9998) ret=79e7a501
0009:trace:seh:raise_exception code=c0000005 flags=0 addr=0x7bc450bd
ip=0x7bc450bd tid=0009
0009:trace:seh:raise_exception  info[0]=00000000
0009:trace:seh:raise_exception  info[1]=00001796
0009:trace:seh:raise_exception  eax=00001796 ebx=7bc9bd90 ecx=7bc9bd90
edx=00110078 esi=00110070 edi=0032f0e4
0009:trace:seh:raise_exception  ebp=0032efe8 esp=0032ef80 cs=0023 ds=002b
es=002b fs=0063 gs=006b flags=00010206
0009:trace:seh:call_stack_handlers calling handler at 0x7a322c04 code=c0000005
flags=0 
--- snip ---

--- snip dlls/advapi32/service.c ---
BOOL WINAPI
CloseServiceHandle( SC_HANDLE hSCObject )
{
    struct sc_handle *obj;
    DWORD err;

TRACE("%p\n", hSCObject);
    if (hSCObject == NULL)
    {
        SetLastError(ERROR_INVALID_HANDLE);
        return FALSE;
    }

obj = (struct sc_handle *)hSCObject;
    __TRY
    {
        err = svcctl_CloseServiceHandle(&obj->server_handle);
    }
    __EXCEPT(rpc_filter)
    {
        err = map_exception_code(GetExceptionCode());
    }
    __ENDTRY
    sc_handle_free( obj );  // <---- will eventually corrupt heap!

if (err != ERROR_SUCCESS)
    {
        SetLastError(err);
        return FALSE;
    }
    return TRUE;
} 
--- snip dlls/advapi32/service.c ---

Change the way how client side service handles are managed to avoid heap
corruption and allow for multiple service handle close calls.

Regards

[Bug 16946] Calling CloseServiceHandle() multiple times on same client side service handle corrupts heap

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

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |dotnet, download, Installer

[Bug 16946] Calling CloseServiceHandle() multiple times on same client side service handle corrupts heap

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

Austin English  changed:

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

--- Comment #1 from Austin English   2009-01-15 16:04:13 ---
*** Bug 16945 has been marked as a duplicate of this bug. ***

[Bug 16946] Calling CloseServiceHandle() multiple times on same client side service handle corrupts heap

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

Dan Kegel  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Target Milestone|---                         |1.2.0

--- Comment #2 from Dan Kegel   2009-01-16 12:55:51 ---
Understood, probably affects a number of apps -> nominating for 1.2.

[Bug 16946] Calling CloseServiceHandle() multiple times on same client side service handle corrupts heap

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

Anastasius Focht  changed:

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

--- Comment #3 from Anastasius Focht   2009-01-20 13:51:41 ---
Hello,

fixed by commit 540fe928424063b930c6076758822b382e734ad7

Regards

[Bug 16946] Calling CloseServiceHandle() multiple times on same client side service handle corrupts heap

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

Alexandre Julliard  changed:

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

--- Comment #4 from Alexandre Julliard   2009-01-30 11:04:57 ---
Closing bugs fixed in 1.1.14.

[Bug 16946] Calling CloseServiceHandle() multiple times on same client side service handle corrupts heap

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

Anastasius Focht  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
      Fixed by SHA1|                            |540fe928424063b930c60767588
                   |                            |22b382e734ad7

--- Comment #5 from Anastasius Focht  2011-10-12 03:13:38 CDT ---
Hello,

filling/correcting fields ...

Regards

[Bug 16946] Calling CloseServiceHandle() multiple times on same client side service handle corrupts heap

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

Anastasius Focht  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Hardware|Other                       |x86
                 OS|other                       |Linux

[Bug 16946] Calling CloseServiceHandle() multiple times on same client side service handle corrupts heap

[WineHQ Bugzilla]

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

Anastasius Focht  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|http://www.nbc.com/Video/NB |https://web.archive.org/web
                   |CDirectInstaller.exe        |/20080423004530/http://www.
                   |                            |nbc.com/Video/NBCDirectInst
                   |                            |aller.exe