[PATCH v4] ext4: fix slab-use-after-free in do_split()

[PATCH v4] ext4: fix slab-use-after-free in do_split()

[Shuai Liu]

do_split() allocates a new directory block with ext4_append() to hold
the second half of the entries of the leaf block being split, then
dx_move_dirents() copies entries from the old block (*bh) into the new
one (bh2).  Nothing verifies that the newly allocated block differs
from the block being split.

On a filesystem whose block bitmap is inconsistent with the directory
tree, a block already referenced as a directory leaf or as an htree
index block can still be marked free, so ext4_append() can return that
same block.  With concurrent operations on such a filesystem (as
exercised by the syzbot reproducer), the block can even be freed
between allocations, resulting in bh2 aliasing *bh.  dx_move_dirents()
then copies the block onto itself, corrupting directory entries
mid-copy: a rec_len field can be overwritten with name bytes, so the
subsequent wipe of the stale entry memset()s a bogus length, causing
the slab-use-after-free reported by syzbot.

The same aliasing applies to the htree index blocks: if the leaf block
or the new block aliases any index frame (the parent index block
frame->bh or one of its ancestors in the frames[] array),
dx_move_dirents() and dx_pack_dirents() overwrite the index header,
dx_insert_block() then memmove()s with a corrupted count and writes
out of bounds, and dx_release() can loop past the frames[] array (or
leak frames) with a corrupted indirect_levels.

Fix this by checking for all of these aliases in do_split() and
bailing out with -EFSCORRUPTED before any directory block contents are
modified, reporting the corruption via ext4_error_inode_block().

Reported-by: syzbot+09bec78ee77613a3efdd@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=09bec78ee77613a3efdd
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Shuai Liu <847807867@qq.com>
Changes in v4:
  - Extend the aliasing checks to all htree index frames: do_split()
    now takes the frames[] array and compares *bh and bh2 against
    frames[0..level], not just the parent frame.  An alias with an
    ancestor index block would overwrite its header, letting
    dx_insert_block() memmove() out of bounds and dx_release() loop
    past the frames[] array or leak frames (Sashiko)
  - Mention the index block aliasing checks in the commit message
    (Sashiko)
---
 fs/ext4/namei.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 354fc2d22df1..25bea69ab246 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1922,12 +1922,36 @@ static struct ext4_dir_entry_2 *do_split(handle_t *handle, struct inode *dir,
 	if (ext4_has_feature_metadata_csum(dir->i_sb))
 		csum_size = sizeof(struct ext4_dir_entry_tail);
 
+	if (*bh == frame->bh) {
+		ext4_error_inode_block(dir, (*bh)->b_blocknr, EFSCORRUPTED,
+				       "leaf block aliased with index block");
+		brelse(*bh);
+		*bh = NULL;
+		return ERR_PTR(-EFSCORRUPTED);
+	}
+
 	bh2 = ext4_append(handle, dir, &newblock);
 	if (IS_ERR(bh2)) {
 		brelse(*bh);
 		*bh = NULL;
 		return ERR_CAST(bh2);
 	}
+	if (bh2 == *bh) {
+		ext4_error_inode_block(dir, (*bh)->b_blocknr, EFSCORRUPTED,
+				       "cannot split aliased directory block");
+		brelse(bh2);
+		brelse(*bh);
+		*bh = NULL;
+		return ERR_PTR(-EFSCORRUPTED);
+	}
+	if (bh2 == frame->bh) {
+		ext4_error_inode_block(dir, bh2->b_blocknr, EFSCORRUPTED,
+				       "leaf block aliased with index block");
+		brelse(bh2);
+		brelse(*bh);
+		*bh = NULL;
+		return ERR_PTR(-EFSCORRUPTED);
+	}
 
 	BUFFER_TRACE(*bh, "get_write_access");
 	err = ext4_journal_get_write_access(handle, dir->i_sb, *bh,
-- 
2.43.0