Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Possible false positive unaddr on NtGdiPolyPolyDraw parameter #1 #809

Closed
derekbruening opened this issue Nov 28, 2014 · 6 comments
Closed

Possible false positive unaddr on NtGdiPolyPolyDraw parameter #1 #809

derekbruening opened this issue Nov 28, 2014 · 6 comments
Labels
Bug-FalsePositive FalsePos-Syscall Migrated Priority-High Status-Fixed

Comments

@derekbruening
Copy link
Contributor

From rnk@google.com on February 28, 2012 14:32:05

Encountered when running chrome unit_tests FirstRunBubbleTest.CreateAndClose.

[ RUN ] FirstRunBubbleTest.CreateAndClose
Dr.M
Dr.M Error #1: UNADDRESSABLE ACCESS: reading 0x148ff3b8-0x148ff3c0 8 byte(s) within 0x148ff3b8-0x148ff3c0
Dr.M # 0 system call NtGdiPolyPolyDraw parameter #1
Dr.M # 1 gfx::Path::CreateNativeRegion [ui\gfx\path_win.cc:21]
Dr.M # 2 views::NativeWidgetWin::ResetWindowRegion [ui\views\widget\native_widget_win.cc:2417]
Dr.M # 3 views::NativeWidgetWin::OnSize [ui\views\widget\native_widget_win.cc:1948]
Dr.M # 4 views::NativeWidgetWin::_ProcessWindowMessage [ui\views\widget\native_widget_win.h:354]
Dr.M # 5 views::NativeWidgetWin::ProcessWindowMessage [ui\views\widget\native_widget_win.h:279]
Dr.M # 6 views::NativeWidgetWin::OnWndProc [ui\views\widget\native_widget_win.cc:1164]
Dr.M # 7 ui::WindowImpl::WndProc [ui\base\win\window_impl.cc:196]
Dr.M # 8 base::win::WrappedWindowProc<&ui::WindowImpl::WndProc> [base\win\wrapped_window_proc.h:60]
Dr.M # 9 USER32.dll!gapfnScSendMessage
Dr.M #10 USER32.dll!GetThreadDesktop
Dr.M #11 USER32.dll!LoadStringW
Dr.M #12 USER32.dll!gapfnScSendMessage
Dr.M #13 UxTheme.dll!? +0x0 (0x734d0b64 <UxTheme.dll+0x10b64>)
Dr.M #14 UxTheme.dll!? +0x0 (0x734d0b96 <UxTheme.dll+0x10b96>)
Dr.M #15 USER32.dll!GetPropW
Dr.M #16 views::NativeWidgetWin::OnWndProc [ui\views\widget\native_widget_win.cc:1165]
Dr.M #17 ui::WindowImpl::WndProc [ui\base\win\window_impl.cc:196]
Dr.M #18 base::win::WrappedWindowProc<&ui::WindowImpl::WndProc> [base\win\wrapped_window_proc.h:60]
Dr.M #19 USER32.dll!gapfnScSendMessage
Dr.M #20 USER32.dll!GetDC
Dr.M #21 USER32.dll!GetThreadDesktop
Dr.M #22 USER32.dll!LoadStringW
Dr.M #23 ntdll.dll!KiUserCallbackDispatcher
Dr.M #24 ui::CenterAndSizeWindow [ui\base\win\hwnd_util.cc:155]
Dr.M #25 views::NativeWidgetWin::CenterWindow [ui\views\widget\native_widget_win.cc:668]
Dr.M #26 views::Widget::SetInitialBounds [ui\views\widget\widget.cc:1218]
Dr.M #27 views::Widget::Init [ui\views\widget\widget.cc:341]
Dr.M #28 views::anonymous namespace'::CreateBorderWidget [ui\views\bubble\bubble_delegate.cc:89] \~~Dr.M~~#29views::BubbleDelegateView::CreateBubble [ui\views\bubble\bubble_delegate.cc:148] \~~Dr.M~~#30browser::CreateViewsBubble [chrome\browser\ui\views\window.cc:77] \~~Dr.M~~#31FirstRunBubble::ShowBubble [chrome\browser\ui\views\first_run_bubble.cc:35] \~~Dr.M~~#32FirstRunBubbleTest_CreateAndClose_Test::TestBody [chrome\browser\ui\views\first_run_bubble_unittest.cc:45] \~~Dr.M~~#33testing::internal::HandleExceptionsInMethodIfSupported<testing::Test,void> [testing\gtest\src\gtest.cc:2145] \~~Dr.M~~ Note: @0:12:07.991 in thread 2724 \~~Dr.M~~ Note: next higher malloc: 0x148ff3c8-0x148ff3cc \~~Dr.M~~ Note: prev lower malloc: 0x148ff398-0x148ff39c \~~Dr.M~~ \~~Dr.M~~ Error#2: UNADDRESSABLE ACCESS: reading 0x148ff268-0x148ff270 8 byte(s) within 0x148ff268-0x148ff270 \~~Dr.M~~ # 0 system call NtGdiPolyPolyDraw parameter#1 \~~Dr.M~~ # 1 gfx::Path::CreateNativeRegion [ui\gfx\path_win.cc:21] \~~Dr.M~~ # 2 views::NativeWidgetWin::ResetWindowRegion [ui\views\widget\native_widget_win.cc:2417] \~~Dr.M~~ # 3 views::NativeWidgetWin::OnSize [ui\views\widget\native_widget_win.cc:1948] \~~Dr.M~~ # 4 views::NativeWidgetWin::_ProcessWindowMessage [ui\views\widget\native_widget_win.h:354] \~~Dr.M~~ # 5 views::NativeWidgetWin::ProcessWindowMessage [ui\views\widget\native_widget_win.h:279] \~~Dr.M~~ # 6 views::NativeWidgetWin::OnWndProc [ui\views\widget\native_widget_win.cc:1164] \~~Dr.M~~ # 7 ui::WindowImpl::WndProc [ui\base\win\window_impl.cc:196] \~~Dr.M~~ # 8 base::win::WrappedWindowProc<&ui::WindowImpl::WndProc> [base\win\wrapped_window_proc.h:60] \~~Dr.M~~ # 9 USER32.dll!gapfnScSendMessage \~~Dr.M~~#10USER32.dll!GetThreadDesktop \~~Dr.M~~#11USER32.dll!LoadStringW \~~Dr.M~~#12USER32.dll!gapfnScSendMessage \~~Dr.M~~#13UxTheme.dll!? +0x0 (0x734d0b64 \<UxTheme.dll+0x10b64>) \~~Dr.M~~#14UxTheme.dll!? +0x0 (0x734d0b96 \<UxTheme.dll+0x10b96>) \~~Dr.M~~#15USER32.dll!GetPropW \~~Dr.M~~#16views::NativeWidgetWin::OnWndProc [ui\views\widget\native_widget_win.cc:1165] \~~Dr.M~~#17ui::WindowImpl::WndProc [ui\base\win\window_impl.cc:196] \~~Dr.M~~#18base::win::WrappedWindowProc<&ui::WindowImpl::WndProc> [base\win\wrapped_window_proc.h:60] \~~Dr.M~~#19USER32.dll!gapfnScSendMessage \~~Dr.M~~#20USER32.dll!GetDC \~~Dr.M~~#21USER32.dll!GetThreadDesktop \~~Dr.M~~#22USER32.dll!LoadStringW \~~Dr.M~~#23ntdll.dll!KiUserCallbackDispatcher \~~Dr.M~~#24views::NativeWidgetWin::SetBounds [ui\views\widget\native_widget_win.cc:811] \~~Dr.M~~#25views::Widget::SetBounds [ui\views\widget\widget.cc:445] \~~Dr.M~~#26views::BubbleDelegateView::SizeToContents [ui\views\bubble\bubble_delegate.cc:277] \~~Dr.M~~#27views::BubbleDelegateView::CreateBubble [ui\views\bubble\bubble_delegate.cc:151] \~~Dr.M~~#28browser::CreateViewsBubble [chrome\browser\ui\views\window.cc:77] \~~Dr.M~~#29FirstRunBubble::ShowBubble [chrome\browser\ui\views\first_run_bubble.cc:35] \~~Dr.M~~#30FirstRunBubbleTest_CreateAndClose_Test::TestBody [chrome\browser\ui\views\first_run_bubble_unittest.cc:45] \~~Dr.M~~#31` testing::internal::HandleExceptionsInMethodIfSupportedtesting::Test,void [testing\gtest\src\gtest.cc:2145]
Dr.M Note: @0:12:08.101 in thread 2724
Dr.M Note: next higher malloc: 0x148ff278-0x148ff27c
Dr.M Note: prev lower malloc: 0x148ff258-0x148ff25c
[ OK ] FirstRunBubbleTest.CreateAndClose (5120 ms)

