diff mbox series

[V2] ext4: Add a sanity check for next dentry when insert

Message ID tencent_2EB5A7DB06DD92D88651C9B3EED8AEF38C06@qq.com
State New
Headers show
Series [V2] ext4: Add a sanity check for next dentry when insert | expand

Commit Message

Edward Adam Davis Oct. 28, 2024, 2:07 p.m. UTC
Syzbot reported a use-after-free in ext4_insert_dentry.

Before copying the file name to the next directory entry, it is necessary to
confirm whether there is enough space in the next directory entry.
When the space is insufficient, it will not be inserted and an error code
-EINVAL will be returned.

Reported-by: syzbot+0c99c3f90699936c1e77@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=0c99c3f90699936c1e77
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
V1 -> V2: change check_next_dentry to static and comments

 fs/ext4/ext4.h   |  2 +-
 fs/ext4/inline.c |  4 +++-
 fs/ext4/namei.c  | 32 ++++++++++++++++++++++++++------
 3 files changed, 30 insertions(+), 8 deletions(-)

Comments

kernel test robot Nov. 4, 2024, 6:43 a.m. UTC | #1
Hello,

kernel test robot noticed "xfstests.generic.080.fail" on:

commit: d29093707e013ca381d404c4444413df49c719c1 ("[PATCH V2] ext4: Add a sanity check for next dentry when insert")
url: https://github.com/intel-lab-lkp/linux/commits/Edward-Adam-Davis/ext4-Add-a-sanity-check-for-next-dentry-when-insert/20241028-220910
base: https://git.kernel.org/cgit/linux/kernel/git/tytso/ext4.git dev
patch link: https://lore.kernel.org/all/tencent_2EB5A7DB06DD92D88651C9B3EED8AEF38C06@qq.com/
patch subject: [PATCH V2] ext4: Add a sanity check for next dentry when insert

in testcase: xfstests
version: xfstests-x86_64-891f4995-1_20241028
with following parameters:

	disk: 4HDD
	fs: ext4
	fs2: smbv2
	test: generic-080



config: x86_64-rhel-8.3-func
compiler: gcc-12
test machine: 4 threads Intel(R) Core(TM) i5-6500 CPU @ 3.20GHz (Skylake) with 32G memory

(please refer to attached dmesg/kmsg for entire log/backtrace)




If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202411041103.a030928c-oliver.sang@intel.com

2024-10-31 17:45:57 mount /dev/sda1 /fs/sda1
2024-10-31 17:45:58 mkdir -p /smbv2//cifs/sda1
2024-10-31 17:45:58 export FSTYP=cifs
2024-10-31 17:45:58 export TEST_DEV=//localhost/fs/sda1
2024-10-31 17:45:58 export TEST_DIR=/smbv2//cifs/sda1
2024-10-31 17:45:58 export CIFS_MOUNT_OPTIONS=-ousername=root,password=pass,noperm,vers=2.0,mfsymlinks,actimeo=0
2024-10-31 17:45:58 echo generic/080
2024-10-31 17:45:58 ./check -E tests/cifs/exclude.incompatible-smb2.txt -E tests/cifs/exclude.very-slow.txt generic/080
FSTYP         -- cifs
PLATFORM      -- Linux/x86_64 lkp-skl-d05 6.12.0-rc1-00004-gd29093707e01 #1 SMP PREEMPT_DYNAMIC Wed Oct 30 22:27:17 CST 2024

