| From ee82c27b677264cb96058e06b03e89d4d69982a6 Mon Sep 17 00:00:00 2001 |
| From: Jan Kara <jack@suse.cz> |
| Date: Mon, 10 Feb 2020 15:43:16 +0100 |
| Subject: [PATCH] ext4: fix checksum errors with indexed dirs |
| |
| commit 48a34311953d921235f4d7bbd2111690d2e469cf upstream. |
| |
| DIR_INDEX has been introduced as a compat ext4 feature. That means that |
| even kernels / tools that don't understand the feature may modify the |
| filesystem. This works because for kernels not understanding indexed dir |
| format, internal htree nodes appear just as empty directory entries. |
| Index dir aware kernels then check the htree structure is still |
| consistent before using the data. This all worked reasonably well until |
| metadata checksums were introduced. The problem is that these |
| effectively made DIR_INDEX only ro-compatible because internal htree |
| nodes store checksums in a different place than normal directory blocks. |
| Thus any modification ignorant to DIR_INDEX (or just clearing |
| EXT4_INDEX_FL from the inode) will effectively cause checksum mismatch |
| and trigger kernel errors. So we have to be more careful when dealing |
| with indexed directories on filesystems with checksumming enabled. |
| |
| 1) We just disallow loading any directory inodes with EXT4_INDEX_FL when |
| DIR_INDEX is not enabled. This is harsh but it should be very rare (it |
| means someone disabled DIR_INDEX on existing filesystem and didn't run |
| e2fsck), e2fsck can fix the problem, and we don't want to answer the |
| difficult question: "Should we rather corrupt the directory more or |
| should we ignore that DIR_INDEX feature is not set?" |
| |
| 2) When we find out htree structure is corrupted (but the filesystem and |
| the directory should in support htrees), we continue just ignoring htree |
| information for reading but we refuse to add new entries to the |
| directory to avoid corrupting it more. |
| |
| Link: https://lore.kernel.org/r/20200210144316.22081-1-jack@suse.cz |
| Fixes: dbe89444042a ("ext4: Calculate and verify checksums for htree nodes") |
| Reviewed-by: Andreas Dilger <adilger@dilger.ca> |
| Signed-off-by: Jan Kara <jack@suse.cz> |
| Signed-off-by: Theodore Ts'o <tytso@mit.edu> |
| Cc: stable@kernel.org |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c |
| index 37081ffb8952..57a74e110986 100644 |
| --- a/fs/ext4/dir.c |
| +++ b/fs/ext4/dir.c |
| @@ -127,12 +127,14 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx) |
| if (err != ERR_BAD_DX_DIR) { |
| return err; |
| } |
| - /* |
| - * We don't set the inode dirty flag since it's not |
| - * critical that it get flushed back to the disk. |
| - */ |
| - ext4_clear_inode_flag(file_inode(file), |
| - EXT4_INODE_INDEX); |
| + /* Can we just clear INDEX flag to ignore htree information? */ |
| + if (!ext4_has_metadata_csum(sb)) { |
| + /* |
| + * We don't set the inode dirty flag since it's not |
| + * critical that it gets flushed back to the disk. |
| + */ |
| + ext4_clear_inode_flag(inode, EXT4_INODE_INDEX); |
| + } |
| } |
| |
| if (ext4_has_inline_data(inode)) { |
| diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h |
| index 14540afaa64a..9393553cbbbf 100644 |
| --- a/fs/ext4/ext4.h |
| +++ b/fs/ext4/ext4.h |
| @@ -2445,8 +2445,11 @@ void ext4_insert_dentry(struct inode *inode, |
| struct ext4_filename *fname); |
| static inline void ext4_update_dx_flag(struct inode *inode) |
| { |
| - if (!ext4_has_feature_dir_index(inode->i_sb)) |
| + if (!ext4_has_feature_dir_index(inode->i_sb)) { |
| + /* ext4_iget() should have caught this... */ |
| + WARN_ON_ONCE(ext4_has_feature_metadata_csum(inode->i_sb)); |
| ext4_clear_inode_flag(inode, EXT4_INODE_INDEX); |
| + } |
| } |
| static const unsigned char ext4_filetype_table[] = { |
| DT_UNKNOWN, DT_REG, DT_DIR, DT_CHR, DT_BLK, DT_FIFO, DT_SOCK, DT_LNK |
| diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c |
| index 83a25d4244c6..8b46f8e259bf 100644 |
| --- a/fs/ext4/inode.c |
| +++ b/fs/ext4/inode.c |
| @@ -5000,6 +5000,18 @@ struct inode *__ext4_iget(struct super_block *sb, unsigned long ino, |
| ret = -EFSCORRUPTED; |
| goto bad_inode; |
| } |
| + /* |
| + * If dir_index is not enabled but there's dir with INDEX flag set, |
| + * we'd normally treat htree data as empty space. But with metadata |
| + * checksumming that corrupts checksums so forbid that. |
| + */ |
| + if (!ext4_has_feature_dir_index(sb) && ext4_has_metadata_csum(sb) && |
| + ext4_test_inode_flag(inode, EXT4_INODE_INDEX)) { |
| + ext4_error_inode(inode, function, line, 0, |
| + "iget: Dir with htree data on filesystem without dir_index feature."); |
| + ret = -EFSCORRUPTED; |
| + goto bad_inode; |
| + } |
| ei->i_disksize = inode->i_size; |
| #ifdef CONFIG_QUOTA |
| ei->i_reserved_quota = 0; |
| diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c |
| index 8c4128a45359..488a4b92daa9 100644 |
| --- a/fs/ext4/namei.c |
| +++ b/fs/ext4/namei.c |
| @@ -2214,6 +2214,13 @@ static int ext4_add_entry(handle_t *handle, struct dentry *dentry, |
| retval = ext4_dx_add_entry(handle, &fname, dir, inode); |
| if (!retval || (retval != ERR_BAD_DX_DIR)) |
| goto out; |
| + /* Can we just ignore htree data? */ |
| + if (ext4_has_metadata_csum(sb)) { |
| + EXT4_ERROR_INODE(dir, |
| + "Directory has corrupted htree index."); |
| + retval = -EFSCORRUPTED; |
| + goto out; |
| + } |
| ext4_clear_inode_flag(dir, EXT4_INODE_INDEX); |
| dx_fallback++; |
| ext4_mark_inode_dirty(handle, dir); |
| -- |
| 2.7.4 |
| |