Skip to content

Commit

Permalink
ocfs2: Morph the haphazard OCFS2_IS_VALID_DINODE() checks.
Browse files Browse the repository at this point in the history 
Random places in the code would check a dinode bh to see if it was
valid.  Not only did they do different levels of validation, they
handled errors in different ways.

The previous commit unified inode block reads, validating all block
reads in the same place.  Thus, these haphazard checks are no longer
necessary.  Rather than eliminate them, however, we change them to
BUG_ON() checks.  This ensures the assumptions remain true.  All of the
code paths to these checks have been audited to ensure they come from a
validated inode read.

Signed-off-by: Joel Becker <joel.becker@oracle.com>
Signed-off-by: Mark Fasheh <mfasheh@suse.com>
  • Loading branch information
Joel Becker authored and Mark Fasheh committed Jan 5, 2009
1 parent b657c95 commit 10995aa
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 75 deletions.
50 changes: 21 additions & 29 deletions fs/ocfs2/alloc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,20 +187,12 @@ static int ocfs2_dinode_insert_check(struct inode *inode,
static int ocfs2_dinode_sanity_check(struct inode *inode,
struct ocfs2_extent_tree *et)
{
int ret = 0;
struct ocfs2_dinode *di;
struct ocfs2_dinode *di = et->et_object;

BUG_ON(et->et_ops != &ocfs2_dinode_et_ops);
BUG_ON(!OCFS2_IS_VALID_DINODE(di));

di = et->et_object;
if (!OCFS2_IS_VALID_DINODE(di)) {
ret = -EIO;
ocfs2_error(inode->i_sb,
"Inode %llu has invalid path root",
(unsigned long long)OCFS2_I(inode)->ip_blkno);
}

return ret;
return 0;
}

static void ocfs2_dinode_fill_root_el(struct ocfs2_extent_tree *et)
Expand Down Expand Up @@ -5380,13 +5372,13 @@ int ocfs2_truncate_log_append(struct ocfs2_super *osb,
start_cluster = ocfs2_blocks_to_clusters(osb->sb, start_blk);

di = (struct ocfs2_dinode *) tl_bh->b_data;
tl = &di->id2.i_dealloc;
if (!OCFS2_IS_VALID_DINODE(di)) {
OCFS2_RO_ON_INVALID_DINODE(osb->sb, di);
status = -EIO;
goto bail;
}

/* tl_bh is loaded from ocfs2_truncate_log_init(). It's validated
* by the underlying call to ocfs2_read_inode_block(), so any
* corruption is a code bug */
BUG_ON(!OCFS2_IS_VALID_DINODE(di));

tl = &di->id2.i_dealloc;
tl_count = le16_to_cpu(tl->tl_count);
mlog_bug_on_msg(tl_count > ocfs2_truncate_recs_per_inode(osb->sb) ||
tl_count == 0,
Expand Down Expand Up @@ -5536,13 +5528,13 @@ int __ocfs2_flush_truncate_log(struct ocfs2_super *osb)
BUG_ON(mutex_trylock(&tl_inode->i_mutex));

di = (struct ocfs2_dinode *) tl_bh->b_data;
tl = &di->id2.i_dealloc;
if (!OCFS2_IS_VALID_DINODE(di)) {
OCFS2_RO_ON_INVALID_DINODE(osb->sb, di);
status = -EIO;
goto out;
}

/* tl_bh is loaded from ocfs2_truncate_log_init(). It's validated
* by the underlying call to ocfs2_read_inode_block(), so any
* corruption is a code bug */
BUG_ON(!OCFS2_IS_VALID_DINODE(di));

tl = &di->id2.i_dealloc;
num_to_flush = le16_to_cpu(tl->tl_used);
mlog(0, "Flush %u records from truncate log #%llu\n",
num_to_flush, (unsigned long long)OCFS2_I(tl_inode)->ip_blkno);
Expand Down Expand Up @@ -5697,13 +5689,13 @@ int ocfs2_begin_truncate_log_recovery(struct ocfs2_super *osb,
}

di = (struct ocfs2_dinode *) tl_bh->b_data;
tl = &di->id2.i_dealloc;
if (!OCFS2_IS_VALID_DINODE(di)) {
OCFS2_RO_ON_INVALID_DINODE(tl_inode->i_sb, di);
status = -EIO;
goto bail;
}

/* tl_bh is loaded from ocfs2_get_truncate_log_info(). It's
* validated by the underlying call to ocfs2_read_inode_block(),
* so any corruption is a code bug */
BUG_ON(!OCFS2_IS_VALID_DINODE(di));

tl = &di->id2.i_dealloc;
if (le16_to_cpu(tl->tl_used)) {
mlog(0, "We'll have %u logs to recover\n",
le16_to_cpu(tl->tl_used));
Expand Down
17 changes: 5 additions & 12 deletions fs/ocfs2/journal.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -587,17 +587,11 @@ static int ocfs2_journal_toggle_dirty(struct ocfs2_super *osb,
mlog_entry_void();

fe = (struct ocfs2_dinode *)bh->b_data;
if (!OCFS2_IS_VALID_DINODE(fe)) {
/* This is called from startup/shutdown which will
* handle the errors in a specific manner, so no need
* to call ocfs2_error() here. */
mlog(ML_ERROR, "Journal dinode %llu has invalid "
"signature: %.*s",
(unsigned long long)le64_to_cpu(fe->i_blkno), 7,
fe->i_signature);
status = -EIO;
goto out;
}

/* The journal bh on the osb always comes from ocfs2_journal_init()
* and was validated there inside ocfs2_inode_lock_full(). It's a
* code bug if we mess it up. */
BUG_ON(!OCFS2_IS_VALID_DINODE(fe));

flags = le32_to_cpu(fe->id1.journal1.ij_flags);
if (dirty)
Expand All @@ -613,7 +607,6 @@ static int ocfs2_journal_toggle_dirty(struct ocfs2_super *osb,
if (status < 0)
mlog_errno(status);

out:
mlog_exit(status);
return status;
}
Expand Down
8 changes: 0 additions & 8 deletions fs/ocfs2/ocfs2.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -444,14 +444,6 @@ static inline int ocfs2_uses_extended_slot_map(struct ocfs2_super *osb)
#define OCFS2_IS_VALID_DINODE(ptr) \
(!strcmp((ptr)->i_signature, OCFS2_INODE_SIGNATURE))

#define OCFS2_RO_ON_INVALID_DINODE(__sb, __di) do { \
typeof(__di) ____di = (__di); \
ocfs2_error((__sb), \
"Dinode # %llu has bad signature %.*s", \
(unsigned long long)le64_to_cpu((____di)->i_blkno), 7, \
(____di)->i_signature); \
} while (0)

#define OCFS2_IS_VALID_EXTENT_BLOCK(ptr) \
(!strcmp((ptr)->h_signature, OCFS2_EXTENT_BLOCK_SIGNATURE))

Expand Down
10 changes: 4 additions & 6 deletions fs/ocfs2/resize.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,10 @@ int ocfs2_group_extend(struct inode * inode, int new_clusters)

fe = (struct ocfs2_dinode *)main_bm_bh->b_data;

/* main_bm_bh is validated by inode read inside ocfs2_inode_lock(),
* so any corruption is a code bug. */
BUG_ON(!OCFS2_IS_VALID_DINODE(fe));

if (le16_to_cpu(fe->id2.i_chain.cl_cpg) !=
ocfs2_group_bitmap_size(osb->sb) * 8) {
mlog(ML_ERROR, "The disk is too old and small. "
Expand All @@ -322,12 +326,6 @@ int ocfs2_group_extend(struct inode * inode, int new_clusters)
goto out_unlock;
}

if (!OCFS2_IS_VALID_DINODE(fe)) {
OCFS2_RO_ON_INVALID_DINODE(main_bm_inode->i_sb, fe);
ret = -EIO;
goto out_unlock;
}

first_new_cluster = le32_to_cpu(fe->i_clusters);
lgd_blkno = ocfs2_which_cluster_group(main_bm_inode,
first_new_cluster - 1);
Expand Down
36 changes: 16 additions & 20 deletions fs/ocfs2/suballoc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -441,11 +441,11 @@ static int ocfs2_reserve_suballoc_bits(struct ocfs2_super *osb,
ac->ac_alloc_slot = slot;

fe = (struct ocfs2_dinode *) bh->b_data;
if (!OCFS2_IS_VALID_DINODE(fe)) {
OCFS2_RO_ON_INVALID_DINODE(alloc_inode->i_sb, fe);
status = -EIO;
goto bail;
}

/* The bh was validated by the inode read inside
* ocfs2_inode_lock(). Any corruption is a code bug. */
BUG_ON(!OCFS2_IS_VALID_DINODE(fe));

if (!(fe->i_flags & cpu_to_le32(OCFS2_CHAIN_FL))) {
ocfs2_error(alloc_inode->i_sb, "Invalid chain allocator %llu",
(unsigned long long)le64_to_cpu(fe->i_blkno));
Expand Down Expand Up @@ -931,11 +931,6 @@ static int ocfs2_relink_block_group(handle_t *handle,
struct ocfs2_group_desc *bg = (struct ocfs2_group_desc *) bg_bh->b_data;
struct ocfs2_group_desc *prev_bg = (struct ocfs2_group_desc *) prev_bg_bh->b_data;

if (!OCFS2_IS_VALID_DINODE(fe)) {
OCFS2_RO_ON_INVALID_DINODE(alloc_inode->i_sb, fe);
status = -EIO;
goto out;
}
if (!OCFS2_IS_VALID_GROUP_DESC(bg)) {
OCFS2_RO_ON_INVALID_GROUP_DESC(alloc_inode->i_sb, bg);
status = -EIO;
Expand Down Expand Up @@ -1392,11 +1387,11 @@ static int ocfs2_claim_suballoc_bits(struct ocfs2_super *osb,
BUG_ON(!ac->ac_bh);

fe = (struct ocfs2_dinode *) ac->ac_bh->b_data;
if (!OCFS2_IS_VALID_DINODE(fe)) {
OCFS2_RO_ON_INVALID_DINODE(osb->sb, fe);
status = -EIO;
goto bail;
}

/* The bh was validated by the inode read during
* ocfs2_reserve_suballoc_bits(). Any corruption is a code bug. */
BUG_ON(!OCFS2_IS_VALID_DINODE(fe));

if (le32_to_cpu(fe->id1.bitmap1.i_used) >=
le32_to_cpu(fe->id1.bitmap1.i_total)) {
ocfs2_error(osb->sb, "Chain allocator dinode %llu has %u used "
Expand Down Expand Up @@ -1782,11 +1777,12 @@ int ocfs2_free_suballoc_bits(handle_t *handle,

mlog_entry_void();

if (!OCFS2_IS_VALID_DINODE(fe)) {
OCFS2_RO_ON_INVALID_DINODE(alloc_inode->i_sb, fe);
status = -EIO;
goto bail;
}
/* The alloc_bh comes from ocfs2_free_dinode() or
* ocfs2_free_clusters(). The callers have all locked the
* allocator and gotten alloc_bh from the lock call. This
* validates the dinode buffer. Any corruption that has happended
* is a code bug. */
BUG_ON(!OCFS2_IS_VALID_DINODE(fe));
BUG_ON((count + start_bit) > ocfs2_bits_per_group(cl));

mlog(0, "%llu: freeing %u bits from group %llu, starting at %u\n",
Expand Down

0 comments on commit 10995aa

Please sign in to comment.