[PATCH] user32/menu: Inside HiliteMenuItem free menu earlier to avoid crash

[Fabian Maurer]

MENU_SelectItem sends a message,
and we must not hold the lock when that happens

Fixes bug 45457.

Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de>
---
 dlls/user32/menu.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/dlls/user32/menu.c b/dlls/user32/menu.c
index bae45002b7..cd6e9fff2a 100644
--- a/dlls/user32/menu.c
+++ b/dlls/user32/menu.c
@@ -3859,17 +3859,21 @@ BOOL WINAPI HiliteMenuItem( HWND hWnd, HMENU hMenu, UINT wItemID,
 {
     POPUPMENU *menu;
     UINT pos;
+    HMENU handle_menu;
 
     TRACE("(%p, %p, %04x, %04x);\n", hWnd, hMenu, wItemID, wHilite);
 
     if (!(menu = find_menu_item(hMenu, wItemID, wHilite, &pos))) return FALSE;
 
+    handle_menu = menu->obj.handle;
+    release_menu_ptr(menu);
+
     if (menu->FocusedItem != pos)
     {
-        MENU_HideSubPopups( hWnd, menu->obj.handle, FALSE, 0 );
-        MENU_SelectItem( hWnd, menu->obj.handle, pos, TRUE, 0 );
+        MENU_HideSubPopups( hWnd, handle_menu, FALSE, 0 );
+        MENU_SelectItem( hWnd, handle_menu, pos, TRUE, 0 );
     }
-    release_menu_ptr(menu);
+
     return TRUE;
 }
 
-- 
2.18.0

[Dmitry Timoshkov]

Fabian Maurer <dark.shadow4(a)web.de> wrote:
> if (!(menu = find_menu_item(hMenu, wItemID, wHilite, &pos))) return FALSE;
>
> +    handle_menu = menu->obj.handle;
> +    release_menu_ptr(menu);
> +
>      if (menu->FocusedItem != pos)
>      {

Now you are accessing the menu contents after the release_menu_ptr().

-- 
Dmitry.

[Fabian Maurer]

On Freitag, 20. Juli 2018 16:00:11 CEST Dmitry Timoshkov wrote:
> Now you are accessing the menu contents after the 
> release_menu_ptr().

Where? I'm passing the handle, and that should be fine.

MENU_HideSubPopups and MENU_SelectItem are using 
MENU_GetMenu to get the structure again, I don't see an issue here.

Regards,
Fabian Maurer

[Fabian Maurer]

Never mind,
sending and updated patch. Completely missed 
that.

Regards,
Fabian Maurer