From 5e998de248337b2609bb8ffa35782e107aea77b9 Mon Sep 17 00:00:00 2001 From: Julen Urizar Compains Date: Mon, 30 Mar 2026 12:01:28 +0200 Subject: [PATCH] [KERNEL32][KERNEL32_APITEST] Fix the pipe buffer overflow situation into Iosb.Status in the ReadFile (#6724) Change the way ReadFile and NTReadFile behaves under ReactOS, and fit more what Wine does. Fixes Calibre conversion failure. JIRA issue: CORE-17376 --- dll/win32/kernel32/client/file/rw.c | 36 +++--- .../rostests/apitests/kernel32/CMakeLists.txt | 1 + modules/rostests/apitests/kernel32/Pipes.c | 114 ++++++++++++++++++ modules/rostests/apitests/kernel32/testlist.c | 4 +- 4 files changed, 134 insertions(+), 21 deletions(-) create mode 100644 modules/rostests/apitests/kernel32/Pipes.c diff --git a/dll/win32/kernel32/client/file/rw.c b/dll/win32/kernel32/client/file/rw.c index 0c6905723e0..f6c66becb48 100644 --- a/dll/win32/kernel32/client/file/rw.c +++ b/dll/win32/kernel32/client/file/rw.c @@ -95,16 +95,14 @@ WriteFile(IN HANDLE hFile, if (NT_SUCCESS(Status)) Status = Iosb.Status; } - if (NT_SUCCESS(Status)) - { - /* - * lpNumberOfBytesWritten must not be NULL here, in fact Win doesn't - * check that case either and crashes (only after the operation - * completed). - */ - *lpNumberOfBytesWritten = Iosb.Information; - } - else + /* + * lpNumberOfBytesWritten must not be NULL here, in fact Win doesn't + * check that case either and crashes (only after the operation + * completed). + */ + *lpNumberOfBytesWritten = Iosb.Information; + + if (!NT_SUCCESS(Status)) { BaseSetLastNTError(Status); return FALSE; @@ -219,16 +217,14 @@ ReadFile(IN HANDLE hFile, return TRUE; } - if (NT_SUCCESS(Status)) - { - /* - * lpNumberOfBytesRead must not be NULL here, in fact Win doesn't - * check that case either and crashes (only after the operation - * completed). - */ - *lpNumberOfBytesRead = Iosb.Information; - } - else + /* + * lpNumberOfBytesRead must not be NULL here, in fact Win doesn't + * check that case either and crashes (only after the operation + * completed). + */ + *lpNumberOfBytesRead = Iosb.Information; + + if (!NT_SUCCESS(Status)) { BaseSetLastNTError(Status); return FALSE; diff --git a/modules/rostests/apitests/kernel32/CMakeLists.txt b/modules/rostests/apitests/kernel32/CMakeLists.txt index 7f3eca36b54..bba783049c3 100644 --- a/modules/rostests/apitests/kernel32/CMakeLists.txt +++ b/modules/rostests/apitests/kernel32/CMakeLists.txt @@ -32,6 +32,7 @@ list(APPEND SOURCE lstrlen.c Mailslot.c MultiByteToWideChar.c + Pipes.c PrivMoveFileIdentityW.c QueueUserAPC.c SetComputerNameExW.c diff --git a/modules/rostests/apitests/kernel32/Pipes.c b/modules/rostests/apitests/kernel32/Pipes.c new file mode 100644 index 00000000000..e3fe95e83b4 --- /dev/null +++ b/modules/rostests/apitests/kernel32/Pipes.c @@ -0,0 +1,114 @@ +/* + * PROJECT: ReactOS API tests + * LICENSE: GPL-2.0-or-later (https://spdx.org/licenses/GPL-2.0-or-later) + * PURPOSE: Test for pipe (CORE-17376) + * COPYRIGHT: Copyright 2024 Simone Mario Lombardo + * Copyright 2024 Julen Urizar Compains + */ + +#include "precomp.h" + +static PCWSTR g_PipeName = L"\\\\.\\pipe\\rostest_pipe"; + +#define MINBUFFERSIZE 1 +#define MAXBUFFERSIZE 255 +#define TEST_MESSAGE "Test" +#define TEST_MESSAGE_SIZE (sizeof(TEST_MESSAGE) - sizeof(ANSI_NULL)) + +static DWORD g_dwReadBufferSize; + +DWORD WINAPI PipeWriter(_In_ PVOID Param) +{ + HANDLE hPipe = (HANDLE)Param; + DWORD cbWritten = 0; + BOOL Success = WriteFile(hPipe, TEST_MESSAGE, TEST_MESSAGE_SIZE, &cbWritten, NULL); + + ok(Success, "WriteFile() failed, last error = 0x%lx\n", GetLastError()); + ok_int(cbWritten, TEST_MESSAGE_SIZE); + + return 0; +} + +DWORD WINAPI PipeReader(_In_ PVOID Param) +{ + HANDLE hPipe = (HANDLE)Param; + CHAR outMsg[MAXBUFFERSIZE]; + + DWORD cbRead = 0; + BOOL Success = ReadFile(hPipe, outMsg, g_dwReadBufferSize, &cbRead, NULL); + + if (g_dwReadBufferSize == MINBUFFERSIZE) + ok(!Success, "ReadFile() succeeded unexpectedly\n"); + else + ok(Success, "ReadFile() failed, last error = 0x%lx\n", GetLastError()); + + ok(cbRead != 0, "cbRead == 0\n"); + + if (g_dwReadBufferSize == MINBUFFERSIZE) + ok_hex(GetLastError(), ERROR_MORE_DATA); + + return 0; +} + +VOID StartTestCORE17376(_In_ DWORD adReadBufferSize) +{ + HANDLE hPipeReader, hPipeWriter, hThreadReader, hThreadWriter; + trace("adReadBufferSize = %lu - START\n", adReadBufferSize); + + g_dwReadBufferSize = adReadBufferSize; + + hPipeReader = CreateNamedPipeW( + g_PipeName, + PIPE_ACCESS_DUPLEX, + PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE, + 1, + MAXBUFFERSIZE, + MAXBUFFERSIZE, + 0, + NULL); + ok(hPipeReader != INVALID_HANDLE_VALUE, "CreateNamedPipeW failed\n"); + + if (hPipeReader == INVALID_HANDLE_VALUE) + return; + + hThreadReader = CreateThread(NULL, 0, PipeReader, hPipeReader, 0, NULL); + ok(hThreadReader != INVALID_HANDLE_VALUE, "CreateThread failed\n"); + + hPipeWriter = CreateFileW( + g_PipeName, + GENERIC_READ | GENERIC_WRITE, + FILE_SHARE_READ | FILE_SHARE_WRITE, + NULL, + OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, + NULL); + ok(hPipeWriter != INVALID_HANDLE_VALUE, "CreateFileW failed\n"); + + if (hPipeWriter != INVALID_HANDLE_VALUE) + { + hThreadWriter = CreateThread(NULL, 0, PipeWriter, hPipeWriter, 0, NULL); + ok(hThreadWriter != INVALID_HANDLE_VALUE, "CreateThread failed\n"); + + if (hThreadWriter != INVALID_HANDLE_VALUE) + { + WaitForSingleObject(hThreadWriter, INFINITE); + CloseHandle(hThreadWriter); + } + CloseHandle(hPipeWriter); + } + + if (hThreadReader != INVALID_HANDLE_VALUE) + { + WaitForSingleObject(hThreadReader, INFINITE); + CloseHandle(hThreadReader); + } + CloseHandle(hPipeReader); + + trace("adReadBufferSize = %lu - COMPLETED\n", adReadBufferSize); +} + +START_TEST(Pipes) +{ + StartTestCORE17376(MINBUFFERSIZE); + StartTestCORE17376(MAXBUFFERSIZE); +} diff --git a/modules/rostests/apitests/kernel32/testlist.c b/modules/rostests/apitests/kernel32/testlist.c index 79c15c9d71b..3bec920a41c 100644 --- a/modules/rostests/apitests/kernel32/testlist.c +++ b/modules/rostests/apitests/kernel32/testlist.c @@ -31,6 +31,7 @@ extern void func_lstrcpynW(void); extern void func_lstrlen(void); extern void func_Mailslot(void); extern void func_MultiByteToWideChar(void); +extern void func_Pipes(void); extern void func_PrivMoveFileIdentityW(void); extern void func_QueueUserAPC(void); extern void func_SetComputerNameExW(void); @@ -45,6 +46,7 @@ extern void func_WideCharToMultiByte(void); const struct test winetest_testlist[] = { + { "ActCtxWithXmlNamespaces", func_ActCtxWithXmlNamespaces }, { "ConsoleCP", func_ConsoleCP }, { "CreateProcess", func_CreateProcess }, { "DefaultActCtx", func_DefaultActCtx }, @@ -73,6 +75,7 @@ const struct test winetest_testlist[] = { "lstrlen", func_lstrlen }, { "MailslotRead", func_Mailslot }, { "MultiByteToWideChar", func_MultiByteToWideChar }, + { "Pipes", func_Pipes }, { "PrivMoveFileIdentityW", func_PrivMoveFileIdentityW }, { "QueueUserAPC", func_QueueUserAPC }, { "SetComputerNameExW", func_SetComputerNameExW }, @@ -84,6 +87,5 @@ const struct test winetest_testlist[] = { "TunnelCache", func_TunnelCache }, { "UEFIFirmware", func_UEFIFirmware }, { "WideCharToMultiByte", func_WideCharToMultiByte }, - { "ActCtxWithXmlNamespaces", func_ActCtxWithXmlNamespaces }, { 0, 0 } };