]> git.ipfire.org Git - thirdparty/linux.git/commitdiff
ext4: check dot and dotdot of dx_root before making dir indexed
authorBaokun Li <libaokun1@huawei.com>
Tue, 2 Jul 2024 13:23:48 +0000 (21:23 +0800)
committerTheodore Ts'o <tytso@mit.edu>
Thu, 11 Jul 2024 03:25:12 +0000 (23:25 -0400)
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: syzbot+ae688d469e36fb5138d0@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=ae688d469e36fb5138d0
Fixes: ac27a0ec112a ("[PATCH] ext4: initial copy of files from ext3")
Cc: stable@kernel.org
Signed-off-by: Baokun Li <libaokun1@huawei.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Link: https://patch.msgid.link/20240702132349.2600605-2-libaokun@huaweicloud.com
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
fs/ext4/namei.c

index a630b27a4cc6e7caf57301831d2eeb68f8827c1f..6917c1de8f412f4ea9020fcfc5478a76a3c9ae37 100644 (file)
@@ -2217,6 +2217,52 @@ static int add_dirent_to_buf(handle_t *handle, struct ext4_filename *fname,
        return err ? err : err2;
 }
 
+static bool ext4_check_dx_root(struct inode *dir, struct dx_root *root)
+{
+       struct fake_dirent *fde;
+       const char *error_msg;
+       unsigned int rlen;
+       unsigned int blocksize = dir->i_sb->s_blocksize;
+       char *blockend = (char *)root + dir->i_sb->s_blocksize;
+
+       fde = &root->dot;
+       if (unlikely(fde->name_len != 1)) {
+               error_msg = "invalid name_len for '.'";
+               goto corrupted;
+       }
+       if (unlikely(strncmp(root->dot_name, ".", fde->name_len))) {
+               error_msg = "invalid name for '.'";
+               goto corrupted;
+       }
+       rlen = ext4_rec_len_from_disk(fde->rec_len, blocksize);
+       if (unlikely((char *)fde + rlen >= blockend)) {
+               error_msg = "invalid rec_len for '.'";
+               goto corrupted;
+       }
+
+       fde = &root->dotdot;
+       if (unlikely(fde->name_len != 2)) {
+               error_msg = "invalid name_len for '..'";
+               goto corrupted;
+       }
+       if (unlikely(strncmp(root->dotdot_name, "..", fde->name_len))) {
+               error_msg = "invalid name for '..'";
+               goto corrupted;
+       }
+       rlen = ext4_rec_len_from_disk(fde->rec_len, blocksize);
+       if (unlikely((char *)fde + rlen >= blockend)) {
+               error_msg = "invalid rec_len for '..'";
+               goto corrupted;
+       }
+
+       return true;
+
+corrupted:
+       EXT4_ERROR_INODE(dir, "Corrupt dir, %s, running e2fsck is recommended",
+                        error_msg);
+       return false;
+}
+
 /*
  * This converts a one block unindexed directory to a 3 block indexed
  * 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,
                brelse(bh);
                return retval;
        }
+
        root = (struct dx_root *) bh->b_data;
+       if (!ext4_check_dx_root(dir, root)) {
+               brelse(bh);
+               return -EFSCORRUPTED;
+       }
 
        /* The 0th block becomes the root, move the dirents out */
        fde = &root->dotdot;
        de = (struct ext4_dir_entry_2 *)((char *)fde +
                ext4_rec_len_from_disk(fde->rec_len, blocksize));
-       if ((char *) de >= (((char *) root) + blocksize)) {
-               EXT4_ERROR_INODE(dir, "invalid rec_len for '..'");
-               brelse(bh);
-               return -EFSCORRUPTED;
-       }
        len = ((char *) root) + (blocksize - csum_size) - (char *) de;
 
        /* Allocate new block for the 0th block's dirents */