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.

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

-replace_malloc 64-bit causes csrss READ_CONSOLE_OUTPUT_ATTRIB request to fail #1221

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

Comments

@derekbruening
Copy link
Contributor

From bruen...@google.com on May 07, 2013 21:25:50

The symptom here is an extra handle leak in the x64 handle test with -replace_malloc:

Error #5: HANDLE LEAK: Kernel Handle 0x00000000000000d4 was opened but not closed:
#0 system call NtOpenKey
#1 KERNELBASE.dll!OpenRegKey (0x000007fefe28910a <KERNELBASE.dll+0x910a>) modid:7
#2 KERNELBASE.dll!MakeNamedLocaleHashNode (0x000007fefe289274 <KERNELBASE.dll+0x9274>) modid:7
#3 KERNELBASE.dll!OpenRegistryInternationalKey (0x000007fefe2889c3 <KERNELBASE.dll+0x89c3>) modid:7
#4 KERNELBASE.dll!OpenRegistryInternationalKeyWrapper (0x000007fefe2981d1 <KERNELBASE.dll+0x181d1>) modid:7
#5 KERNELBASE.dll!MakeNamedLocaleHashNode (0x000007fefe2888fe <KERNELBASE.dll+0x88fe>) modid:7
#6 KERNELBASE.dll!TlsGetValue (0x000007fefe2a0bc7 <KERNELBASE.dll+0x20bc7>) modid:7
#7 IMM32.dll!ImmLocalAlloc (0x000007feffb21497 <IMM32.dll+0x1497>) modid:12
#8 KERNELBASE.dll!GetLocaleInfoW (0x000007fefe28e7a2 <KERNELBASE.dll+0xe7a2>) modid:7
#9 GDI32.dll!TranslateCharsetInfo (0x000007fefe526231 <GDI32.dll+0x6231>) modid:9
#10 IMM32.dll!LoadImeDpi (0x000007feffb22559 <IMM32.dll+0x2559>) modid:12
#11 ntdll.dll!RtlFreeSid (0x0000000077ae9484 <ntdll.dll+0x29484>) modid:4
#12 GDI32.dll!GetTextExtentPointW (0x000007fefe527121 <GDI32.dll+0x7121>) modid:9
#13 USER32.dll!UpdateLayeredWindowIndirect (0x00000000773eb77b <USER32.dll+0xb77b>) modid:2
#14 IMM32.dll!ImmLoadIME (0x000007feffb224ae <IMM32.dll+0x24ae>) modid:12
#15 USER32.dll!RegisterClassExW (0x00000000773f131a <USER32.dll+0x1131a>) modid:2
#16 USER32.dll!RegisterClassExW (0x00000000773f1047 <USER32.dll+0x11047>) modid:2
#17 USER32.dll!RegisterClassExW (0x00000000773f1196 <USER32.dll+0x11196>) modid:2
#18 USER32.dll!TranslateMessageEx (0x00000000773f9b43 <USER32.dll+0x19b43>) modid:2
#19 USER32.dll!SetWindowTextW (0x00000000773f72cb <USER32.dll+0x172cb>) modid:2
#20 USER32.dll!IsDialogMessageW (0x00000000773f6829 <USER32.dll+0x16829>) modid:2
#21 USER32.dll!SendMessageTimeoutW (0x00000000773f0397 <USER32.dll+0x10397>) modid:2
#22 USER32.dll!SendMessageTimeoutW (0x00000000773f05d8 <USER32.dll+0x105d8>) modid:2
#23 USER32.dll!CreateWindowExA (0x00000000773ea350 <USER32.dll+0xa350>) modid:2
#24 test_window_handles [c:\src\drmemory\git\src\tests\handle.cpp:186](0x000000013f3e1098 <handle.exe+0x1098) modid:6
#25 main [c:\src\drmemory\git\src\tests\handle.cpp:242](0x000000013f3e1245 <handle.exe+0x1245) modid:6
Note: @0:00:02.598 in thread 672616
error end

The app execution diverges from a wrapping run:

