hrev50820 adds 3 changesets to branch 'master'
old head: 7894dfde260a8285baade68cbd44b1d052113e42
new head: 39f437f77e5c352955208116360a43814412b263
overview:
http://cgit.haiku-os.org/haiku/log/?qt=range&q=39f437f77e5c+%5E7894dfde260a
----------------------------------------------------------------------------
9bcf23b1a25b: bfs: Made stream code more robust against bad data.
* Avoid divisions by zero. This closes ticket #13094.
80d54534fae5: Coding style cleanup.
39f437f77e5c: bfs: Always check if NodeGetter succeeded.
* There were quite a few cases that just assumed that the disk access
would succeed.
* This also fixes bug #12962.
[ Axel Dörfler <axeld@xxxxxxxxxxxxxxxx> ]
----------------------------------------------------------------------------
6 files changed, 107 insertions(+), 47 deletions(-)
.../kernel/file_systems/bfs/Attribute.cpp | 5 +-
.../kernel/file_systems/bfs/BlockAllocator.cpp | 7 +-
.../kernel/file_systems/bfs/CachedBlock.h | 79 +++++++++++---------
src/add-ons/kernel/file_systems/bfs/Inode.cpp | 52 +++++++++++--
src/add-ons/kernel/file_systems/bfs/Inode.h | 4 +-
src/add-ons/kernel/file_systems/bfs/Query.cpp | 7 +-
############################################################################
Commit: 9bcf23b1a25b6e169979948f750e07f5eb06bb5c
URL: http://cgit.haiku-os.org/haiku/commit/?id=9bcf23b1a25b
Author: Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>
Date: Wed Jan 4 20:07:16 2017 UTC
Ticket: https://dev.haiku-os.org/ticket/13094
bfs: Made stream code more robust against bad data.
* Avoid divisions by zero. This closes ticket #13094.
----------------------------------------------------------------------------
diff --git a/src/add-ons/kernel/file_systems/bfs/Inode.cpp
b/src/add-ons/kernel/file_systems/bfs/Inode.cpp
index db2d98f..378c817 100644
--- a/src/add-ons/kernel/file_systems/bfs/Inode.cpp
+++ b/src/add-ons/kernel/file_systems/bfs/Inode.cpp
@@ -1,5 +1,5 @@
/*
- * Copyright 2001-2014, Axel Dörfler, axeld@xxxxxxxxxxxxxxxx.
+ * Copyright 2001-2017, Axel Dörfler, axeld@xxxxxxxxxxxxxxxx.
* This file may be used under the terms of the MIT License.
*/
@@ -1431,6 +1431,8 @@ Inode::FindBlockRun(off_t pos, block_run& run, off_t&
offset)
int32 indirectSize;
get_double_indirect_sizes(data->double_indirect.Length(),
fVolume->BlockSize(), runsPerBlock, directSize,
indirectSize);
+ if (directSize <= 0 || indirectSize <= 0)
+ RETURN_ERROR(B_BAD_DATA);
off_t start = pos - data->MaxIndirectRange();
int32 index = start / indirectSize;
@@ -1920,6 +1922,8 @@ Inode::_GrowStream(Transaction& transaction, off_t size)
int32 indirectSize;
get_double_indirect_sizes(data->double_indirect.Length(),
fVolume->BlockSize(), runsPerBlock, directSize,
indirectSize);
+ if (directSize <= 0 || indirectSize <= 0)
+ return B_BAD_DATA;
off_t start = data->MaxDoubleIndirectRange()
- data->MaxIndirectRange();
@@ -2031,6 +2035,8 @@ Inode::_FreeStaticStreamArray(Transaction& transaction,
int32 level,
indirectSize = double_indirect_max_direct_size(run.Length(),
fVolume->BlockSize());
}
+ if (indirectSize <= 0)
+ return B_BAD_DATA;
off_t start;
if (size > offset)
############################################################################
Commit: 80d54534fae53d938d8319ca1bd016368c319327
URL: http://cgit.haiku-os.org/haiku/commit/?id=80d54534fae5
Author: Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>
Date: Wed Jan 4 20:29:59 2017 UTC
Coding style cleanup.
----------------------------------------------------------------------------
diff --git a/src/add-ons/kernel/file_systems/bfs/CachedBlock.h
b/src/add-ons/kernel/file_systems/bfs/CachedBlock.h
index 34ff725..5bbc297 100644
--- a/src/add-ons/kernel/file_systems/bfs/CachedBlock.h
+++ b/src/add-ons/kernel/file_systems/bfs/CachedBlock.h
@@ -1,11 +1,12 @@
/*
- * Copyright 2001-2008, Axel Dörfler, axeld@xxxxxxxxxxxxxxxx.
+ * Copyright 2001-2017, Axel Dörfler, axeld@xxxxxxxxxxxxxxxx.
* This file may be used under the terms of the MIT License.
*/
#ifndef CACHED_BLOCK_H
#define CACHED_BLOCK_H
-//! interface for the block cache
+
+//! Interface for the block cache
#include "system_dependencies.h"
@@ -21,41 +22,44 @@
// convenient to use them).
class CachedBlock {
- public:
- CachedBlock(Volume* volume);
- CachedBlock(Volume* volume, off_t block);
- CachedBlock(Volume* volume, block_run run);
- CachedBlock(CachedBlock* cached);
- ~CachedBlock();
-
- inline void Keep();
- inline void Unset();
-
- inline const uint8* SetTo(off_t block, off_t base, size_t
length);
- inline const uint8* SetTo(off_t block);
- inline const uint8* SetTo(block_run run);
- inline uint8* SetToWritable(Transaction& transaction, off_t
block,
- off_t base, size_t length, bool empty = false);
- inline uint8* SetToWritable(Transaction& transaction, off_t
block,
- bool empty = false);
- inline uint8* SetToWritable(Transaction& transaction, block_run
run,
- bool empty = false);
- inline status_t MakeWritable(Transaction& transaction);
-
- const uint8* Block() const { return fBlock; }
- off_t BlockNumber() const { return fBlockNumber; }
- uint32 BlockSize() const { return fVolume->BlockSize(); }
- uint32 BlockShift() const { return fVolume->BlockShift(); }
-
- private:
- CachedBlock(const CachedBlock& other);
- CachedBlock& operator=(const CachedBlock& other);
- // no implementation
-
- protected:
- Volume *fVolume;
- off_t fBlockNumber;
- uint8 *fBlock;
+public:
+
CachedBlock(Volume* volume);
+
CachedBlock(Volume* volume, off_t block);
+
CachedBlock(Volume* volume, block_run run);
+
CachedBlock(CachedBlock* cached);
+ ~CachedBlock();
+
+ inline void Keep();
+ inline void Unset();
+
+ inline const uint8* SetTo(off_t block, off_t base,
size_t length);
+ inline const uint8* SetTo(off_t block);
+ inline const uint8* SetTo(block_run run);
+ inline uint8* SetToWritable(Transaction&
transaction,
+ off_t
block, off_t base, size_t length,
+ bool
empty = false);
+ inline uint8* SetToWritable(Transaction&
transaction,
+ off_t
block, bool empty = false);
+ inline uint8* SetToWritable(Transaction&
transaction,
+
block_run run, bool empty = false);
+ inline status_t MakeWritable(Transaction&
transaction);
+
+ const uint8* Block() const { return fBlock; }
+ off_t BlockNumber() const {
return fBlockNumber; }
+ uint32 BlockSize() const
+ {
return fVolume->BlockSize(); }
+ uint32 BlockShift() const
+ {
return fVolume->BlockShift(); }
+
+private:
+ CachedBlock(const CachedBlock& other);
+ CachedBlock& operator=(const CachedBlock& other);
+ // no implementation
+
+protected:
+ Volume* fVolume;
+ off_t fBlockNumber;
+ uint8* fBlock;
};
@@ -196,4 +200,5 @@ CachedBlock::MakeWritable(Transaction& transaction)
transaction.ID());
}
+
#endif // CACHED_BLOCK_H
############################################################################
Revision: hrev50820
Commit: 39f437f77e5c352955208116360a43814412b263
URL: http://cgit.haiku-os.org/haiku/commit/?id=39f437f77e5c
Author: Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>
Date: Wed Jan 4 20:32:28 2017 UTC
Ticket: https://dev.haiku-os.org/ticket/12962
bfs: Always check if NodeGetter succeeded.
* There were quite a few cases that just assumed that the disk access
would succeed.
* This also fixes bug #12962.
----------------------------------------------------------------------------
diff --git a/src/add-ons/kernel/file_systems/bfs/Attribute.cpp
b/src/add-ons/kernel/file_systems/bfs/Attribute.cpp
index 3ae4122..1c6d38b 100644
--- a/src/add-ons/kernel/file_systems/bfs/Attribute.cpp
+++ b/src/add-ons/kernel/file_systems/bfs/Attribute.cpp
@@ -1,5 +1,5 @@
/*
- * Copyright 2004-2008, Axel Dörfler, axeld@xxxxxxxxxxxxxxxx.
+ * Copyright 2004-2017, Axel Dörfler, axeld@xxxxxxxxxxxxxxxx.
* This file may be used under the terms of the MIT License.
*/
@@ -83,6 +83,9 @@ Attribute::Get(const char* name)
// try to find it in the small data region
if (recursive_lock_lock(&fInode->SmallDataLock()) == B_OK) {
fNodeGetter.SetToNode(fInode);
+ if (fNodeGetter.Node() == NULL)
+ return B_IO_ERROR;
+
fSmall = fInode->FindSmallData(fNodeGetter.Node(), (const
char*)name);
if (fSmall != NULL)
return B_OK;
diff --git a/src/add-ons/kernel/file_systems/bfs/BlockAllocator.cpp
b/src/add-ons/kernel/file_systems/bfs/BlockAllocator.cpp
index 5361a75..4ce6e8a 100644
--- a/src/add-ons/kernel/file_systems/bfs/BlockAllocator.cpp
+++ b/src/add-ons/kernel/file_systems/bfs/BlockAllocator.cpp
@@ -1,5 +1,5 @@
/*
- * Copyright 2001-2014, Axel Dörfler, axeld@xxxxxxxxxxxxxxxx.
+ * Copyright 2001-2017, Axel Dörfler, axeld@xxxxxxxxxxxxxxxx.
* This file may be used under the terms of the MIT License.
*/
@@ -1546,6 +1546,11 @@ BlockAllocator::CheckNextNode(check_control* control)
&& inode->IsRegularNode()) {
RecursiveLocker locker(inode->SmallDataLock());
NodeGetter node(fVolume, inode);
+ if (node.Node() == NULL) {
+ fCheckCookie->control.errors |=
BFS_COULD_NOT_OPEN;
+ fCheckCookie->control.status = B_IO_ERROR;
+ return B_OK;
+ }
const char* localName = inode->Name(node.Node());
if (localName == NULL || strcmp(localName, name)) {
diff --git a/src/add-ons/kernel/file_systems/bfs/Inode.cpp
b/src/add-ons/kernel/file_systems/bfs/Inode.cpp
index 378c817..df0ceb6 100644
--- a/src/add-ons/kernel/file_systems/bfs/Inode.cpp
+++ b/src/add-ons/kernel/file_systems/bfs/Inode.cpp
@@ -346,11 +346,14 @@ Inode::Inode(Volume* volume, ino_t id)
{
PRINT(("Inode::Inode(volume = %p, id = %Ld) @ %p\n", volume, id, this));
- UpdateNodeFromDisk();
-
rw_lock_init(&fLock, "bfs inode");
recursive_lock_init(&fSmallDataLock, "bfs inode small data");
+ if (UpdateNodeFromDisk() != B_OK) {
+ // TODO: the error code gets eaten
+ return;
+ }
+
// these two will help to maintain the indices
fOldSize = Size();
fOldLastModified = LastModified();
@@ -381,6 +384,11 @@ Inode::Inode(Volume* volume, Transaction& transaction,
ino_t id, mode_t mode,
recursive_lock_init(&fSmallDataLock, "bfs inode small data");
NodeGetter node(volume, transaction, this, true);
+ if (node.Node() == NULL) {
+ FATAL(("Could not read inode block %" B_PRId64 "!\n",
BlockNumber()));
+ return;
+ }
+
memset(&fNode, 0, sizeof(bfs_inode));
// Initialize the bfs_inode structure -- it's not written back to disk
@@ -490,12 +498,19 @@ Inode::WriteBack(Transaction& transaction)
}
-void
+status_t
Inode::UpdateNodeFromDisk()
{
NodeGetter node(fVolume, this);
+ if (node.Node() == NULL) {
+ FATAL(("Failed to read block %" B_PRId64 " from disk!\n",
+ BlockNumber()));
+ return B_IO_ERROR;
+ }
+
memcpy(&fNode, node.Node(), sizeof(bfs_inode));
fNode.flags &= HOST_ENDIAN_TO_BFS_INT32(INODE_PERMANENT_FLAGS);
+ return B_OK;
}
@@ -916,6 +931,9 @@ status_t
Inode::GetName(char* buffer, size_t size) const
{
NodeGetter node(fVolume, this);
+ if (node.Node() == NULL)
+ return B_IO_ERROR;
+
RecursiveLocker locker(fSmallDataLock);
const char* name = Name(node.Node());
@@ -940,6 +958,9 @@ Inode::SetName(Transaction& transaction, const char* name)
return B_BAD_VALUE;
NodeGetter node(fVolume, transaction, this);
+ if (node.Node() == NULL)
+ return B_IO_ERROR;
+
const char nameTag[2] = {FILE_NAME_NAME, 0};
return _AddSmallData(transaction, node, nameTag, FILE_NAME_TYPE, 0,
@@ -1013,6 +1034,9 @@ Inode::ReadAttribute(const char* name, int32 type, off_t
pos, uint8* buffer,
// search in the small_data section (which has to be locked first)
{
NodeGetter node(fVolume, this);
+ if (node.Node() == NULL)
+ return B_IO_ERROR;
+
RecursiveLocker locker(fSmallDataLock);
small_data* smallData = FindSmallData(node.Node(), name);
@@ -1079,6 +1103,9 @@ Inode::WriteAttribute(Transaction& transaction, const
char* name, int32 type,
// save the old attribute data
NodeGetter node(fVolume, transaction, this);
+ if (node.Node() == NULL)
+ return B_IO_ERROR;
+
recursive_lock_lock(&fSmallDataLock);
small_data* smallData = FindSmallData(node.Node(), name);
@@ -1148,6 +1175,9 @@ Inode::WriteAttribute(Transaction& transaction, const
char* name, int32 type,
// check if the data fits into the small_data section again
NodeGetter node(fVolume, transaction, this);
+ if (node.Node() == NULL)
+ return B_IO_ERROR;
+
status = _AddSmallData(transaction, node, name, type, pos,
buffer,
*_length);
@@ -1214,6 +1244,8 @@ Inode::RemoveAttribute(Transaction& transaction, const
char* name)
Index index(fVolume);
bool hasIndex = index.SetTo(name) == B_OK;
NodeGetter node(fVolume, this);
+ if (node.Node() == NULL)
+ return B_IO_ERROR;
// update index for attributes in the small_data section
{
@@ -2431,7 +2463,8 @@ void
Inode::TransactionDone(bool success)
{
if (!success) {
- // revert any changes made to the cached bfs_inode
+ // Revert any changes made to the cached bfs_inode
+ // TODO: return code gets eaten
UpdateNodeFromDisk();
}
}
@@ -2822,6 +2855,9 @@ AttributeIterator::GetNext(char* name, size_t* _length,
uint32* _type,
if (fCurrentSmallData >= 0) {
NodeGetter nodeGetter(fInode->GetVolume(), fInode);
+ if (nodeGetter.Node() == NULL)
+ return B_IO_ERROR;
+
const bfs_inode* node = nodeGetter.Node();
const small_data* item = ((bfs_inode*)node)->SmallDataStart();
diff --git a/src/add-ons/kernel/file_systems/bfs/Inode.h
b/src/add-ons/kernel/file_systems/bfs/Inode.h
index 1a51804..1d20f54 100644
--- a/src/add-ons/kernel/file_systems/bfs/Inode.h
+++ b/src/add-ons/kernel/file_systems/bfs/Inode.h
@@ -1,5 +1,5 @@
/*
- * Copyright 2001-2016, Axel Dörfler, axeld@xxxxxxxxxxxxxxxx.
+ * Copyright 2001-2017, Axel Dörfler, axeld@xxxxxxxxxxxxxxxx.
* This file may be used under the terms of the MIT License.
*/
#ifndef INODE_H
@@ -45,7 +45,7 @@ public:
recursive_lock& SmallDataLock() { return
fSmallDataLock; }
status_t WriteBack(Transaction&
transaction);
- void UpdateNodeFromDisk();
+ status_t UpdateNodeFromDisk();
bool IsContainer() const
{
return S_ISDIR(Mode()); }
diff --git a/src/add-ons/kernel/file_systems/bfs/Query.cpp
b/src/add-ons/kernel/file_systems/bfs/Query.cpp
index 6df6a6e..1ec3c20 100644
--- a/src/add-ons/kernel/file_systems/bfs/Query.cpp
+++ b/src/add-ons/kernel/file_systems/bfs/Query.cpp
@@ -1,5 +1,5 @@
/*
- * Copyright 2001-2014, Axel Dörfler, axeld@xxxxxxxxxxxxxxxx.
+ * Copyright 2001-2017, Axel Dörfler, axeld@xxxxxxxxxxxxxxxx.
* Copyright 2010, Clemens Zeidler <haiku@xxxxxxxxxxxxxxxxxx>
* This file may be used under the terms of the MIT License.
*/
@@ -558,6 +558,8 @@ Equation::Match(Inode* inode, const char* attributeName,
int32 type,
} else if (!strcmp(fAttribute, "name")) {
// we need to lock before accessing Inode::Name()
nodeGetter.SetToNode(inode);
+ if (nodeGetter.Node() == NULL)
+ return B_IO_ERROR;
recursive_lock_lock(&inode->SmallDataLock());
locked = true;
@@ -589,6 +591,9 @@ Equation::Match(Inode* inode, const char* attributeName,
int32 type,
// then for attributes in the small_data section, and finally
for the
// real attributes
nodeGetter.SetToNode(inode);
+ if (nodeGetter.Node() == NULL)
+ return B_IO_ERROR;
+
Inode* attribute;
recursive_lock_lock(&inode->SmallDataLock());
