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
This commit is contained in:
Ingo Weinhold
2008-07-20 16:29:12 +00:00
parent 762472641e
commit e700fc1d5c
+22 -23
View File
@@ -171,7 +171,7 @@ FileMap::_MakeSpace(size_t count)
{ {
if (count <= CACHED_FILE_EXTENTS) { if (count <= CACHED_FILE_EXTENTS) {
// just use the reserved area in the file_cache_ref structure // 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 // the new size is smaller than the minimal array size
file_extent *array = fIndirect.array; file_extent *array = fIndirect.array;
memcpy(fDirect, array, sizeof(file_extent) * count); memcpy(fDirect, array, sizeof(file_extent) * count);
@@ -270,10 +270,13 @@ FileMap::_InvalidateAfter(off_t offset)
uint32 index; uint32 index;
file_extent* extent = _FindExtent(offset, &index); file_extent* extent = _FindExtent(offset, &index);
if (extent != NULL) { 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; 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; off_t end = offset + size;
while (mapOffset < end) { while (status == B_OK && mapOffset < end) {
// We don't have the requested extents yet, retrieve them // We don't have the requested extents yet, retrieve them
size_t vecCount = maxVecs; size_t vecCount = maxVecs;
status = vfs_get_file_map(Vnode(), mapOffset, ~0UL, vecs, &vecCount); status = vfs_get_file_map(Vnode(), mapOffset, ~0UL, vecs, &vecCount);
if (status < B_OK && status != B_BUFFER_OVERFLOW) if (status == B_OK || status == B_BUFFER_OVERFLOW)
return status; status = _Add(vecs, vecCount, mapOffset);
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) 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].offset = fileExtent->disk.offset + offset;
vecs[0].length = fileExtent->disk.length - 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; *_count = 1;
return B_OK; 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 // copy the rest of the vecs
size -= vecs[0].length; size -= vecs[0].length;
uint32 vecIndex = 1;
for (index = 1; index < fCount;) { while (true) {
fileExtent++; fileExtent++;
vecs[index] = fileExtent->disk; vecs[vecIndex++] = fileExtent->disk;
index++;
if (size <= fileExtent->disk.length) if (size <= fileExtent->disk.length) {
if (size < fileExtent->disk.length)
vecs[vecIndex - 1].length = size;
break; break;
}
if (index >= maxVecs) { if (vecIndex >= maxVecs) {
*_count = index; *_count = vecIndex;
return B_BUFFER_OVERFLOW; return B_BUFFER_OVERFLOW;
} }
size -= fileExtent->disk.length; size -= fileExtent->disk.length;
} }
*_count = index; *_count = vecIndex;
return B_OK; return B_OK;
} }