2022-12-29 07:40:11

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH] ext4: Fix possible use-after-free in ext4_find_extent

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=be6e90ce70987950e6deb3bac8418344ca8b96cd
Reported-by: [email protected]
Cc: [email protected]
Signed-off-by: Tudor Ambarus <[email protected]>
---
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",
--
2.34.1


2022-12-29 22:41:41

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] ext4: Fix possible use-after-free in ext4_find_extent

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-possible-use-after-free-in-ext4_find_extent/20221229-153646
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.org
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/c88d21ba221fa1b9e4b6b34efbee21080e35c193
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 <[email protected]>

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

--
0-DAY CI Kernel Test Service
https://01.org/lkp


Attachments:
(No filename) (4.77 kB)
config (124.93 kB)
Download all attachments