diff mbox series

ext4: Add a sanity check for next dentry when insert

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

Commit Message

Edward Adam Davis Oct. 27, 2024, 11:09 a.m. UTC 
Syzbot reported a use-after-free in ext4_insert_dentry.

Before inserting 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>
---
 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 Oct. 27, 2024, 3:45 p.m. UTC | #1 
Hi Edward,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tytso-ext4/dev]
[also build test WARNING on linus/master v6.12-rc4 next-20241025]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Edward-Adam-Davis/ext4-Add-a-sanity-check-for-next-dentry-when-insert/20241027-191200
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link:    https://lore.kernel.org/r/tencent_E4CFC65D09852ECE2EF28C83A7C3C6E41206%40qq.com
patch subject: [PATCH] ext4: Add a sanity check for next dentry when insert
config: x86_64-rhel-8.3 (https://download.01.org/0day-ci/archive/20241027/202410272114.DrZ8huEU-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241027/202410272114.DrZ8huEU-lkp@intel.com/reproduce)

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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410272114.DrZ8huEU-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> fs/ext4/namei.c:2087:5: warning: no previous prototype for 'ext4_check_next_dentry' [-Wmissing-prototypes]
    2087 | int ext4_check_next_dentry(struct inode *dir,
         |     ^~~~~~~~~~~~~~~~~~~~~~


vim +/ext4_check_next_dentry +2087 fs/ext4/namei.c

  2086	
> 2087	int ext4_check_next_dentry(struct inode *dir,
  2088				struct inode *inode,
  2089				struct ext4_dir_entry_2 *de,
  2090				int buf_size,
  2091				struct ext4_filename *fname)
  2092	{
  2093		int nlen, rlen;
  2094	
  2095		nlen = ext4_dir_rec_len(de->name_len, dir);
  2096		rlen = ext4_rec_len_from_disk(de->rec_len, buf_size);
  2097		if (de->inode) {
  2098			struct ext4_dir_entry_2 *nde =
  2099				(struct ext4_dir_entry_2 *)((char *)de + nlen);
  2100			nde->rec_len = ext4_rec_len_to_disk(rlen - nlen, buf_size);
  2101			de->rec_len = ext4_rec_len_to_disk(nlen, buf_size);
  2102			de = nde;
  2103			rlen = ext4_rec_len_from_disk(de->rec_len, buf_size);
  2104			return fname_len(fname) > rlen - EXT4_BASE_DIR_LEN;
  2105		}
  2106	
  2107		return 0;
  2108	}
  2109
kernel test robot Oct. 27, 2024, 4:06 p.m. UTC | #2 
Hi Edward,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tytso-ext4/dev]
[also build test WARNING on linus/master v6.12-rc4 next-20241025]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Edward-Adam-Davis/ext4-Add-a-sanity-check-for-next-dentry-when-insert/20241027-191200
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link:    https://lore.kernel.org/r/tencent_E4CFC65D09852ECE2EF28C83A7C3C6E41206%40qq.com
patch subject: [PATCH] ext4: Add a sanity check for next dentry when insert
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20241027/202410272335.nwupXeQD-lkp@intel.com/config)
compiler: clang version 19.1.2 (https://github.com/llvm/llvm-project 7ba7d8e2f7b6445b60679da826210cdde29eaf8b)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241027/202410272335.nwupXeQD-lkp@intel.com/reproduce)

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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410272335.nwupXeQD-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from fs/ext4/namei.c:29:
   In file included from include/linux/pagemap.h:8:
   In file included from include/linux/mm.h:2213:
   include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     504 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     505 |                            item];
         |                            ~~~~
   include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     511 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     512 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     518 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     524 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     525 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