Here's the syscall table line from syscall_wingdi.c:
{0,"NtGdiPolyPolyDraw", OK, 20, {{1,sizeof(POINT),R,}, {2,-3,R|SYSARG_SIZE_IN_ELEMENTS,sizeof(ULONG)}, }},

The docs for the function called from Chrome: http://msdn.microsoft.com/en-us/library/dd183511(v=vs.85).aspx I'd need to look closer to verify that our table entry is right.

Original issue: http://code.google.com/p/drmemory/issues/detail?id=809
@derekbruening
Copy link
Contributor Author

From bruen...@google.com on February 28, 2012 12:04:08

I added all the NtGdi* table entries based on wininc/ntgdi.h

@derekbruening
Copy link
Contributor Author

From zhao...@google.com on September 12, 2013 13:04:21

Issue 1328 has been merged into this issue.

@derekbruening
Copy link
Contributor Author

From zhao...@google.com on September 12, 2013 13:04:49

Status: Started
Owner: zhao...@google.com
Labels: -Priority-Medium Priority-High

@derekbruening
Copy link
Contributor Author

From bruen...@google.com on September 13, 2013 19:26:26

Taking over as I can reproduce this

Owner: bruen...@google.com

@derekbruening
Copy link
Contributor Author

From bruen...@google.com on September 14, 2013 08:20:43