in event_basic_block(tag=0x000007fefe28920e)
in event_basic_block(tag=0x000007fefe289217)
in event_basic_block(tag=0x0000000077b06580)
in event_basic_block(tag=0x0000000077b0658d)
in event_basic_block(tag=0x0000000077b0659d)
in event_basic_block(tag=0x0000000077b065d1)
in event_basic_block(tag=0x000007fefe289230)
in event_basic_block(tag=0x0000000077b14c10)
in event_basic_block(tag=0x0000000077b14c2b)
in event_basic_block(tag=0x0000000077b14c3c)
in event_basic_block(tag=0x0000000077b14c70)
in event_basic_block(tag=0x0000000077b14c7d)
in event_basic_block(tag=0x0000000077af39cb)
in event_basic_block(tag=0x0000000077af39e9)
in event_basic_block(tag=0x0000000077af39fb)
in event_basic_block(tag=0x0000000077b14c86)
app xsp=0x0000000000aae0d8
arg 0 = 0x18
arg 1 = 0xaae140
arg 2 = 0xaae140
system call #31==31.0 NtRequestWaitReplyPort
#0 ntdll.dll!CsrClientCallServer+0x88 (0x0000000077b14c98 <ntdll.dll+0x54c98>) modid:0
#1 fp=0x0000000000aae0d0 parent=0x0000000000000000 ntdll.dll!CsrClientCallServer+0x87 (0x0000000077b14c98 <ntdll.dll+0x54c98>) modid:0
#2 fp=0x0000000000aae100 parent=0x0000000002cf01f0 KERNELBASE.dll!MakeNamedLocaleHashNode+0xffffffffffffc080 (0x000007fefe289251 <KERNELBASE.dll+0x9251>) modid:0
#3 fp=0x0000000000aae128 parent=0x00000000710d67c9 KERNELBASE.dll!MakeLocHashNodeFromSystemLocale+0x37 (0x000007fefe28d638 <KERNELBASE.dll+0xd638>) modid:0
#4 fp=0x0000000000aae2b0 parent=0x0000000000000000 KERNELBASE.dll!TlsGetValue +0x40c6 (0x000007fefe2a0bc7 <KERNELBASE.dll+0x20bc7>) modid:0
...
in event_basic_block(tag=0x0000000077b14c98)
in event_basic_block(tag=0x0000000077af3a0f)
in event_basic_block(tag=0x0000000077af3a27)
in event_basic_block(tag=0x0000000077af3a3a)
in event_basic_block(tag=0x0000000077b14ca1)
in event_basic_block(tag=0x0000000077b14ca9)
in event_basic_block(tag=0x000007fefe289251)
in event_basic_block(tag=0x000007fefe28926b)
in event_basic_block(tag=0x0000000077af8670)
replace_RtlFreeHeap heap=0x0000000000010000 flags=0x0 ptr=0x0000000002cf01f0
set pattern value at 0x0000000002cf01f0-0x0000000002cf07e8 in delay-freed block
replace_free_common 0x0000000002cf01f0 == request=1528, alloc=1536, arena=0x0000000002cf0000
in event_basic_block(tag=0x000007fefe289274)
exiting heap routine: NOT adding nop-if-mem-unaddr checks
in event_basic_block(tag=0x000007fefe289282)
in event_basic_block(tag=0x000007fefe2888ec) <=== divergence
in event_basic_block(tag=0x000007fefe288940)
in event_basic_block(tag=0x000007fefe2937fa)
in event_basic_block(tag=0x000007fefe2981c2)
in event_basic_block(tag=0x000007fefe2889a0)
in event_basic_block(tag=0x000007fefe2889aa)
in event_basic_block(tag=0x000007fefe28915a)
in event_basic_block(tag=0x0000000077ae7f70)
in event_basic_block(tag=0x0000000077ae72a0)
in event_basic_block(tag=0x0000000077ae74b7)
in event_basic_block(tag=0x0000000077b11610)

