ext4: don't append a directory block already mapped in the inode

PATCH In review Adriano Cordova 3 participants 3 messages Oct 5 ext4 file system

Share & tools

Adriano Cordova

author

2026-10-05 00:35 UTC #0 lore raw

PATCH v2 1/1 ext4: don't append a directory block already mapped in the inode

ext4_append() grows a directory by one block. It checks that the target logical block is a hole, but a corrupt block bitmap can still make the allocator hand back a physical block that is already in use by this inode. The in-memory copy of a block is keyed by its physical block number, so the "new" block and that existing one are the same memory; callers that split a directory (make_indexed_dir()/do_split()) then move entries between two aliased buffers and corrupt the directory, until a bogus rec_len read from the middle of a name runs the wipe out of bounds:

  BUG: KASAN: slab-use-after-free in dx_move_dirents [inline]
  Write of size 90458 ...

Reject the block and report the corrupt bitmap instead of corrupting memory.

Reported-by [email protected]

Closes https://syzkaller.appspot.com/bug?extid=09bec78ee77613a3efdd

Tested-by [email protected]

Signed-off-by Adriano Cordova <[email protected]>

---
Changes in v2:
- Rework after review of v1 ("ext4: wipe moved dirents with their real length"). dx_make_map() already validates each entry, so the bad rec_len does not come from disk. The problem is that the block allocator was handing back a block already mapped by the inode.

 fs/ext4/namei.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

fs/ext4/namei.c +19 -0

index 3b9740c1c16d..ff6013306b74 100644
@@ -83,6 +83,25 @@ static struct buffer_head *ext4_append(handle_t *handle,
 	bh = ext4_bread(handle, inode, *block, EXT4_GET_BLOCKS_CREATE);
 	if (IS_ERR(bh))
 		return bh;
+
+	for (map.m_lblk = 0; map.m_lblk < *block; map.m_lblk += map.m_len) {
+		map.m_len = *block - map.m_lblk;
+		err = ext4_map_blocks(NULL, inode, &map, 0);
+		if (err < 0)
+			goto out;
+		if (err == 0) {
+			map.m_len = 1;
+			continue;
+		}
+		if (unlikely(map.m_pblk == bh->b_blocknr)) {
+			EXT4_ERROR_INODE(inode,
+					 "new block %llu already mapped",
+					 (unsigned long long)bh->b_blocknr);
+			err = -EFSCORRUPTED;
+			goto out;
+		}
+	}
+
 	inode->i_size += inode->i_sb->s_blocksize;
 	EXT4_I(inode)->i_disksize = inode->i_size;
 	err = ext4_mark_inode_dirty(handle, inode);

Signature · 1 line

2.51.0

[email protected]

AI review

2026-10-05 00:46 UTC #1 lore raw

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] ext4: ext4_append() misses aliases in multi-block extents - [High] ext4: ext4_append() forces O(N)…

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] ext4: ext4_append() misses aliases in multi-block extents
- [High] ext4: ext4_append() forces O(N) iteration over holes leading to soft lockup
- [High] ext4: ext4_append() escalates transient errors to filesystem corruption --

commit 4e22a7e845c3a3e2ac2696b510b59d9b11640408
Author: Adriano Cordova <[email protected]>
ext4: don't append a directory block already mapped in the inode

This commit adds a check in ext4_append to ensure a newly allocated directory block is not already mapped by the inode. It iterates through the inode's logical blocks to verify the new physical block does not alias an existing one.

Closes https://syzkaller.appspot.com/bug?extid=09bec78ee77613a3efdd

Quoted 3 lines

fs/ext4/namei.c +0 -0

[ ... ]