I can repro, but interestingly I can't w/ -no_check_uninitialized.

% ~/drmemory/git/build_x86_dbg/bin/drmemory.exe -no_check_gdi -pause_at_error -no_count_leaks -batch -dr d:/derek/dr/git/exports -- ./unit_tests.exe --gtest_filter=FirstRunBubbleTest.CreateAndClose

Error #1: UNADDRESSABLE ACCESS: reading 0x072c4880-0x072c4888 8 byte(s) within 0x072c4880-0x072c4888

0 system call NtGdiPolyPolyDraw parameter #1

1 GDI32.dll!CreatePolygonRgn (0x774ce41e <GDI32.dll+0x1e41e>) modid:118

2 ui.dll!gfx::CreateHRGNFromSkPath [e:\derek\chromium\src\ui\gfx\path_win.cc:22](0x67ae3a9d <ui.dll+0x63a9d) modid:50

3 views.dll!views::HWNDMessageHandler::ResetWindowRegion [e:\derek\chromium\src\ui\views\win\hwnd_message_handler.cc:1133](0x636454f2 <views.dll+0x854f2) modid:12

4 views.dll!views::HWNDMessageHandler::OnSize [e:\derek\chromium\src\ui\views\win\hwnd_message_handler.cc:1982](0x63645dbf <views.dll+0x85dbf) modid:12

5 views.dll!views::HWNDMessageHandler::ProcessWindowMessage [e:\derek\chromium\src\ui\views\win\hwnd_message_handler.h:236](0x6363d5f9 <views.dll+0x7d5f9) modid:12

6 views.dll!views::HWNDMessageHandler::OnWndProc [e:\derek\chromium\src\ui\views\win\hwnd_message_handler.cc:875](0x63647a66 <views.dll+0x87a66) modid:12

7 ui.dll!ui::WindowImpl::WndProc [e:\derek\chromium\src\ui\base\win\window_impl.cc:265](0x67ac8637 <ui.dll+0x48637) modid:50

8 ui.dll!base::win::WrappedWindowProc<&ui::WindowImpl::WndProc> [e:\derek\chromium\src\base\win\wrapped_window_proc.h:76](0x67ac86a0 <ui.dll+0x486a0) modid:50

9 USER32.dll!InternalCallWinProc (0x770b62fa <USER32.dll+0x162fa>) modid:114

#10 USER32.dll!UserCallWinProcCheckWow (0x770b6d3a <USER32.dll+0x16d3a>) modid:114
#11 USER32.dll!RealDefWindowProcWorker (0x770b90c9 <USER32.dll+0x190c9>) modid:114
Note: @0:00:15.521 in thread 6564
Note: next higher malloc: 0x072c48d0-0x072c4900
Note: refers to 0 byte(s) beyond last valid byte in prior malloc
Note: prev lower malloc: 0x072c4880-0x072c4880

return ::CreatePolygonRgn(windows_points.get(), point_count, ALTERNATE);

0:000> Uf GDI32!CreatePolygonRgn
GDI32!CreatePolygonRgn:
774ce406 8bff mov edi,edi
774ce408 55 push ebp
774ce409 8bec mov ebp,esp
774ce40b 6a06 push 0x6 <-- GdiPolyPolyRgn
774ce40d 6a01 push 0x1
774ce40f 8d450c lea eax,[ebp+0xc]
774ce412 50 push eax <-- &cPoints
774ce413 ff7508 push dword ptr [ebp+0x8] <-- lppt
774ce416 ff7510 push dword ptr [ebp+0x10] <-- fnPolyFillMode
774ce419 e8b4e5ffff call GDI32!NtGdiPolyPolyDraw (774cc9d2)
774ce41e 5d pop ebp
774ce41f c20c00 ret 0xc