>> fs/ext4/namei.c:2087:5: warning: no previous prototype for function 'ext4_check_next_dentry' [-Wmissing-prototypes]
    2087 | int ext4_check_next_dentry(struct inode *dir,
         |     ^
   fs/ext4/namei.c:2087:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
    2087 | int ext4_check_next_dentry(struct inode *dir,
         | ^
         | static 
   5 warnings generated.


vim +/ext4_check_next_dentry +2087 fs/ext4/namei.c

  2086	
> 2087	int ext4_check_next_dentry(struct inode *dir,
  2088				struct inode *inode,
  2089				struct ext4_dir_entry_2 *de,
  2090				int buf_size,
  2091				struct ext4_filename *fname)
  2092	{
  2093		int nlen, rlen;
  2094	
  2095		nlen = ext4_dir_rec_len(de->name_len, dir);
  2096		rlen = ext4_rec_len_from_disk(de->rec_len, buf_size);
  2097		if (de->inode) {
  2098			struct ext4_dir_entry_2 *nde =
  2099				(struct ext4_dir_entry_2 *)((char *)de + nlen);
  2100			nde->rec_len = ext4_rec_len_to_disk(rlen - nlen, buf_size);
  2101			de->rec_len = ext4_rec_len_to_disk(nlen, buf_size);
  2102			de = nde;
  2103			rlen = ext4_rec_len_from_disk(de->rec_len, buf_size);
  2104			return fname_len(fname) > rlen - EXT4_BASE_DIR_LEN;
  2105		}
  2106	
  2107		return 0;
  2108	}
  2109
kernel test robot Oct. 28, 2024, 2:14 p.m. UTC | #3 
Hi Edward,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tytso-ext4/dev]
[also build test WARNING on linus/master v6.12-rc5 next-20241028]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Edward-Adam-Davis/ext4-Add-a-sanity-check-for-next-dentry-when-insert/20241027-191200
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link:    https://lore.kernel.org/r/tencent_E4CFC65D09852ECE2EF28C83A7C3C6E41206%40qq.com
patch subject: [PATCH] ext4: Add a sanity check for next dentry when insert
config: x86_64-randconfig-121-20241028 (https://download.01.org/0day-ci/archive/20241028/202410282131.FBDMC1Gq-lkp@intel.com/config)
compiler: clang version 19.1.2 (https://github.com/llvm/llvm-project 7ba7d8e2f7b6445b60679da826210cdde29eaf8b)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241028/202410282131.FBDMC1Gq-lkp@intel.com/reproduce)

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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410282131.FBDMC1Gq-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> fs/ext4/namei.c:2087:5: sparse: sparse: symbol 'ext4_check_next_dentry' was not declared. Should it be static?
   fs/ext4/namei.c: note: in included file (through include/linux/mmzone.h, include/linux/gfp.h, include/linux/xarray.h, ...):
   include/linux/page-flags.h:237:46: sparse: sparse: self-comparison always evaluates to false
   include/linux/page-flags.h:237:46: sparse: sparse: self-comparison always evaluates to false
   fs/ext4/namei.c: note: in included file:
   fs/ext4/ext4.h:2429:9: sparse: sparse: self-comparison always evaluates to false

vim +/ext4_check_next_dentry +2087 fs/ext4/namei.c

  2086	
> 2087	int ext4_check_next_dentry(struct inode *dir,
  2088				struct inode *inode,
  2089				struct ext4_dir_entry_2 *de,
  2090				int buf_size,
  2091				struct ext4_filename *fname)
  2092	{
  2093		int nlen, rlen;
  2094	
  2095		nlen = ext4_dir_rec_len(de->name_len, dir);
  2096		rlen = ext4_rec_len_from_disk(de->rec_len, buf_size);
  2097		if (de->inode) {
  2098			struct ext4_dir_entry_2 *nde =
  2099				(struct ext4_dir_entry_2 *)((char *)de + nlen);
  2100			nde->rec_len = ext4_rec_len_to_disk(rlen - nlen, buf_size);
  2101			de->rec_len = ext4_rec_len_to_disk(nlen, buf_size);
  2102			de = nde;
  2103			rlen = ext4_rec_len_from_disk(de->rec_len, buf_size);
  2104			return fname_len(fname) > rlen - EXT4_BASE_DIR_LEN;
  2105		}
  2106	
  2107		return 0;
  2108	}
  2109
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..843d23391b0c 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,
+int ext4_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 (ext4_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