From 01ed50c4b4d02c78458cbb6c0ed5edaef4800868 Mon Sep 17 00:00:00 2001 From: Luke Street Date: Sun, 5 Oct 2025 23:24:50 -0600 Subject: [PATCH] Fix pipe reads; add tests for NtReadFile & pipes --- CMakeLists.txt | 36 +++++++++++++++ dll/crt.cpp | 4 +- dll/kernel32/fileapi.cpp | 18 +++++++- dll/kernel32/internal.h | 4 +- dll/ntdll.cpp | 50 +++++++++++++-------- src/errors.h | 1 + src/files.cpp | 36 ++++++++++++--- test/test_heap.c | 2 +- test/test_ntreadfile.c | 88 ++++++++++++++++++++++++++++++++++++ test/test_pipe_io.c | 96 ++++++++++++++++++++++++++++++++++++++++ 10 files changed, 304 insertions(+), 31 deletions(-) create mode 100644 test/test_ntreadfile.c create mode 100644 test/test_pipe_io.c diff --git a/CMakeLists.txt b/CMakeLists.txt index b737eba..0f08444 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -298,6 +298,28 @@ if(BUILD_TESTING) ${CMAKE_CURRENT_SOURCE_DIR}/test/test_ntquery.c ${CMAKE_CURRENT_SOURCE_DIR}/test/test_assert.h) + add_custom_command( + OUTPUT ${WIBO_TEST_BIN_DIR}/test_ntreadfile.exe + COMMAND ${WIBO_MINGW_CC} -Wall -Wextra -O2 + -I${CMAKE_CURRENT_SOURCE_DIR}/test + -o test_ntreadfile.exe + ${CMAKE_CURRENT_SOURCE_DIR}/test/test_ntreadfile.c + WORKING_DIRECTORY ${WIBO_TEST_BIN_DIR} + DEPENDS + ${CMAKE_CURRENT_SOURCE_DIR}/test/test_ntreadfile.c + ${CMAKE_CURRENT_SOURCE_DIR}/test/test_assert.h) + + add_custom_command( + OUTPUT ${WIBO_TEST_BIN_DIR}/test_pipe_io.exe + COMMAND ${WIBO_MINGW_CC} -Wall -Wextra -O2 + -I${CMAKE_CURRENT_SOURCE_DIR}/test + -o test_pipe_io.exe + ${CMAKE_CURRENT_SOURCE_DIR}/test/test_pipe_io.c + WORKING_DIRECTORY ${WIBO_TEST_BIN_DIR} + DEPENDS + ${CMAKE_CURRENT_SOURCE_DIR}/test/test_pipe_io.c + ${CMAKE_CURRENT_SOURCE_DIR}/test/test_assert.h) + add_custom_command( OUTPUT ${WIBO_TEST_BIN_DIR}/test_sysdir.exe COMMAND ${WIBO_MINGW_CC} -Wall -Wextra -O2 @@ -328,6 +350,8 @@ if(BUILD_TESTING) ${WIBO_TEST_BIN_DIR}/test_virtualquery.exe ${WIBO_TEST_BIN_DIR}/test_rtl.exe ${WIBO_TEST_BIN_DIR}/test_ntquery.exe + ${WIBO_TEST_BIN_DIR}/test_ntreadfile.exe + ${WIBO_TEST_BIN_DIR}/test_pipe_io.exe ${WIBO_TEST_BIN_DIR}/test_sysdir.exe) if(CMAKE_CONFIGURATION_TYPES) @@ -412,6 +436,18 @@ if(BUILD_TESTING) WORKING_DIRECTORY ${WIBO_TEST_BIN_DIR} DEPENDS wibo.build_fixtures) + add_test(NAME wibo.test_ntreadfile + COMMAND $ ${WIBO_TEST_BIN_DIR}/test_ntreadfile.exe) + set_tests_properties(wibo.test_ntreadfile PROPERTIES + WORKING_DIRECTORY ${WIBO_TEST_BIN_DIR} + DEPENDS wibo.build_fixtures) + + add_test(NAME wibo.test_pipe_io + COMMAND $ ${WIBO_TEST_BIN_DIR}/test_pipe_io.exe) + set_tests_properties(wibo.test_pipe_io PROPERTIES + WORKING_DIRECTORY ${WIBO_TEST_BIN_DIR} + DEPENDS wibo.build_fixtures) + add_test(NAME wibo.test_sysdir COMMAND $ ${WIBO_TEST_BIN_DIR}/test_sysdir.exe) set_tests_properties(wibo.test_sysdir PROPERTIES diff --git a/dll/crt.cpp b/dll/crt.cpp index af99059..49bc36d 100644 --- a/dll/crt.cpp +++ b/dll/crt.cpp @@ -279,10 +279,10 @@ void WIN_ENTRY exit(int status) { (*it)(); } } - ::exit(status); + ::_exit(status); } -void WIN_ENTRY _cexit(void) { +void WIN_ENTRY _cexit() { HOST_CONTEXT_GUARD(); DEBUG_LOG("_cexit()\n"); TIB *tib = wibo::getThreadTibForHost(); diff --git a/dll/kernel32/fileapi.cpp b/dll/kernel32/fileapi.cpp index aef8678..85777d9 100644 --- a/dll/kernel32/fileapi.cpp +++ b/dll/kernel32/fileapi.cpp @@ -521,7 +521,8 @@ BOOL WIN_FUNC FlushFileBuffers(HANDLE hFile) { BOOL WIN_FUNC ReadFile(HANDLE hFile, LPVOID lpBuffer, DWORD nNumberOfBytesToRead, LPDWORD lpNumberOfBytesRead, LPOVERLAPPED lpOverlapped) { HOST_CONTEXT_GUARD(); - DEBUG_LOG("ReadFile(%p, %u)\n", hFile, nNumberOfBytesToRead); + DEBUG_LOG("ReadFile(%p, %p, %u, %p, %p)\n", hFile, lpBuffer, nNumberOfBytesToRead, lpNumberOfBytesRead, + lpOverlapped); wibo::lastError = ERROR_SUCCESS; HandleMeta meta{}; @@ -543,6 +544,10 @@ BOOL WIN_FUNC ReadFile(HANDLE hFile, LPVOID lpBuffer, DWORD nNumberOfBytesToRead return FALSE; } + if (lpNumberOfBytesRead && (!file->overlapped || lpOverlapped == nullptr)) { + *lpNumberOfBytesRead = 0; + } + std::optional offset; bool updateFilePointer = true; if (lpOverlapped != nullptr) { @@ -561,6 +566,16 @@ BOOL WIN_FUNC ReadFile(HANDLE hFile, LPVOID lpBuffer, DWORD nNumberOfBytesToRead wibo::lastError = wibo::winErrorFromErrno(io.unixError); } else if (io.reachedEnd && io.bytesTransferred == 0) { completionStatus = STATUS_END_OF_FILE; + if (file->isPipe) { + wibo::lastError = ERROR_BROKEN_PIPE; + if (lpOverlapped != nullptr) { + lpOverlapped->Internal = completionStatus; + lpOverlapped->InternalHigh = 0; + signalOverlappedEvent(lpOverlapped); + } + DEBUG_LOG("-> ERROR_BROKEN_PIPE\n"); + return FALSE; + } } if (lpNumberOfBytesRead && (!file->overlapped || lpOverlapped == nullptr)) { @@ -573,6 +588,7 @@ BOOL WIN_FUNC ReadFile(HANDLE hFile, LPVOID lpBuffer, DWORD nNumberOfBytesToRead signalOverlappedEvent(lpOverlapped); } + DEBUG_LOG("-> %u bytes read, error %d\n", io.bytesTransferred, wibo::lastError); return io.unixError == 0; } diff --git a/dll/kernel32/internal.h b/dll/kernel32/internal.h index 5889897..f2cbda4 100644 --- a/dll/kernel32/internal.h +++ b/dll/kernel32/internal.h @@ -29,13 +29,13 @@ struct FileObject final : FsObject { off64_t filePos = 0; bool overlapped = false; bool appendOnly = false; - bool seekable = true; + bool isPipe = false; explicit FileObject(int fd) : FsObject(kType, fd) { if (fd >= 0) { off64_t pos = lseek64(fd, 0, SEEK_CUR); if (pos == -1 && errno == ESPIPE) { - seekable = false; + isPipe = true; } else if (pos >= 0) { filePos = pos; } diff --git a/dll/ntdll.cpp b/dll/ntdll.cpp index 0cb04c1..a0b6447 100644 --- a/dll/ntdll.cpp +++ b/dll/ntdll.cpp @@ -121,6 +121,9 @@ BOOL WIN_FUNC ResetEvent(HANDLE hEvent); namespace ntdll { +constexpr LARGE_INTEGER FILE_WRITE_TO_END_OF_FILE = static_cast(-1); +constexpr LARGE_INTEGER FILE_USE_FILE_POINTER_POSITION = static_cast(-2); + NTSTATUS WIN_FUNC NtReadFile(HANDLE FileHandle, HANDLE Event, PIO_APC_ROUTINE ApcRoutine, PVOID ApcContext, PIO_STATUS_BLOCK IoStatusBlock, PVOID Buffer, ULONG Length, PLARGE_INTEGER ByteOffset, PULONG Key) { @@ -132,48 +135,59 @@ NTSTATUS WIN_FUNC NtReadFile(HANDLE FileHandle, HANDLE Event, PIO_APC_ROUTINE Ap (void)Key; if (!IoStatusBlock) { - wibo::lastError = ERROR_INVALID_PARAMETER; return STATUS_INVALID_PARAMETER; } + IoStatusBlock->Information = 0; auto file = wibo::handles().getAs(FileHandle); if (!file || !file->valid()) { - wibo::lastError = ERROR_INVALID_HANDLE; IoStatusBlock->Status = STATUS_INVALID_HANDLE; IoStatusBlock->Information = 0; return STATUS_INVALID_HANDLE; } - bool handleOverlapped = file->overlapped; - std::optional offset; - bool updateFilePointer = !handleOverlapped; - if (ByteOffset) { - offset = static_cast(*ByteOffset); - } else if (handleOverlapped) { - updateFilePointer = false; + bool useOverlapped = file->overlapped; + bool useCurrentFilePosition = (ByteOffset == nullptr); + if (!useCurrentFilePosition && *ByteOffset == FILE_USE_FILE_POINTER_POSITION) { + useCurrentFilePosition = true; } + std::optional offset; + if (!useCurrentFilePosition) { + offset = static_cast(*ByteOffset); + } + + if (useOverlapped && useCurrentFilePosition) { + IoStatusBlock->Status = STATUS_INVALID_PARAMETER; + IoStatusBlock->Information = 0; + return STATUS_INVALID_PARAMETER; + } + + Pin ev; if (Event) { - kernel32::ResetEvent(Event); + ev = wibo::handles().getAs(Event); + if (!ev) { + IoStatusBlock->Status = STATUS_INVALID_HANDLE; + IoStatusBlock->Information = 0; + return STATUS_INVALID_HANDLE; + } + ev->reset(); } + bool updateFilePointer = !useOverlapped; auto io = files::read(file.get(), Buffer, Length, offset, updateFilePointer); - DWORD winError = ERROR_SUCCESS; NTSTATUS status = STATUS_SUCCESS; if (io.unixError != 0) { - winError = wibo::winErrorFromErrno(io.unixError); - status = wibo::statusFromWinError(winError); + status = wibo::statusFromErrno(io.unixError); } else if (io.reachedEnd && io.bytesTransferred == 0) { - winError = ERROR_HANDLE_EOF; - status = STATUS_END_OF_FILE; + status = file->isPipe ? STATUS_PIPE_BROKEN : STATUS_END_OF_FILE; } IoStatusBlock->Status = status; IoStatusBlock->Information = static_cast(io.bytesTransferred); - wibo::lastError = winError; - if (Event) { - kernel32::SetEvent(Event); + if (ev && (status == STATUS_SUCCESS || status == STATUS_END_OF_FILE)) { + ev->set(); } DEBUG_LOG("-> 0x%x\n", status); diff --git a/src/errors.h b/src/errors.h index acdc303..b9f5088 100644 --- a/src/errors.h +++ b/src/errors.h @@ -55,6 +55,7 @@ typedef int NTSTATUS; #define STATUS_PENDING ((NTSTATUS)0x00000103) #define STATUS_NOT_SUPPORTED ((NTSTATUS)0xC00000BB) #define STATUS_UNEXPECTED_IO_ERROR ((NTSTATUS)0xC00000E9) +#define STATUS_PIPE_BROKEN ((NTSTATUS)0xC000014B) typedef int HRESULT; #define S_OK ((HRESULT)0x00000000) diff --git a/src/files.cpp b/src/files.cpp index 25b292b..4bc575b 100644 --- a/src/files.cpp +++ b/src/files.cpp @@ -156,6 +156,30 @@ IOResult read(FileObject *file, void *buffer, size_t bytesToRead, const std::opt // Sanity check: if no offset is given, we must update the file pointer assert(offset.has_value() || updateFilePointer); + if (file->isPipe) { + std::lock_guard lk(file->m); + size_t chunk = bytesToRead > SSIZE_MAX ? SSIZE_MAX : bytesToRead; + uint8_t *in = static_cast(buffer); + ssize_t rc; + while (true) { + rc = ::read(file->fd, in, chunk); + if (rc == -1 && errno == EINTR) { + continue; + } + break; + } + if (rc == -1) { + result.unixError = errno ? errno : EIO; + return result; + } + if (rc == 0) { + result.reachedEnd = true; + return result; + } + result.bytesTransferred = static_cast(rc); + return result; + } + const auto doRead = [&](off64_t pos) { size_t total = 0; size_t remaining = bytesToRead; @@ -167,14 +191,12 @@ IOResult read(FileObject *file, void *buffer, size_t bytesToRead, const std::opt if (errno == EINTR) { continue; } - result.bytesTransferred = total; result.unixError = errno ? errno : EIO; - return; + break; } if (rc == 0) { - result.bytesTransferred = total; result.reachedEnd = true; - return; + break; } total += static_cast(rc); remaining -= static_cast(rc); @@ -185,7 +207,7 @@ IOResult read(FileObject *file, void *buffer, size_t bytesToRead, const std::opt if (updateFilePointer || !offset.has_value()) { std::lock_guard lk(file->m); - off64_t pos = offset.value_or(file->filePos); + const off64_t pos = offset.value_or(file->filePos); doRead(pos); if (updateFilePointer) { file->filePos = pos + static_cast(result.bytesTransferred); @@ -211,7 +233,7 @@ IOResult write(FileObject *file, const void *buffer, size_t bytesToWrite, const // Sanity check: if no offset is given, we must update the file pointer assert(offset.has_value() || updateFilePointer); - if (file->appendOnly || !file->seekable) { + if (file->appendOnly || file->isPipe) { std::lock_guard lk(file->m); size_t total = 0; size_t remaining = bytesToWrite; @@ -234,7 +256,7 @@ IOResult write(FileObject *file, const void *buffer, size_t bytesToWrite, const } result.bytesTransferred = total; if (updateFilePointer) { - off64_t pos = file->seekable ? lseek64(file->fd, 0, SEEK_CUR) : 0; + off64_t pos = file->isPipe ? 0 : lseek64(file->fd, 0, SEEK_CUR); if (pos >= 0) { file->filePos = pos; } else if (result.unixError == 0) { diff --git a/test/test_heap.c b/test/test_heap.c index 08907db..887e18f 100644 --- a/test/test_heap.c +++ b/test/test_heap.c @@ -29,7 +29,7 @@ int main(void) { } SetLastError(0); - void *inPlace = HeapReAlloc(processHeap, HEAP_REALLOC_IN_PLACE_ONLY, grown, 128); + void *inPlace = HeapReAlloc(processHeap, HEAP_REALLOC_IN_PLACE_ONLY, grown, 2048); TEST_CHECK(inPlace == NULL); TEST_CHECK_EQ(ERROR_NOT_ENOUGH_MEMORY, GetLastError()); diff --git a/test/test_ntreadfile.c b/test/test_ntreadfile.c new file mode 100644 index 0000000..73b38c0 --- /dev/null +++ b/test/test_ntreadfile.c @@ -0,0 +1,88 @@ +#include "test_assert.h" + +#define WIN32_LEAN_AND_MEAN +#include +#include +#include + +#ifndef STATUS_SUCCESS +#define STATUS_SUCCESS ((NTSTATUS)0x00000000L) +#endif + +#ifndef STATUS_END_OF_FILE +#define STATUS_END_OF_FILE ((NTSTATUS)0xC0000011L) +#endif + +static const char *kTempFileName = "ntreadfile_fixture.tmp"; + +typedef NTSTATUS(NTAPI *NtReadFile_t)(HANDLE, HANDLE, PIO_APC_ROUTINE, PVOID, PIO_STATUS_BLOCK, PVOID, ULONG, + PLARGE_INTEGER, PULONG); + +static NtReadFile_t load_ntreadfile(void) { + HMODULE mod = GetModuleHandleW(L"ntdll.dll"); + if (!mod) { + mod = LoadLibraryW(L"ntdll.dll"); + } + TEST_CHECK(mod != NULL); + FARPROC proc = GetProcAddress(mod, "NtReadFile"); + TEST_CHECK(proc != NULL); + NtReadFile_t fn = NULL; + TEST_CHECK(sizeof(fn) == sizeof(proc)); + memcpy(&fn, &proc, sizeof(fn)); + return fn; +} + +static void write_fixture(HANDLE file, const char *data, DWORD length) { + DWORD written = 0; + TEST_CHECK(WriteFile(file, data, length, &written, NULL)); + TEST_CHECK_EQ(length, written); + TEST_CHECK(SetFilePointer(file, 0, NULL, FILE_BEGIN) != INVALID_SET_FILE_POINTER); +} + +int main(void) { + NtReadFile_t fn = load_ntreadfile(); + + DeleteFileA(kTempFileName); + HANDLE file = CreateFileA(kTempFileName, GENERIC_READ | GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, + FILE_ATTRIBUTE_NORMAL, NULL); + TEST_CHECK(file != INVALID_HANDLE_VALUE); + + const char payload[] = "hello"; + write_fixture(file, payload, (DWORD)(sizeof(payload) - 1)); + + HANDLE event = CreateEventA(NULL, TRUE, TRUE, NULL); + TEST_CHECK(event != NULL); + + IO_STATUS_BLOCK iosb; + memset(&iosb, 0, sizeof(iosb)); + char buffer[6]; + memset(buffer, 0, sizeof(buffer)); + SetLastError(ERROR_GEN_FAILURE); + DWORD before = GetLastError(); + NTSTATUS status = fn(file, event, NULL, NULL, &iosb, buffer, 5, NULL, NULL); + TEST_CHECK_EQ((NTSTATUS)STATUS_SUCCESS, status); + TEST_CHECK_EQ(5u, (unsigned int)iosb.Information); + TEST_CHECK(memcmp(buffer, payload, 5) == 0); + TEST_CHECK_EQ(before, GetLastError()); + TEST_CHECK_EQ(WAIT_OBJECT_0, WaitForSingleObject(event, 0)); + TEST_CHECK(ResetEvent(event)); + + LARGE_INTEGER useCurrent; + useCurrent.QuadPart = -2; + IO_STATUS_BLOCK eofIosb; + memset(&eofIosb, 0, sizeof(eofIosb)); + memset(buffer, 0, sizeof(buffer)); + SetLastError(ERROR_GEN_FAILURE); + before = GetLastError(); + status = fn(file, event, NULL, NULL, &eofIosb, buffer, sizeof(buffer), &useCurrent, NULL); + TEST_CHECK_EQ((NTSTATUS)STATUS_END_OF_FILE, status); + TEST_CHECK_EQ(0u, (unsigned int)eofIosb.Information); + TEST_CHECK_EQ(before, GetLastError()); + TEST_CHECK(buffer[0] == 0); + TEST_CHECK_EQ(WAIT_OBJECT_0, WaitForSingleObject(event, 0)); + + TEST_CHECK(CloseHandle(event)); + TEST_CHECK(CloseHandle(file)); + TEST_CHECK(DeleteFileA(kTempFileName)); + return 0; +} diff --git a/test/test_pipe_io.c b/test/test_pipe_io.c new file mode 100644 index 0000000..26bcdc0 --- /dev/null +++ b/test/test_pipe_io.c @@ -0,0 +1,96 @@ +#include "test_assert.h" + +#define WIN32_LEAN_AND_MEAN +#include +#include + +#include + +#ifndef STATUS_SUCCESS +#define STATUS_SUCCESS ((NTSTATUS)0x00000000L) +#endif + +#ifndef STATUS_PIPE_BROKEN +#define STATUS_PIPE_BROKEN ((NTSTATUS)0xC000014BL) +#endif + +typedef NTSTATUS(NTAPI *NtReadFile_t)(HANDLE, HANDLE, PIO_APC_ROUTINE, PVOID, PIO_STATUS_BLOCK, PVOID, ULONG, + PLARGE_INTEGER, PULONG); + +static NtReadFile_t load_ntreadfile(void) { + HMODULE mod = GetModuleHandleW(L"ntdll.dll"); + if (!mod) { + TEST_CHECK_EQ(ERROR_MOD_NOT_FOUND, GetLastError()); + mod = LoadLibraryW(L"ntdll.dll"); + } + TEST_CHECK(mod != NULL); + FARPROC proc = GetProcAddress(mod, "NtReadFile"); + TEST_CHECK(proc != NULL); + NtReadFile_t fn = NULL; + TEST_CHECK(sizeof(fn) == sizeof(proc)); + memcpy(&fn, &proc, sizeof(fn)); + return fn; +} + +static void write_bytes(HANDLE handle, const char *data, size_t length) { + DWORD written = 0; + TEST_CHECK(WriteFile(handle, data, (DWORD)length, &written, NULL)); + TEST_CHECK_EQ((DWORD)length, written); +} + +int main(void) { + NtReadFile_t fn = load_ntreadfile(); + + HANDLE readPipe = NULL; + HANDLE writePipe = NULL; + TEST_CHECK(CreatePipe(&readPipe, &writePipe, NULL, 0)); + + const char msgReadFile[] = "pipe-read"; + write_bytes(writePipe, msgReadFile, strlen(msgReadFile)); + + char buffer[64]; + memset(buffer, 0, sizeof(buffer)); + DWORD bytesRead = 0; + TEST_CHECK(ReadFile(readPipe, buffer, sizeof(buffer), &bytesRead, NULL)); + TEST_CHECK_EQ((DWORD)strlen(msgReadFile), bytesRead); + TEST_CHECK(memcmp(buffer, msgReadFile, bytesRead) == 0); + + const char msgNtRead[] = "ntread"; + write_bytes(writePipe, msgNtRead, strlen(msgNtRead)); + + HANDLE event = CreateEventA(NULL, TRUE, FALSE, NULL); + TEST_CHECK(event != NULL); + + IO_STATUS_BLOCK iosb; + memset(&iosb, 0, sizeof(iosb)); + memset(buffer, 0, sizeof(buffer)); + NTSTATUS status = fn(readPipe, event, NULL, NULL, &iosb, buffer, sizeof(buffer), NULL, NULL); + TEST_CHECK_EQ((NTSTATUS)STATUS_SUCCESS, status); + TEST_CHECK_EQ((ULONG_PTR)strlen(msgNtRead), iosb.Information); + TEST_CHECK(memcmp(buffer, msgNtRead, iosb.Information) == 0); + TEST_CHECK_EQ(WAIT_OBJECT_0, WaitForSingleObject(event, 0)); + + TEST_CHECK(CloseHandle(writePipe)); + writePipe = NULL; + + bytesRead = 123; + SetLastError(ERROR_GEN_FAILURE); + BOOL ok = ReadFile(readPipe, buffer, sizeof(buffer), &bytesRead, NULL); + TEST_CHECK(!ok); + TEST_CHECK_EQ(0u, (unsigned int)bytesRead); + TEST_CHECK_EQ(ERROR_BROKEN_PIPE, GetLastError()); + TEST_CHECK(ResetEvent(event)); + + memset(&iosb, 0, sizeof(iosb)); + status = fn(readPipe, event, NULL, NULL, &iosb, buffer, sizeof(buffer), NULL, NULL); + TEST_CHECK_EQ((NTSTATUS)STATUS_PIPE_BROKEN, status); + TEST_CHECK_EQ(0u, (unsigned int)iosb.Information); + TEST_CHECK_EQ(WAIT_TIMEOUT, WaitForSingleObject(event, 0)); + + TEST_CHECK(CloseHandle(event)); + TEST_CHECK(CloseHandle(readPipe)); + if (writePipe) { + TEST_CHECK(CloseHandle(writePipe)); + } + return 0; +}