i2p/haiku-beta6
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

* Fixed two bugs found by Ingo: the tempVec array bounds weren't checked when

Browse Source 
filling them which could have written over the stack, and their iovec length
  was set for the wrong iovec, potentially clobbering any memory.
* The first tempVec was usually empty, anyway, as the wrong iovec was chosen
  to start from (usually one too early).
* The tempVec loop is now repeated until the whole fileVec is completed.
* Minor cleanup.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@20476 a95241bf-73f2-0310-859d-f6bbb57e9c96
This commit is contained in:
Axel Dörfler
2007-03-31 09:54:56 +00:00
parent dab183a745
commit 061816eefa
1 changed files with 97 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/system/kernel/cache/file_cache.cpp
Vendored
+97 -68
View File
@@ -32,6 +32,7 @@
// maximum number of iovecs per request // maximum number of iovecs per request
#define MAX_IO_VECS 64 // 256 kB #define MAX_IO_VECS 64 // 256 kB
#define MAX_FILE_IO_VECS 32 #define MAX_FILE_IO_VECS 32
#define MAX_TEMP_IO_VECS 8
#define CACHED_FILE_EXTENTS 2 #define CACHED_FILE_EXTENTS 2
// must be smaller than MAX_FILE_IO_VECS // must be smaller than MAX_FILE_IO_VECS
@@ -190,7 +191,7 @@ add_to_iovec(iovec *vecs, int32 &index, int32 max, addr_t address, size_t size)
static file_extent * static file_extent *
find_file_extent(file_cache_ref *ref, off_t offset, uint32 *_index) find_file_extent(file_cache_ref *ref, off_t offset, uint32 *_index)
{ {
// ToDo: do binary search // TODO: do binary search
for (uint32 index = 0; index < ref->map.count; index++) { for (uint32 index = 0; index < ref->map.count; index++) {
file_extent *extent = ref->map[index]; file_extent *extent = ref->map[index];
@@ -303,37 +304,45 @@ get_file_map(file_cache_ref *ref, off_t offset, size_t size,
} }
/*!
Does the dirty work of translating the request into actual disk offsets
and reads to or writes from the supplied iovecs as specified by \a doWrite.
*/
static status_t static status_t
pages_io(file_cache_ref *ref, off_t offset, const iovec *vecs, size_t count, pages_io(file_cache_ref *ref, off_t offset, const iovec *vecs, size_t count,
size_t *_numBytes, bool doWrite) size_t *_numBytes, bool doWrite)
{ {
TRACE(("pages_io: ref = %p, offset = %Ld, size = %lu, %s\n", ref, offset, TRACE(("pages_io: ref = %p, offset = %Ld, size = %lu, vecCount = %lu, %s\n", ref, offset,
*_numBytes, doWrite ? "write" : "read")); *_numBytes, count, doWrite ? "write" : "read"));
// translate the iovecs into direct device accesses // translate the iovecs into direct device accesses
file_io_vec fileVecs[MAX_FILE_IO_VECS]; file_io_vec fileVecs[MAX_FILE_IO_VECS];
size_t fileVecCount = MAX_FILE_IO_VECS; size_t fileVecCount = MAX_FILE_IO_VECS;
size_t numBytes = *_numBytes; size_t numBytes = *_numBytes;
status_t status = get_file_map(ref, offset, numBytes, fileVecs, &fileVecCount); status_t status = get_file_map(ref, offset, numBytes, fileVecs,
&fileVecCount);
if (status < B_OK) { if (status < B_OK) {
TRACE(("get_file_map(offset = %Ld, numBytes = %lu) failed\n", offset, TRACE(("get_file_map(offset = %Ld, numBytes = %lu) failed\n", offset,
numBytes)); numBytes));
return status; return status;
} }
// ToDo: handle array overflow gracefully! // TODO: handle array overflow gracefully!
#ifdef TRACE_FILE_CACHE #ifdef TRACE_FILE_CACHE
dprintf("got %lu file vecs for %Ld:%lu:\n", fileVecCount, offset, numBytes); dprintf("got %lu file vecs for %Ld:%lu:\n", fileVecCount, offset, numBytes);
for (size_t i = 0; i < fileVecCount; i++) for (size_t i = 0; i < fileVecCount; i++) {
dprintf("[%lu] offset = %Ld, size = %Ld\n", i, fileVecs[i].offset, fileVecs[i].length); dprintf(" [%lu] offset = %Ld, size = %Ld\n",
i, fileVecs[i].offset, fileVecs[i].length);
}
#endif #endif
if (fileVecCount == 0) { if (fileVecCount == 0) {
// There are no file vecs at this offset, so we're obviously trying // There are no file vecs at this offset, so we're obviously trying
// to access the file outside of its bounds // to access the file outside of its bounds
TRACE(("pages_io: access outside of vnode %p at offset %Ld\n", ref->vnode, offset)); TRACE(("pages_io: access outside of vnode %p at offset %Ld\n",
ref->vnode, offset));
return B_BAD_VALUE; return B_BAD_VALUE;
} }
@@ -353,9 +362,10 @@ pages_io(file_cache_ref *ref, off_t offset, const iovec *vecs, size_t count,
if (status < B_OK) if (status < B_OK)
return status; return status;
// ToDo: this is a work-around for buggy device drivers! // TODO: this is a work-around for buggy device drivers!
// When our own drivers honour the length, we can: // When our own drivers honour the length, we can:
// a) also use this direct I/O for writes (otherwise, it would overwrite precious data) // a) also use this direct I/O for writes (otherwise, it would
// overwrite precious data)
// b) panic if the term below is true (at least for writes) // b) panic if the term below is true (at least for writes)
if (size > fileVecs[0].length) { if (size > fileVecs[0].length) {
//dprintf("warning: device driver %p doesn't respect total length in read_pages() call!\n", ref->device); //dprintf("warning: device driver %p doesn't respect total length in read_pages() call!\n", ref->device);
@@ -373,7 +383,7 @@ pages_io(file_cache_ref *ref, off_t offset, const iovec *vecs, size_t count,
*_numBytes = size; *_numBytes = size;
return B_OK; return B_OK;
} }
fileVecIndex = 1; fileVecIndex = 1;
} else { } else {
fileVecIndex = 0; fileVecIndex = 0;
@@ -387,61 +397,79 @@ pages_io(file_cache_ref *ref, off_t offset, const iovec *vecs, size_t count,
// first, find out where we have to continue in our iovecs // first, find out where we have to continue in our iovecs
uint32 i = 0; uint32 i = 0;
for (; i < count; i++) { for (; i < count; i++) {
if (size <= vecs[i].iov_len) if (size < vecs[i].iov_len)
break; break;
size -= vecs[i].iov_len; size -= vecs[i].iov_len;
} }
size_t vecOffset = size; size_t vecOffset = size;
size_t bytesLeft = numBytes - size;
for (; fileVecIndex < fileVecCount; fileVecIndex++) { for (; fileVecIndex < fileVecCount; fileVecIndex++) {
file_io_vec &fileVec = fileVecs[fileVecIndex]; file_io_vec &fileVec = fileVecs[fileVecIndex];
iovec tempVecs[8]; off_t fileOffset = fileVec.offset;
uint32 tempCount = 1; off_t fileLeft = fileVec.length;
tempVecs[0].iov_base = (void *)((addr_t)vecs[i].iov_base + vecOffset); fileLeft = min_c(fileVec.length, bytesLeft);
size = min_c(vecs[i].iov_len - vecOffset, fileVec.length); TRACE(("FILE VEC [%lu] length %Ld\n", fileVecIndex, fileLeft));
tempVecs[0].iov_len = size;
TRACE(("fill vec %ld, offset = %lu, size = %lu\n", i, vecOffset, size)); while (fileLeft > 0) {
iovec tempVecs[MAX_TEMP_IO_VECS];
uint32 tempCount = 1;
if (size >= fileVec.length) size = min_c(vecs[i].iov_len - vecOffset, fileLeft);
vecOffset += size;
else
vecOffset = 0;
while (size < fileVec.length && ++i < count) { tempVecs[0].iov_base = (void *)((addr_t)vecs[i].iov_base + vecOffset);
tempVecs[tempCount].iov_base = vecs[i].iov_base; tempVecs[0].iov_len = size;
tempCount++;
// is this iovec larger than the file_io_vec? if (size >= fileLeft)
if (vecs[i].iov_len + size > fileVec.length) { vecOffset += size;
size += tempVecs[tempCount].iov_len = vecOffset = fileVec.length - size; else
break; vecOffset = 0;
while (size < fileLeft && ++i < count
&& tempCount < MAX_TEMP_IO_VECS) {
TRACE(("fill vec %ld, offset = %lu, size = %lu\n",
i, vecOffset, size));
tempVecs[tempCount].iov_base = vecs[i].iov_base;
// is this iovec larger than the file_io_vec?
if (vecs[i].iov_len + size > fileLeft) {
size += tempVecs[tempCount].iov_len
= vecOffset = fileLeft - size;
tempCount++;
break;
}
size += tempVecs[tempCount].iov_len = vecs[i].iov_len;
tempCount++;
} }
size += tempVecs[tempCount].iov_len = vecs[i].iov_len; size_t bytes = size;
} if (doWrite) {
status = vfs_write_pages(ref->device, ref->cookie, fileOffset,
tempVecs, tempCount, &bytes, false);
} else {
status = vfs_read_pages(ref->device, ref->cookie, fileOffset,
tempVecs, tempCount, &bytes, false);
}
if (status < B_OK)
return status;
size_t bytes = size; totalSize += size;
if (doWrite) { bytesLeft -= size;
status = vfs_write_pages(ref->device, ref->cookie, fileVec.offset, tempVecs, fileOffset += size;
tempCount, &bytes, false); fileLeft -= size;
} else { //dprintf("-> file left = %Lu\n", fileLeft);
status = vfs_read_pages(ref->device, ref->cookie, fileVec.offset, tempVecs,
tempCount, &bytes, false);
}
if (status < B_OK)
return status;
totalSize += size; if (size != bytes) {
// there are no more bytes, let's bail out
if (size != bytes) { *_numBytes = totalSize;
// there are no more bytes, let's bail out return B_OK;
*_numBytes = totalSize; }
return B_OK;
} }
} }
@@ -449,11 +477,11 @@ pages_io(file_cache_ref *ref, off_t offset, const iovec *vecs, size_t count,
} }
/** This function is called by read_into_cache() (and from there only) - it /*!
* can only handle a certain amount of bytes, and read_into_cache() makes This function is called by read_into_cache() (and from there only) - it
* sure that it matches that criterion. can only handle a certain amount of bytes, and read_into_cache() makes
*/ sure that it matches that criterion.
*/
static inline status_t static inline status_t
read_chunk_into_cache(file_cache_ref *ref, off_t offset, size_t size, read_chunk_into_cache(file_cache_ref *ref, off_t offset, size_t size,
int32 pageOffset, addr_t buffer, size_t bufferSize) int32 pageOffset, addr_t buffer, size_t bufferSize)
@@ -480,11 +508,12 @@ read_chunk_into_cache(file_cache_ref *ref, off_t offset, size_t size,
vm_cache_insert_page(cache, page, offset + pos); vm_cache_insert_page(cache, page, offset + pos);
addr_t virtualAddress; addr_t virtualAddress;
if (vm_get_physical_page(page->physical_page_number * B_PAGE_SIZE, &virtualAddress, PHYSICAL_PAGE_CAN_WAIT) < B_OK) if (vm_get_physical_page(page->physical_page_number * B_PAGE_SIZE,
&virtualAddress, PHYSICAL_PAGE_CAN_WAIT) < B_OK)
panic("could not get physical page"); panic("could not get physical page");
add_to_iovec(vecs, vecCount, MAX_IO_VECS, virtualAddress, B_PAGE_SIZE); add_to_iovec(vecs, vecCount, MAX_IO_VECS, virtualAddress, B_PAGE_SIZE);
// ToDo: check if the array is large enough! // TODO: check if the array is large enough (currently panics)!
} }
mutex_unlock(&cache->lock); mutex_unlock(&cache->lock);
@@ -499,9 +528,11 @@ read_chunk_into_cache(file_cache_ref *ref, off_t offset, size_t size,
for (int32 i = 0; i < vecCount; i++) { for (int32 i = 0; i < vecCount; i++) {
addr_t base = (addr_t)vecs[i].iov_base; addr_t base = (addr_t)vecs[i].iov_base;
size_t size = vecs[i].iov_len; size_t size = vecs[i].iov_len;
for (size_t pos = 0; pos < size; pos += B_PAGE_SIZE, base += B_PAGE_SIZE) for (size_t pos = 0; pos < size;
pos += B_PAGE_SIZE, base += B_PAGE_SIZE) {
vm_put_physical_page(base); vm_put_physical_page(base);
}
} }
mutex_lock(&cache->lock); mutex_lock(&cache->lock);
@@ -544,14 +575,14 @@ read_chunk_into_cache(file_cache_ref *ref, off_t offset, size_t size,
} }
/** This function reads \a size bytes directly from the file into the cache. /*!
* If \a bufferSize does not equal zero, \a bufferSize bytes from the data This function reads \a size bytes directly from the file into the cache.
* read in are also copied to the provided \a buffer. If \a bufferSize does not equal zero, \a bufferSize bytes from the data
* This function always allocates all pages; it is the responsibility of the read in are also copied to the provided \a buffer.
* calling function to only ask for yet uncached ranges. This function always allocates all pages; it is the responsibility of the
* The cache_ref lock must be hold when calling this function. calling function to only ask for yet uncached ranges.
*/ The cache_ref lock must be hold when calling this function.
*/
static status_t static status_t
read_into_cache(file_cache_ref *ref, off_t offset, size_t size, addr_t buffer, size_t bufferSize) read_into_cache(file_cache_ref *ref, off_t offset, size_t size, addr_t buffer, size_t bufferSize)
{ {
@@ -1016,8 +1047,7 @@ file_cache_control(const char *subsystem, uint32 function, void *buffer, size_t
} }
// #pragma mark - // #pragma mark - private kernel API
// kernel public API
extern "C" void extern "C" void
@@ -1164,8 +1194,7 @@ file_cache_init(void)
} }
// #pragma mark - // #pragma mark - public FS API
// public FS API
extern "C" void * extern "C" void *