syzbot reported a use-after-free Read in ext4_find_extent that is hit when using a corrupted file system. The bug was reported on Android 5.15, but using the same reproducer triggers the bug on v6.2-rc1 as well.
Fix the use-after-free by checking the extent header magic. An alternative would be to check the values of EXT4_{FIRST,LAST}_{EXTENT,INDEX} used in ext4_ext_binsearch() and ext4_ext_binsearch_idx(), so that we make sure that pointers returned by EXT4_{FIRST,LAST}_{EXTENT,INDEX} don't exceed the bounds of the extent tree node. But this alternative will not squash the bug for the cases where eh->eh_entries fit into eh->eh_max. We could also try to check the sanity of the path, but costs more than checking just the header magic, so stick to the header magic sanity check.
Link: https://syzkaller.appspot.com/bug?id=be6e90ce70987950e6deb3bac8418344ca8b96c... Reported-by: syzbot+0827b4b52b5ebf65f219@syzkaller.appspotmail.com Cc: stable@vger.kernel.org Signed-off-by: Tudor Ambarus tudor.ambarus@linaro.org --- fs/ext4/extents.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c index 9de1c9d1a13d..18367767afd7 100644 --- a/fs/ext4/extents.c +++ b/fs/ext4/extents.c @@ -894,6 +894,12 @@ ext4_find_extent(struct inode *inode, ext4_lblk_t block, gfp_flags |= __GFP_NOFAIL;
eh = ext_inode_hdr(inode); + if (le16_to_cpu(eh->eh_magic) != EXT4_EXT_MAGIC) { + EXT4_ERROR_INODE(inode, "Extent header has invalid magic."); + ret = -EFSCORRUPTED; + goto err; + } + depth = ext_depth(inode); if (depth < 0 || depth > EXT4_MAX_EXTENT_DEPTH) { EXT4_ERROR_INODE(inode, "inode has invalid extent depth: %d",
Hi Tudor,
I love your patch! Perhaps something to improve:
[auto build test WARNING on tytso-ext4/dev] [also build test WARNING on linus/master v6.2-rc1 next-20221226] [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/Tudor-Ambarus/ext4-Fix-possib... base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev patch link: https://lore.kernel.org/r/20221229073606.1711522-1-tudor.ambarus%40linaro.or... patch subject: [PATCH] ext4: Fix possible use-after-free in ext4_find_extent config: x86_64-randconfig-s023 compiler: gcc-11 (Debian 11.3.0-8) 11.3.0 reproduce: # apt-get install sparse # sparse version: v0.6.4-39-gce1a6720-dirty # https://github.com/intel-lab-lkp/linux/commit/c88d21ba221fa1b9e4b6b34efbee21... git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Tudor-Ambarus/ext4-Fix-possible-use-after-free-in-ext4_find_extent/20221229-153646 git checkout c88d21ba221fa1b9e4b6b34efbee21080e35c193 # save the config file mkdir build_dir && cp config build_dir/.config make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 olddefconfig make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 SHELL=/bin/bash fs/ext4/
If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot lkp@intel.com
sparse warnings: (new ones prefixed by >>)
fs/ext4/extents.c:897:42: sparse: sparse: restricted __le16 degrades to integer
vim +897 fs/ext4/extents.c
881 882 struct ext4_ext_path * 883 ext4_find_extent(struct inode *inode, ext4_lblk_t block, 884 struct ext4_ext_path **orig_path, int flags) 885 { 886 struct ext4_extent_header *eh; 887 struct buffer_head *bh; 888 struct ext4_ext_path *path = orig_path ? *orig_path : NULL; 889 short int depth, i, ppos = 0; 890 int ret; 891 gfp_t gfp_flags = GFP_NOFS; 892 893 if (flags & EXT4_EX_NOFAIL) 894 gfp_flags |= __GFP_NOFAIL; 895 896 eh = ext_inode_hdr(inode);
897 if (le16_to_cpu(eh->eh_magic) != EXT4_EXT_MAGIC) {
898 EXT4_ERROR_INODE(inode, "Extent header has invalid magic."); 899 ret = -EFSCORRUPTED; 900 goto err; 901 } 902 903 depth = ext_depth(inode); 904 if (depth < 0 || depth > EXT4_MAX_EXTENT_DEPTH) { 905 EXT4_ERROR_INODE(inode, "inode has invalid extent depth: %d", 906 depth); 907 ret = -EFSCORRUPTED; 908 goto err; 909 } 910 911 if (path) { 912 ext4_ext_drop_refs(path); 913 if (depth > path[0].p_maxdepth) { 914 kfree(path); 915 *orig_path = path = NULL; 916 } 917 } 918 if (!path) { 919 /* account possible depth increase */ 920 path = kcalloc(depth + 2, sizeof(struct ext4_ext_path), 921 gfp_flags); 922 if (unlikely(!path)) 923 return ERR_PTR(-ENOMEM); 924 path[0].p_maxdepth = depth + 1; 925 } 926 path[0].p_hdr = eh; 927 path[0].p_bh = NULL; 928 929 i = depth; 930 if (!(flags & EXT4_EX_NOCACHE) && depth == 0) 931 ext4_cache_extents(inode, eh); 932 /* walk through the tree */ 933 while (i) { 934 ext_debug(inode, "depth %d: num %d, max %d\n", 935 ppos, le16_to_cpu(eh->eh_entries), le16_to_cpu(eh->eh_max)); 936 937 ext4_ext_binsearch_idx(inode, path + ppos, block); 938 path[ppos].p_block = ext4_idx_pblock(path[ppos].p_idx); 939 path[ppos].p_depth = i; 940 path[ppos].p_ext = NULL; 941 942 bh = read_extent_tree_block(inode, path[ppos].p_idx, --i, flags); 943 if (IS_ERR(bh)) { 944 ret = PTR_ERR(bh); 945 goto err; 946 } 947 948 eh = ext_block_hdr(bh); 949 ppos++; 950 path[ppos].p_bh = bh; 951 path[ppos].p_hdr = eh; 952 } 953 954 path[ppos].p_depth = i; 955 path[ppos].p_ext = NULL; 956 path[ppos].p_idx = NULL; 957 958 /* find extent */ 959 ext4_ext_binsearch(inode, path + ppos, block); 960 /* if not an empty leaf */ 961 if (path[ppos].p_ext) 962 path[ppos].p_block = ext4_ext_pblock(path[ppos].p_ext); 963 964 ext4_ext_show_path(inode, path); 965 966 return path; 967 968 err: 969 ext4_free_ext_path(path); 970 if (orig_path) 971 *orig_path = NULL; 972 return ERR_PTR(ret); 973 } 974
linux-stable-mirror@lists.linaro.org