[haiku-commits] r35743 - haiku/trunk/src/add-ons/kernel/file_systems/bfs
- From: axeld@xxxxxxxxxxxxxxxx
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Wed, 3 Mar 2010 20:38:57 +0100 (CET)
Author: axeld
Date: 2010-03-03 20:38:56 +0100 (Wed, 03 Mar 2010)
New Revision: 35743
Changeset: http://dev.haiku-os.org/changeset/35743/haiku
Ticket: http://dev.haiku-os.org/ticket/3264
Modified:
haiku/trunk/src/add-ons/kernel/file_systems/bfs/BlockAllocator.cpp
haiku/trunk/src/add-ons/kernel/file_systems/bfs/BlockAllocator.h
haiku/trunk/src/add-ons/kernel/file_systems/bfs/bfs_control.h
haiku/trunk/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp
Log:
* Removed the cookie field of the check_control structure; that really was
a stupid idea. Instead, the already existing fCheckCookie member is used.
* bfs_ioctl() now accesses all userland buffers safely, this should help with
#3264, and move the crash where it belongs.
* Changes not yet tested; they only affect checkfs.
Modified: haiku/trunk/src/add-ons/kernel/file_systems/bfs/BlockAllocator.cpp
===================================================================
--- haiku/trunk/src/add-ons/kernel/file_systems/bfs/BlockAllocator.cpp
2010-03-03 19:20:51 UTC (rev 35742)
+++ haiku/trunk/src/add-ons/kernel/file_systems/bfs/BlockAllocator.cpp
2010-03-03 19:38:56 UTC (rev 35743)
@@ -1177,7 +1177,7 @@
bool
-BlockAllocator::_IsValidCheckControl(check_control* control)
+BlockAllocator::_IsValidCheckControl(const check_control* control)
{
if (control == NULL
|| control->magic != BFS_IOCTL_CHECK_MAGIC) {
@@ -1190,7 +1190,7 @@
status_t
-BlockAllocator::StartChecking(check_control* control)
+BlockAllocator::StartChecking(const check_control* control)
{
if (!_IsValidCheckControl(control))
return B_BAD_VALUE;
@@ -1212,8 +1212,8 @@
return B_NO_MEMORY;
}
- check_cookie* cookie = new check_cookie();
- if (cookie == NULL) {
+ fCheckCookie = new check_cookie();
+ if (fCheckCookie == NULL) {
free(fCheckBitmap);
fCheckBitmap = NULL;
mutex_unlock(&fLock);
@@ -1229,19 +1229,15 @@
_SetCheckBitmapAt(block);
}
- cookie->stack.Push(fVolume->Root());
- cookie->stack.Push(fVolume->Indices());
- cookie->iterator = NULL;
- control->cookie = cookie;
+ fCheckCookie->stack.Push(fVolume->Root());
+ fCheckCookie->stack.Push(fVolume->Indices());
+ fCheckCookie->iterator = NULL;
- fCheckCookie = cookie;
- // to be able to restore nicely if "chkbfs" exited abnormally
-
// Put removed vnodes to the stack -- they are not reachable by
traversing
// the file system anymore.
InodeList::Iterator iterator = fVolume->RemovedInodes().GetIterator();
while (Inode* inode = iterator.Next()) {
- cookie->stack.Push(inode->BlockRun());
+ fCheckCookie->stack.Push(inode->BlockRun());
}
// TODO: check reserved area in bitmap!
@@ -1253,21 +1249,15 @@
status_t
BlockAllocator::StopChecking(check_control* control)
{
- check_cookie* cookie;
- if (control == NULL)
- cookie = fCheckCookie;
- else
- cookie = (check_cookie*)control->cookie;
-
- if (cookie == NULL)
+ if (fCheckCookie == NULL)
return B_ERROR;
- if (cookie->iterator != NULL) {
- delete cookie->iterator;
- cookie->iterator = NULL;
+ if (fCheckCookie->iterator != NULL) {
+ delete fCheckCookie->iterator;
+ fCheckCookie->iterator = NULL;
// the current directory inode is still locked in memory
- put_vnode(fVolume->FSVolume(),
fVolume->ToVnode(cookie->current));
+ put_vnode(fVolume->FSVolume(),
fVolume->ToVnode(fCheckCookie->current));
}
if (fVolume->IsReadOnly()) {
@@ -1351,8 +1341,8 @@
free(fCheckBitmap);
fCheckBitmap = NULL;
+ delete fCheckCookie;
fCheckCookie = NULL;
- delete cookie;
mutex_unlock(&fLock);
fVolume->GetJournal(0)->Unlock(NULL, true);
@@ -1366,22 +1356,21 @@
if (!_IsValidCheckControl(control))
return B_BAD_VALUE;
- check_cookie* cookie = (check_cookie*)control->cookie;
fVolume->SetCheckingThread(find_thread(NULL));
while (true) {
- if (cookie->iterator == NULL) {
- if (!cookie->stack.Pop(&cookie->current)) {
+ if (fCheckCookie->iterator == NULL) {
+ if (!fCheckCookie->stack.Pop(&fCheckCookie->current)) {
// no more runs on the stack, we are obviously
finished!
control->status = B_ENTRY_NOT_FOUND;
return B_ENTRY_NOT_FOUND;
}
- Vnode vnode(fVolume, cookie->current);
+ Vnode vnode(fVolume, fCheckCookie->current);
Inode* inode;
if (vnode.Get(&inode) != B_OK) {
FATAL(("check: Could not open inode at %"
B_PRIdOFF "\n",
- fVolume->ToBlock(cookie->current)));
+
fVolume->ToBlock(fCheckCookie->current)));
continue;
}
@@ -1404,15 +1393,15 @@
BPlusTree* tree = inode->Tree();
if (tree == NULL) {
FATAL(("check: could not open b+tree from inode
at %" B_PRIdOFF
- "\n",
fVolume->ToBlock(cookie->current)));
+ "\n",
fVolume->ToBlock(fCheckCookie->current)));
continue;
}
- cookie->parent = inode;
- cookie->parent_mode = inode->Mode();
+ fCheckCookie->parent = inode;
+ fCheckCookie->parent_mode = inode->Mode();
- cookie->iterator = new TreeIterator(tree);
- if (cookie->iterator == NULL)
+ fCheckCookie->iterator = new TreeIterator(tree);
+ if (fCheckCookie->iterator == NULL)
RETURN_ERROR(B_NO_MEMORY);
// the inode must stay locked in memory until the
iterator is freed
@@ -1432,15 +1421,16 @@
uint16 length;
ino_t id;
- status_t status = cookie->iterator->GetNextEntry(name, &length,
+ status_t status = fCheckCookie->iterator->GetNextEntry(name,
&length,
B_FILE_NAME_LENGTH, &id);
if (status == B_ENTRY_NOT_FOUND) {
// there are no more entries in this iterator, free it
and go on
- delete cookie->iterator;
- cookie->iterator = NULL;
+ delete fCheckCookie->iterator;
+ fCheckCookie->iterator = NULL;
// unlock the directory's inode from memory
- put_vnode(fVolume->FSVolume(),
fVolume->ToVnode(cookie->current));
+ put_vnode(fVolume->FSVolume(),
+ fVolume->ToVnode(fCheckCookie->current));
continue;
} else if (status == B_OK) {
@@ -1460,8 +1450,8 @@
control->errors |= BFS_COULD_NOT_OPEN;
if ((control->flags & BFS_REMOVE_INVALID) != 0)
{
- status =
_RemoveInvalidNode(cookie->parent,
- cookie->iterator->Tree(), NULL,
name);
+ status =
_RemoveInvalidNode(fCheckCookie->parent,
+ fCheckCookie->iterator->Tree(),
NULL, name);
} else
status = B_ERROR;
@@ -1501,22 +1491,22 @@
// Check for the correct mode of the node (if the mode
of the
// file don't fit to its parent, there is a serious
problem)
- if (((cookie->parent_mode & S_ATTR_DIR) != 0
+ if (((fCheckCookie->parent_mode & S_ATTR_DIR) != 0
&& !inode->IsAttribute())
- || ((cookie->parent_mode & S_INDEX_DIR) != 0
+ || ((fCheckCookie->parent_mode & S_INDEX_DIR)
!= 0
&& !inode->IsIndex())
- || (is_directory(cookie->parent_mode)
+ || (is_directory(fCheckCookie->parent_mode)
&& !inode->IsRegularNode())) {
FATAL(("inode at %" B_PRIdOFF " is of wrong
type: %o (parent "
"%o at %" B_PRIdOFF ")!\n",
inode->BlockNumber(),
- inode->Mode(), cookie->parent_mode,
- cookie->parent->BlockNumber()));
+ inode->Mode(),
fCheckCookie->parent_mode,
+ fCheckCookie->parent->BlockNumber()));
// if we are allowed to fix errors, we should
remove the file
if ((control->flags & BFS_REMOVE_WRONG_TYPES)
!= 0
&& (control->flags &
BFS_FIX_BITMAP_ERRORS) != 0) {
- status =
_RemoveInvalidNode(cookie->parent, NULL, inode,
- name);
+ status =
_RemoveInvalidNode(fCheckCookie->parent, NULL,
+ inode, name);
} else
status = B_ERROR;
@@ -1527,7 +1517,7 @@
// push the directory on the stack so that it will be
scanned later
if (inode->IsContainer() && !inode->IsIndex())
- cookie->stack.Push(inode->BlockRun());
+ fCheckCookie->stack.Push(inode->BlockRun());
else {
// check it now
control->status = CheckInode(inode, control);
@@ -1757,10 +1747,8 @@
return status;
// If the inode has an attribute directory, push it on the stack
- if (!inode->Attributes().IsZero()) {
- check_cookie* cookie = (check_cookie*)control->cookie;
- cookie->stack.Push(inode->Attributes());
- }
+ if (!inode->Attributes().IsZero())
+ fCheckCookie->stack.Push(inode->Attributes());
if (inode->IsSymLink() && (inode->Flags() & INODE_LONG_SYMLINK) == 0) {
// symlinks may not have a valid data stream
Modified: haiku/trunk/src/add-ons/kernel/file_systems/bfs/BlockAllocator.h
===================================================================
--- haiku/trunk/src/add-ons/kernel/file_systems/bfs/BlockAllocator.h
2010-03-03 19:20:51 UTC (rev 35742)
+++ haiku/trunk/src/add-ons/kernel/file_systems/bfs/BlockAllocator.h
2010-03-03 19:38:56 UTC (rev 35743)
@@ -46,7 +46,7 @@
int32 group,
uint16 start, uint16 numBlocks,
uint16 minimum,
block_run& run);
- status_t StartChecking(check_control*
control);
+ status_t StartChecking(const
check_control* control);
status_t StopChecking(check_control*
control);
status_t CheckNextNode(check_control*
control);
@@ -74,7 +74,7 @@
#ifdef DEBUG_ALLOCATION_GROUPS
void _CheckGroup(int32 group) const;
#endif
- bool
_IsValidCheckControl(check_control* control);
+ bool _IsValidCheckControl(const
check_control* control);
bool _CheckBitmapIsUsedAt(off_t
block) const;
void _SetCheckBitmapAt(off_t block);
Modified: haiku/trunk/src/add-ons/kernel/file_systems/bfs/bfs_control.h
===================================================================
--- haiku/trunk/src/add-ons/kernel/file_systems/bfs/bfs_control.h
2010-03-03 19:20:51 UTC (rev 35742)
+++ haiku/trunk/src/add-ons/kernel/file_systems/bfs/bfs_control.h
2010-03-03 19:38:56 UTC (rev 35743)
@@ -51,7 +51,6 @@
uint64 freed;
} stats;
status_t status;
- void* cookie;
};
/* values for the flags field */
Modified: haiku/trunk/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp
===================================================================
--- haiku/trunk/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp
2010-03-03 19:20:51 UTC (rev 35742)
+++ haiku/trunk/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp
2010-03-03 19:38:56 UTC (rev 35743)
@@ -618,23 +618,21 @@
Volume* volume = (Volume*)_volume->private_volume;
- // TODO: Access user buffers safely!
-
switch (cmd) {
case BFS_IOCTL_VERSION:
{
- uint32 *version = (uint32*)buffer;
-
- *version = 0x10000;
- return B_OK;
+ uint32 version = 0x10000;
+ return user_memcpy(buffer, &version, sizeof(uint32));
}
case BFS_IOCTL_START_CHECKING:
{
// start checking
BlockAllocator& allocator = volume->Allocator();
- check_control* control = (check_control*)buffer;
+ check_control control;
+ if (user_memcpy(&control, buffer,
sizeof(check_control)) != B_OK)
+ return B_BAD_ADDRESS;
- status_t status = allocator.StartChecking(control);
+ status_t status = allocator.StartChecking(&control);
if (status == B_OK) {
file_cookie* cookie = (file_cookie*)_cookie;
cookie->open_mode |= BFS_OPEN_MODE_CHECKING;
@@ -646,13 +644,15 @@
{
// stop checking
BlockAllocator& allocator = volume->Allocator();
- check_control* control = (check_control*)buffer;
+ check_control control;
- status_t status = allocator.StopChecking(control);
+ status_t status = allocator.StopChecking(&control);
if (status == B_OK) {
file_cookie* cookie = (file_cookie*)_cookie;
cookie->open_mode &= ~BFS_OPEN_MODE_CHECKING;
}
+ if (status == B_OK)
+ status = user_memcpy(buffer, &control,
sizeof(check_control));
return status;
}
@@ -660,9 +660,13 @@
{
// check next
BlockAllocator& allocator = volume->Allocator();
- check_control* control = (check_control*)buffer;
+ check_control control;
- return allocator.CheckNextNode(control);
+ status_t status = allocator.CheckNextNode(&control);
+ if (status == B_OK)
+ status = user_memcpy(buffer, &control,
sizeof(check_control));
+
+ return status;
}
case BFS_IOCTL_UPDATE_BOOT_BLOCK:
{
Other related posts:
- » [haiku-commits] r35743 - haiku/trunk/src/add-ons/kernel/file_systems/bfs - axeld
