ext4: validate readdir offset before accessing dirent
PATCH 1/1 ext4: validate readdir offset before accessing dirent
From: Yao Kai <[email protected]>
commit bc4b7b0414c33b2c8898eb04386df0d21a13dad8 upstream.
A corrupted directory can trigger the following KASAN report when ext4_readdir() resumes from an invalid position:
BUG: KASAN: use-after-free in __ext4_check_dir_entry+0x5ef/0x820 Read of size 2 at addr ffff88810a646000 by task repro_linear/509 Call Trace: <TASK> dump_stack_lvl+0x53/0x70 print_report+0xd0/0x630 kasan_report+0xce/0x100 __ext4_check_dir_entry+0x5ef/0x820 ext4_readdir+0xcde/0x2b70 iterate_dir+0x1a1/0x520 __x64_sys_getdents64+0x12b/0x220 do_syscall_64+0xf9/0x540 entry_SYSCALL_64_after_hwframe+0x77/0x7f </TASK>
KASAN reports use-after-free because the out-of-bounds access lands in an adjacent freed page. The directory buffer itself is still referenced.
ext4_dir_llseek() invalidates the directory cookie so that ext4_readdir() rescans directory entries from the start of the block. The rescan checks only the lower bound of rec_len before advancing. A corrupted rec_len can therefore place the offset where the block has insufficient space for a complete directory entry. The rescan itself may dereference that truncated entry, or the main loop may pass it to __ext4_check_dir_entry(). The latter reads de->rec_len before validating the range. For example:
block offset 0 4092 4096
|---- de1.rec_len = 4092 -----|----|
de2.inode
| de2.rec_len
^ OOB, reported as UAFde2 starts at offset 4092 in this 4 KiB block. Its four-byte inode fits in the block, but its rec_len starts at offset 4096 and crosses the boundary.
The minimum safe length is inode-dependent. Encrypted and casefolded directory entries need eight additional hash bytes, while a valid metadata checksum tail is only 12 bytes.
Cache the metadata checksum feature state and derive the minimum directory entry length from the on-disk format. Use it to bound both the rescan and the offset passed to the main loop. Report an offset in a truncated block tail and skip the remainder of the block, while continuing to accept an offset exactly at the block boundary.
--- fs/ext4/dir.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)
fs/ext4/dir.c +18 -2
@@ -136,6 +136,7 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx)struct super_block *sb = inode->i_sb;struct buffer_head *bh =NULL ;struct fscrypt_str fstr =FSTR_INIT (NULL ,0 ); +bool has_csum =ext4_has_metadata_csum (sb); err =fscrypt_prepare_readdir (inode);if (err)
@@ -147,7 +148,7 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx)return err;/* Can we just clear INDEX flag to ignore htree information? */ -if (!ext4_has_metadata_csum (sb)) { +if (!has_csum) {/* * We don't set the inode dirty flag since it's not * critical that it gets flushed back to the disk.
@@ -233,7 +234,10 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx)* dirent right now. Scan from the start of the block * to make sure. */ if (!inode_eq_iversion (inode, file->f_version)) { -for (i =0 ; i < sb->s_blocksize && i < offset; ) { +for (i =0 ; + i <= sb->s_blocksize - +ext4_dir_rec_len (1 , has_csum ?NULL : inode) && + i < offset;) { de = (struct ext4_dir_entry_2 *) (bh->b_data + i);/* It's too expensive to do a full
@@ -255,6 +259,17 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx) file->f_version =inode_query_iversion (inode); } +if (unlikely (offset < sb->s_blocksize && + offset > sb->s_blocksize - +ext4_dir_rec_len (1 , has_csum ?NULL : inode))) { +EXT4_ERROR_FILE (file, bh->b_blocknr, +"bad entry in directory: %s - offset=%u, size=%lu" , +"directory entry too close to block end" , + offset, sb->s_blocksize); + ctx->pos =round_up (ctx->pos, sb->s_blocksize); + goto next_block; + } +while (ctx->pos < inode->i_size && offset < sb->s_blocksize) { de = (struct ext4_dir_entry_2 *) (bh->b_data + offset);
@@ -310,6 +325,7 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx) ctx->pos +=ext4_rec_len_from_disk (de->rec_len, sb->s_blocksize); } +next_block:if ((ctx->pos < inode->i_size) && !dir_relax_shared (inode)) goto done;brelse (bh);
base-commit: 0aebf2870d53a8d97e02a99e2103ea0b38950c1b
Signature · 1 line
2.56.0
[email protected]
AI reviewThank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] ext4: unchecked rec_len upper bound in ext4_readdir() rescan loop --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ext4: unchecked rec_len upper bound in ext4_readdir() rescan loop --
commit d2979e729161116125f8d03cf9aa1731cdfdef1e
Author: Yao Kai <[email protected]>
ext4: validate readdir offset before accessing dirent
This commit bounds the directory entry rescan loop in ext4_readdir() and validates the block offset before accessing directory entries. When an offset falls in a truncated block tail where a complete entry cannot fit, it reports an error and skips to the next block.
Quoted 4 lines
fs/ext4/dir.c +0
-0
[ ... ]
Quoted 16 lines
fs/ext4/dir.c +4
-1@@ -233,7 +234,10 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx)* dirent right now. Scan from the start of the block * to make sure. */ if (!inode_eq_iversion (inode, file->f_version)) { -for (i =0 ; i < sb->s_blocksize && i < offset; ) { +for (i =0 ; + i <= sb->s_blocksize - +ext4_dir_rec_len (1 , has_csum ?NULL : inode) && + i < offset;) { de = (struct ext4_dir_entry_2 *) (bh->b_data + i);/* It's too expensive to do a full @@ -255,6 +259,17 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx) file->f_version =inode_query_iversion (inode); }
fs/ext4/dir.c static int ext4_readdir(struct file *file, struct dir_context *ctx) · line 271
Quoted 10 lines
fs/ext4/dir.c +10
-0+if (unlikely (offset < sb->s_blocksize && + offset > sb->s_blocksize - +ext4_dir_rec_len (1 , has_csum ?NULL : inode))) { +EXT4_ERROR_FILE (file, bh->b_blocknr, +"bad entry in directory: %s - offset=%u, size=%lu" , +"directory entry too close to block end" , + offset, sb->s_blocksize); + ctx->pos =round_up (ctx->pos, sb->s_blocksize); + goto next_block; + }
[Severity: Medium]
What happens in ext4_readdir() if a corrupted directory entry in the rescan loop has a de->rec_len that pushes i past sb->s_blocksize?
During the rescan loop when !inode_eq_iversion(inode, file->f_version):
if (ext4_rec_len_from_disk(de->rec_len, sb->s_blocksize) < ext4_dir_rec_len(1, inode)) break; i += ext4_rec_len_from_disk(de->rec_len, sb->s_blocksize); } offset = i; ctx->pos = (ctx->pos & ~(sb->s_blocksize - 1)) | offset;
file->f_version = inode_query_iversion(inode);
Only the lower bound of de->rec_len is checked before advancing i. If de->rec_len is corrupted such that i + rlen > sb->s_blocksize (for example, rec_len = 4100 in a 4096-byte block), the loop terminates with offset > sb->s_blocksize, corrupts ctx->pos via the bitwise OR, and updates file->f_version.
Because the new validation check requires offset < sb->s_blocksize, an offset > sb->s_blocksize bypasses both this check and the subsequent while (ctx->pos < inode->i_size && offset < sb->s_blocksize) loop without reporting the corrupted block or rounding ctx->pos to a block boundary.
On the next iteration of the outer loop, file->f_version now matches the inode version, so the rescan is skipped and offset = ctx->pos & (sb->s_blocksize - 1) can land at an unvalidated mid-entry offset or in the tail of an uncorrupted subsequent block, triggering EXT4_ERROR_FILE() on the wrong block and skipping its entries.
Should the condition check offset != sb->s_blocksize && offset > sb->s_blocksize - ext4_dir_rec_len(1, has_csum ? NULL : inode) (or cap the rescan step so i cannot exceed sb->s_blocksize)?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1