vs wrap:
app xsp=0x0000000000a7de58
arg 0 = 0x18
arg 1 = 0xa7dec0
arg 2 = 0xa7dec0
system call #31==31.0 NtRequestWaitReplyPort
in event_basic_block(tag=0x0000000077b14c98)
in event_basic_block(tag=0x0000000077af3a0f)
in event_basic_block(tag=0x0000000077af3a27)
in event_basic_block(tag=0x0000000077af3a3a)
in event_basic_block(tag=0x0000000077b14ca1)
in event_basic_block(tag=0x0000000077b14ca9)
in event_basic_block(tag=0x000007fefe289251)
in event_basic_block(tag=0x000007fefe289257)
in event_basic_block(tag=0x000007fefe28926b)
in event_basic_block(tag=0x0000000077af8670)
entering alloc routine 0x0000000077b13200 RtlFreeHeap type=34 rec=0 adj=0
free-pre heap=0x0000000000010000 ptr=0x0000000000010aa0 size=0x5f8 => 0x0000000000010a90 real size = 0x618
inserting into delay_free_tree (queue idx=161): 0x0000000000010a90-0x00000000000110a8 1560 bytes redzone=1
delayed free queue not full: delaying 161-th free of 0x0000000000010a90-0x00000000000110a8 auxarg=0x0000000000010000
set pattern value at 0x0000000000010aa0-0x0000000000011098 in delay-freed block
free-pre client 34 changing base from 0x0000000000010a90 to 0x0000000000000000
changed base => pt->alloc_base=0x0000000000000010
in event_basic_block(tag=0x000007fefe289274)
inside heap routine: adding nop-if-mem-unaddr checks
exiting heap routine: NOT adding nop-if-mem-unaddr checks
leaving alloc routine 0x0000000077b13200 RtlFreeHeap rec=1 adj=1
in event_basic_block(tag=0x000007fefe289282)
in event_basic_block(tag=0x000007fefe28928a) <=== divergence
in event_basic_block(tag=0x000007fefe2892b1)
in event_basic_block(tag=0x000007fefe285830)
in event_basic_block(tag=0x000007fefe285871)
in event_basic_block(tag=0x000007fefe28587b)
in event_basic_block(tag=0x000007fefe285887)
in event_basic_block(tag=0x000007fefe285883)
in event_basic_block(tag=0x000007fefe2858a8)
in event_basic_block(tag=0x000007fefe2858c9)

0:000> U 0x000007fefe28926b
KERNELBASE!MakeNamedLocaleHashNode+0x45b:
000007fefe28926b 488bcf mov rcx,rdi 000007fefe28926e ff1594210400 call qword ptr [KERNELBASE!_imp_CsrFreeCaptureBuffer (000007fefe2cb408)] 000007fefe289274 6683bbe405000001 cmp word ptr [rbx+5E4h],1
000007fefe28927c 0f856af6ffff jne KERNELBASE!MakeNamedLocaleHashNode+0x4f5 (000007fefe2888ec)
000007fefe289282 85f6 test esi,esi 000007fefe289284 0f8862f6ffff js KERNELBASE!MakeNamedLocaleHashNode+0x4f5 (000007fefe2888ec) 000007fefe28928a 488bbc24c8010000 mov rdi,qword ptr [rsp+1C8h]
000007fe`fe289292 488bb424c0010000 mov rsi,qword ptr [rsp+1C0h]

0:000> U 0x0000000077af8670
ntdll!CsrFreeCaptureBuffer:
0000000077af8670 4c8bc1 mov r8 ,rcx 0000000077af8673 488b0d069e0f00 mov rcx,qword ptr [ntdll!CsrPortHeap (0000000077bf2480)] 0000000077af867a 33d2 xor edx,edx
0000000077af867c e97fab0100 jmp ntdll!RtlFreeHeap (0000000077b13200)

esi came from:
0:000> U 0x000007fefe289230
KERNELBASE!MakeNamedLocaleHashNode+0x420:
000007fefe289230 458d4c2410 lea r9d,[ r12 +10h] 000007fefe289235 488d4c2430 lea rcx,[rsp+30h]
000007fefe28923a 41b81b000100 mov r8d,1001Bh 000007fefe289240 488bd7 mov rdx,rdi
000007fefe289243 c7442478cc050000 mov dword ptr [rsp+78h],5CCh 000007fefe28924b ff157f210400 call qword ptr [KERNELBASE!_imp_CsrClientCallServer (000007fefe2cb3d0)] 000007fefe289251 8bf0 mov esi,eax
000007fe`fe2...

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

