From 5c56d775a9ef7f4dc3508b3ea4f2377324b6a575 Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Wed, 20 Nov 2024 15:47:56 -0500 Subject: [PATCH] kernel/fs: Fix handling of partial requests in do_synchronous_iterative_vnode_io. If get_vecs returns 0 and no vecs, then we need to quit iterating. Otherwise we'll just loop forever and leak memory. Also add an assert in IOBuffer::GetNextVirtualVec that would have caught this problem, and the memory leak it results in. --- src/system/kernel/device_manager/IORequest.cpp | 5 +++-- src/system/kernel/fs/vfs_request_io.cpp | 15 +++++++++++---- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/system/kernel/device_manager/IORequest.cpp b/src/system/kernel/device_manager/IORequest.cpp index 2937261014..7d29b7456b 100644 --- a/src/system/kernel/device_manager/IORequest.cpp +++ b/src/system/kernel/device_manager/IORequest.cpp @@ -165,9 +165,10 @@ IOBuffer::GetNextVirtualVec(void*& _cookie, iovec& vector) } if (cookie->vec_index == 0 - && (fVecCount > 1 || fVecs[0].length > B_PAGE_SIZE)) { + && (fVecCount > 1 || fVecs[0].length > B_PAGE_SIZE)) { void* mappedAddress; addr_t mappedSize; + ASSERT(cookie->mapped_area == -1); // TODO: This is a potential violation of the VIP requirement, since // vm_map_physical_memory_vecs() allocates memory without special flags! @@ -216,9 +217,9 @@ void IOBuffer::FreeVirtualVecCookie(void* _cookie) { virtual_vec_cookie* cookie = (virtual_vec_cookie*)_cookie; + if (cookie->mapped_area >= 0) delete_area(cookie->mapped_area); - cookie->PutPhysicalPageIfNeeded(); free_etc(cookie, fVIP ? HEAP_PRIORITY_VIP : 0); diff --git a/src/system/kernel/fs/vfs_request_io.cpp b/src/system/kernel/fs/vfs_request_io.cpp index 2b991c540d..cfb40264ff 100644 --- a/src/system/kernel/fs/vfs_request_io.cpp +++ b/src/system/kernel/fs/vfs_request_io.cpp @@ -292,8 +292,9 @@ do_synchronous_iterative_vnode_io(struct vnode* vnode, void* openCookie, generic_size_t length = request->Length(); status_t error = B_OK; + bool partial = false; - for (; error == B_OK && length > 0 + for (; error == B_OK && length > 0 && !partial && buffer->GetNextVirtualVec(virtualVecCookie, vector) == B_OK;) { uint8* vecBase = (uint8*)vector.iov_base; generic_size_t vecLength = min_c(vector.iov_len, length); @@ -309,8 +310,12 @@ do_synchronous_iterative_vnode_io(struct vnode* vnode, void* openCookie, fileVecs[0].length = vecLength; fileVecCount = 1; } - if (error != B_OK || fileVecCount == 0) + if (error != B_OK) break; + if (fileVecCount == 0) { + partial = true; + break; + } for (size_t i = 0; i < fileVecCount; i++) { const file_io_vec& fileVec = fileVecs[i]; @@ -325,15 +330,17 @@ do_synchronous_iterative_vnode_io(struct vnode* vnode, void* openCookie, vecBase += transferred; vecLength -= transferred; - if (transferred != toTransfer) + if (transferred != toTransfer) { + partial = true; break; + } } } } buffer->FreeVirtualVecCookie(virtualVecCookie); - bool partial = length > 0; + partial = (partial || length > 0); size_t bytesTransferred = request->Length() - length; request->SetTransferredBytes(partial, bytesTransferred); if (finished != NULL)