All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chandan Babu R <chandanrlinux@gmail.com>
To: linux-xfs@vger.kernel.org
Cc: Chandan Babu R <chandanrlinux@gmail.com>,
	darrick.wong@oracle.com, david@fromorbit.com, hch@infradead.org
Subject: [PATCH V3 00/10] Bail out if transaction can cause extent count to overflow
Date: Thu, 20 Aug 2020 11:13:39 +0530	[thread overview]
Message-ID: <20200820054349.5525-1-chandanrlinux@gmail.com> (raw)

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-v3.

Changelog:
V2 -> V3:
  1. Move the definition of xfs_iext_count_may_overflow() from
     libxfs/xfs_trans_resv.c to libxfs/xfs_inode_fork.c. Also, I tried
     to make xfs_iext_count_may_overflow() an inline function by
     placing the definition in libxfs/xfs_inode_fork.h. However this
     required that the definition of 'struct xfs_inode' be available,
     since xfs_iext_count_may_overflow() uses a 'struct xfs_inode *'
     type variable.
  2. Handle XFS_COW_FORK within xfs_iext_count_may_overflow() by
     returning a success value.
  3. Rename XFS_IEXT_ADD_CNT to XFS_IEXT_ADD_NOSPLIT_CNT. Thanks to
     Darrick for the suggesting the new name.
  4. Expand comments to make use of 80 columns.

V1 -> V2:
  1. Rename helper function from xfs_trans_resv_ext_cnt() to
     xfs_iext_count_may_overflow().
  2. Define and use macros to represent fs operations and the
     corresponding increase in extent count.
  3. Split the patches based on the fs operation being performed.

Chandan Babu R (10):
  xfs: Add helper for checking per-inode extent count overflow
  xfs: Check for extent overflow when trivally adding a new extent
  xfs: Check for extent overflow when deleting an extent
  xfs: Check for extent overflow when adding/removing xattrs
  xfs: Check for extent overflow when adding/removing dir entries
  xfs: Check for extent overflow when writing to unwritten extent
  xfs: Check for extent overflow when inserting a hole
  xfs: Check for extent overflow when moving extent from cow to data
    fork
  xfs: Check for extent overflow when remapping an extent
  xfs: Check for extent overflow when swapping extents

 fs/xfs/libxfs/xfs_attr.c       | 13 ++++++
 fs/xfs/libxfs/xfs_bmap.c       |  6 +++
 fs/xfs/libxfs/xfs_inode_fork.c | 23 +++++++++++
 fs/xfs/libxfs/xfs_inode_fork.h | 75 ++++++++++++++++++++++++++++++++++
 fs/xfs/xfs_bmap_item.c         |  4 ++
 fs/xfs/xfs_bmap_util.c         | 30 ++++++++++++++
 fs/xfs/xfs_dquot.c             |  8 +++-
 fs/xfs/xfs_inode.c             | 27 ++++++++++++
 fs/xfs/xfs_iomap.c             | 10 +++++
 fs/xfs/xfs_reflink.c           | 10 +++++
 fs/xfs/xfs_rtalloc.c           |  5 +++
 fs/xfs/xfs_symlink.c           |  5 +++
 12 files changed, 215 insertions(+), 1 deletion(-)

-- 
2.28.0


             reply	other threads:[~2020-08-20  5:44 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-20  5:43 Chandan Babu R [this message]
2020-08-20  5:43 ` [PATCH V3 01/10] xfs: Add helper for checking per-inode extent count overflow Chandan Babu R
2020-08-31 16:08   ` Darrick J. Wong
2020-08-31 16:44     ` Darrick J. Wong
2020-09-01  9:44       ` Chandan Babu R
2020-08-20  5:43 ` [PATCH V3 02/10] xfs: Check for extent overflow when trivally adding a new extent Chandan Babu R
2020-08-31 16:12   ` Darrick J. Wong
2020-08-20  5:43 ` [PATCH V3 03/10] xfs: Check for extent overflow when deleting an extent Chandan Babu R
2020-08-31 16:34   ` Darrick J. Wong
2020-09-01  9:44     ` Chandan Babu R
2020-08-20  5:43 ` [PATCH V3 04/10] xfs: Check for extent overflow when adding/removing xattrs Chandan Babu R
2020-08-31 16:37   ` Darrick J. Wong
2020-09-01  9:44     ` Chandan Babu R
2020-08-20  5:43 ` [PATCH V3 05/10] xfs: Check for extent overflow when adding/removing dir entries Chandan Babu R
2020-08-31 16:41   ` Darrick J. Wong
2020-08-20  5:43 ` [PATCH V3 06/10] xfs: Check for extent overflow when writing to unwritten extent Chandan Babu R
2020-08-31 16:45   ` Darrick J. Wong
2020-08-20  5:43 ` [PATCH V3 07/10] xfs: Check for extent overflow when inserting a hole Chandan Babu R
2020-08-31 16:46   ` Darrick J. Wong
2020-09-01  9:44     ` Chandan Babu R
2020-08-20  5:43 ` [PATCH V3 08/10] xfs: Check for extent overflow when moving extent from cow to data fork Chandan Babu R
2020-08-31 16:29   ` Darrick J. Wong
2020-09-01  9:45     ` Chandan Babu R
2020-08-20  5:43 ` [PATCH V3 09/10] xfs: Check for extent overflow when remapping an extent Chandan Babu R
2020-08-31 16:23   ` Darrick J. Wong
2020-09-01  9:45     ` Chandan Babu R
2020-08-20  5:43 ` [PATCH V3 10/10] xfs: Check for extent overflow when swapping extents Chandan Babu R
2020-08-31 16:20   ` Darrick J. Wong
2020-09-01  9:45     ` Chandan Babu R

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200820054349.5525-1-chandanrlinux@gmail.com \
    --to=chandanrlinux@gmail.com \
    --cc=darrick.wong@oracle.com \
    --cc=david@fromorbit.com \
    --cc=hch@infradead.org \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.