Skip to content

Commit 50ea741

Browse files
LiBaokun96tytso
authored andcommitted
ext4: check dot and dotdot of dx_root before making dir indexed
Syzbot reports a issue as follows: ============================================ BUG: unable to handle page fault for address: ffffed11022e24fe PGD 23ffee067 P4D 23ffee067 PUD 0 Oops: Oops: 0000 [#1] PREEMPT SMP KASAN PTI CPU: 0 PID: 5079 Comm: syz-executor306 Not tainted 6.10.0-rc5-g55027e689933 #0 Call Trace: <TASK> make_indexed_dir+0xdaf/0x13c0 fs/ext4/namei.c:2341 ext4_add_entry+0x222a/0x25d0 fs/ext4/namei.c:2451 ext4_rename fs/ext4/namei.c:3936 [inline] ext4_rename2+0x26e5/0x4370 fs/ext4/namei.c:4214 [...] ============================================ The immediate cause of this problem is that there is only one valid dentry for the block to be split during do_split, so split==0 results in out of bounds accesses to the map triggering the issue. do_split unsigned split dx_make_map count = 1 split = count/2 = 0; continued = hash2 == map[split - 1].hash; ---> map[4294967295] The maximum length of a filename is 255 and the minimum block size is 1024, so it is always guaranteed that the number of entries is greater than or equal to 2 when do_split() is called. But syzbot's crafted image has no dot and dotdot in dir, and the dentry distribution in dirblock is as follows: bus dentry1 hole dentry2 free |xx--|xx-------------|...............|xx-------------|...............| 0 12 (8+248)=256 268 256 524 (8+256)=264 788 236 1024 So when renaming dentry1 increases its name_len length by 1, neither hole nor free is sufficient to hold the new dentry, and make_indexed_dir() is called. In make_indexed_dir() it is assumed that the first two entries of the dirblock must be dot and dotdot, so bus and dentry1 are left in dx_root because they are treated as dot and dotdot, and only dentry2 is moved to the new leaf block. That's why count is equal to 1. Therefore add the ext4_check_dx_root() helper function to add more sanity checks to dot and dotdot before starting the conversion to avoid the above issue. Reported-by: [email protected] Closes: https://syzkaller.appspot.com/bug?extid=ae688d469e36fb5138d0 Fixes: ac27a0e ("[PATCH] ext4: initial copy of files from ext3") Cc: [email protected] Signed-off-by: Baokun Li <[email protected]> Reviewed-by: Jan Kara <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Theodore Ts'o <[email protected]>
1 parent 83f4414 commit 50ea741

File tree

1 file changed

+51
-5
lines changed

1 file changed

+51
-5
lines changed

fs/ext4/namei.c

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2217,6 +2217,52 @@ static int add_dirent_to_buf(handle_t *handle, struct ext4_filename *fname,
22172217
return err ? err : err2;
22182218
}
22192219

2220+
static bool ext4_check_dx_root(struct inode *dir, struct dx_root *root)
2221+
{
2222+
struct fake_dirent *fde;
2223+
const char *error_msg;
2224+
unsigned int rlen;
2225+
unsigned int blocksize = dir->i_sb->s_blocksize;
2226+
char *blockend = (char *)root + dir->i_sb->s_blocksize;
2227+
2228+
fde = &root->dot;
2229+
if (unlikely(fde->name_len != 1)) {
2230+
error_msg = "invalid name_len for '.'";
2231+
goto corrupted;
2232+
}
2233+
if (unlikely(strncmp(root->dot_name, ".", fde->name_len))) {
2234+
error_msg = "invalid name for '.'";
2235+
goto corrupted;
2236+
}
2237+
rlen = ext4_rec_len_from_disk(fde->rec_len, blocksize);
2238+
if (unlikely((char *)fde + rlen >= blockend)) {
2239+
error_msg = "invalid rec_len for '.'";
2240+
goto corrupted;
2241+
}
2242+
2243+
fde = &root->dotdot;
2244+
if (unlikely(fde->name_len != 2)) {
2245+
error_msg = "invalid name_len for '..'";
2246+
goto corrupted;
2247+
}
2248+
if (unlikely(strncmp(root->dotdot_name, "..", fde->name_len))) {
2249+
error_msg = "invalid name for '..'";
2250+
goto corrupted;
2251+
}
2252+
rlen = ext4_rec_len_from_disk(fde->rec_len, blocksize);
2253+
if (unlikely((char *)fde + rlen >= blockend)) {
2254+
error_msg = "invalid rec_len for '..'";
2255+
goto corrupted;
2256+
}
2257+
2258+
return true;
2259+
2260+
corrupted:
2261+
EXT4_ERROR_INODE(dir, "Corrupt dir, %s, running e2fsck is recommended",
2262+
error_msg);
2263+
return false;
2264+
}
2265+
22202266
/*
22212267
* This converts a one block unindexed directory to a 3 block indexed
22222268
* directory, and adds the dentry to the indexed directory.
@@ -2251,17 +2297,17 @@ static int make_indexed_dir(handle_t *handle, struct ext4_filename *fname,
22512297
brelse(bh);
22522298
return retval;
22532299
}
2300+
22542301
root = (struct dx_root *) bh->b_data;
2302+
if (!ext4_check_dx_root(dir, root)) {
2303+
brelse(bh);
2304+
return -EFSCORRUPTED;
2305+
}
22552306

22562307
/* The 0th block becomes the root, move the dirents out */
22572308
fde = &root->dotdot;
22582309
de = (struct ext4_dir_entry_2 *)((char *)fde +
22592310
ext4_rec_len_from_disk(fde->rec_len, blocksize));
2260-
if ((char *) de >= (((char *) root) + blocksize)) {
2261-
EXT4_ERROR_INODE(dir, "invalid rec_len for '..'");
2262-
brelse(bh);
2263-
return -EFSCORRUPTED;
2264-
}
22652311
len = ((char *) root) + (blocksize - csum_size) - (char *) de;
22662312

22672313
/* Allocate new block for the 0th block's dirents */

0 commit comments

Comments
 (0)