[PATCH 0/2] Fixes for CodeQL analysis of MSVC's CL.EXE
CodeQL has a mode where it'll run a compiler and trace its execution, in order to have some internal representation, which it then ingests into its database, for later analysis. I managed to get MSVC's CL.EXE running in Wine, from the latest EWDK, but when I tried to trace its execution using CodeQL, I ran into trouble. These two fixes resolve that issue. I can't send these to Gitlab because I don't have the ability to fork repos there yet. I'll open a ticket for that. Jason A. Donenfeld (2): ntdll, server: allow file-backed SEC_RESERVE sections cmd: link against kernel32 for CreateProcessW dlls/ntdll/unix/virtual.c | 13 ++++++++++--- programs/cmd/Makefile.in | 2 +- server/mapping.c | 6 ++++-- 3 files changed, 15 insertions(+), 6 deletions(-) -- 2.53.0
LMDB and other applications create file-backed memory sections with SEC_RESERVE, mapping a view larger than the file and committing pages later with VirtualAlloc(MEM_COMMIT). This pattern fails in Wine because get_mapping_flags() strips SEC_RESERVE from file-backed sections, create_mapping() skips committed-range tracking for them, allocate_virtual_memory() rejects MEM_COMMIT on all SEC_FILE views, and both virtual_map_section() and the server's map_view handler reject oversized views. Fix all of these, and map only the file-backed portion of the view, leaving the excess as anonymous reserved address space. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> --- dlls/ntdll/unix/virtual.c | 13 ++++++++++--- server/mapping.c | 6 ++++-- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index 9297a5f..bec60ca 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -3550,7 +3550,8 @@ static unsigned int virtual_map_section( HANDLE handle, PVOID *addr_ptr, ULONG_P if (*size_ptr) { size = *size_ptr; - if (size > full_size - offset.QuadPart) return STATUS_INVALID_VIEW_SIZE; + if (size > full_size - offset.QuadPart && !(sec_flags & SEC_RESERVE)) + return STATUS_INVALID_VIEW_SIZE; } else { @@ -3576,7 +3577,12 @@ static unsigned int virtual_map_section( HANDLE handle, PVOID *addr_ptr, ULONG_P if (res) goto done; TRACE( "handle=%p size=%lx offset=%s\n", handle, size, wine_dbgstr_longlong(offset.QuadPart) ); - res = map_file_into_view( view, unix_handle, 0, size, offset.QuadPart, vprot, needs_close ); + { + size_t map_size = size; + if ((sec_flags & SEC_RESERVE) && size > full_size - offset.QuadPart) + map_size = ROUND_SIZE( 0, full_size - offset.QuadPart, page_mask ); + res = map_file_into_view( view, unix_handle, 0, map_size, offset.QuadPart, vprot, needs_close ); + } if (res == STATUS_SUCCESS) { /* file mappings must always be accessible */ @@ -5149,7 +5155,8 @@ static NTSTATUS allocate_virtual_memory( void **ret, SIZE_T *size_ptr, ULONG typ else /* commit the pages */ { if (!(view = find_view( base, size ))) status = STATUS_NOT_MAPPED_VIEW; - else if (view->protect & SEC_FILE) status = STATUS_ALREADY_COMMITTED; + else if ((view->protect & SEC_FILE) && !(view->protect & SEC_RESERVE)) + status = STATUS_ALREADY_COMMITTED; else if (view->protect & VPROT_FREE_PLACEHOLDER) status = STATUS_CONFLICTING_ADDRESSES; else if (!(status = set_protection( view, base, size, protect )) && (view->protect & SEC_RESERVE)) { diff --git a/server/mapping.c b/server/mapping.c index f9a80bc..72291eb 100644 --- a/server/mapping.c +++ b/server/mapping.c @@ -1107,7 +1107,7 @@ static unsigned int get_mapping_flags( obj_handle_t handle, unsigned int flags ) /* fall through */ case SEC_RESERVE: if (flags & SEC_LARGE_PAGES) break; - if (handle) return SEC_FILE | (flags & (SEC_NOCACHE | SEC_WRITECOMBINE)); + if (handle) return SEC_FILE | (flags & (SEC_RESERVE | SEC_NOCACHE | SEC_WRITECOMBINE)); return flags; } set_error( STATUS_INVALID_PARAMETER ); @@ -1193,6 +1193,7 @@ static struct mapping *create_mapping( struct object *root, const struct unicode } if (!grow_file( unix_fd, mapping->size )) goto error; } + if ((flags & SEC_RESERVE) && !(mapping->committed = create_ranges())) goto error; } else /* Anonymous mapping (no associated file) */ { @@ -1706,7 +1707,8 @@ DECL_HANDLER(map_view) if ((mapping->flags & SEC_IMAGE) || req->start >= mapping->size || req->start + req->size < req->start || - req->start + req->size > round_size( mapping->size, page_mask )) + (!(mapping->flags & SEC_RESERVE) && + req->start + req->size > round_size( mapping->size, page_mask ))) { set_error( STATUS_INVALID_PARAMETER ); goto done; -- 2.53.0
Such changes need detailed tests added to ntdll/tests/virtual.c (there are also related tests in kernel32/tests/virtual.c). Then, if those tests confirm that mapping with SEC_RESERVE with larger size are allowed, it is interesting what happens if those mappings beyond the end of file are committed (will it fail? or grow file? or else?) and that will likely need more changes. On 4/12/26 17:13, Jason A. Donenfeld via Wine-Devel wrote:
LMDB and other applications create file-backed memory sections with SEC_RESERVE, mapping a view larger than the file and committing pages later with VirtualAlloc(MEM_COMMIT). This pattern fails in Wine because get_mapping_flags() strips SEC_RESERVE from file-backed sections, create_mapping() skips committed-range tracking for them, allocate_virtual_memory() rejects MEM_COMMIT on all SEC_FILE views, and both virtual_map_section() and the server's map_view handler reject oversized views. Fix all of these, and map only the file-backed portion of the view, leaving the excess as anonymous reserved address space.
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> --- dlls/ntdll/unix/virtual.c | 13 ++++++++++--- server/mapping.c | 6 ++++-- 2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index 9297a5f..bec60ca 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -3550,7 +3550,8 @@ static unsigned int virtual_map_section( HANDLE handle, PVOID *addr_ptr, ULONG_P if (*size_ptr) { size = *size_ptr; - if (size > full_size - offset.QuadPart) return STATUS_INVALID_VIEW_SIZE; + if (size > full_size - offset.QuadPart && !(sec_flags & SEC_RESERVE)) + return STATUS_INVALID_VIEW_SIZE; } else { @@ -3576,7 +3577,12 @@ static unsigned int virtual_map_section( HANDLE handle, PVOID *addr_ptr, ULONG_P if (res) goto done;
TRACE( "handle=%p size=%lx offset=%s\n", handle, size, wine_dbgstr_longlong(offset.QuadPart) ); - res = map_file_into_view( view, unix_handle, 0, size, offset.QuadPart, vprot, needs_close ); + { + size_t map_size = size; + if ((sec_flags & SEC_RESERVE) && size > full_size - offset.QuadPart) + map_size = ROUND_SIZE( 0, full_size - offset.QuadPart, page_mask ); + res = map_file_into_view( view, unix_handle, 0, map_size, offset.QuadPart, vprot, needs_close ); + } if (res == STATUS_SUCCESS) { /* file mappings must always be accessible */ @@ -5149,7 +5155,8 @@ static NTSTATUS allocate_virtual_memory( void **ret, SIZE_T *size_ptr, ULONG typ else /* commit the pages */ { if (!(view = find_view( base, size ))) status = STATUS_NOT_MAPPED_VIEW; - else if (view->protect & SEC_FILE) status = STATUS_ALREADY_COMMITTED; + else if ((view->protect & SEC_FILE) && !(view->protect & SEC_RESERVE)) + status = STATUS_ALREADY_COMMITTED; else if (view->protect & VPROT_FREE_PLACEHOLDER) status = STATUS_CONFLICTING_ADDRESSES; else if (!(status = set_protection( view, base, size, protect )) && (view->protect & SEC_RESERVE)) { diff --git a/server/mapping.c b/server/mapping.c index f9a80bc..72291eb 100644 --- a/server/mapping.c +++ b/server/mapping.c @@ -1107,7 +1107,7 @@ static unsigned int get_mapping_flags( obj_handle_t handle, unsigned int flags ) /* fall through */ case SEC_RESERVE: if (flags & SEC_LARGE_PAGES) break; - if (handle) return SEC_FILE | (flags & (SEC_NOCACHE | SEC_WRITECOMBINE)); + if (handle) return SEC_FILE | (flags & (SEC_RESERVE | SEC_NOCACHE | SEC_WRITECOMBINE)); return flags; } set_error( STATUS_INVALID_PARAMETER ); @@ -1193,6 +1193,7 @@ static struct mapping *create_mapping( struct object *root, const struct unicode } if (!grow_file( unix_fd, mapping->size )) goto error; } + if ((flags & SEC_RESERVE) && !(mapping->committed = create_ranges())) goto error; } else /* Anonymous mapping (no associated file) */ { @@ -1706,7 +1707,8 @@ DECL_HANDLER(map_view) if ((mapping->flags & SEC_IMAGE) || req->start >= mapping->size || req->start + req->size < req->start || - req->start + req->size > round_size( mapping->size, page_mask )) + (!(mapping->flags & SEC_RESERVE) && + req->start + req->size > round_size( mapping->size, page_mask ))) { set_error( STATUS_INVALID_PARAMETER ); goto done;
On Mon, Apr 13, 2026 at 2:08 AM Paul Gofman <gofmanp@gmail.com> wrote:
Such changes need detailed tests added to ntdll/tests/virtual.c (there are also related tests in kernel32/tests/virtual.c). Then, if those tests confirm that mapping with SEC_RESERVE with larger size are allowed, it is interesting what happens if those mappings beyond the end of file are committed (will it fail? or grow file? or else?) and that will likely need more changes.
Yes indeed; this isn't quite complete. I've added some tests and reversed more windows behavior. I'll send a v2 shortly.
Detours-based tracing tools hook kernel32's CreateProcessW to intercept child process creation. Since cmd.exe currently links only against kernelbase, its calls to CreateProcessW resolve directly to kernelbase and never hit the hook. Add kernel32 to cmd.exe's imports so that CreateProcessW resolves from there, allowing Detours hooks to intercept process creation as intended. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> --- programs/cmd/Makefile.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/cmd/Makefile.in b/programs/cmd/Makefile.in index cc91373..343fe59 100644 --- a/programs/cmd/Makefile.in +++ b/programs/cmd/Makefile.in @@ -1,5 +1,5 @@ MODULE = cmd.exe -IMPORTS = shell32 user32 advapi32 kernelbase +IMPORTS = shell32 user32 advapi32 kernel32 kernelbase EXTRADLLFLAGS = -mconsole -municode -- 2.53.0
participants (2)
-
Jason A. Donenfeld -
Paul Gofman