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: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
index 7fb8df7b6a43d3ba9566741ebec5820e685f1874..6b3a32f75dadb88e51aaab3271b5b81ddec7453d 100644 (file)
--- a/fs/ext4/dir.c
+++ b/fs/ext4/dir.c
@@ -124,12 +124,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 567a6c7af677c6eb2b4b93932f33cea5869fe698..bc8ef27378193fc49e3f602b5b79b6a07e8c030e 100644 (file)
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2375,8 +2375,11 @@ int ext4_insert_dentry(struct inode *dir,
                       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 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 8133e6529994793f7a8d50a2f3bc480affd001eb..42209a1a6c2112e61967b01b82c0e1e6c0db83ac 100644 (file)
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4594,6 +4594,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,
+                                "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 f0ce535d514c704a1bc4de3d1104251e735df5db..d74dee7de546b5aab02cf2d652201a4832ea15d2 100644 (file)
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -2148,6 +2148,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);