[PATCH 0/1] MR10793: cfgmgr32: Catch invalid handles in CM_Unregister_Notification.

[Tim Clem (＠tclem)]

This is effectively the cfgmgr32 version of !10648. And weirdly again it's Valve software having issues. Since CM_[Un]register_Notification was implemented in !7615, Team Fortress 2 and Counter-Strike 2 have been crashing on exit trying to unregister an invalid HCMNOTIFICATION. My investigation really makes it seem like that's their bug, not ours. And either way, as with HDEVNOTIFYs, unregistering invalid HCMNOTIFICATIONs doesn't crash on native.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10793

[PATCH 1/1] cfgmgr32: Catch invalid handles in CM_Unregister_Notification.

[Tim Clem]

From: Tim Clem <[e-mail]>

---
 dlls/cfgmgr32/notification.c   | 25 ++++++++++++++++++++++---
 dlls/cfgmgr32/tests/cfgmgr32.c | 10 ++++++++++
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/dlls/cfgmgr32/notification.c b/dlls/cfgmgr32/notification.c
index e0b3893430f..12ed121f3d8 100644
--- a/dlls/cfgmgr32/notification.c
+++ b/dlls/cfgmgr32/notification.c
@@ -18,6 +18,7 @@
  */
 
 #include "cfgmgr32_private.h"
+#include "wine/exception.h"
 
 WINE_DEFAULT_DEBUG_CHANNEL(setupapi);
 
@@ -41,8 +42,11 @@ static const char *debugstr_CM_NOTIFY_FILTER( const CM_NOTIFY_FILTER *filter )
     }
 }
 
+#define CM_NOTIFY_CONTEXT_MAGIC 0xbeef4dad
+
 struct cm_notify_context
 {
+    DWORD magic;
     HDEVNOTIFY notify;
     void *user_data;
     PCM_NOTIFY_CALLBACK callback;
@@ -155,6 +159,7 @@ static CONFIGRET create_notify_context( const CM_NOTIFY_FILTER *filter, HCMNOTIF
 
     if (!(ctx = calloc( 1, sizeof(*ctx) ))) return CR_OUT_OF_MEMORY;
 
+    ctx->magic = CM_NOTIFY_CONTEXT_MAGIC;
     ctx->user_data = user_data;
     ctx->callback = callback;
     if (!(ctx->notify = I_ScRegisterDeviceNotification( ctx, ¬ify_filter.header, devnotify_callback )))
@@ -191,13 +196,27 @@ CONFIGRET WINAPI CM_Register_Notification( CM_NOTIFY_FILTER *filter, void *conte
 CONFIGRET WINAPI CM_Unregister_Notification( HCMNOTIFICATION notify )
 {
     struct cm_notify_context *ctx = notify;
+    CONFIGRET ret = CR_SUCCESS;
 
     TRACE( "(%p)\n", notify );
 
     if (!notify) return CR_INVALID_DATA;
 
-    I_ScUnregisterDeviceNotification( ctx->notify );
-    free( ctx );
+    __TRY
+    {
+        if (ctx->magic == CM_NOTIFY_CONTEXT_MAGIC)
+        {
+            I_ScUnregisterDeviceNotification( ctx->notify );
+            free( ctx );
+        }
+        else
+            ret = CR_INVALID_DATA;
+    }
+    __EXCEPT_PAGE_FAULT
+    {
+        ret = CR_FAILURE;
+    }
+    __ENDTRY
 
-    return CR_SUCCESS;
+    return ret;
 }
diff --git a/dlls/cfgmgr32/tests/cfgmgr32.c b/dlls/cfgmgr32/tests/cfgmgr32.c
index b5af953d229..75fb67b7eed 100644
--- a/dlls/cfgmgr32/tests/cfgmgr32.c
+++ b/dlls/cfgmgr32/tests/cfgmgr32.c
@@ -431,6 +431,16 @@ static void test_CM_Register_Notification( void )
         }
         winetest_pop_context();
     }
+
+    /* Some unregister edge cases */
+    ret = pCM_Unregister_Notification( NULL );
+    ok( ret == CR_INVALID_DATA, "Expected CR_INVALID_DATA, got %#lx\n", ret );
+
+    ret = pCM_Unregister_Notification( (HCMNOTIFICATION)"valid pointer but not a handle" );
+    ok( ret == CR_INVALID_DATA, "Expected CR_INVALID_DATA, got %#lx\n", ret );
+
+    ret = pCM_Unregister_Notification( (HCMNOTIFICATION)0xdeadbeef );
+    ok( ret == CR_FAILURE, "Expected CR_FAILURE, got %#lx\n", ret );
 }
 
 static void check_device_path_casing(const WCHAR *original_path)
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/10793