Re: [PATCH] ext4: validate fiemap iomap begin offset and length value

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



++ mailing list.
Sorry somehow it got dropped.


On 4/19/20 7:21 AM, Ritesh Harjani wrote:
Hello Murphy,

I guess the patch to fix this issue was recently submitted.
Could you please test your reproducer, xfstest and ltp
tests on below patch too. And let me know if we can add your Tested-by:

https://patchwork.ozlabs.org/project/linux-ext4/patch/1a2dc8f198e1225ddd40833de76b60c7ee20d22d.1587024137.git.riteshh@xxxxxxxxxxxxx/

-ritesh

On 4/19/20 5:02 AM, Murphy Zhou wrote:
Sometimes crazy userspace values can be here causing overflow issue.

After moved ext4_fiemap to using the iomap framework in
   commit d3b6f23f7167 ("ext4: move ext4_fiemap to use iomap framework")
we can hit the WARN_ON at fs/iomap/apply.c:51, then get an EIO error
running xfstests generic/009 (and some others) on ext4 based overlayfs.

The minimal reproducer is:
-------------------------------------
fallocate -l 256M test.img
mkfs.ext4 -Fq -b 4096 -I 256 test.img
mkdir -p test
mount -o loop test.img test || exit
pushd test
rm -rf l u w m
mkdir -p l u w m
mount -t overlay -o lowerdir=l,upperdir=u,workdir=w overlay m || exit
xfs_io -f -c "pwrite 0 4096" -c "fiemap"  m/tf
umount m
rm -rf l u w m
popd
umount -d test
rm -rf test test.img
-------------------------------------

Because we run fiemap command wo/ the offset and length parameters,
xfs_io set values based on fs blocksize etc which is got from
the mounted fs. These values xfs_io passed are way larger on overlayfs
than ext4 directly. So we can't reproduce this directly on ext4 or xfs.
I tried to call ioctl directly with large length value but failed to
reproduce this.

I did not try to get what values xfs_io exactly passing in, but I
confirmed that overflowed value when it made into _ext4_fiemap.
It's a length of 0x7fffffffffffffff which will mess up the calculation
of map.m_lblk and map.m_len, make map.m_len to be 0, then hit WARN_ON
and get EIO in iomap_apply.

Fixing this by ensuring the offset and length values wont exceed
EXT4_MAX_LOGICAL_BLOCK. Also make sure that the length would not
be zero because of crazy overflowed values.

This patch has been tested with LTP/xfstests showing no new issue.

Signed-off-by: Murphy Zhou <jencce.kernel@xxxxxxxxx>
Fixes: d3b6f23f7167 ("ext4: move ext4_fiemap to use iomap framework")
---
  fs/ext4/inode.c | 17 ++++++++++++++---
  1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index e416096..3620417 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3523,6 +3523,8 @@ static int ext4_iomap_begin_report(struct inode *inode, loff_t offset,
      int ret;
      bool delalloc = false;
      struct ext4_map_blocks map;
+    ext4_lblk_t last_lblk;
+    ext4_lblk_t lblk;
      u8 blkbits = inode->i_blkbits;
      if ((offset >> blkbits) > EXT4_MAX_LOGICAL_BLOCK)
@@ -3540,9 +3542,18 @@ static int ext4_iomap_begin_report(struct inode *inode, loff_t offset,
      /*
       * Calculate the first and last logical block respectively.
       */
-    map.m_lblk = offset >> blkbits;
-    map.m_len = min_t(loff_t, (offset + length - 1) >> blkbits,
-              EXT4_MAX_LOGICAL_BLOCK) - map.m_lblk + 1;
+    lblk = offset >> blkbits;
+    last_lblk = (offset + length - 1) >> blkbits;
+
+    if (last_lblk >= EXT4_MAX_LOGICAL_BLOCK)
+        last_lblk = EXT4_MAX_LOGICAL_BLOCK - 1;
+    if (lblk >= EXT4_MAX_LOGICAL_BLOCK)
+        lblk = EXT4_MAX_LOGICAL_BLOCK - 1;
+
+    map.m_lblk = lblk;
+    map.m_len = last_lblk - lblk + 1;
+    if (map.m_len == 0 )
+        map.m_len = 1;
      /*
       * Fiemap callers may call for offset beyond s_bitmap_maxbytes.





[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux