search

DynamoRIO / drmemory

Memory Debugger for Windows, Linux, Mac, and Android
Other
2.42k stars 259 forks source link

Possible false positive report on NtUserMessageCall COPYDATASTRUCT.lpData #1329

Closed derekbruening closed 9 years ago

derekbruening commented 9 years ago

From zhao...@google.com on September 12, 2013 21:35:15

http://build.chromium.org/p/chromium.fyi/builders/Windows%20Unit%20%28DrMemory%20full%29%20%282%29/builds/405/steps/memory%20test%3A%20unit/logs/stdio [----------] 1 test from DisplaySettingsProviderWinTest [ RUN ] DisplaySettingsProviderWinTest.GetDesktopBarThicknessFromBounds Dr.M Dr.M Error #1: UNINITIALIZED READ: reading 0x0018f6fc-0x0018f700 4 byte(s) within 0x0018f6d0-0x0018f710 Dr.M # 0 system call NtUserMessageCall COPYDATASTRUCT.lpData
Dr.M # 1 USER32.dll!SendMessageW +0x4b (0x750096c5 <USER32.dll+0x196c5>) Dr.M # 2 SHELL32.dll!SHAppBarMessage +0xcb (0x75ce3304 <SHELL32.dll+0x13304>) Dr.M # 3 views.dll!views::GetTopmostAutoHideTaskbarForEdge [ui\views\widget\monitor_win.cc:32] Dr.M # 4 DisplaySettingsProviderWin::CheckTaskbars [chrome\browser\ui\panels\display_settings_provider_win.cc:147] Dr.M # 5 DisplaySettingsProviderWin::OnDisplaySettingsChanged [chrome\browser\ui\panels\display_settings_provider_win.cc:37] Dr.M # 6 DisplaySettingsProviderWinTest::MockDisplaySettingsProviderWin::MockDisplaySettingsProviderWin [chrome\browser\ui\panels\display_settings_provider_win_unittest.cc:17] Dr.M # 7 DisplaySettingsProviderWinTest_GetDesktopBarThicknessFromBounds_Test::TestBody [chrome\browser\ui\panels\display_settings_provider_win_unittest.cc:32] Dr.M # 8 testing::internal::HandleExceptionsInMethodIfSupportedtesting::Test,void [testing\gtest\src\gtest.cc:2051] Dr.M Note: @0:31:15.621 in thread 772

Original issue: http://code.google.com/p/drmemory/issues/detail?id=1329

derekbruening commented 9 years ago

From bruen...@google.com on September 13, 2013 11:52:51

Owner: bruen...@google.com

derekbruening commented 9 years ago

From bruen...@google.com on September 13, 2013 13:09:50

% ~/drmemory/git/build_x86_dbg/bin/drmemory.exe -pause_at_uninitialized -no_count_leaks -batch -dr d:/derek/dr/git/exports -- ./unit_tests.exe --gtest_filter=DisplaySettingsProviderWinTest.GetDesktopBarThicknessFromBounds Dr.M Dr. Memory version 1.6.1548 Dr.M Running "./unit_tests.exe --gtest_filter=DisplaySettingsProviderWinTest.GetDesktopBarThicknessFromBounds" Note: Google Test filter = DisplaySettingsProviderWinTest.GetDesktopBarThicknessFromBounds [==========] Running 1 test from 1 test case. [----------] Global test environment set-up. [----------] 1 test from DisplaySettingsProviderWinTest [ RUN ] DisplaySettingsProviderWinTest.GetDesktopBarThicknessFromBounds Dr.M Dr.M Error #1: UNINITIALIZED READ: reading 0x0014f714-0x0014f718 4 byte(s) within 0x0014f6e8-0x0014f728 Dr.M # 0 system call NtUserMessageCall COPYDATASTRUCT.lpData
Dr.M # 1 USER32.dll!SendMessageW Dr.M # 2 SHELL32.dll!SHAppBarMessage Dr.M # 3 views.dll!views::GetTopmostAutoHideTaskbarForEdge [e:\derek\chromium\src\ui\views\widget\monitor_win.cc:32] Dr.M # 4 DisplaySettingsProviderWin::CheckTaskbars [e:\derek\chromium\src\chrome\browser\ui\panels\display_settings_provider_win.cc:147] Dr.M # 5 DisplaySettingsProviderWin::OnDisplaySettingsChanged [e:\derek\chromium\src\chrome\browser\ui\panels\display_settings_provider_win.cc:37] Dr.M # 6 DisplaySettingsProviderWinTest_GetDesktopBarThicknessFromBounds_Test::TestBody [e:\derek\chromium\src\chrome\browser\ui\panels\display_settings_provider_win_unittest.cc:32] Dr.M # 7 testing::internal::HandleExceptionsInMethodIfSupportedtesting::Test,void [e:\derek\chromium\src\testing\gtest\src\gtest.cc:2051] Dr.M # 8 testing::Test::Run [e:\derek\chromium\src\testing\gtest\src\gtest.cc:2068] Dr.M # 9 testing::TestCase::Run [e:\derek\chromium\src\testing\gtest\src\gtest.cc:2351] Dr.M #10 testing::internal::UnitTestImpl::RunAllTests [e:\derek\chromium\src\testing\gtest\src\gtest.cc:4177] Dr.M #11 testing::internal::HandleExceptionsInMethodIfSupportedtesting::internal::UnitTestImpl,bool [e:\derek\chromium\src\testing\gtest\src\gtest.cc:2051] Dr.M Note: @0:00:20.057 in thread 3876

