[PATCH v2 0/1] MR7035: server: Avoid crash when handle table is allocated but not yet filled.
I was experimenting with a Wine tree built with ASan enabled at PE side. With it I tried to run the test suite, but every now and then I received a crash in wineserver. Because the management structure for the ASan shadow makes each wine process using several gigabytes memory I had a taskmgr.exe running all the time. I think this crash is a result of this taskmgr calling `NtQuerySystemInformation(SystemHandleInformation)` in conjunction with having many new processes created by the tests. This patch leaves `enum_handles` early if the last member has still the value -1 from `alloc_handle_table`. ``` Program received signal SIGSEGV, Segmentation fault. enum_handles (process=0x55ec0db97780, user=0x7ffce90cee30) at .../wine/wine/server/handle.c:846 846 if (!entry->ptr) continue; 1: x/i $pc => 0x55ec0b25ae6e <enum_handles+158>: cmpq $0x0,(%rdx) (rr) bt #0 enum_handles (process=0x55ec0db97780, user=0x7ffce90cee30) at .../wine/wine/server/handle.c:846 #1 0x000055ec0b268908 in enum_processes (cb=cb(a)entry=0x55ec0b25add0 <enum_handles>, user=user(a)entry=0x7ffce90cee30) at .../wine/wine/server/process.c:1112 #2 0x000055ec0b25c868 in req_get_system_handles (req=<optimized out>, reply=0x7ffce90cee90) at .../wine/wine/server/handle.c:875 #3 0x000055ec0b27d6eb in call_req_handler (thread=thread(a)entry=0x55ec0db62c10) at .../wine/wine/server/request.c:304 #4 0x000055ec0b27e62a in read_request (thread=thread(a)entry=0x55ec0db62c10) at .../wine/wine/server/request.c:338 #5 0x000055ec0b2896c0 in thread_poll_event (fd=<optimized out>, event=1) at .../wine/wine/server/thread.c:389 #6 0x000055ec0b2560b7 in fd_poll_event (event=<optimized out>, fd=<optimized out>) at .../wine/wine/server/fd.c:503 #7 main_loop_epoll () at .../wine/wine/server/fd.c:597 #8 0x000055ec0b25633e in main_loop () at .../wine/wine/server/fd.c:953 #9 0x000055ec0b246aa2 in main (argc=3, argv=0x7ffce90cf6c8) at .../wine/wine/server/main.c:238 (rr) print table->last $38 = -1 (rr) print table->count $39 = 32 ``` -- v2: server: Avoid crash when handle table is allocated but not yet filled. https://gitlab.winehq.org/wine/wine/-/merge_requests/7035
From: Bernhard Übelacker <bernhardu(a)mailbox.org> --- server/handle.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/handle.c b/server/handle.c index e65831b3b22..c43b19c1160 100644 --- a/server/handle.c +++ b/server/handle.c @@ -836,7 +836,7 @@ static int enum_handles( struct process *process, void *user ) struct handle_table *table = process->handles; struct handle_entry *entry; struct handle_info *handle; - unsigned int i; + int i; if (!table) return 0; @@ -896,7 +896,7 @@ static int enum_process_handles_cb( struct process *process, void *user ) struct enum_process_handles_info *info = user; struct handle_table *table = process->handles; struct handle_entry *entry; - unsigned int i; + int i; if (!table) return 0; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7035
v2: - Replaced check for -1 by changing type of variable `i` from unsigned to signed. - Added the same check to `enum_process_handles_cb`. Thanks @epo for the review, this matches way better the surrounding code. I have pushed v2 with your recommendation. For convenience following triggers the issue quite fast, when the cmd-loop runs at the same time as the querysysteminformation.exe: ```c /* x86_64-w64-mingw32-gcc -g -O0 querysysteminformation.c -o querysysteminformation.exe -lntdll wine querysysteminformation.exe wine cmd /C "for /L %%i in (0,1,1000) do @hostname" */ #include <windows.h> #include <winternl.h> int main() { LONG status; ULONG ulSize; ULONG BufferSize = 0x10000; LPBYTE SysHandleInfoData = HeapAlloc(GetProcessHeap(), 0, BufferSize); while (1) { status = NtQuerySystemInformation(SystemHandleInformation, SysHandleInfoData, BufferSize, &ulSize); } } ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7035#note_90699
participants (1)
-
Bernhard Übelacker