On Mon, May 23, 2022 at 02:15:44 PM +0300, Amir Goldstein wrote: > On Sun, Jan 10, 2021 at 6:10 PM Chandan Babu R <chandanrlinux@xxxxxxxxx> wrote: >> >> XFS does not check for possible overflow of per-inode extent counter >> fields when adding extents to either data or attr fork. >> >> For e.g. >> 1. Insert 5 million xattrs (each having a value size of 255 bytes) and >> then delete 50% of them in an alternating manner. >> >> 2. On a 4k block sized XFS filesystem instance, the above causes 98511 >> extents to be created in the attr fork of the inode. >> >> xfsaild/loop0 2008 [003] 1475.127209: probe:xfs_inode_to_disk: (ffffffffa43fb6b0) if_nextents=98511 i_ino=131 >> >> 3. The incore inode fork extent counter is a signed 32-bit >> quantity. However, the on-disk extent counter is an unsigned 16-bit >> quantity and hence cannot hold 98511 extents. >> >> 4. The following incorrect value is stored in the xattr extent counter, >> # xfs_db -f -c 'inode 131' -c 'print core.naextents' /dev/loop0 >> core.naextents = -32561 >> >> This patchset adds a new helper function >> (i.e. xfs_iext_count_may_overflow()) to check for overflow of the >> per-inode data and xattr extent counters and invokes it before >> starting an fs operation (e.g. creating a new directory entry). With >> this patchset applied, XFS detects counter overflows and returns with >> an error rather than causing a silent corruption. >> >> The patchset has been tested by executing xfstests with the following >> mkfs.xfs options, >> 1. -m crc=0 -b size=1k >> 2. -m crc=0 -b size=4k >> 3. -m crc=0 -b size=512 >> 4. -m rmapbt=1,reflink=1 -b size=1k >> 5. -m rmapbt=1,reflink=1 -b size=4k >> >> The patches can also be obtained from >> https://github.com/chandanr/linux.git at branch xfs-reserve-extent-count-v14. >> >> I have two patches that define the newly introduced error injection >> tags in xfsprogs >> (https://lore.kernel.org/linux-xfs/20201104114900.172147-1-chandanrlinux@xxxxxxxxx/). >> >> I have also written tests >> (https://github.com/chandanr/xfstests/commits/extent-overflow-tests) >> for verifying the checks introduced in the kernel. >> > > Hi Chandan and XFS folks, > > As you may have heard, I am working on producing a series of > xfs patches for stable v5.10.y. > > My patch selection is documented at [1]. > I am in the process of testing the backport patches against the 5.10.y > baseline using Luis' kdevops [2] fstests runner. > > The configurations that we are testing are: > 1. -m rmbat=0,reflink=1 -b size=4k (default) > 2. -m crc=0 -b size=4k > 3. -m crc=0 -b size=512 > 4. -m rmapbt=1,reflink=1 -b size=1k > 5. -m rmapbt=1,reflink=1 -b size=4k > > This patch set is the only largish series that I selected, because: > - It applies cleanly to 5.10.y > - I evaluated it as low risk and high value > - Chandan has written good regression tests > > I intend to post the rest of the individual selected patches > for review in small batches after they pass the tests, but w.r.t this > patch set - > > Does anyone object to including it in the stable kernel > after it passes the tests? > Hi Amir, The following three commits will have to be skipped from the series, 1. 02092a2f034fdeabab524ae39c2de86ba9ffa15a xfs: Check for extent overflow when renaming dir entries 2. 0dbc5cb1a91cc8c44b1c75429f5b9351837114fd xfs: Check for extent overflow when removing dir entries 3. f5d92749191402c50e32ac83dd9da3b910f5680f xfs: Check for extent overflow when adding dir entries The maximum size of a directory data fork is ~96GiB. This is much smaller than what can be accommodated by the existing data fork extent counter (i.e. 2^31 extents). Also the corresponding test (i.e. xfs/533) has been removed from fstests. Please refer to https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/commit/?id=9ae10c882550c48868e7c0baff889bb1a7c7c8e9 -- chandan