@derekbruening
Copy link
Contributor Author

From bruen...@google.com on May 08, 2013 11:53:38

**** DONE is ntdll!CsrPortHeap a special heap somehow? yes! it's shared memory created by NtConnectPort
CLOSED: [2013-05-08 Wed 14:41]
- State "DONE" from "TODO" [2013-05-08 Wed 14:41]

native run:
0:000> ? poi(ntdll!CsrPortHeap)
Evaluate expression: 65536 = 0000000000010000 0:000> p ntdll!CsrAllocateCaptureBuffer+0x79: 0000000077af3979 e822fa0100 call ntdll!RtlAllocateHeap (0000000077b133a0) 0:000> r rax=0000000000000002 rbx=0000000000000001 rcx=0000000000010000 rdx=0000000000000000 rsi=000000000000026c rdi=0000000000000000 rip=0000000077af3979 rsp=00000000009de2a0 rbp=0000000000000003 r8 =000000000000026c r9 =0000000000000240 r10 =0000000000000000 0:000> p ntdll!CsrAllocateCaptureBuffer+0x7e: 0000000077af397e 4885c0 test rax,rax
0:000> dd 10a90 10a90+26c
0000000000010a90 baadf00d baadf00d baadf00d baadf00d 0000000000010aa0 baadf00d baadf00d baadf00d baadf00d
...
0000000000010ce0 baadf00d baadf00d baadf00d baadf00d 0000000000010cf0 baadf00d baadf00d baadf00d abababab

0:000> !vprot 10a90
BaseAddress: 0000000000010000
AllocationBase: 0000000000010000
AllocationProtect: 00000004 PAGE_READWRITE
RegionSize: 0000000000010000
State: 00001000 MEM_COMMIT
Protect: 00000004 PAGE_READWRITE
Type: 00040000 MEM_MAPPED

hmm, so it's not HEAP_CREATE_ENABLE_EXECUTE, but it's MEM_MAPPED

it's created here:
0:000> kn

Child-SP RetAddr Call Site

00 00000000009ee7a0 0000000077b043b5 ntdll!CsrpConnectToServer+0x426
01 00000000009ee960 000007fefe28deea ntdll!CsrClientConnectToServer+0x230
02 00000000009eeb70 0000000077afb0d8 KERNELBASE!KernelBaseDllInitialize+0x148
03 00000000009eee00 0000000077b042ed ntdll!LdrpRunInitializeRoutines+0x1fe
04 00000000009eefd0 0000000077b01dc8 ntdll!LdrGetProcedureAddressEx+0x2aa
05 00000000009ef140 0000000077b01a17 ntdll!LdrpInitializeProcess+0x1a0c
06 00000000009ef630 0000000077aec32e ntdll! ?? ::FNODOBFM::string'+0x29220 07 00000000009ef6a0 00000000`00000000 ntdll!LdrInitializeThunk+0xe

