Skip to contentSkip to conversation

ext4: validate readdir offset before accessing dirent

PATCH New Eric Biggers 2 participants 2 messages Oct 28 Stable branch

lore
Share & tools
lore
#0 lore raw

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 UAF

de2 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
index 6682b8ab11f1c..bc53ea82161d3 100644
@@ -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
#1 lore raw
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 --

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
index 6682b8ab11f1c..bc53ea82161d3 100644

[ ... ]

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