APPBARDATA taskbar_data = { sizeof(APPBARDATA), NULL, 0, edge }; HWND taskbar = reinterpret_cast(SHAppBarMessage(ABM_GETAUTOHIDEBAR, &taskbar_data)); http://msdn.microsoft.com/en-us/library/windows/desktop/bb762108(v=vs.85).aspx ABM_GETAUTOHIDEBAR (0x00000007) Retrieves the handle to the autohide appbar associated with a particular edge of the screen.

0:000> kb =@@(mc->ebp) @@(mc->esp) 0x770b96c5 ChildEBP RetAddr Args to Child
0014f68c 770b96c5 04a14f90 00000000 ff4e124c USER32!SendMessageW+0x7f 0014f6b0 75a73304 00010066 0000004a 00000000 USER32!SendMessageW+0x7f 0014f72c 6566bf6a 00000007 0014f744 072770cc SHELL32!SHAppBarMessage+0xfc 0014f76c 02dc2a11 00000003 00010001 e99e64d8 views!views::GetTopmostAutoHideTaskbarForEdge+0x4a [e:\derek\chromium\src\ui\views\widget\monitor_win.cc @ 32]

0:000> dt views!APPBARDATA 0014f744 +0x000 cbSize : 0x24 +0x004 hWnd : (null) +0x008 uCallbackMessage : 0 +0x00c uEdge : 3 +0x010 rc : tagRECT (== 0) +0x020 lParam : 0

0:000> dds @@(mc->esp) 0014f650 770c2161 USER32!SendMessageWorker+0x5e9 0014f654 00010066 hWnd 0014f658 0000004a Msg == WM_COPYDATA 0014f65c 00000000 wParam 0014f660 0014f6dc lParam 0014f664 00000000 ResultInfo 0014f668 000002b1 dwType 0014f66c 00000000 Ansi

0:000> dt drmemorylib!COPYDATASTRUCT 0014f6dc +0x000 dwData : 0 +0x004 cbData : 0x40 +0x008 lpData : 0x0014f6e8 0:000> dt safe Local var @ 0x2292ed0c Type tagCOPYDATASTRUCT +0x000 dwData : 0 +0x004 cbData : 0x40 +0x008 lpData : 0x0014f6e8 0:000> dd 0014f6e8 0014f6e8 00000028 00000000 00000000 00000003 0014f6f8 00000000 00000000 00000000 00000000 0014f708 00000000 00000000 00000007 00000000 <== this dword is uninit 0014f718 00000000 00000000 00000ca8 656fa1d8

The copied data can be in any format: up to sender and receiver. Could easily be padding in there. This is a case where checking at WinAPI layer would be superior to syscall layer.

*\ CANCELED understand details here CLOSED: [2013-09-13 Fri 16:07]