ntdll!CsrpConnectToServer+0x3c6:
0000000077b04796 4c8b842498000000 mov r8 ,qword ptr [rsp+98h] 0000000077b0479e 488d05ab200000 lea rax,[ntdll!LdrSetAppCompatDllRedirectionCallback (0000000077b06850)] 0000000077b047a5 448d4e01 lea r9d,[rsi+1]
0000000077b047a9 488bd7 mov rdx,rdi 0000000077b047ac 4889842488010000 mov qword ptr [rsp+188h],rax
0000000077b047b4 488d842440010000 lea rax,[rsp+140h] 0000000077b047bc b900800000 mov ecx,8000h
0000000077b047c1 4889442428 mov qword ptr [rsp+28h],rax 0000000077b047c6 c784244001000060000000 mov dword ptr [rsp+140h],60h
0000000077b047d1 48c784247801000000100000 mov qword ptr [rsp+178h],1000h 0000000077b047dd 4889742420 mov qword ptr [rsp+20h],rsi
0000000077b047e2 4c89842480010000 mov qword ptr [rsp+180h], r8 0000000077b047ea e8818cffff call ntdll!RtlCreateHeap (0000000077afd470) 0000000077b047ef 4889058adc0e00 mov qword ptr [ntdll!CsrPortHeap (00000000`77bf2480)],rax

Theory: this memory needs to be shared with the csrss process.
Wow64 works b/c it goes through the wow64 layer, where the real shared
memory is kept.

RtlCreateHeap does take a base as 2nd arg: so rdx, so was in rdi.

NtConnectPort is what maps the memory:

ntdll!CsrpConnectToServer+0x1ad:
0000000077b0457d 488b442468 mov rax,qword ptr [rsp+68h] 0000000077b04582 4c8d8c2480000000 lea r9 ,[rsp+80h]
0000000077b0458a 4c8d442470 lea r8 ,[rsp+70h] 0000000077b0458f 4889842488000000 mov qword ptr [rsp+88h],rax
0000000077b04597 8b442460 mov eax,dword ptr [rsp+60h] 0000000077b0459b 488d0d4e630f00 lea rcx,[ntdll!CsrPortHandle (0000000077bfa8f0)] 0000000077b045a2 4889842498000000 mov qword ptr [rsp+98h],rax
0000000077b045aa 488d8424c8010000 lea rax,[rsp+1C8h] 0000000077b045b2 488bd5 mov rdx,rbp
0000000077b045b5 4889442438 mov qword ptr [rsp+38h],rax 0000000077b045ba 488d842410010000 lea rax,[rsp+110h]
0000000077b045c2 c784248000000030000000 mov dword ptr [rsp+80h],30h 0000000077b045cd 4889442430 mov qword ptr [rsp+30h],rax
0000000077b045d2 488d8424d0010000 lea rax,[rsp+1D0h] 0000000077b045da 89b42490000000 mov dword ptr [rsp+90h],esi
0000000077b045e1 4889442428 mov qword ptr [rsp+28h],rax 0000000077b045e6 488d8424b0000000 lea rax,[rsp+0B0h]
0000000077b045ee 4889b424a0000000 mov qword ptr [rsp+0A0h],rsi 0000000077b045f6 4889b424a8000000 mov qword ptr [rsp+0A8h],rsi
0000000077b045fe c78424b000000018000000 mov dword ptr [rsp+0B0h],18h 0000000077b04609 4889b424b8000000 mov qword ptr [rsp+0B8h],rsi
0000000077b04611 4889442420 mov qword ptr [rsp+20h],rax 0000000077b04616 4889b424c0000000 mov qword ptr [rsp+0C0h],rsi
0000000077b0461e c78424c801000030000000 mov dword ptr [rsp+1C8h],30h 0000000077b04629 e802d60000 call ntdll!ZwConnectPort (00000000`77b11c30)

Args:
0:000> r
rax=0000000000a2ebf0 rbx=0000000000a30036 rcx=0000000077bfa8f0
rdx=0000000077bf25a0 rsi=0000000000000000 rdi=0000000000000026
rip=0000000077b04629 rsp=0000000000a2eb40 rbp=0000000077bf25a0 r8 =0000000000a2ebb0 r9 =0000000000a2ebc0 r10 =0000000000000000
0:000> dqs rsp
0000000000a2eb40 0000000000000004
0000000000a2eb48 0000000000a2ef68
0000000000a2eb50 0000000000000000
0000000000a2eb58 000007fefe2800e8 KERNELBASE!GetEnvironmentVariableA (KERNELBASE+0xe8)
0000000000a2eb60 0000000000a2ebf0
0000000000a2eb68 0000000000a2ed10
0000000000a2eb70 0000000000a2ec50
0000000000a2eb78 0000000000a2ed08
0000000000a2eb80 0000000077bc6270 ntdll!$$VProc_ImageExportDirectory
0000000000a2eb88 0000000000000000
=>
PortName:
0:000> dq 77bf25a0
0000000077bf25a0 0000000000420036 0000000000a33740 0:000> du 00a33740 0000000000a33740 "\Sessions\1\Windows\ApiPortectio"
0000000000a33780 "n........ﻮﻮﻮﻮﻮﻮﻮ" 0:000> dq a2ebc0 0000000000a2ebc0 0000000000000030 0000000000000014
0:000> dq 00a2ebf0
0000000000a2ebf0 0000000000000018 00000000`00000000

It queries a section to get the base of what was mmapped:
0:000> U 77b0464f L10
ntdll!CsrpConnectToServer+0x27f:
0000000077b0464f 4c8d8424f8000000 lea r8 ,[rsp+0F8h] 0000000077b04657 488b5860 mov rbx,qword ptr [rax+60h]
0000000077b0465b 488b842418010000 mov rax,qword ptr [rsp+118h] 0000000077b04663 33d2 xor edx,edx
0000000077b04665 48898388000000 mov qword ptr [rbx+88h],rax 0000000077b0466c 488b842420010000 mov rax,qword ptr [rsp+120h]
0000000077b04674 4889742420 mov qword ptr [rsp+20h],rsi 0000000077b04679 48898398000000 mov qword ptr [rbx+98h],rax
0000000077b04680 488b4c2450 mov rcx,qword ptr [rsp+50h] 0000000077b04685 e896d10000 call ntdll!NtQuerySection (0000000077b11820) 0000000077b0468a 85c0 test eax,eax
0000000077b0468c 0f88dba30500 js ntdll! ?? ::FNODOBFM::string'+0x1e2bd (0000000077b5ea6d) 0000000077b04692 488b842408010000 mov rax,qword ptr [rsp+108h]
00000000`77b0469a 4883f8ff cmp rax,0FFFFFFFFFFFFFFFFh

I don't quite get it though: SECTION_BASIC_INFORMATION has the base 1st
thing, so shouldn't it be at rsp+0F8h, and not rsp+108h (I observed the
code: it's at the latter)? The info code is 0.

**** TODO solution: query pre-us heaps

I'll have to query the memory and if MEM_MAPPED I won't be able to use my own memory.
Two possibilities:

A) forward to native routines -- not easy since DR will redirect direct calls
B) find end of that heap by walking, and allocate my own stuff there

