ext4: refactor code to read directory blocks into ext4_read_dirblock()
The code to read in directory blocks and verify their metadata
checksums was replicated in ten different places across
fs/ext4/namei.c, and the code was buggy in subtle ways in a number of
those replicated sites. In some cases, ext4_error() was called with a
training newline. In others, in particularly in empty_dir(), it was
possible to call ext4_dirent_csum_verify() on an index block, which
would trigger false warnings requesting the system adminsitrator to
run e2fsck.
By refactoring the code, we make the code more readable, as well as
shrinking the compiled object file by over 700 bytes and 50 lines of
code.
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 3e1529c..0e28c74 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -83,6 +83,86 @@
return bh;
}
+static int ext4_dx_csum_verify(struct inode *inode,
+ struct ext4_dir_entry *dirent);
+
+typedef enum {
+ EITHER, INDEX, DIRENT
+} dirblock_type_t;
+
+#define ext4_read_dirblock(inode, block, type) \
+ __ext4_read_dirblock((inode), (block), (type), __LINE__)
+
+static struct buffer_head *__ext4_read_dirblock(struct inode *inode,
+ ext4_lblk_t block,
+ dirblock_type_t type,
+ unsigned int line)
+{
+ struct buffer_head *bh;
+ struct ext4_dir_entry *dirent;
+ int err = 0, is_dx_block = 0;
+
+ bh = ext4_bread(NULL, inode, block, 0, &err);
+ if (!bh) {
+ if (err == 0) {
+ ext4_error_inode(inode, __func__, line, block,
+ "Directory hole found");
+ return ERR_PTR(-EIO);
+ }
+ __ext4_warning(inode->i_sb, __func__, line,
+ "error reading directory block "
+ "(ino %lu, block %lu)", inode->i_ino,
+ (unsigned long) block);
+ return ERR_PTR(err);
+ }
+ dirent = (struct ext4_dir_entry *) bh->b_data;
+ /* Determine whether or not we have an index block */
+ if (is_dx(inode)) {
+ if (block == 0)
+ is_dx_block = 1;
+ else if (ext4_rec_len_from_disk(dirent->rec_len,
+ inode->i_sb->s_blocksize) ==
+ inode->i_sb->s_blocksize)
+ is_dx_block = 1;
+ }
+ if (!is_dx_block && type == INDEX) {
+ ext4_error_inode(inode, __func__, line, block,
+ "directory leaf block found instead of index block");
+ return ERR_PTR(-EIO);
+ }
+ if (!EXT4_HAS_RO_COMPAT_FEATURE(inode->i_sb,
+ EXT4_FEATURE_RO_COMPAT_METADATA_CSUM) ||
+ buffer_verified(bh))
+ return bh;
+
+ /*
+ * An empty leaf block can get mistaken for a index block; for
+ * this reason, we can only check the index checksum when the
+ * caller is sure it should be an index block.
+ */
+ if (is_dx_block && type == INDEX) {
+ if (ext4_dx_csum_verify(inode, dirent))
+ set_buffer_verified(bh);
+ else {
+ ext4_error_inode(inode, __func__, line, block,
+ "Directory index failed checksum");
+ brelse(bh);
+ return ERR_PTR(-EIO);
+ }
+ }
+ if (!is_dx_block) {
+ if (ext4_dirent_csum_verify(inode, dirent))
+ set_buffer_verified(bh);
+ else {
+ ext4_error_inode(inode, __func__, line, block,
+ "Directory block failed checksum");
+ brelse(bh);
+ return ERR_PTR(-EIO);
+ }
+ }
+ return bh;
+}
+
#ifndef assert
#define assert(test) J_ASSERT(test)
#endif
@@ -604,9 +684,9 @@
u32 hash;
frame->bh = NULL;
- if (!(bh = ext4_bread(NULL, dir, 0, 0, err))) {
- if (*err == 0)
- *err = ERR_BAD_DX_DIR;
+ bh = ext4_read_dirblock(dir, 0, INDEX);
+ if (IS_ERR(bh)) {
+ *err = PTR_ERR(bh);
goto fail;
}
root = (struct dx_root *) bh->b_data;
@@ -643,15 +723,6 @@
goto fail;
}
- if (!buffer_verified(bh) &&
- !ext4_dx_csum_verify(dir, (struct ext4_dir_entry *)bh->b_data)) {
- ext4_warning(dir->i_sb, "Root failed checksum");
- brelse(bh);
- *err = ERR_BAD_DX_DIR;
- goto fail;
- }
- set_buffer_verified(bh);
-
entries = (struct dx_entry *) (((char *)&root->info) +
root->info.info_length);
@@ -709,23 +780,13 @@
frame->entries = entries;
frame->at = at;
if (!indirect--) return frame;
- if (!(bh = ext4_bread(NULL, dir, dx_get_block(at), 0, err))) {
- if (!(*err))
- *err = ERR_BAD_DX_DIR;
+ bh = ext4_read_dirblock(dir, dx_get_block(at), INDEX);
+ if (IS_ERR(bh)) {
+ *err = PTR_ERR(bh);
goto fail2;
}
entries = ((struct dx_node *) bh->b_data)->entries;
- if (!buffer_verified(bh) &&
- !ext4_dx_csum_verify(dir,
- (struct ext4_dir_entry *)bh->b_data)) {
- ext4_warning(dir->i_sb, "Node failed checksum");
- brelse(bh);
- *err = ERR_BAD_DX_DIR;
- goto fail2;
- }
- set_buffer_verified(bh);
-
if (dx_get_limit(entries) != dx_node_limit (dir)) {
ext4_warning(dir->i_sb,
"dx entry: limit != node limit");
@@ -783,7 +844,7 @@
{
struct dx_frame *p;
struct buffer_head *bh;
- int err, num_frames = 0;
+ int num_frames = 0;
__u32 bhash;
p = frame;
@@ -822,26 +883,9 @@
* block so no check is necessary
*/
while (num_frames--) {
- if (!(bh = ext4_bread(NULL, dir, dx_get_block(p->at),
- 0, &err))) {
- if (!err) {
- ext4_error(dir->i_sb,
- "Directory hole detected on inode %lu\n",
- dir->i_ino);
- return -EIO;
- }
- return err; /* Failure */
- }
-
- if (!buffer_verified(bh) &&
- !ext4_dx_csum_verify(dir,
- (struct ext4_dir_entry *)bh->b_data)) {
- ext4_warning(dir->i_sb, "Node failed checksum");
- brelse(bh);
- return -EIO;
- }
- set_buffer_verified(bh);
-
+ bh = ext4_read_dirblock(dir, dx_get_block(p->at), INDEX);
+ if (IS_ERR(bh))
+ return PTR_ERR(bh);
p++;
brelse(p->bh);
p->bh = bh;
@@ -867,23 +911,9 @@
dxtrace(printk(KERN_INFO "In htree dirblock_to_tree: block %lu\n",
(unsigned long)block));
- if (!(bh = ext4_bread(NULL, dir, block, 0, &err))) {
- if (!err) {
- err = -EIO;
- ext4_error(dir->i_sb,
- "Directory hole detected on inode %lu\n",
- dir->i_ino);
- }
- return err;
- }
-
- if (!buffer_verified(bh) &&
- !ext4_dirent_csum_verify(dir,
- (struct ext4_dir_entry *)bh->b_data)) {
- brelse(bh);
- return -EIO;
- }
- set_buffer_verified(bh);
+ bh = ext4_read_dirblock(dir, block, DIRENT);
+ if (IS_ERR(bh))
+ return PTR_ERR(bh);
de = (struct ext4_dir_entry_2 *) bh->b_data;
top = (struct ext4_dir_entry_2 *) ((char *) de +
@@ -1337,26 +1367,11 @@
return NULL;
do {
block = dx_get_block(frame->at);
- if (!(bh = ext4_bread(NULL, dir, block, 0, err))) {
- if (!(*err)) {
- *err = -EIO;
- ext4_error(dir->i_sb,
- "Directory hole detected on inode %lu\n",
- dir->i_ino);
- }
+ bh = ext4_read_dirblock(dir, block, DIRENT);
+ if (IS_ERR(bh)) {
+ *err = PTR_ERR(bh);
goto errout;
}
-
- if (!buffer_verified(bh) &&
- !ext4_dirent_csum_verify(dir,
- (struct ext4_dir_entry *)bh->b_data)) {
- EXT4_ERROR_INODE(dir, "checksumming directory "
- "block %lu", (unsigned long)block);
- brelse(bh);
- *err = -EIO;
- goto errout;
- }
- set_buffer_verified(bh);
retval = search_dirblock(bh, dir, d_name,
block << EXT4_BLOCK_SIZE_BITS(sb),
res_dir);
@@ -1920,22 +1935,10 @@
}
blocks = dir->i_size >> sb->s_blocksize_bits;
for (block = 0; block < blocks; block++) {
- if (!(bh = ext4_bread(handle, dir, block, 0, &retval))) {
- if (!retval) {
- retval = -EIO;
- ext4_error(inode->i_sb,
- "Directory hole detected on inode %lu\n",
- inode->i_ino);
- }
- return retval;
- }
- if (!buffer_verified(bh) &&
- !ext4_dirent_csum_verify(dir,
- (struct ext4_dir_entry *)bh->b_data)) {
- brelse(bh);
- return -EIO;
- }
- set_buffer_verified(bh);
+ bh = ext4_read_dirblock(dir, block, DIRENT);
+ if (IS_ERR(bh))
+ return PTR_ERR(bh);
+
retval = add_dirent_to_buf(handle, dentry, inode, NULL, bh);
if (retval != -ENOSPC) {
brelse(bh);
@@ -1986,22 +1989,13 @@
return err;
entries = frame->entries;
at = frame->at;
-
- if (!(bh = ext4_bread(handle, dir, dx_get_block(frame->at), 0, &err))) {
- if (!err) {
- err = -EIO;
- ext4_error(dir->i_sb,
- "Directory hole detected on inode %lu\n",
- dir->i_ino);
- }
+ bh = ext4_read_dirblock(dir, dx_get_block(frame->at), DIRENT);
+ if (IS_ERR(bh)) {
+ err = PTR_ERR(bh);
+ bh = NULL;
goto cleanup;
}
- if (!buffer_verified(bh) &&
- !ext4_dirent_csum_verify(dir, (struct ext4_dir_entry *)bh->b_data))
- goto journal_error;
- set_buffer_verified(bh);
-
BUFFER_TRACE(bh, "get_write_access");
err = ext4_journal_get_write_access(handle, bh);
if (err)
@@ -2352,6 +2346,7 @@
struct buffer_head *dir_block = NULL;
struct ext4_dir_entry_2 *de;
struct ext4_dir_entry_tail *t;
+ ext4_lblk_t block = 0;
unsigned int blocksize = dir->i_sb->s_blocksize;
int csum_size = 0;
int err;
@@ -2368,16 +2363,9 @@
goto out;
}
- inode->i_size = EXT4_I(inode)->i_disksize = blocksize;
- if (!(dir_block = ext4_bread(handle, inode, 0, 1, &err))) {
- if (!err) {
- err = -EIO;
- ext4_error(inode->i_sb,
- "Directory hole detected on inode %lu\n",
- inode->i_ino);
- }
+ inode->i_size = 0;
+ if (!(dir_block = ext4_append(handle, inode, &block, &err)))
goto out;
- }
BUFFER_TRACE(dir_block, "get_write_access");
err = ext4_journal_get_write_access(handle, dir_block);
if (err)
@@ -2477,26 +2465,14 @@
}
sb = inode->i_sb;
- if (inode->i_size < EXT4_DIR_REC_LEN(1) + EXT4_DIR_REC_LEN(2) ||
- !(bh = ext4_bread(NULL, inode, 0, 0, &err))) {
- if (err)
- EXT4_ERROR_INODE(inode,
- "error %d reading directory lblock 0", err);
- else
- ext4_warning(inode->i_sb,
- "bad directory (dir #%lu) - no data block",
- inode->i_ino);
+ if (inode->i_size < EXT4_DIR_REC_LEN(1) + EXT4_DIR_REC_LEN(2)) {
+ EXT4_ERROR_INODE(inode, "invalid size");
return 1;
}
- if (!buffer_verified(bh) &&
- !ext4_dirent_csum_verify(inode,
- (struct ext4_dir_entry *)bh->b_data)) {
- EXT4_ERROR_INODE(inode, "checksum error reading directory "
- "lblock 0");
- brelse(bh);
- return -EIO;
- }
- set_buffer_verified(bh);
+ bh = ext4_read_dirblock(inode, 0, EITHER);
+ if (IS_ERR(bh))
+ return 1;
+
de = (struct ext4_dir_entry_2 *) bh->b_data;
de1 = ext4_next_entry(de, sb->s_blocksize);
if (le32_to_cpu(de->inode) != inode->i_ino ||
@@ -2519,29 +2495,9 @@
err = 0;
brelse(bh);
lblock = offset >> EXT4_BLOCK_SIZE_BITS(sb);
- bh = ext4_bread(NULL, inode, lblock, 0, &err);
- if (!bh) {
- if (err)
- EXT4_ERROR_INODE(inode,
- "error %d reading directory "
- "lblock %u", err, lblock);
- else
- ext4_warning(inode->i_sb,
- "bad directory (dir #%lu) - no data block",
- inode->i_ino);
-
- offset += sb->s_blocksize;
- continue;
- }
- if (!buffer_verified(bh) &&
- !ext4_dirent_csum_verify(inode,
- (struct ext4_dir_entry *)bh->b_data)) {
- EXT4_ERROR_INODE(inode, "checksum error "
- "reading directory lblock 0");
- brelse(bh);
- return -EIO;
- }
- set_buffer_verified(bh);
+ bh = ext4_read_dirblock(inode, lblock, EITHER);
+ if (IS_ERR(bh))
+ return 1;
de = (struct ext4_dir_entry_2 *) bh->b_data;
}
if (ext4_check_dir_entry(inode, NULL, de, bh,
@@ -3004,13 +2960,9 @@
struct buffer_head *bh;
if (!ext4_has_inline_data(inode)) {
- if (!(bh = ext4_bread(handle, inode, 0, 0, retval))) {
- if (!*retval) {
- *retval = -EIO;
- ext4_error(inode->i_sb,
- "Directory hole detected on inode %lu\n",
- inode->i_ino);
- }
+ bh = ext4_read_dirblock(inode, 0, EITHER);
+ if (IS_ERR(bh)) {
+ *retval = PTR_ERR(bh);
return NULL;
}
*parent_de = ext4_next_entry(
@@ -3089,11 +3041,6 @@
&inlined);
if (!dir_bh)
goto end_rename;
- if (!inlined && !buffer_verified(dir_bh) &&
- !ext4_dirent_csum_verify(old_inode,
- (struct ext4_dir_entry *)dir_bh->b_data))
- goto end_rename;
- set_buffer_verified(dir_bh);
if (le32_to_cpu(parent_de->inode) != old_dir->i_ino)
goto end_rename;
retval = -EMLINK;