From 7020e10ab8032996a4bf6d9c17ac9735f198ec26 Mon Sep 17 00:00:00 2001 From: Jim906 Date: Fri, 27 Dec 2024 10:47:37 -0500 Subject: [PATCH] userlandfs: Fix file_cache_read * Prevent the userlandfs server from calling clone_area on an area that might have already been deleted. * _HandleRequest(FileCacheReadRequest*) waits for a reply from the server when bytesRead > 0. This ensures that the server has time to use the area holding the returned data, before that area is deleted when the RequestAllocator goes out of scope in the kernel add-on. However, if bytesRead is 0, the server will still call clone_area, even though by that time the area has probably been deleted. This leads to a B_BAD_VALUE error when the FS tries to use the emulated file_cache_read at the end of a file, which differs from the normal behavior of file_cache_read. * _HandleRequest(ReadFromIORequestRequest*) has similar logic in that it waits for a server reply, but not if size == 0. It's possible that a similar problem could occur here. This test can be dropped if no requests with size 0 are ever sent from the server to begin with. * For other _HandleRequest overrides, the kernel never waits for a server reply, and this causes no problems. This could be because the size of data returned fits in the port buffer, so no external area needs to be created by RequestAllocator::AllocateAddress. Change-Id: If070901c25d446e00e67a74a7883808d8a38dae2 Reviewed-on: https://review.haiku-os.org/c/haiku/+/8721 Tested-by: Commit checker robot Reviewed-by: waddlesplash --- .../userlandfs/kernel_add_on/KernelRequestHandler.cpp | 2 +- .../kernel/file_systems/userlandfs/private/Requests.cpp | 7 ++++--- .../file_systems/userlandfs/server/fuse/FUSEVolume.cpp | 3 +++ .../userlandfs/server/haiku/haiku_kernel_emu.cpp | 3 +++ 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/add-ons/kernel/file_systems/userlandfs/kernel_add_on/KernelRequestHandler.cpp b/src/add-ons/kernel/file_systems/userlandfs/kernel_add_on/KernelRequestHandler.cpp index 6bfc4fb225..c0d040361a 100644 --- a/src/add-ons/kernel/file_systems/userlandfs/kernel_add_on/KernelRequestHandler.cpp +++ b/src/add-ons/kernel/file_systems/userlandfs/kernel_add_on/KernelRequestHandler.cpp @@ -803,7 +803,7 @@ KernelRequestHandler::_HandleRequest(ReadFromIORequestRequest* request) reply->error = result; // send the reply - if (reply->error == B_OK && size > 0) { + if (reply->error == B_OK) { SingleReplyRequestHandler handler(RECEIPT_ACK_REPLY); return fPort->SendRequest(&allocator, &handler); } diff --git a/src/add-ons/kernel/file_systems/userlandfs/private/Requests.cpp b/src/add-ons/kernel/file_systems/userlandfs/private/Requests.cpp index 420aec650e..5156ccd42f 100644 --- a/src/add-ons/kernel/file_systems/userlandfs/private/Requests.cpp +++ b/src/add-ons/kernel/file_systems/userlandfs/private/Requests.cpp @@ -5,12 +5,12 @@ #include "Debug.h" #include "Requests.h" -#define _ADD_ADDRESS(_address, _flags) \ +#define _ADD_ADDRESS(_address, _flags) { \ if (*count >= MAX_REQUEST_ADDRESS_COUNT) \ return B_BAD_VALUE; \ infos[*count].address = &_address; \ infos[*count].flags = _flags; \ - infos[(*count)++].max_size = INT32_MAX; // TODO:... + infos[(*count)++].max_size = INT32_MAX; } // TODO:... #define ADD_ADDRESS(address) _ADD_ADDRESS(address, 0) #define ADD_STRING(address) _ADD_ADDRESS(address, ADDRESS_IS_STRING) @@ -309,7 +309,8 @@ NotifyQueryRequest::GetAddressInfos(AddressInfo* infos, int32* count) status_t FileCacheReadReply::GetAddressInfos(AddressInfo* infos, int32* count) { - ADD_ADDRESS(buffer); + if (bytesRead > 0) + ADD_ADDRESS(buffer); return B_OK; } diff --git a/src/add-ons/kernel/file_systems/userlandfs/server/fuse/FUSEVolume.cpp b/src/add-ons/kernel/file_systems/userlandfs/server/fuse/FUSEVolume.cpp index c5f60bd367..3273d6a488 100644 --- a/src/add-ons/kernel/file_systems/userlandfs/server/fuse/FUSEVolume.cpp +++ b/src/add-ons/kernel/file_systems/userlandfs/server/fuse/FUSEVolume.cpp @@ -1022,6 +1022,9 @@ FUSEVolume::DoIO(void* _node, void* _cookie, const IORequestInfo& requestInfo) FUSENode* node = (FUSENode*)_node; FileCookie* cookie = (FileCookie*)_cookie; + if (requestInfo.length == 0) + return B_OK; + NodeReadLocker nodeLocker(this, node, true); if (nodeLocker.Status() != B_OK) RETURN_ERROR(nodeLocker.Status()); diff --git a/src/add-ons/kernel/file_systems/userlandfs/server/haiku/haiku_kernel_emu.cpp b/src/add-ons/kernel/file_systems/userlandfs/server/haiku/haiku_kernel_emu.cpp index 62e8c46aed..fc94c43347 100644 --- a/src/add-ons/kernel/file_systems/userlandfs/server/haiku/haiku_kernel_emu.cpp +++ b/src/add-ons/kernel/file_systems/userlandfs/server/haiku/haiku_kernel_emu.cpp @@ -384,6 +384,9 @@ io_request_length(const io_request* request) status_t read_from_io_request(io_request* _request, void* buffer, size_t size) { + if (size == 0) + return B_OK; + HaikuKernelIORequest* request = (HaikuKernelIORequest*)_request; // send the request