From e677f5aa675fce95e7558cf58fee40afd6df554d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Axel=20D=C3=B6rfler?= Date: Sat, 10 Jan 2009 22:16:48 +0000 Subject: [PATCH] * Fixed two possible crashing bugs in the file system check code: the checker was attached to the inode instead of the file descriptor, causing it to call BlockAllocator::StopChecking() whenever anyone closed the device's root node during the check. The other bug was that it tried to fix errors even if the device was read-only. * Added TODO that ioctl() accesses userland buffers unsafely. * Removed no longer valid or superfluous comments. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@28869 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- .../file_systems/bfs/BlockAllocator.cpp | 5 ++++ src/add-ons/kernel/file_systems/bfs/bfs.h | 6 +++-- .../file_systems/bfs/kernel_interface.cpp | 26 +++++++------------ 3 files changed, 18 insertions(+), 19 deletions(-) diff --git a/src/add-ons/kernel/file_systems/bfs/BlockAllocator.cpp b/src/add-ons/kernel/file_systems/bfs/BlockAllocator.cpp index 979d2d775b..18e2d83aff 100644 --- a/src/add-ons/kernel/file_systems/bfs/BlockAllocator.cpp +++ b/src/add-ons/kernel/file_systems/bfs/BlockAllocator.cpp @@ -1189,6 +1189,11 @@ BlockAllocator::StopChecking(check_control* control) put_vnode(fVolume->FSVolume(), fVolume->ToVnode(cookie->current)); } + if (fVolume->IsReadOnly()) { + // We can't fix errors on this volume + control->flags &= ~BFS_FIX_BITMAP_ERRORS; + } + // if CheckNextNode() could completely work through, we can // fix any damages of the bitmap if (control != NULL && control->status == B_ENTRY_NOT_FOUND) { diff --git a/src/add-ons/kernel/file_systems/bfs/bfs.h b/src/add-ons/kernel/file_systems/bfs/bfs.h index aaa84e1f47..a2c1619458 100644 --- a/src/add-ons/kernel/file_systems/bfs/bfs.h +++ b/src/add-ons/kernel/file_systems/bfs/bfs.h @@ -223,8 +223,7 @@ enum inode_flags { INODE_WAS_WRITTEN = 0x00020000, // The rest is only used by the file system check functionality - INODE_DONT_FREE_SPACE = 0x00080000, - INODE_CHECK_RUNNING = 0x00200000 + INODE_DONT_FREE_SPACE = 0x00080000 }; //************************************** @@ -235,6 +234,9 @@ struct file_cookie { int open_mode; }; +#define BFS_OPEN_MODE_USER_MASK 0x7fffffff +#define BFS_OPEN_MODE_CHECKING 0x80000000 + // notify every second if the file size has changed #define INODE_NOTIFICATION_INTERVAL 1000000LL diff --git a/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp b/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp index 3d04461289..1d163d8ff6 100644 --- a/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp +++ b/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp @@ -584,7 +584,8 @@ bfs_ioctl(fs_volume* _volume, fs_vnode* _node, void* _cookie, ulong cmd, buffer, bufferLength)); Volume* volume = (Volume*)_volume->private_volume; - Inode* inode = (Inode*)_node->private_node; + + // TODO: Access user buffers safely! switch (cmd) { case BFS_IOCTL_VERSION: @@ -602,8 +603,8 @@ bfs_ioctl(fs_volume* _volume, fs_vnode* _node, void* _cookie, ulong cmd, status_t status = allocator.StartChecking(control); if (status == B_OK) { - inode->Node().flags - |= HOST_ENDIAN_TO_BFS_INT32(INODE_CHECK_RUNNING); + file_cookie* cookie = (file_cookie*)_cookie; + cookie->open_mode |= BFS_OPEN_MODE_CHECKING; } return status; @@ -616,8 +617,8 @@ bfs_ioctl(fs_volume* _volume, fs_vnode* _node, void* _cookie, ulong cmd, status_t status = allocator.StopChecking(control); if (status == B_OK) { - inode->Node().flags - &= HOST_ENDIAN_TO_BFS_INT32(~INODE_CHECK_RUNNING); + file_cookie* cookie = (file_cookie*)_cookie; + cookie->open_mode &= ~BFS_OPEN_MODE_CHECKING; } return status; @@ -1164,7 +1165,7 @@ bfs_open(fs_volume* _volume, fs_vnode* _node, int openMode, void** _cookie) // opening a directory read-only is allowed, although you can't read // any data from it. - if (inode->IsDirectory() && openMode & O_RWMASK) { + if (inode->IsDirectory() && (openMode & O_RWMASK) != 0) { openMode = openMode & ~O_RWMASK; // TODO: for compatibility reasons, we don't return an error here... // e.g. "copyattr" tries to do that @@ -1176,22 +1177,13 @@ bfs_open(fs_volume* _volume, fs_vnode* _node, int openMode, void** _cookie) if (status < B_OK) RETURN_ERROR(status); - // We could actually use the cookie to keep track of: - // - the last block_run - // - the location in the data_stream (indirect, double indirect, - // position in block_run array) - // - // This could greatly speed up continuous reads of big files, especially - // in the indirect block section. - file_cookie* cookie = new(std::nothrow) file_cookie; if (cookie == NULL) RETURN_ERROR(B_NO_MEMORY); ObjectDeleter cookieDeleter(cookie); // initialize the cookie - cookie->open_mode = openMode; - // needed by e.g. bfs_write() for O_APPEND + cookie->open_mode = openMode & BFS_OPEN_MODE_USER_MASK; cookie->last_size = inode->Size(); cookie->last_notification = system_time(); @@ -1381,7 +1373,7 @@ bfs_free_cookie(fs_volume* _volume, fs_vnode* _node, void* _cookie) if (status == B_OK) transaction.Done(); - if ((inode->Flags() & INODE_CHECK_RUNNING) != 0) { + if ((cookie->open_mode & BFS_OPEN_MODE_CHECKING) != 0) { // "chkbfs" exited abnormally, so we have to stop it here... FATAL(("check process was aborted!\n")); volume->Allocator().StopChecking(NULL);