fs/ext4/namei.c static struct buffer_head *ext4_append(handle_t *handle, · line 91

Quoted 10 lines

fs/ext4/namei.c +6 -0

@@ -83,6 +83,25 @@ static struct buffer_head *ext4_append(handle_t *handle,
 	bh = ext4_bread(handle, inode, *block, EXT4_GET_BLOCKS_CREATE);
 	if (IS_ERR(bh))
 		return bh;
+
+	for (map.m_lblk = 0; map.m_lblk < *block; map.m_lblk += map.m_len) {
+		map.m_len = *block - map.m_lblk;
+		err = ext4_map_blocks(NULL, inode, &map, 0);
+		if (err < 0)
+			goto out;

[Severity: High]
Could this cause a regression where transient errors are escalated to filesystem corruption?

The goto out path calls ext4_std_error(inode->i_sb, err), which marks the filesystem as corrupted.

If ext4_map_blocks() returns a transient error like -ENOMEM or -EIO, should it instead propagate the error directly back to the caller, similar to how the earlier ext4_map_blocks() call handles them before the loop?

fs/ext4/namei.c static struct buffer_head *ext4_append(handle_t *handle, · line 95

Quoted 4 lines

fs/ext4/namei.c +4 -0

+		if (err == 0) {
+			map.m_len = 1;
+			continue;
+		}

[Severity: High]
Does this force an iteration over large holes, potentially causing a performance regression?

When ext4_map_blocks() returns 0 for a hole, it populates map.m_len with the full length of the hole.

Overriding map.m_len to 1 discards the hole size and forces the loop to iterate block-by-block. For directories with large sparse holes, could this lead to severe performance degradation and soft lockups?

fs/ext4/namei.c static struct buffer_head *ext4_append(handle_t *handle, · line 96

Quoted 1 line

fs/ext4/namei.c +1 -0

+		if (unlikely(map.m_pblk == bh->b_blocknr)) {

[Severity: High]
Can this equality check miss aliases if the mapped extent spans multiple blocks, allowing a memory corruption regression?

Because ext4_map_blocks() can return a multi-block extent with map.m_len > 1, the newly allocated block bh->b_blocknr might fall in the middle of a previously allocated multi-block extent.

If bh->b_blocknr is within the range from map.m_pblk to map.m_pblk + map.m_len - 1, this exact match check will evaluate to false, failing to prevent the alias.

Should this check account for the extent length?

Quoted 11 lines

fs/ext4/namei.c +8 -0

+			EXT4_ERROR_INODE(inode,
+					 "new block %llu already mapped",
+					 (unsigned long long)bh->b_blocknr);
+			err = -EFSCORRUPTED;
+			goto out;
+		}
+	}
+
 	inode->i_size += inode->i_sb->s_blocksize;
 	EXT4_I(inode)->i_disksize = inode->i_size;
 	err = ext4_mark_inode_dirty(handle, inode);

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Jan Kara

reviewer

2026-10-05 11:24 UTC #2 lore raw

In reply to Adriano Cordova

Hello!

Quoted 12 lines · Adriano Cordova

ext4_append() grows a directory by one block. It checks that the target logical block is a hole, but a corrupt block bitmap can still make the allocator hand back a physical block that is already in use by this inode. The in-memory copy of a block is keyed by its physical block number, so the "new" block and that existing one are the same memory; callers that split a directory (make_indexed_dir()/do_split()) then move entries between two aliased buffers and corrupt the directory, until a bogus rec_len read from the middle of a name runs the wipe out of bounds:
  BUG: KASAN: slab-use-after-free in dx_move_dirents [inline]
  Write of size 90458 ...

Quoted 7 lines

Reject the block and report the corrupt bitmap instead of corrupting memory.
Reported-by [email protected]
Closes https://syzkaller.appspot.com/bug?extid=09bec78ee77613a3efdd
Tested-by [email protected]
Signed-off-by Adriano Cordova <[email protected]>

Good that you tracked down the reason for the corruption. But what you do below isn't really a good fix and I don't think you've put too much thought into it. Firstly, it would work only in the specific case this syzkaller reproducer triggers where the same block is claimed twice by the same inode (but other inodes can end up claiming the block as well!). Secondly, it would heavily slow down appending to large directories.

Frankly, I don't think this case of multiply claimed blocks is easy to deal with. To properly solve it you would need something like storing in the struct buffer_head the type of metadata (and perhaps inode & offset owning it) when loading metadata from the disk and then validating this when getting the buffer head from cache. But it's a lot of work and practically only to make fuzzer of disk images happy so I'm not really sure it's worth it.

Honza

Quoted 43 lines

 fs/ext4/namei.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

fs/ext4/namei.c +19 -0

index 3b9740c1c16d..ff6013306b74 100644
@@ -83,6 +83,25 @@ static struct buffer_head *ext4_append(handle_t *handle,
 	bh = ext4_bread(handle, inode, *block, EXT4_GET_BLOCKS_CREATE);
 	if (IS_ERR(bh))
 		return bh;
+
+	for (map.m_lblk = 0; map.m_lblk < *block; map.m_lblk += map.m_len) {
+		map.m_len = *block - map.m_lblk;
+		err = ext4_map_blocks(NULL, inode, &map, 0);
+		if (err < 0)
+			goto out;
+		if (err == 0) {
+			map.m_len = 1;
+			continue;
+		}
+		if (unlikely(map.m_pblk == bh->b_blocknr)) {
+			EXT4_ERROR_INODE(inode,
+					 "new block %llu already mapped",
+					 (unsigned long long)bh->b_blocknr);
+			err = -EFSCORRUPTED;
+			goto out;
+		}
+	}
+
 	inode->i_size += inode->i_sb->s_blocksize;
 	EXT4_I(inode)->i_disksize = inode->i_size;
 	err = ext4_mark_inode_dirty(handle, inode);

Signature · 1 line

2.51.0

Signature · 2 lines

Jan Kara <[email protected]>
SUSE Labs, CR