generic/080       - output mismatch (see /lkp/benchmarks/xfstests/results//generic/080.out.bad)
    --- tests/generic/080.out	2024-10-28 16:28:46.000000000 +0000
    +++ /lkp/benchmarks/xfstests/results//generic/080.out.bad	2024-10-31 17:46:01.599410948 +0000
    @@ -1,2 +1,3 @@
     QA output created by 080
     Silence is golden.
    +rm: cannot remove '/smbv2/cifs/sda1/mmap_mtime_testfile': Permission denied
    ...
    (Run 'diff -u /lkp/benchmarks/xfstests/tests/generic/080.out /lkp/benchmarks/xfstests/results//generic/080.out.bad'  to see the entire diff)
Ran: generic/080
Failures: generic/080
Failed 1 of 1 tests




The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20241104/202411041103.a030928c-oliver.sang@intel.com
diff mbox series

Patch

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 44b0d418143c..e07ac540ed00 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2834,7 +2834,7 @@  extern int ext4_find_dest_de(struct inode *dir, struct inode *inode,
 			     void *buf, int buf_size,
 			     struct ext4_filename *fname,
 			     struct ext4_dir_entry_2 **dest_de);
-void ext4_insert_dentry(struct inode *dir, struct inode *inode,
+int ext4_insert_dentry(struct inode *dir, struct inode *inode,
 			struct ext4_dir_entry_2 *de,
 			int buf_size,
 			struct ext4_filename *fname);
diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index 3536ca7e4fcc..e318b13459d1 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -1022,7 +1022,9 @@  static int ext4_add_dirent_to_inline(handle_t *handle,
 					    EXT4_JTR_NONE);
 	if (err)
 		return err;
-	ext4_insert_dentry(dir, inode, de, inline_size, fname);
+	err = ext4_insert_dentry(dir, inode, de, inline_size, fname);
+	if (err)
+		return err;
 
 	ext4_show_inline_dir(dir, iloc->bh, inline_start, inline_size);
 
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 790db7eac6c2..1c9fedf36fb0 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -2084,24 +2084,38 @@  int ext4_find_dest_de(struct inode *dir, struct inode *inode,
 	return 0;
 }
 
-void ext4_insert_dentry(struct inode *dir,
+static int check_next_dentry(struct inode *dir,
 			struct inode *inode,
 			struct ext4_dir_entry_2 *de,
 			int buf_size,
 			struct ext4_filename *fname)
 {
-
 	int nlen, rlen;
 
 	nlen = ext4_dir_rec_len(de->name_len, dir);
 	rlen = ext4_rec_len_from_disk(de->rec_len, buf_size);
 	if (de->inode) {
-		struct ext4_dir_entry_2 *de1 =
+		struct ext4_dir_entry_2 *nde =
 			(struct ext4_dir_entry_2 *)((char *)de + nlen);
-		de1->rec_len = ext4_rec_len_to_disk(rlen - nlen, buf_size);
+		nde->rec_len = ext4_rec_len_to_disk(rlen - nlen, buf_size);
 		de->rec_len = ext4_rec_len_to_disk(nlen, buf_size);
-		de = de1;
+		de = nde;
+		rlen = ext4_rec_len_from_disk(de->rec_len, buf_size);
+		return fname_len(fname) > rlen - EXT4_BASE_DIR_LEN;
 	}
+
+	return 0;
+}
+
+int ext4_insert_dentry(struct inode *dir,
+			struct inode *inode,
+			struct ext4_dir_entry_2 *de,
+			int buf_size,
+			struct ext4_filename *fname)
+{
+	if (check_next_dentry(dir, inode, de, buf_size, fname))
+		return -EINVAL;
+
 	de->file_type = EXT4_FT_UNKNOWN;
 	de->inode = cpu_to_le32(inode->i_ino);
 	ext4_set_de_type(inode->i_sb, de, inode->i_mode);
@@ -2114,6 +2128,8 @@  void ext4_insert_dentry(struct inode *dir,
 		EXT4_DIRENT_HASHES(de)->minor_hash =
 						cpu_to_le32(hinfo->minor_hash);
 	}
+
+	return 0;
 }
 
 /*
@@ -2151,7 +2167,11 @@  static int add_dirent_to_buf(handle_t *handle, struct ext4_filename *fname,
 	}
 
 	/* By now the buffer is marked for journaling */
-	ext4_insert_dentry(dir, inode, de, blocksize, fname);
+	err = ext4_insert_dentry(dir, inode, de, blocksize, fname);
+	if (err) {
+		ext4_std_error(dir->i_sb, err);
+		return err;
+	}
 
 	/*
 	 * XXX shouldn't update any times until successful