hrev54101 adds 2 changesets to branch 'master'
old head: 8ff2f81f6bf95180a6230b8ad6db008a5a3c38e4
new head: c1c239fefb6198f014b0d62c9de459fee3f509dc
overview:
https://git.haiku-os.org/haiku/log/?qt=range&q=c1c239fefb61+%5E8ff2f81f6bf9
----------------------------------------------------------------------------
d939cacf6bb5: kernel: Add assert in IOBuffer::SetVecs() that the passed length
is the actual length.
These are, at present, triggered on rare invocations of vfs_read_pages.
See #15912.
c1c239fefb61: kernel: Add mechanism in IOBuffer & IORequest to clamp the last
iovec.
This is needed by CreateSubRequest, which creates a sub-request
using some subset of the passed IO vectors. In the case that the
last vector will not be fully used, we need to clamp its size
in the sub-request to the remaining length.
do_iterative_fd_io, used by vfs_read_pages (which is in turn
used by the file cache) used this to split up requests
into their constituent block-run requests. So, previously,
the invalid IO requests created by this could, under one possible
interpretation, overwrite valid file data and cause disk corruption.
It is slightly unfortunate that generic_size_t has no unsigned
equivalent, so we are left with 0 as the magic number here,
instead of -1. However, the passed "length" remains unchanged,
so any callers that pass the wrong value for lastVecSize
will be trapped by the assert added in the previous commit
Fixes #15912 (the assert added in the previous commit),
and potentially disk corruption caused by this.
[ Augustin Cavalier <waddlesplash@xxxxxxxxx> ]
----------------------------------------------------------------------------
2 files changed, 21 insertions(+), 8 deletions(-)
src/system/kernel/device_manager/IORequest.cpp | 25 ++++++++++++++++------
src/system/kernel/device_manager/IORequest.h | 4 +++-
############################################################################
Commit: d939cacf6bb5b85a3d0a54f416c40b1e8f2a4add
URL: https://git.haiku-os.org/haiku/commit/?id=d939cacf6bb5
Author: Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date: Mon Apr 27 04:50:34 2020 UTC
Ticket: https://dev.haiku-os.org/ticket/15912
kernel: Add assert in IOBuffer::SetVecs() that the passed length is the actual
length.
These are, at present, triggered on rare invocations of vfs_read_pages.
See #15912.
----------------------------------------------------------------------------
diff --git a/src/system/kernel/device_manager/IORequest.cpp
b/src/system/kernel/device_manager/IORequest.cpp
index 1c0ae43139..f638979af7 100644
--- a/src/system/kernel/device_manager/IORequest.cpp
+++ b/src/system/kernel/device_manager/IORequest.cpp
@@ -126,6 +126,14 @@ IOBuffer::SetVecs(generic_size_t firstVecOffset, const
generic_io_vec* vecs,
fLength = length;
fPhysical = (flags & B_PHYSICAL_IO_REQUEST) != 0;
fUser = !fPhysical && IS_USER_ADDRESS(vecs[0].base);
+
+#if KDEBUG
+ generic_size_t actualLength = 0;
+ for (size_t i = 0; i < fVecCount; i++)
+ actualLength += fVecs[i].length;
+
+ ASSERT(actualLength == fLength);
+#endif
}
############################################################################
Revision: hrev54101
Commit: c1c239fefb6198f014b0d62c9de459fee3f509dc
URL: https://git.haiku-os.org/haiku/commit/?id=c1c239fefb61
Author: Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date: Tue Apr 28 02:30:08 2020 UTC
Ticket: https://dev.haiku-os.org/ticket/15912
kernel: Add mechanism in IOBuffer & IORequest to clamp the last iovec.
This is needed by CreateSubRequest, which creates a sub-request
using some subset of the passed IO vectors. In the case that the
last vector will not be fully used, we need to clamp its size
in the sub-request to the remaining length.
do_iterative_fd_io, used by vfs_read_pages (which is in turn
used by the file cache) used this to split up requests
into their constituent block-run requests. So, previously,
the invalid IO requests created by this could, under one possible
interpretation, overwrite valid file data and cause disk corruption.
It is slightly unfortunate that generic_size_t has no unsigned
equivalent, so we are left with 0 as the magic number here,
instead of -1. However, the passed "length" remains unchanged,
so any callers that pass the wrong value for lastVecSize
will be trapped by the assert added in the previous commit
Fixes #15912 (the assert added in the previous commit),
and potentially disk corruption caused by this.
----------------------------------------------------------------------------
diff --git a/src/system/kernel/device_manager/IORequest.cpp
b/src/system/kernel/device_manager/IORequest.cpp
index f638979af7..9c38a0a104 100644
--- a/src/system/kernel/device_manager/IORequest.cpp
+++ b/src/system/kernel/device_manager/IORequest.cpp
@@ -112,8 +112,8 @@ IOBuffer::Delete()
void
-IOBuffer::SetVecs(generic_size_t firstVecOffset, const generic_io_vec* vecs,
- uint32 count, generic_size_t length, uint32 flags)
+IOBuffer::SetVecs(generic_size_t firstVecOffset, generic_size_t lastVecSize,
+ const generic_io_vec* vecs, uint32 count, generic_size_t length, uint32
flags)
{
memcpy(fVecs, vecs, sizeof(generic_io_vec) * count);
@@ -121,6 +121,8 @@ IOBuffer::SetVecs(generic_size_t firstVecOffset, const
generic_io_vec* vecs,
fVecs[0].base += firstVecOffset;
fVecs[0].length -= firstVecOffset;
}
+ if (lastVecSize > 0)
+ fVecs[count - 1].length = lastVecSize;
fVecCount = count;
fLength = length;
@@ -750,8 +752,8 @@ IORequest::Init(off_t offset, generic_addr_t buffer,
generic_size_t length,
status_t
IORequest::Init(off_t offset, generic_size_t firstVecOffset,
- const generic_io_vec* vecs, size_t count, generic_size_t length, bool
write,
- uint32 flags)
+ generic_size_t lastVecSize, const generic_io_vec* vecs, size_t count,
+ generic_size_t length, bool write, uint32 flags)
{
ASSERT(offset >= 0);
@@ -759,7 +761,7 @@ IORequest::Init(off_t offset, generic_size_t firstVecOffset,
if (fBuffer == NULL)
return B_NO_MEMORY;
- fBuffer->SetVecs(firstVecOffset, vecs, count, length, flags);
+ fBuffer->SetVecs(firstVecOffset, lastVecSize, vecs, count, length,
flags);
fOwner = NULL;
fOffset = offset;
@@ -825,8 +827,9 @@ IORequest::CreateSubRequest(off_t parentOffset, off_t
offset,
if (subRequest == NULL)
return B_NO_MEMORY;
- status_t error = subRequest->Init(offset, vecOffset, vecs + startVec,
- endVec - startVec + 1, length, fIsWrite, fFlags &
~B_DELETE_IO_REQUEST);
+ status_t error = subRequest->Init(offset, vecOffset, remainingLength,
+ vecs + startVec, endVec - startVec + 1, length, fIsWrite,
+ fFlags & ~B_DELETE_IO_REQUEST);
if (error != B_OK) {
delete subRequest;
return error;
diff --git a/src/system/kernel/device_manager/IORequest.h
b/src/system/kernel/device_manager/IORequest.h
index 4601f7a70b..1e754b8c12 100644
--- a/src/system/kernel/device_manager/IORequest.h
+++ b/src/system/kernel/device_manager/IORequest.h
@@ -40,6 +40,7 @@ public:
bool IsUser() const { return
fUser; }
void SetVecs(generic_size_t
firstVecOffset,
+
generic_size_t lastVecSize,
const
generic_io_vec* vecs, uint32 count,
generic_size_t length, uint32 flags);
@@ -219,10 +220,11 @@ struct IORequest : IORequestChunk,
DoublyLinkedListLinkImpl<IORequest> {
status_t Init(off_t offset,
const generic_io_vec* vecs,
size_t
count, generic_size_t length,
bool
write, uint32 flags)
- {
return Init(offset, 0, vecs, count,
+ {
return Init(offset, 0, 0, vecs, count,
length, write, flags); }
status_t Init(off_t offset,
generic_size_t firstVecOffset,
+
generic_size_t lastVecSize,
const
generic_io_vec* vecs, size_t count,
generic_size_t length, bool write,
uint32
flags);
