Skip to content

Commit 3ca80d1

Browse files
committed
Cygwin: format_process_maps: avoid crashing PID when fetching heap info
To fetch heap info for a process in our /proc/PID/maps emulation, we call RtlQueryProcessDebugInformation on this process since commit b4966f9 ("(heap_info::heap_info): Rearrange using RtlQueryProcessDebugInformation"). However, it turns out that this call can crash the targeted process, if it's called from multiple threads or processes in parallel. Worse, the entire code from creating the debug buffer, over fetching the debug info, subsequent collecting the information from said debug buffer, up to destroying the buffer, needs to be guarded against parallel access. We do this by adding a global mutex object, serializing access to the debug info of a process. Reported-by: Christian Franke <[email protected]> Fixes: b4966f9 ("(heap_info::heap_info): Rearrange using RtlQueryProcessDebugInformation") Signed-off-by: Corinna Vinschen <[email protected]> (cherry picked from commit f075f3ba2720b4cca65e63b67c865959ba6b6c92)
1 parent 284048f commit 3ca80d1

File tree

2 files changed

+34
-3
lines changed

2 files changed

+34
-3
lines changed

winsup/cygwin/fhandler/process.cc

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -621,14 +621,39 @@ struct heap_info
621621
: heap_vm_chunks (NULL)
622622
{
623623
PDEBUG_BUFFER buf;
624+
wchar_t mtx_name [32];
625+
HANDLE mtx;
624626
NTSTATUS status;
625627
PDEBUG_HEAP_ARRAY harray;
626628

629+
/* We need a global mutex here, because RtlQueryProcessDebugInformation
630+
is neither thread-safe, nor multi-process-safe. If it's called in
631+
parallel on the same process it can crash that process. We can't
632+
avoid this if a non-Cygwin app calls RtlQueryProcessDebugInformation
633+
on the same process in parallel, but we can avoid Cygwin processes
634+
crashing process PID just because they open /proc/PID/maps in parallel
635+
by serializing RtlQueryProcessDebugInformation on the same process.
636+
637+
Note that the mutex guards the entire code from
638+
RtlCreateQueryDebugBuffer to RtlDestroyQueryDebugBuffer including the
639+
code accessing the debug buffer. Apparently the debug buffer needs
640+
safeguarded against parallel access all the time it's used!!! */
641+
__small_swprintf (mtx_name, L"cyg-heapinfo-mtx-%u", pid);
642+
mtx = CreateMutexW (&sec_none_nih, FALSE, mtx_name);
643+
if (!mtx)
644+
return;
645+
WaitForSingleObject (mtx, INFINITE);
627646
buf = RtlCreateQueryDebugBuffer (16 * 65536, FALSE);
647+
if (buf)
648+
status = RtlQueryProcessDebugInformation (pid,
649+
PDI_HEAPS | PDI_HEAP_BLOCKS,
650+
buf);
628651
if (!buf)
629-
return;
630-
status = RtlQueryProcessDebugInformation (pid, PDI_HEAPS | PDI_HEAP_BLOCKS,
631-
buf);
652+
{
653+
ReleaseMutex (mtx);
654+
CloseHandle (mtx);
655+
return;
656+
}
632657
if (NT_SUCCESS (status)
633658
&& (harray = (PDEBUG_HEAP_ARRAY) buf->HeapInformation) != NULL)
634659
for (ULONG hcnt = 0; hcnt < harray->Count; ++hcnt)
@@ -653,6 +678,8 @@ struct heap_info
653678
}
654679
}
655680
RtlDestroyQueryDebugBuffer (buf);
681+
ReleaseMutex (mtx);
682+
CloseHandle (mtx);
656683
}
657684

658685
char *fill_if_match (char *base, ULONG type, char *dest)

winsup/cygwin/release/3.6.4

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,7 @@ Fixes:
2121

2222
- Fix unexpected blocking mode change by pipe_data_available()
2323
Addresses: https://github.com/git-for-windows/git/issues/5682#issuecomment-2997428207
24+
25+
- Fix potential crashing a process PID by accessing /proc/PID/maps
26+
in parallel.
27+
Addresses: https://cygwin.com/pipermail/cygwin/2025-May/258198.html

0 commit comments

Comments
 (0)