ABM_GETAUTOHIDEBAR wants to retrieve a handle: how does it get it back?

SHELL32!SHAppBarMessage+0xea: 75a732f2 8d45b0 lea eax,[ebp-0x50] 75a732f5 50 push eax 75a732f6 ff7304 push dword ptr [ebx+0x4] 75a732f9 6a4a push 0x4a 75a732fb ff75a8 push dword ptr [ebp-0x58] 75a732fe ff154420a675 call dword ptr [SHELL32!_imp__SendMessageW (75a62044)] 75a73304 8945ac mov [ebp-0x54],eax 75a73307 8b45ec mov eax,[ebp-0x14] 75a7330a 0b45f0 or eax,[ebp-0x10] 75a7330d 0f85837d1a00 jne SHELL32!SHAppBarMessage+0x107 (75c1b096)

SHELL32!SHAppBarMessage+0x107: 75c1b096 ff75f4 push dword ptr [ebp-0xc] 75c1b099 8d45bc lea eax,[ebp-0x44] 75c1b09c 6a28 push 0x28 75c1b09e 50 push eax 75c1b09f ff75ec push dword ptr [ebp-0x14] 75c1b0a2 e8dde00800 call SHELL32!CopyOut (75ca9184) 75c1b0a7 85c0 test eax,eax 75c1b0a9 7427 jz SHELL32!SHAppBarMessage+0x140 (75c1b0d2)

SHELL32!SHAppBarMessage+0x143: 75a73313 8b45ac mov eax,[ebp-0x54]

SHELL32!SHAppBarMessage+0x146: 75a73316 8b4dfc mov ecx,[ebp-0x4] 75a73319 5f pop edi 75a7331a 5e pop esi 75a7331b 33cd xor ecx,ebp 75a7331d 5b pop ebx 75a7331e e8dd630a00 call SHELL32!__security_check_cookie (75b19700) 75a73323 c9 leave 75a73324 c20800 ret 0x8

ebp-0x50 holds: 0:000> dd eax 0018f6c4 00000000 00000040 0018f6d0 00000028 0018f6d4 00000000 00000000 00000003 00000000 0018f6e4 00000000 00000000 00000000 00000000 0018f6f4 00000000 00000007 00000000 00000000 0018f704 00000000 00000ca8 178b434c ce401f58

APPBARDATA is copied earlier: 75c1b077 8d45bc lea eax,[ebp-0x44] 75c1b07a 6a28 push 0x28 75c1b07c 50 push eax 75c1b07d e8a6e00800 call SHELL32!CopyIn (75ca9128)

All other fields are filled in, skipping over ebp-0x18 (the uninit field): 75a732ab 8955e0 mov [ebp-0x20],edx 75a732ae 8975e4 mov [ebp-0x1c],esi <== 0x7, param 75a732b1 897dec mov [ebp-0x14],edi <== zero 75a732b4 897df0 mov [ebp-0x10],edi <== zero

PID of window creator is put into ebp-0xc ==> 0xca8 == 3240 == explorer.exe: 75a7329b 8d45f4 lea eax,[ebp-0xc] 75a7329e 50 push eax 75a7329f ff75a8 push dword ptr [ebp-0x58] 75a732b7 ff15441ca675 call dword ptr [SHELL32!_imp__GetWindowThreadProcessId (75a61c44)]

The return value of SendMessage WM_COPYDATA is just whether it was processed or not (bool). The handle sought for must be @ebp-0x14? CopyOut takes arg1..arg4 and calls memcpy(arg2, SHLockShared(arg1, arg4), arg3), passing here: ebp-0x14, APPBARDATA copy, sizeof(APPBARDATA), pid

Anyway, punting on fully understanding how this is all implemented: not worth further time.

*\ TODO solutions

Two choices:

1) Only check WM_COPYDATA for addressability, since the data format is unknown. Risk: false negatives.

2) Suppress or special-case known instances of padding. Here we don't have enough info to special-case so we'd suppress.

derekbruening commented 9 years ago

From derek.br...@gmail.com on September 13, 2013 13:41:10

This issue was closed by revision r1551 .

Status: Fixed