Should also look for +x for HEAP_CREATE_ENABLE_EXECUTE

@derekbruening
Copy link
Contributor Author

From derek.br...@gmail.com on May 09, 2013 13:57:15

This issue was closed by revision r1353 .

Status: Fixed

timniederhausen pushed a commit to timniederhausen/gn that referenced this issue Nov 26, 2016
In bug 464430 we are aiming to lockdown CSRSS.
However this causes a problem with the dumping of the heap, which has necessitated the change to base/trace_event/winheap_dump_provider_win.cc

The dumping of the heaps get a list of process heaps (for the renderer) and dumps each of these. One of these heaps is the one used by CSRSS for its communication between the client (this process) and the CSRSS server (csrss.exe). For more background see [0] and [1].
When the handle to ALPC port to CSRSS is closed in the sandbox lockdown, the server destroys this heap. However as this is only meant to happen as part of process termination, there is no cleanup inside the client process. So the client process is left thinking this process heap exists, when it does not.

It is possible to destroy this heap right before the ALPC port is closed, however the 2 options I've experimented with both require use of undocumented features. This is not desired in general, and from my observations, the internal heap structures change a lot from version to version, so we really can't rely on them.

The solution implemented here is to handle the invalid heaps when they are being dumped, by gracefully supporting failures, rather than CHECK()ing. In particular, the call to HeapLock() is performing the heap validation for us, if this fails we assume that we can't read the heap, which is now considered ok. As an extra check, we add a CHECK() to ensure that at least one heap was dumped successfully.

It would be an option, perhaps, to plumb through this lockdown state in to the heap dumping code, however that would require a very large amount of changes so it has not been done.

[0]: http://j00ru.vexillium.org/?p=527
[1]: DynamoRIO/drmemory#1221

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
BUG=464430

Review-Url: https://codereview.chromium.org/2242953002
Cr-Original-Commit-Position: refs/heads/master@{#425523}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: c56e1ffa7c82070f974f065c1a5b0bc99165d32e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant