POSIX subsystem scaffold, NT syscall wrappers, shell UI fixes, and console-logon utility#3
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a POSIX subsystem scaffold, including a user-mode runtime DLL, a stub kernel driver, and documentation outlining the architectural strategy. It also adds several NT syscall wrappers in ntos2nd, a utility script for toggling console logon, and various UI improvements to the shell components. Feedback highlights a bug in the POSIX runtime where Win32 error codes are incorrectly passed to an NTSTATUS translation function, and a permission issue in the PowerShell utility regarding system file ownership. Additionally, improvements were suggested for type safety in the syscall wrapper headers and safer string handling in the POSIX process spawning logic.
| resp->ntstatus = (int32_t)GetLastError(); | ||
| return PosixTranslateNtStatusToErrno(resp->ntstatus); |
There was a problem hiding this comment.
The PosixTranslateNtStatusToErrno function is designed to handle NTSTATUS values (e.g., 0xC0000008), but it is being passed a Win32 error code from GetLastError(). Win32 error codes (like ERROR_FILE_NOT_FOUND = 2) do not match the NTSTATUS constants used in the switch statement, which will cause most errors to fall through to the default case (EIO). You should either use a Win32-to-NTSTATUS mapping or update the translation function to handle Win32 error codes.
| if ($PSCmdlet.ShouldProcess($Path, "Rename to $disabledPath")) { | ||
| Rename-Item -LiteralPath $Path -NewName ([System.IO.Path]::GetFileName($disabledPath)) -ErrorAction Stop | ||
| Write-Host "Disabled: $Path" | ||
| } |
There was a problem hiding this comment.
Renaming system binaries like authui.dll in System32 typically fails with 'Access Denied' even for Administrators because these files are owned by TrustedInstaller. To successfully rename them, the script needs to take ownership and modify the ACLs first. Consider using takeown.exe and icacls.exe before attempting the rename.
if ($PSCmdlet.ShouldProcess($Path, "Rename to $disabledPath")) {
# Take ownership and grant permissions to allow renaming system files
$null = takeown.exe /f $Path /a
$null = icacls.exe $Path /grant "*S-1-5-32-544:F" # Administrators group SID
Rename-Item -LiteralPath $Path -NewName ([System.IO.Path]::GetFileName($disabledPath)) -ErrorAction Stop
Write-Host "Disabled: $Path"
}
| NTSTATUS NTAPI NtOpenProcessWrap(PHANDLE ProcessHandle, ACCESS_MASK DesiredAccess, void* ObjectAttributes, void* ClientId); | ||
| NTSTATUS NTAPI NtReadFileWrap(HANDLE FileHandle, HANDLE Event, PVOID ApcRoutine, PVOID ApcContext, void* IoStatusBlock, PVOID Buffer, ULONG Length, PLARGE_INTEGER ByteOffset, PULONG Key); | ||
| NTSTATUS NTAPI NtWriteFileWrap(HANDLE FileHandle, HANDLE Event, PVOID ApcRoutine, PVOID ApcContext, void* IoStatusBlock, const VOID* Buffer, ULONG Length, PLARGE_INTEGER ByteOffset, PULONG Key); |
There was a problem hiding this comment.
The function prototypes in this header use void* for parameters that have specific types in the implementation (e.g., POBJECT_ATTRIBUTES, PCLIENT_ID, PIO_STATUS_BLOCK in NTCall.c). This reduces type safety for consumers of the header. It is recommended to use the specific types, potentially by moving the type definitions to a shared header included by both files.
| strncat(cmdLine, req->path, sizeof(cmdLine) - 1); | ||
| if (req->argv_blob[0]) { | ||
| strncat(cmdLine, " ", sizeof(cmdLine) - strlen(cmdLine) - 1); | ||
| strncat(cmdLine, req->argv_blob, sizeof(cmdLine) - strlen(cmdLine) - 1); | ||
| } |
There was a problem hiding this comment.
The construction of cmdLine using strncat is slightly inconsistent. While the first call (line 34) is safe because the buffer was just zeroed, the subsequent calls correctly use sizeof(cmdLine) - strlen(cmdLine) - 1. For better maintainability and to prevent potential overflows if the code is refactored, all strncat calls should use the remaining buffer size calculation. Additionally, ensure that req->path and req->argv_blob are null-terminated before concatenation to avoid reading past the source buffers.
char cmdLine[4096];
cmdLine[0] = '\0';
strncat(cmdLine, req->path, sizeof(cmdLine) - strlen(cmdLine) - 1);
if (req->argv_blob[0]) {
strncat(cmdLine, " ", sizeof(cmdLine) - strlen(cmdLine) - 1);
strncat(cmdLine, req->argv_blob, sizeof(cmdLine) - strlen(cmdLine) - 1);
}
Motivation
NTOS2NDHandleSyscallfrom user code.authui.dll.Description
ntos2nd/posix/including a user-mode runtime (posixsubsystem.c), kernel stub driver (posixsubsys_driver.c), ABI header (posix_abi.h), and exports file (posixsubsystem.def), and wire installation intoCMakeLists.txtasposixsubsystemwith header install.ntos2nd/NTCall.cand declare them in the new headerntos2nd/NTCall.h(wrappers:NtCloseWrap,NtOpenProcessWrap,NtReadFileWrap,NtWriteFileWrap,NtAllocateVirtualMemoryWrap), and document coverage inntos2nd/README.md.Appunhandled exception signature, replaceWindows.System.ProcessLauncherusage with a localTryLaunchhelper usingProcess.Start, wireSystemTrayandStartMenuintoMainWindow, adjust XAML namespaces and removeDropShadowattribute, and ensure timer/type references compile (System.Timers.Timer).utils/Enable-ConsoleLogon.ps1to toggle/restoreauthui.dllfor console-style logon (supports offline-WindowsRootand-Restore) and update top-levelREADME.mdwith a short utilities section and usage examples.Testing
dotnet build mswindows/shelland the shell assembly compiles successfully (no build errors).cmake --build buildforntos2ndand the newposixsubsystemtarget andntos2ndlink succeeded.ctestin thentos2ndtest tree and reported tests passed (no failing tests).Codex Task