From e2202b2ddbfafd3dcbd46c7cf79d2074f19da962 Mon Sep 17 00:00:00 2001 From: Jonas 'Sortie' Termansen Date: Thu, 9 Jul 2015 18:06:45 +0200 Subject: [PATCH] Fix extfs unhandled allocation failures. This is not sufficient. The operator new calls are dangerous right now because they throw exceptions (not handled) on error instead of returning NULL. This needs to be changed to operator new nothrow instead. --- ext/blockgroup.cpp | 4 ++++ ext/device.cpp | 14 ++++++++++++-- ext/extfs.cpp | 6 ++++++ ext/filesystem.cpp | 15 +++++++++++++++ ext/fuse.cpp | 29 ++++++++++++++++------------- ext/inode.cpp | 34 +++++++++++++++++++++++++++------- 6 files changed, 80 insertions(+), 22 deletions(-) diff --git a/ext/blockgroup.cpp b/ext/blockgroup.cpp index 028549a9..e073427e 100644 --- a/ext/blockgroup.cpp +++ b/ext/blockgroup.cpp @@ -91,6 +91,8 @@ uint32_t BlockGroup::AllocateBlock() { uint32_t block_id = data->bg_block_bitmap + block_alloc_chunk; block_bitmap_chunk = filesystem->device->GetBlock(block_id); + if ( !block_bitmap_chunk ) + return 0; block_bitmap_chunk_i = 0; } uint32_t chunk_offset = block_alloc_chunk * num_chunk_bits; @@ -138,6 +140,8 @@ uint32_t BlockGroup::AllocateInode() { uint32_t block_id = data->bg_inode_bitmap + inode_alloc_chunk; inode_bitmap_chunk = filesystem->device->GetBlock(block_id); + if ( !inode_bitmap_chunk ) + return 0; inode_bitmap_chunk_i = 0; } uint32_t chunk_offset = inode_alloc_chunk * num_chunk_bits; diff --git a/ext/device.cpp b/ext/device.cpp index 2b761dd9..4cc44eed 100644 --- a/ext/device.cpp +++ b/ext/device.cpp @@ -92,8 +92,13 @@ Block* Device::GetBlock(uint32_t block_id) { if ( Block* block = GetCachedBlock(block_id) ) return block; + uint8_t* data = new uint8_t[block_size]; + if ( !data ) // TODO: Use operator new nothrow! + return NULL; Block* block = new Block(this, block_id); - block->block_data = new uint8_t[block_size]; + if ( !block ) // TODO: Use operator new nothrow! + return delete[] data, (Block*) NULL; + block->block_data = data; off_t file_offset = (off_t) block_size * (off_t) block_id; preadall(fd, block->block_data, block_size, file_offset); block->Prelink(); @@ -109,8 +114,13 @@ Block* Device::GetBlockZeroed(uint32_t block_id) block->FinishWrite(); return block; } + uint8_t* data = new uint8_t[block_size]; + if ( !data ) // TODO: Use operator new nothrow! + return NULL; Block* block = new Block(this, block_id); - block->block_data = new uint8_t[block_size]; + if ( !block ) // TODO: Use operator new nothrow! + return delete[] data, (Block*) NULL; + block->block_data = data; memset(block->block_data, 0, block_size); block->Prelink(); block->BeginWrite(); diff --git a/ext/extfs.cpp b/ext/extfs.cpp index 4ceeab3c..9abb0453 100644 --- a/ext/extfs.cpp +++ b/ext/extfs.cpp @@ -384,9 +384,15 @@ int main(int argc, char* argv[]) uint32_t block_size = 1024U << sb.s_log_block_size; Device* dev = new Device(fd, device_path, block_size, write); + if ( !dev ) // TODO: Use operator new nothrow! + error(1, errno, "malloc"); Filesystem* fs = new Filesystem(dev, mount_path); + if ( !fs ) // TODO: Use operator new nothrow! + error(1, errno, "malloc"); fs->block_groups = new BlockGroup*[fs->num_groups]; + if ( !fs->block_groups ) // TODO: Use operator new nothrow! + error(1, errno, "malloc"); for ( size_t i = 0; i < fs->num_groups; i++ ) fs->block_groups[i] = NULL; diff --git a/ext/filesystem.cpp b/ext/filesystem.cpp index 0679d4d0..b447dffe 100644 --- a/ext/filesystem.cpp +++ b/ext/filesystem.cpp @@ -43,6 +43,7 @@ Filesystem::Filesystem(Device* device, const char* mount_path) uint64_t sb_offset = 1024; uint32_t sb_block_id = sb_offset / device->block_size; this->sb_block = device->GetBlock(sb_block_id); + assert(sb_block); // TODO: This can fail. this->sb = (struct ext_superblock*) (sb_block->block_data + sb_offset % device->block_size); this->device = device; @@ -140,7 +141,11 @@ BlockGroup* Filesystem::GetBlockGroup(uint32_t group_id) uint32_t offset = (group_id * group_size) % block_size; Block* block = device->GetBlock(block_id); + if ( !block ) + return (BlockGroup*) NULL; BlockGroup* group = new BlockGroup(this, group_id); + if ( !group ) // TODO: Use operator new nothrow! + return block->Unref(), (BlockGroup*) NULL; group->data_block = block; uint8_t* buf = group->data_block->block_data + offset; group->data = (struct ext_blockgrpdesc*) buf; @@ -163,13 +168,19 @@ Inode* Filesystem::GetInode(uint32_t inode_id) uint32_t tabel_index = (inode_id-1) % sb->s_inodes_per_group; assert(group_id < num_groups); BlockGroup* group = GetBlockGroup(group_id); + if ( !group ) + return (Inode*) NULL; uint32_t tabel_block = group->data->bg_inode_table; group->Unref(); uint32_t block_id = tabel_block + (tabel_index * inode_size) / block_size; uint32_t offset = (tabel_index * inode_size) % block_size; Block* block = device->GetBlock(block_id); + if ( !block ) + return (Inode*) NULL; Inode* inode = new Inode(this, inode_id); + if ( !inode ) + return block->Unref(), (Inode*) NULL; inode->data_block = block; uint8_t* buf = inode->data_block->block_data + offset; inode->data = (struct ext_inode*) buf; @@ -229,6 +240,8 @@ void Filesystem::FreeBlock(uint32_t block_id) uint32_t group_id = (block_id - sb->s_first_data_block) / sb->s_blocks_per_group; assert(group_id < num_groups); BlockGroup* group = GetBlockGroup(group_id); + if ( !group ) + return; group->FreeBlock(block_id); group->Unref(); } @@ -240,6 +253,8 @@ void Filesystem::FreeInode(uint32_t inode_id) uint32_t group_id = (inode_id-1) / sb->s_inodes_per_group; assert(group_id < num_groups); BlockGroup* group = GetBlockGroup(group_id); + if ( !group ) + return; group->FreeInode(inode_id); group->Unref(); } diff --git a/ext/fuse.cpp b/ext/fuse.cpp index 1582e73d..dd6f70cd 100644 --- a/ext/fuse.cpp +++ b/ext/fuse.cpp @@ -77,7 +77,8 @@ Inode* ext2_fuse_resolve_path(const char* path) { Filesystem* fs = FUSE_FS; Inode* inode = fs->GetInode(EXT2_ROOT_INO); - assert(inode); + if ( !inode ) + return (Inode*) NULL; while ( path[0] ) { if ( *path == '/' ) @@ -88,12 +89,12 @@ Inode* ext2_fuse_resolve_path(const char* path) continue; } size_t elem_len = strcspn(path, "/"); - char* elem = new char[elem_len+1]; - memcpy(elem, path, elem_len); - elem[elem_len] = '\0'; + char* elem = strndup(path, elem_len); + if ( !elem ) + return inode->Unref(), errno = ENOTDIR, (Inode*) NULL; path += elem_len; Inode* next = inode->Open(elem, O_RDONLY, 0); - delete[] elem; + free(elem); inode->Unref(); if ( !next ) return NULL; @@ -108,7 +109,8 @@ Inode* ext2_fuse_parent_dir(const char** path_ptr) const char* path = *path_ptr; Filesystem* fs = FUSE_FS; Inode* inode = fs->GetInode(EXT2_ROOT_INO); - assert(inode); + if ( !inode ) + return (Inode*) NULL; while ( strchr(path, '/') ) { if ( *path == '/' ) @@ -119,12 +121,12 @@ Inode* ext2_fuse_parent_dir(const char** path_ptr) continue; } size_t elem_len = strcspn(path, "/"); - char* elem = new char[elem_len+1]; - memcpy(elem, path, elem_len); - elem[elem_len] = '\0'; + char* elem = strndup(path, elem_len); + if ( !elem ) + return inode->Unref(), errno = ENOTDIR, (Inode*) NULL; path += elem_len; Inode* next = inode->Open(elem, O_RDONLY, 0); - delete[] elem; + free(elem); inode->Unref(); if ( !next ) return (Inode*) NULL; @@ -492,11 +494,12 @@ int ext2_fuse_readdir(const char* /*path*/, void* buf, fuse_fill_dir_t filler, const struct ext_dirent* entry = (const struct ext_dirent*) block_data; if ( entry->inode && entry->name_len && (!rec_num || !rec_num--) ) { - char* entry_name = new char[entry->name_len+1]; + char* entry_name = strndup(entry->name, entry->name_len); + if ( !entry_name ) + return block->Unref(), inode->Unref(), -errno; memcpy(entry_name, entry->name, entry->name_len); - entry_name[entry->name_len] = '\0'; bool full = filler(buf, entry_name, NULL, 0); - delete[] entry_name; + free(entry_name); if ( full ) { block->Unref(); diff --git a/ext/inode.cpp b/ext/inode.cpp index 7fb6a7d4..c8037e36 100644 --- a/ext/inode.cpp +++ b/ext/inode.cpp @@ -427,6 +427,8 @@ Inode* Inode::Open(const char* elem, int flags, mode_t mode) file_type != EXT2_FT_SYMLINK ) return errno = ENOTDIR, (Inode*) NULL; Inode* inode = filesystem->GetInode(inode_id); + if ( !inode ) + return (Inode*) NULL; if ( flags & O_DIRECTORY && !EXT2_S_ISDIR(inode->Mode()) && !EXT2_S_ISLNK(inode->Mode()) ) @@ -449,6 +451,8 @@ Inode* Inode::Open(const char* elem, int flags, mode_t mode) if ( !result_inode_id ) return NULL; Inode* result = filesystem->GetInode(result_inode_id); + if ( !result ) + return filesystem->FreeInode(result_inode_id), (Inode*) NULL; memset(result->data, 0, sizeof(*result->data)); result->SetMode((mode & S_SETABLE) | S_IFREG); struct timespec now; @@ -673,10 +677,17 @@ Inode* Inode::UnlinkKeep(const char* elem, bool directories, bool force) if ( entry_block_id + 1 != num_blocks ) { Block* last_block = GetBlock(num_blocks-1); - memcpy(block->block_data, last_block->block_data, block_size); - last_block->Unref(); + if ( last_block ) + { + memcpy(block->block_data, last_block->block_data, block_size); + last_block->Unref(); + Truncate(filesize - block_size); + } + } + else + { + Truncate(filesize - block_size); } - Truncate(filesize - block_size); } block->FinishWrite(); @@ -863,6 +874,8 @@ bool Inode::Symlink(const char* elem, const char* dest) return NULL; Inode* result = filesystem->GetInode(result_inode_id); + if ( !result ) + return filesystem->FreeInode(result_inode_id), false; memset(result->data, 0, sizeof(*result->data)); result->SetMode((0777 & S_SETABLE) | EXT2_S_IFLNK); @@ -894,6 +907,8 @@ Inode* Inode::CreateDirectory(const char* path, mode_t mode) return NULL; Inode* result = filesystem->GetInode(result_inode_id); + if ( !result ) + return filesystem->FreeInode(result_inode_id), (Inode*) NULL; memset(result->data, 0, sizeof(*result->data)); result->SetMode((mode & S_SETABLE) | EXT2_S_IFDIR); @@ -901,6 +916,8 @@ Inode* Inode::CreateDirectory(const char* path, mode_t mode) uint32_t group_id = (result->inode_id - 1) / filesystem->sb->s_inodes_per_group; assert(group_id < filesystem->num_groups); BlockGroup* block_group = filesystem->GetBlockGroup(group_id); + if ( !block_group ) + return result->Unref(), (Inode*) NULL; block_group->BeginWrite(); block_group->data->bg_used_dirs_count++; block_group->FinishWrite(); @@ -945,10 +962,13 @@ bool Inode::RemoveDirectory(const char* path) uint32_t group_id = (result->inode_id - 1) / filesystem->sb->s_inodes_per_group; assert(group_id < filesystem->num_groups); BlockGroup* block_group = filesystem->GetBlockGroup(group_id); - block_group->BeginWrite(); - block_group->data->bg_used_dirs_count--; - block_group->FinishWrite(); - block_group->Unref(); + if ( block_group ) + { + block_group->BeginWrite(); + block_group->data->bg_used_dirs_count--; + block_group->FinishWrite(); + block_group->Unref(); + } result->Unref();