From our ntgdi.h:
__kernel_entry W32KAPI ULONG_PTR APIENTRY
NtGdiPolyPolyDraw(
__in HDC hdc,
__in PPOINT ppt,
__in_ecount(ccpt) PULONG pcpt,
__in ULONG ccpt,
__in int iFunc
);

That doesn't look right: fnPolyFillMode is either 1 or 2 and not an HDC.
ReactOS confirms that for GdiPolyPolyRgn that 1st param is not an HDC:
./win32ss/gdi/gdi32/objects/region.c: return (HRGN) NtGdiPolyPolyDraw( (HDC) fnPolyFillMode, (PPOINT) lppt, (PULONG) &cPoints, 1, GdiPolyPolyRgn);

0:000> dds @@(mc->esp)
0021ef04 774ce41e GDI32!CreatePolygonRgn+0x18
0021ef08 00000001
0021ef0c 072c4880
0021ef10 0021ef28
0021ef14 00000001
0021ef18 00000006
0021ef1c 0021ef5c
0021ef20 67ae3a9d ui!gfx::CreateHRGNFromSkPath+0xfd [e:\derek\chromium\src\ui\gfx\path_win.cc @ 22]
0021ef24 072c4880
0021ef28 00000000
0021ef2c 00000001

typedef struct tagPOINT {
LONG x;
LONG y;
} POINT, *PPOINT;

Hmm, it passed in 0 for the POINT array count.

And indeed this doesn't look like a POINT array:
0:000> dc 072c4880
072c4880 6b626577 772e7469 72706265 2e736665 webkit.webprefs.
072c4890 746e6f66 74732e73 61646e61 432e6472 fonts.standard.C
072c48a0 006d6b61 00000000 00000000 00000000 akm.............

But new[0] should be its own malloc chunk (xref issue #1145). Here's the header:

0:000> dc 072c4880-20
072c4860 00000000 00000000 00000000 00000030 ............0...
072c4870 5244000c 00000030 00000000 00000000 ..DR0...........
072c4880 6b626577 772e7469 72706265 2e736665 webkit.webprefs.
072c4890 746e6f66 74732e73 61646e61 432e6472 fonts.standard.C
072c48a0 006d6b61 00000000 00000000 00000000 akm.............
072c48b0 00000000 00000000 00000000 00000030 ............0...
072c48c0 52440008 00060000 00000000 00000000 ..DR............
072c48d0 072702d0 072c49c0 072702d0 072c4920 ..'..I,...'. I,.
0:000> dt drmemorylib!chunk_header_t 072c4868
+0x000 user_data : (null)
+0x004 alloc_size : 0x30
+0x008 flags : 0xc
+0x00a magic : 0x5244
+0x00c u :

Yes, the chunk should be + 0x18 => 0x072c4880:
0:000> x drmemorylib!redzone_beyond_header
063b5158 drmemorylib!redzone_beyond_header = 8
0:000> x drmemorylib!header_size
063b515c drmemorylib!header_size = 0x10

OK, so that's all kosher: it's unaddr b/c it's the malloc chunk padding.

Return value is NULL, so the call does fail with a 0 count.

CreatePolygonRgn returns a handle, but returns syscall value directly: so
why is it a ULONG_PTR? Some callers of it return BOOL though. So is the
syscall returning non-handles for those, or are these handles ok to discard?

Presumably the count is a pointer b/c it's an OUT param for how many points
were used? And presumably the ecount is on the wrong param, right? Should
be on ppt?
=>
No, consider CreatePolyPolygonRgn:

HRGN CreatePolyPolygonRgn(
In const POINT *lppt,
In const INT *lpPolyCounts,
In int nCount,
In int fnPolyFillMode
);

It takes an array of counts of points in each polygon. So this syscall
must be special-cased as it behaves differently depending on the iFunc
passed in.

*** TODO how handle varying return type? HRGN vs BOOL

drsyscall assumes return type is static

@derekbruening
Copy link
Contributor Author

From derek.br...@gmail.com on September 16, 2013 14:34:10

This issue was closed by revision r1552 .

Status: Fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug-FalsePositive FalsePos-Syscall Migrated Priority-High Status-Fixed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@derekbruening