Changeset 26523

Timestamp:

07/20/08 12:29:12 (4 months ago)

Author:

bonefish

Message:

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.

Files:

• haiku/trunk/src/system/kernel/cache/file_map.cpp (modified) (6 diffs)

haiku/trunk/src/system/kernel/cache/file_map.cpp

@@ -172 +172 @@
         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;
@@ -271 +271 @@
         file_extent* extent = _FindExtent(offset, &index);
         if (extent != NULL) {
-                _MakeSpace(index);
-
-                if (extent->offset + extent->disk.length > offset)
+                _MakeSpace(index + 1);
+
+                if (extent->offset + extent->disk.length > offset) {
                         extent->disk.length = offset - extent->offset;
+                        if (extent->disk.length == 0)
+                                _MakeSpace(index);
+                }
         }
 }
@@ -346 +349 @@
         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);
         }
 
@@ -376 +370 @@
         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;
@@ -384 +380 @@
 
         size -= vecs[0].length;
-
-        for (index = 1; index < fCount;) {
+        uint32 vecIndex = 1;
+
+        while (true) {
                 fileExtent++;
 
-                vecs[index] = fileExtent->disk;
-                index++;
-
-                if (size <= fileExtent->disk.length)
+                vecs[vecIndex++] = fileExtent->disk;
+
+                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;
                 }
@@ -402 +401 @@
         }
 
-        *_count = index;
+        *_count = vecIndex;
         return B_OK;
 }