From e700fc1d5cf01984b8d974e5048b52da0b1a75d1 Mon Sep 17 00:00:00 2001 From: Ingo Weinhold Date: Sun, 20 Jul 2008 16:29:12 +0000 Subject: [PATCH] axeld + bonefish + mmlr: * FileMap::_InvalidateAfter(): - Off-by-one error. The last extend (which normally should be kept) was thrown away, too, but still accessed afterwards. Worst case could be a write to free()d memory. - Drop the last extend when it would be truncated to zero size. * FileMap::Translate(): - Incorrect handling of B_BUFFER_OVERFLOW case in the vfs_get_file_map() loop. After the loop the function would return incorrectly, making the caller think all vectors in the provided array had been initialized correctly. This could cause a file system implementation using the file map to read from or write to random disk locations, in the latter case possibly corrupting the file system. - Some readability improvements in the final loop. Removed incorrect check. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@26523 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- src/system/kernel/cache/file_map.cpp | 45 ++++++++++++++-------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/src/system/kernel/cache/file_map.cpp b/src/system/kernel/cache/file_map.cpp index d1299e1834..e0864d8a9d 100644 --- a/src/system/kernel/cache/file_map.cpp +++ b/src/system/kernel/cache/file_map.cpp @@ -171,7 +171,7 @@ FileMap::_MakeSpace(size_t count) { if (count <= CACHED_FILE_EXTENTS) { // just use the reserved area in the file_cache_ref structure - if (count <= CACHED_FILE_EXTENTS && fCount > CACHED_FILE_EXTENTS) { + if (fCount > CACHED_FILE_EXTENTS) { // the new size is smaller than the minimal array size file_extent *array = fIndirect.array; memcpy(fDirect, array, sizeof(file_extent) * count); @@ -270,10 +270,13 @@ FileMap::_InvalidateAfter(off_t offset) uint32 index; file_extent* extent = _FindExtent(offset, &index); if (extent != NULL) { - _MakeSpace(index); + _MakeSpace(index + 1); - if (extent->offset + extent->disk.length > offset) + if (extent->offset + extent->disk.length > offset) { extent->disk.length = offset - extent->offset; + if (extent->disk.length == 0) + _MakeSpace(index); + } } } @@ -345,21 +348,12 @@ FileMap::Translate(off_t offset, size_t size, file_io_vec* vecs, size_t* _count) off_t end = offset + size; - while (mapOffset < end) { + while (status == B_OK && mapOffset < end) { // We don't have the requested extents yet, retrieve them size_t vecCount = maxVecs; status = vfs_get_file_map(Vnode(), mapOffset, ~0UL, vecs, &vecCount); - if (status < B_OK && status != B_BUFFER_OVERFLOW) - return status; - - status_t addStatus = _Add(vecs, vecCount, mapOffset); - if (addStatus != B_OK) { - // only clobber the status in case of failure - status = addStatus; - } - - if (status != B_BUFFER_OVERFLOW) - break; + if (status == B_OK || status == B_BUFFER_OVERFLOW) + status = _Add(vecs, vecCount, mapOffset); } if (status != B_OK) @@ -375,7 +369,9 @@ FileMap::Translate(off_t offset, size_t size, file_io_vec* vecs, size_t* _count) vecs[0].offset = fileExtent->disk.offset + offset; vecs[0].length = fileExtent->disk.length - offset; - if (vecs[0].length >= size || index >= fCount - 1) { + if (vecs[0].length >= size) { + if (vecs[0].length > size) + vecs[0].length = size; *_count = 1; return B_OK; } @@ -383,25 +379,28 @@ FileMap::Translate(off_t offset, size_t size, file_io_vec* vecs, size_t* _count) // copy the rest of the vecs size -= vecs[0].length; + uint32 vecIndex = 1; - for (index = 1; index < fCount;) { + while (true) { fileExtent++; - vecs[index] = fileExtent->disk; - index++; + vecs[vecIndex++] = fileExtent->disk; - if (size <= fileExtent->disk.length) + if (size <= fileExtent->disk.length) { + if (size < fileExtent->disk.length) + vecs[vecIndex - 1].length = size; break; + } - if (index >= maxVecs) { - *_count = index; + if (vecIndex >= maxVecs) { + *_count = vecIndex; return B_BUFFER_OVERFLOW; } size -= fileExtent->disk.length; } - *_count = index; + *_count = vecIndex; return B_OK; }