Please see: https://marc.info/?l=wine-devel&m=174715050805731 as well as the commit messages for more information.
-- v3: ntdll: Report more info about heap problems detected by ASan. ntdll: Show (partial) stack trace in ASan reports. ntdll: Add heap quarantine for ASan. ntdll: Add asan poisoning and redzoning to heap allocator ntdll: Implement ASan fake stack. ntdll: Don't use address of local variables as the frame address. ntdll: During unwind, also check if frame is on fake stack. ntdll: Implement __asan_{un,}poison_memory_region. ntdll: Make sure to not write into poisoned memory in KeUserModeCallback. ntdll: Implement reporting of ASan errors.
This merge request has too many patches to be relayed via email. Please visit the URL below to see the contents of the merge request. https://gitlab.winehq.org/wine/wine/-/merge_requests/8026
changes: KeUserModeCallback is writing into poisoned user/PE stack. unpoison it before writes, should be harmless.
also fixed shadow boundary calculation, and adjusted some includes.
it might conflict with userland apps for example.
thanks for pointing that out, i didn't think of that.
what kind of hidden channel were you thinking about? maybe a table of function pointers exported via a wine private symbol?
On Tue May 13 22:19:30 2025 +0000, Yuxuan Shui wrote:
it might conflict with userland apps for example.
thanks for pointing that out, i didn't think of that. what kind of hidden channel were you thinking about? maybe a table of function pointers exported via a wine private symbol?
oh, and calling `GetModuleHandle` and `GetProcAddress` from winecrt0 is problematic, too. since they are from kernelbase/kernel32.
On Tue May 13 22:26:01 2025 +0000, Yuxuan Shui wrote:
oh, and calling `GetModuleHandle` and `GetProcAddress` from winecrt0 is problematic, too. since they are from kernelbase/kernel32.
Yes, you need to use `LdrGetDllHandle` / `LdrGetProcedureAddress`.
I don't have anything concrete in mind for the hidden channel. It could be an unused slot in PEB/TEB, a hidden symbol, etc..
table of function pointers exported via a wine private symbol?
i think this doesn't work, since some modules (e.g. in `dinput/tests` and `ntoskrnl.exe/tests`) don't link with ntdll.dll, we can't use `LdrGetDllHandle` etc. in our asan thunk.
so looks like unused slot in PEB is the way to go. IIUC at least a page is allocated for the PEB, and PEB doesn't use up the full page? we should be able to put a pointer there.
On Wed May 14 17:18:57 2025 +0000, Yuxuan Shui wrote:
table of function pointers exported via a wine private symbol?
i think this doesn't work, since some modules (e.g. in `dinput/tests` and `ntoskrnl.exe/tests`) don't link with ntdll.dll, we can't use `LdrGetDllHandle` etc. in our asan thunk. so looks like unused slot in PEB is the way to go. IIUC at least a page is allocated for the PEB, and PEB doesn't use up the full page? we should be able to put a pointer there.
Note that there are basically no unused slots in TEB or PEB, if something is unused on Windows it is used by anticheats for its own purposes more often than not (including space). So probably adding anything Wine specific to PEB directly is the last resort. So unless this is used in a special build this probably should be done elsewise.
Also, Wine specific ntdll exports are also exceptional, adding those is probably a no go. Maybe there is another way to do it.
A separate question, is that confirmed that that regardless of these details such massive changes to ntdll (and not some special separate library) to support ASAN (which is not the case in Windows ntdll probably) is a go at all for Wine? IMO it is questionable. If it can only be done using such massive changes in system libraries maybe a better way to go is not to try to link that to ASAN at all and maybe implement some different targeted Wine-specific improvements to heap debugging isntead?
On Wed May 14 17:28:32 2025 +0000, Paul Gofman wrote:
Note that there are basically no unused slots in TEB or PEB, if something is unused on Windows it is used by anticheats for its own purposes more often than not (including space). So probably adding anything Wine specific to PEB directly is the last resort. So unless this is used in a special build this probably should be done elsewise. Also, Wine specific ntdll exports are also exceptional, adding those is probably a no go. Maybe there is another way to do it. A separate question, is that confirmed that that regardless of these details such massive changes to ntdll (and not some special separate library) to support ASAN (which is not the case in Windows ntdll probably) is a go at all for Wine? IMO it is questionable. If it can only be done using such massive changes in system libraries maybe a better way to go is not to try to link that to ASAN at all and maybe implement some different targeted Wine-specific improvements to heap debugging isntead?
Looks like ASAN is integrated into MSVC on Windows: https://learn.microsoft.com/en-us/cpp/sanitizers/asan?view=msvc-170. If maybe it is possible to something to support it like on Windows that would probably be correct way, otherwise I am not sure how it worth it to bring in all that infrastructure into Wine to only be able to build Wine PE side code with ASAN. Improvements to heap debugging (maybe Wine specific with special debug logging flags) seems like a more feasible way to go.
On Wed May 14 17:34:22 2025 +0000, Paul Gofman wrote:
Looks like ASAN is integrated into MSVC on Windows: https://learn.microsoft.com/en-us/cpp/sanitizers/asan?view=msvc-170. If maybe it is possible to something to support it like on Windows that would probably be correct way, otherwise I am not sure how it worth it to bring in all that infrastructure into Wine to only be able to build Wine PE side code with ASAN. Improvements to heap debugging (maybe Wine specific with special debug logging flags) seems like a more feasible way to go.
IIUC ASAN support on MSVC only covers application DLL/EXEs, it doesn't cover system DLLs like kernel32 and ntdll. The purpose of this MR is to find bugs in Wine DLLs themselves, where the application might not have been compiled with -fsanitize=address.
maybe implement some different targeted Wine-specific improvements to heap debugging isntead?
We would lose nothing that way if ASAN was just some extra checks to heap alloc/dealloc. In fact ASAN is not limited to heap memory; the main value of ASAN lies in the extensive instrumentation on every indirect load/store (whether heap, stack, or static), sort of like Valgrind but more lightweight without the VEX IR. For example it can detect erroneous reading beyond buffer boundary, something a heap verifier won't be capable of.
On Wed May 14 23:48:03 2025 +0000, Jinoh Kang wrote:
maybe implement some different targeted Wine-specific improvements to
heap debugging isntead? We would lose nothing that way if ASAN was just some extra checks to heap alloc/dealloc. In fact ASAN is not limited to heap memory; the main value of ASAN lies in the extensive instrumentation on every indirect load/store (whether heap, stack, or static), sort of like Valgrind but more lightweight without the VEX IR. For example it can detect erroneous reading beyond buffer boundary, something a heap verifier won't be capable of.
IMO it doesn't fit very well into Wine purpose. E. g., things like existing or planned gdb integration changes are much smaller, don't expose Wine internals and at the same time help debugging apps under Wine, not just Wine itself. If some limited changes not affecting things at runtime anyhow (including not exposing any Wine-specific stuff) could work that would be great (like there were something about ASAN in ntdll/heap.c, not sure maybe that is supposed to work in some limited sense??), but IMO making so lot of system places revolve around ASAN requirements doesn't worth it. Also, there is so lot of special cases with what Wine is doing, somehow I suspect there would be more than currently considered in the MR.
On Wed May 14 23:53:11 2025 +0000, Paul Gofman wrote:
IMO it doesn't fit very well into Wine purpose. E. g., things like existing or planned gdb integration changes are much smaller, don't expose Wine internals and at the same time help debugging apps under Wine, not just Wine itself. If some limited changes not affecting things at runtime anyhow (including not exposing any Wine-specific stuff) could work that would be great (like there were something about ASAN in ntdll/heap.c, not sure maybe that is supposed to work in some limited sense??), but IMO making so lot of system places revolve around ASAN requirements doesn't worth it. Also, there is so lot of special cases with what Wine is doing, somehow I suspect there would be more than currently considered in the MR.
the additions to ntdll are mostly self-contained in the two `asan_rtl.c` files, if necessary they can be moved out into a separate folder in `dlls/`. and really has no impact on wine whatsoever if asan is not enabled.
the most invasive changes to ntdll are in `heap.c`, but that's not mandatory. i can write an asan specific heap allocator and make the whole thing self-contained.
the rest of the changes are:
- handling `__WINE_FRAME` exceptions. (can we get rid of these and use SEH?) - mapping shadow memory in `virtual_init`. - putting stuff in unused slots in TEB. this we already do for `ntdll_thread_data`, and games with anti-cheat don't run on asan-enabled wine isn't that big of a compromise.
which IMHO is worth it for what asan will give us (look at the long list of bugs already found).
On Thu May 15 12:41:50 2025 +0000, Yuxuan Shui wrote:
changes: KeUserModeCallback is writing into poisoned user/PE stack. unpoison it before writes, should be harmless. also fixed shadow boundary calculation, and adjusted some includes.
i will push new, unorganized changes to [`wip/address-sanitizer`](https://gitlab.winehq.org/yshui/wine/-/tree/wip/address-sanitizer?ref_type=h...), and will only push cleaned up versions here, so i don't create too much noise.
maybe implement some different targeted Wine-specific improvements to heap debugging instead?
what we can do with heap debugging is very very limited compared to what asan offers. and comparing to valgrind, asan is at least an order of magnitude faster, and wine already has code for interfacing with valgrind in ntdll!
to support asan, some changes to ntdll are unavoidable, otherwise we will have to exclude core parts of wine.
i pushed a new version which doesn't require any new exports from ntdll, only a pointer stored on the PEB page.
-
Jinoh Kang (＠iamahuman) -
Paul Gofman (＠gofman)
-
Yuxuan Shui (＠yshui)