Re: ntdll: Fixed LdrUnloadDll may recursion unload same dll

[Sebastian Lackner]

On 09.02.2017 10:39, 谢威 wrote:
> From 3b5efb522f3ef46b009e6a159a605e087e8841a9 Mon Sep 17 00:00:00 2001
> From: Wei xie 
> Date: Thu, 19 Jan 2017 14:55:44 +0800
> Subject: [PATCH] ntdll: Fixed LdrUnloadDll may recursion unload same dll
> Signed-off-by: Wei xie
>
> dlls/ntdll/loader.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c
> index 74feb97..8762917 100644
> --- a/dlls/ntdll/loader.c
> +++ b/dlls/ntdll/loader.c
> @@ -64,7 +64,7 @@ WINE_DECLARE_DEBUG_CHANNEL(pid);
>  typedef DWORD (CALLBACK *DLLENTRYPROC)(HMODULE,DWORD,LPVOID);
>
> static BOOL process_detaching = FALSE;  /* set on process detach to avoid deadlocks with thread detach */
> -static int free_lib_count;   /* recursion depth of LdrUnloadDll calls */
> +static int free_lib_count = 0;   /* recursion depth of LdrUnloadDll calls */

This isn't really necessary, all static variables are initialized to zero.
> static const char * const reason_names[] =
>  {
> @@ -2859,7 +2859,7 @@ static void MODULE_FlushModrefs(void)
> -
> - The loader_section must be locked while calling this function.
>
> */
> -static void MODULE_DecRefCount( WINE_MODREF *wm )
> +static void MODULE_DecRefCount( WINE_MODREF *wm , BOOL bDeps)
>  {
>      int i;
>
> @@ -2872,13 +2872,13 @@ static void MODULE_DecRefCount( WINE_MODREF *wm )
>      --wm->ldr.LoadCount;
>      TRACE("(%s) ldr.LoadCount: %d\n", debugstr_w(wm->ldr.BaseDllName.Buffer), wm->ldr.LoadCount );
> - if ( wm->ldr.LoadCount == 0 )
> -
>
> if ( wm->ldr.LoadCount == 0 && bDeps)
> {
>     wm->ldr.Flags |= LDR_UNLOAD_IN_PROGRESS;
> for ( i = 0; i < wm->nDeps; i++ )
>     if ( wm->deps[i] )
> -
>            MODULE_DecRefCount( wm->deps[i] );
> -
>            MODULE_DecRefCount( wm->deps[i] , TRUE );
>
>    wm->ldr.Flags &= ~LDR_UNLOAD_IN_PROGRESS;
>
> }
>
> @@ -2893,6 +2893,7 @@ NTSTATUS WINAPI LdrUnloadDll( HMODULE hModule )
>  {
>      WINE_MODREF *wm;
>      NTSTATUS retv = STATUS_SUCCESS;
> -
>
> static HMODULE first_recursion_module = 0;
>
> if (process_detaching) return retv;
>
> @@ -2900,13 +2901,19 @@ NTSTATUS WINAPI LdrUnloadDll( HMODULE hModule )
>  RtlEnterCriticalSection( &loader_section );
>
> -
>
> if (0 == free_lib_count)
> -
>    first_recursion_module = hModule;
> -
>
> free_lib_count++;
>    if ((wm = get_modref( hModule )) != NULL)
>    {
>        TRACE("(%s) - START\n", debugstr_w(wm->ldr.BaseDllName.Buffer));
>    /* Recursively decrement reference counts */
> -
>    MODULE_DecRefCount( wm );
> -
>    if (0 != free_lib_count && first_recursion_module == hModule)
> -
>        MODULE_DecRefCount( wm , FALSE );
> -
>    else
> -
>        MODULE_DecRefCount( wm , TRUE );

At the location above free_lib_count will always be != 0 - this means that
your patch basically disables unloading of dependencies completely. Do you
have a small testcase for the problem you are trying to fix?
>      /* Call process detach notifications */
>      if ( free_lib_count <= 1 )
>
> @@ -2922,6 +2929,9 @@ NTSTATUS WINAPI LdrUnloadDll( HMODULE hModule )
>  free_lib_count--;
>
> -
>
> if (0 == free_lib_count)
> -
>    first_recursion_module = 0;
> -
>
> RtlLeaveCriticalSection( &loader_section );
>
> return retv;
>
> -- 2.10.2