Linux-XFS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH RESEND 0/2] Bail out if transaction can cause extent count to overflow
@ 2020-08-01  8:28 Chandan Babu R
  2020-08-01  8:28 ` [PATCH RESEND 1/2] xfs: Add helper for checking per-inode extent count overflow Chandan Babu R
  2020-08-01  8:28 ` [PATCH RESEND 2/2] xfs: Bail out if transaction can cause extent count to overflow Chandan Babu R
  0 siblings, 2 replies; 3+ messages in thread
From: Chandan Babu R @ 2020-08-01  8:28 UTC (permalink / raw)
  To: linux-xfs; +Cc: Chandan Babu R, darrick.wong, david

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  2035 [003]  9643.390490: probe:xfs_iflush_int: (ffffffffac6225c0) if_nextents=98511 inode=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_trans_resv_ext_cnt()) 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-v0.

PS: I am planning to write the code which extends data/xattr extent
counters from 32-bit/16-bit to 64-bit/32-bit on top of these patches.

 fs/xfs/libxfs/xfs_attr.c       | 33 ++++++++++--
 fs/xfs/libxfs/xfs_bmap.c       |  7 +++
 fs/xfs/libxfs/xfs_trans_resv.c | 33 ++++++++++++
 fs/xfs/libxfs/xfs_trans_resv.h |  1 +
 fs/xfs/xfs_bmap_item.c         | 12 +++++
 fs/xfs/xfs_bmap_util.c         | 40 ++++++++++++++
 fs/xfs/xfs_dquot.c             |  7 ++-
 fs/xfs/xfs_inode.c             | 96 ++++++++++++++++++++++++++++++++++
 fs/xfs/xfs_iomap.c             | 19 +++++++
 fs/xfs/xfs_reflink.c           | 35 +++++++++++++
 fs/xfs/xfs_rtalloc.c           |  4 ++
 fs/xfs/xfs_symlink.c           | 18 +++++++
 12 files changed, 301 insertions(+), 4 deletions(-)

-- 
2.27.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH RESEND 1/2] xfs: Add helper for checking per-inode extent count overflow
  2020-08-01  8:28 [PATCH RESEND 0/2] Bail out if transaction can cause extent count to overflow Chandan Babu R
@ 2020-08-01  8:28 ` Chandan Babu R
  2020-08-01  8:28 ` [PATCH RESEND 2/2] xfs: Bail out if transaction can cause extent count to overflow Chandan Babu R
  1 sibling, 0 replies; 3+ messages in thread
From: Chandan Babu R @ 2020-08-01  8:28 UTC (permalink / raw)
  To: linux-xfs; +Cc: Chandan Babu R, darrick.wong, david

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  2035 [003]  9643.390490: probe:xfs_iflush_int: (ffffffffac6225c0) if_nextents=98511 inode=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 attr extent counter,
   # xfs_db -f -c 'inode 131' -c 'print core.naextents' /dev/loop0
   core.naextents = -32561

This commit adds a new helper function (i.e. xfs_trans_resv_ext_cnt())
to check for overflow of the per-inode data and xattr extent counters. A
future patch will use this function to make sure that a transaction will
not cause the extent counter to overflow.

Suggested-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
---
 fs/xfs/libxfs/xfs_trans_resv.c | 33 +++++++++++++++++++++++++++++++++
 fs/xfs/libxfs/xfs_trans_resv.h |  1 +
 2 files changed, 34 insertions(+)

diff --git a/fs/xfs/libxfs/xfs_trans_resv.c b/fs/xfs/libxfs/xfs_trans_resv.c
index d1a0848cb52e..68d9833d403d 100644
--- a/fs/xfs/libxfs/xfs_trans_resv.c
+++ b/fs/xfs/libxfs/xfs_trans_resv.c
@@ -832,6 +832,39 @@ xfs_calc_sb_reservation(
 	return xfs_calc_buf_res(1, mp->m_sb.sb_sectsize);
 }
 
+int
+xfs_trans_resv_ext_cnt(
+	struct xfs_inode	*ip,
+	int			whichfork,
+	int			nr_to_add)
+{
+	struct xfs_ifork	*ifp;
+	uint64_t		max_exts = 0;
+	uint64_t		nr_exts;
+
+	switch (whichfork) {
+	case XFS_DATA_FORK:
+		max_exts = MAXEXTNUM;
+		break;
+
+	case XFS_ATTR_FORK:
+		max_exts = MAXAEXTNUM;
+		break;
+
+	default:
+		ASSERT(0);
+		break;
+	}
+
+	ifp = XFS_IFORK_PTR(ip, whichfork);
+	nr_exts = ifp->if_nextents + nr_to_add;
+
+	if (nr_exts > max_exts)
+		return -EFBIG;
+
+	return 0;
+}
+
 void
 xfs_trans_resv_calc(
 	struct xfs_mount	*mp,
diff --git a/fs/xfs/libxfs/xfs_trans_resv.h b/fs/xfs/libxfs/xfs_trans_resv.h
index 7241ab28cf84..8268a89caa16 100644
--- a/fs/xfs/libxfs/xfs_trans_resv.h
+++ b/fs/xfs/libxfs/xfs_trans_resv.h
@@ -93,5 +93,6 @@ struct xfs_trans_resv {
 
 void xfs_trans_resv_calc(struct xfs_mount *mp, struct xfs_trans_resv *resp);
 uint xfs_allocfree_log_count(struct xfs_mount *mp, uint num_ops);
+int xfs_trans_resv_ext_cnt(struct xfs_inode *ip, int whichfork, int nr_exts);
 
 #endif	/* __XFS_TRANS_RESV_H__ */
-- 
2.20.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH RESEND 2/2] xfs: Bail out if transaction can cause extent count to overflow
  2020-08-01  8:28 [PATCH RESEND 0/2] Bail out if transaction can cause extent count to overflow Chandan Babu R
  2020-08-01  8:28 ` [PATCH RESEND 1/2] xfs: Add helper for checking per-inode extent count overflow Chandan Babu R
@ 2020-08-01  8:28 ` Chandan Babu R
  1 sibling, 0 replies; 3+ messages in thread
From: Chandan Babu R @ 2020-08-01  8:28 UTC (permalink / raw)
  To: linux-xfs; +Cc: Chandan Babu R, darrick.wong, david

This commit computes the maximum number of new extents that can be added
to per-inode forks (data and xattr) at the beginning of a
transaction. It then uses the helper function xfs_trans_resv_ext_cnt()
to verify that the resulting sum does not overflow the corresponding
on-disk inode extent counter.

Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
---
 fs/xfs/libxfs/xfs_attr.c | 33 ++++++++++++--
 fs/xfs/libxfs/xfs_bmap.c |  7 +++
 fs/xfs/xfs_bmap_item.c   | 12 +++++
 fs/xfs/xfs_bmap_util.c   | 40 +++++++++++++++++
 fs/xfs/xfs_dquot.c       |  7 ++-
 fs/xfs/xfs_inode.c       | 96 ++++++++++++++++++++++++++++++++++++++++
 fs/xfs/xfs_iomap.c       | 19 ++++++++
 fs/xfs/xfs_reflink.c     | 35 +++++++++++++++
 fs/xfs/xfs_rtalloc.c     |  4 ++
 fs/xfs/xfs_symlink.c     | 18 ++++++++
 10 files changed, 267 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index d4583a0d1b3f..745bf9293a17 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -142,7 +142,8 @@ xfs_attr_get(
 STATIC int
 xfs_attr_calc_size(
 	struct xfs_da_args	*args,
-	int			*local)
+	int			*local,
+	int			*dsplit)
 {
 	struct xfs_mount	*mp = args->dp->i_mount;
 	int			size;
@@ -157,7 +158,10 @@ xfs_attr_calc_size(
 	if (*local) {
 		if (size > (args->geo->blksize / 2)) {
 			/* Double split possible */
+			*dsplit = 1;
 			nblks *= 2;
+		} else {
+			*dsplit = 0;
 		}
 	} else {
 		/*
@@ -395,7 +399,8 @@ xfs_attr_set(
 	struct xfs_mount	*mp = dp->i_mount;
 	struct xfs_trans_res	tres;
 	bool			rsvd = (args->attr_filter & XFS_ATTR_ROOT);
-	int			error, local;
+	int			error, local, dsplit;
+	int			rsv_exts = 0;
 	unsigned int		total;
 
 	if (XFS_FORCED_SHUTDOWN(dp->i_mount))
@@ -420,7 +425,7 @@ xfs_attr_set(
 		XFS_STATS_INC(mp, xs_attr_set);
 
 		args->op_flags |= XFS_DA_OP_ADDNAME;
-		args->total = xfs_attr_calc_size(args, &local);
+		args->total = xfs_attr_calc_size(args, &local, &dsplit);
 
 		/*
 		 * If the inode doesn't have an attribute fork, add one.
@@ -442,11 +447,19 @@ xfs_attr_set(
 		tres.tr_logcount = XFS_ATTRSET_LOG_COUNT;
 		tres.tr_logflags = XFS_TRANS_PERM_LOG_RES;
 		total = args->total;
+
+		if (local) {
+			if (dsplit)
+				++rsv_exts;
+		} else {
+			rsv_exts += xfs_attr3_rmt_blocks(mp, args->valuelen);
+		}
 	} else {
 		XFS_STATS_INC(mp, xs_attr_remove);
 
 		tres = M_RES(mp)->tr_attrrm;
 		total = XFS_ATTRRM_SPACE_RES(mp);
+		rsv_exts += xfs_attr3_rmt_blocks(mp, XFS_XATTR_SIZE_MAX);
 	}
 
 	/*
@@ -460,6 +473,20 @@ xfs_attr_set(
 
 	xfs_ilock(dp, XFS_ILOCK_EXCL);
 	xfs_trans_ijoin(args->trans, dp, 0);
+
+	rsv_exts += XFS_DAENTER_BLOCKS(mp, XFS_ATTR_FORK);
+
+	if (args->value || xfs_inode_hasattr(dp)) {
+		/*
+		 * XFS_DA_NODE_MAXDEPTH blocks for dabtree.
+		 * One extra block for dabtree in case of a double split.
+		 * Extents for remote attributes.
+		 */
+		error = xfs_trans_resv_ext_cnt(dp, XFS_ATTR_FORK, rsv_exts);
+		if (error)
+			goto out_trans_cancel;
+	}
+
 	if (args->value) {
 		unsigned int	quota_flags = XFS_QMOPT_RES_REGBLKS;
 
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 9c40d5971035..462869ab26b9 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -4527,6 +4527,13 @@ xfs_bmapi_convert_delalloc(
 		return error;
 
 	xfs_ilock(ip, XFS_ILOCK_EXCL);
+
+	if (whichfork == XFS_DATA_FORK) {
+		error = xfs_trans_resv_ext_cnt(ip, whichfork, 1);
+		if (error)
+			goto out_trans_cancel;
+	}
+
 	xfs_trans_ijoin(tp, ip, 0);
 
 	if (!xfs_iext_lookup_extent(ip, ifp, offset_fsb, &bma.icur, &bma.got) ||
diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
index ec3691372e7c..de427444cc8a 100644
--- a/fs/xfs/xfs_bmap_item.c
+++ b/fs/xfs/xfs_bmap_item.c
@@ -519,6 +519,18 @@ xfs_bui_item_recover(
 	}
 	xfs_trans_ijoin(tp, ip, 0);
 
+	/*
+	 * Removing an extent from the middle of an existing extent
+	 * can cause the extent count to increase by 1.
+	 * i.e. | Old extent | Hole | Old extent |
+	 *
+	 * Mapping a new extent into a file can cause the extent
+	 * count to increase by 1.
+	 */
+	error = xfs_trans_resv_ext_cnt(ip, whichfork, 1);
+	if (error)
+		goto err_inode;
+
 	count = bmap->me_len;
 	error = xfs_trans_log_finish_bmap_update(tp, budp, type, ip, whichfork,
 			bmap->me_startoff, bmap->me_startblock, &count, state);
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index afdc7f8e0e70..a8cd0f7c6005 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -822,6 +822,10 @@ xfs_alloc_file_space(
 		if (error)
 			goto error1;
 
+		error = xfs_trans_resv_ext_cnt(ip, XFS_DATA_FORK, 1);
+		if (error)
+			goto error0;
+
 		xfs_trans_ijoin(tp, ip, 0);
 
 		error = xfs_bmapi_write(tp, ip, startoffset_fsb,
@@ -886,6 +890,15 @@ xfs_unmap_extent(
 
 	xfs_trans_ijoin(tp, ip, 0);
 
+	/*
+	 * One extent encompasses the complete file offset range.
+	 * Removing the file offset range causes extent count to
+	 * increase by 1.
+	 */
+	error = xfs_trans_resv_ext_cnt(ip, XFS_DATA_FORK, 1);
+	if (error)
+		goto out_trans_cancel;
+
 	error = xfs_bunmapi(tp, ip, startoffset_fsb, len_fsb, 0, 2, done);
 	if (error)
 		goto out_trans_cancel;
@@ -1155,6 +1168,14 @@ xfs_insert_file_space(
 	xfs_ilock(ip, XFS_ILOCK_EXCL);
 	xfs_trans_ijoin(tp, ip, 0);
 
+	/*
+	 * Splitting the extent mapping containing stop_fsb will cause
+	 * extent count to increase by 1.
+	 */
+	error = xfs_trans_resv_ext_cnt(ip, XFS_DATA_FORK, 1);
+	if (error)
+		goto out_trans_cancel;
+
 	/*
 	 * The extent shifting code works on extent granularity. So, if stop_fsb
 	 * is not the starting block of extent, we need to split the extent at
@@ -1356,6 +1377,25 @@ xfs_swap_extent_rmap(
 		/* Unmap the old blocks in the source file. */
 		while (tirec.br_blockcount) {
 			ASSERT(tp->t_firstblock == NULLFSBLOCK);
+
+			/*
+			 * Removing an initial part of source file's extent and
+			 * adding a new extent (from donor file) in its place
+			 * will cause extent count to increase by 1.
+			 */
+			error = xfs_trans_resv_ext_cnt(ip, XFS_DATA_FORK, 1);
+			if (error)
+				goto out;
+
+			/*
+			 * Removing an initial part of donor file's extent and
+			 * adding a new extent (from source file) in its place
+			 * will cause extent count to increase by 1.
+			 */
+			error = xfs_trans_resv_ext_cnt(tip, XFS_DATA_FORK, 1);
+			if (error)
+				goto out;
+
 			trace_xfs_swap_extent_rmap_remap_piece(tip, &tirec);
 
 			/* Read extent from the source file */
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 04dc2be19c3a..582f050595bc 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -290,8 +290,13 @@ xfs_dquot_disk_alloc(
 		return -ESRCH;
 	}
 
-	/* Create the block mapping. */
 	xfs_trans_ijoin(tp, quotip, XFS_ILOCK_EXCL);
+
+	error = xfs_trans_resv_ext_cnt(quotip, XFS_DATA_FORK, 1);
+	if (error)
+		return error;
+
+	/* Create the block mapping. */
 	error = xfs_bmapi_write(tp, quotip, dqp->q_fileoffset,
 			XFS_DQUOT_CLUSTER_SIZE_FSB, XFS_BMAPI_METADATA, 0, &map,
 			&nmaps);
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 407d6299606d..007a719c2cdf 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1175,6 +1175,24 @@ xfs_create(
 	if (error)
 		goto out_trans_cancel;
 
+	/*
+	 * Directory entry addition can cause the following,
+	 * 1. Data block can be added.
+	 *    A new extent can cause extent count to increase by 1.
+	 * 2. Free disk block can be added.
+	 *    Same behaviour as described above for Data block.
+	 * 3. Dabtree blocks.
+	 *    XFS_DA_NODE_MAXDEPTH blocks can be added. Each of these
+	 *    can be new extents. Hence extent count can increase by
+	 *    XFS_DA_NODE_MAXDEPTH.
+	 * Total = XFS_DA_NODE_MAXDEPTH + 1 + 1;
+	 */
+	error = xfs_trans_resv_ext_cnt(dp, XFS_DATA_FORK,
+			(XFS_DA_NODE_MAXDEPTH + 1 + 1) *
+			mp->m_dir_geo->fsbcount);
+	if (error)
+		goto out_trans_cancel;
+
 	/*
 	 * A newly created regular or special file just has one directory
 	 * entry pointing to them, but a directory also the "." entry
@@ -1391,6 +1409,24 @@ xfs_link(
 	xfs_trans_ijoin(tp, sip, XFS_ILOCK_EXCL);
 	xfs_trans_ijoin(tp, tdp, XFS_ILOCK_EXCL);
 
+	/*
+	 * Creating a new link can cause the following,
+	 * 1. Data block can be added.
+	 *    A new extent can cause extent count to increase by 1.
+	 * 2. Free disk block can be added.
+	 *    Same behaviour as described above for Data block.
+	 * 3. Dabtree blocks.
+	 *    XFS_DA_NODE_MAXDEPTH blocks can be added. Each of these
+	 *    can be new extents. Hence extent count can increase by
+	 *    XFS_DA_NODE_MAXDEPTH.
+	 * Total = XFS_DA_NODE_MAXDEPTH + 1 + 1;
+	 */
+	error = xfs_trans_resv_ext_cnt(tdp, XFS_DATA_FORK,
+			(XFS_DA_NODE_MAXDEPTH + 1 + 1) *
+				mp->m_dir_geo->fsbcount);
+	if (error)
+		goto error_return;
+
 	/*
 	 * If we are using project inheritance, we only allow hard link
 	 * creation in our tree when the project IDs are the same; else
@@ -2861,6 +2897,26 @@ xfs_remove(
 	xfs_trans_ijoin(tp, dp, XFS_ILOCK_EXCL);
 	xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
 
+	/*
+	 * Directory entry removal can cause the following,
+	 * 1. Data block can be freed.
+	 *    3 data blocks can be contiguous. Deletion of a single
+	 *    data block can cause this single extent to be split into
+	 *    two. Hence extent count can increase by 1.
+	 * 2. Free disk block
+	 *    Same behaviour as described above for Data block.
+	 * 3. Dabtree blocks.
+	 *    XFS_DA_NODE_MAXDEPTH blocks can be freed. Each of these
+	 *    blocks can cause a single extent to be split into two.
+	 *    Hence extent count can increase by XFS_DA_NODE_MAXDEPTH.
+	 * Total = XFS_DA_NODE_MAXDEPTH + 1 + 1;
+	 */
+	error = xfs_trans_resv_ext_cnt(dp, XFS_DATA_FORK,
+			(XFS_DA_NODE_MAXDEPTH + 1 + 1) *
+				mp->m_dir_geo->fsbcount);
+	if (error)
+		goto out_trans_cancel;
+
 	/*
 	 * If we're removing a directory perform some additional validation.
 	 */
@@ -3221,6 +3277,46 @@ xfs_rename(
 	if (wip)
 		xfs_trans_ijoin(tp, wip, XFS_ILOCK_EXCL);
 
+	/*
+	 * Directory entry removal can cause the following,
+	 * 1. Data block can be freed.
+	 *    3 data blocks can be contiguous. Deletion of a single
+	 *    data block can cause this single extent to be split into
+	 *    two. Hence extent count can increase by 1.
+	 * 2. Free disk block
+	 *    Same behaviour as described above for Data block.
+	 * 3. Dabtree blocks.
+	 *    XFS_DA_NODE_MAXDEPTH blocks can be freed. Each of these
+	 *    blocks can cause a single extent to be split into two.
+	 *    Hence extent count can increase by XFS_DA_NODE_MAXDEPTH.
+	 * Total = XFS_DA_NODE_MAXDEPTH + 1 + 1;
+	 */
+	error = xfs_trans_resv_ext_cnt(src_dp, XFS_DATA_FORK,
+			(XFS_DA_NODE_MAXDEPTH + 1 + 1) *
+				mp->m_dir_geo->fsbcount);
+	if (error)
+		goto out_trans_cancel;
+
+	/*
+	 * Directory entry addition can cause the following,
+	 * 1. Data block can be added.
+	 *    A new extent can cause extent count to increase by 1.
+	 * 2. Free disk block can be added.
+	 *    Same behaviour as described above for Data block.
+	 * 3. Dabtree blocks.
+	 *    XFS_DA_NODE_MAXDEPTH blocks can be added. Each of these
+	 *    can be new extents. Hence extent count can increase by
+	 *    XFS_DA_NODE_MAXDEPTH.
+	 * Total = XFS_DA_NODE_MAXDEPTH + 1 + 1;
+	 */
+	if (target_ip == NULL) {
+		error = xfs_trans_resv_ext_cnt(target_dp, XFS_DATA_FORK,
+			(XFS_DA_NODE_MAXDEPTH + 1 + 1) *
+				mp->m_dir_geo->fsbcount);
+		if (error)
+			goto out_trans_cancel;
+	}
+
 	/*
 	 * If we are using project inheritance, we only allow renames
 	 * into our tree when the project IDs are the same; else the
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 0e3f62cde375..e7ea3b7bbb0d 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -250,6 +250,14 @@ xfs_iomap_write_direct(
 	if (error)
 		goto out_trans_cancel;
 
+	/*
+	 * Writing to a hole or extending a file can cause
+	 * the extent count to increase by 1.
+	 */
+	error = xfs_trans_resv_ext_cnt(ip, XFS_DATA_FORK, 1);
+	if (error)
+		goto out_trans_cancel;
+
 	xfs_trans_ijoin(tp, ip, 0);
 
 	/*
@@ -561,6 +569,17 @@ xfs_iomap_write_unwritten(
 		if (error)
 			goto error_on_bmapi_transaction;
 
+		/*
+		 * We might be writing to the middle region of an
+		 * existing unwritten extent. This causes the original
+		 * extent to be split into 3 extents
+		 * i.e. | Unwritten | Real | Unwritten |
+		 * Hence extent count can increase by 2.
+		 */
+		error = xfs_trans_resv_ext_cnt(ip, XFS_DATA_FORK, 2);
+		if (error)
+			goto error_on_bmapi_transaction;
+
 		/*
 		 * Modify the unwritten extent state of the buffer.
 		 */
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index aac83f9d6107..45fa89558cdb 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -29,6 +29,7 @@
 #include "xfs_iomap.h"
 #include "xfs_sb.h"
 #include "xfs_ag_resv.h"
+#include "xfs_trans_resv.h"
 
 /*
  * Copy on Write of Shared Blocks
@@ -628,6 +629,17 @@ xfs_reflink_end_cow_extent(
 	xfs_ilock(ip, XFS_ILOCK_EXCL);
 	xfs_trans_ijoin(tp, ip, 0);
 
+	/*
+	 * Extents are unmapped starting from "end_fsb - 1" and moves
+	 * towards offset_fsb.  A data fork extent containing
+	 * "end_fsb - 1" can be split into three parts i.e.
+	 * | Old extent | New extent | Old extent |
+	 * Hence number of extents increases by 2.
+	 */
+	error = xfs_trans_resv_ext_cnt(ip, XFS_DATA_FORK, 2);
+	if (error)
+		goto out_cancel;
+
 	/*
 	 * In case of racing, overlapping AIO writes no COW extents might be
 	 * left by the time I/O completes for the loser of the race.  In that
@@ -1002,6 +1014,7 @@ xfs_reflink_remap_extent(
 	bool			smap_real;
 	bool			dmap_written = xfs_bmap_is_written_extent(dmap);
 	int			nimaps;
+	int			resv_exts = 0;
 	int			error;
 
 	/* Start a rolling transaction to switch the mappings */
@@ -1094,6 +1107,28 @@ xfs_reflink_remap_extent(
 			goto out_cancel;
 	}
 
+	/*
+	 * When unmapping, an extent containing the entire unmap
+	 * range can be split into two extents,
+	 * i.e. | old extent | hole | old extent |
+	 * Hence extent count increases by 1.
+	 */
+	if (smap_real)
+		++resv_exts;
+
+	/*
+	 * Mapping in the new extent into the destination file can
+	 * increase the extent count by 1.
+	 */
+	if (dmap_written)
+		++resv_exts;
+
+	if (resv_exts) {
+		error = xfs_trans_resv_ext_cnt(ip, XFS_DATA_FORK, resv_exts);
+		if (error)
+			goto out_cancel;
+	}
+
 	if (smap_real) {
 		/*
 		 * If the extent we're unmapping is backed by storage (written
diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
index 6209e7b6b895..a2e640f43f1e 100644
--- a/fs/xfs/xfs_rtalloc.c
+++ b/fs/xfs/xfs_rtalloc.c
@@ -787,6 +787,10 @@ xfs_growfs_rt_alloc(
 		xfs_ilock(ip, XFS_ILOCK_EXCL);
 		xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
 
+		error = xfs_trans_resv_ext_cnt(ip, XFS_DATA_FORK, 1);
+		if (error)
+			goto out_trans_cancel;
+
 		/*
 		 * Allocate blocks to the bitmap file.
 		 */
diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
index 8e88a7ca387e..1bc71576289d 100644
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -220,6 +220,24 @@ xfs_symlink(
 	if (error)
 		goto out_trans_cancel;
 
+	/*
+	 * Creating a new symlink can cause the following,
+	 * 1. Data block can be added.
+	 *    A new extent can cause extent count to increase by 1.
+	 * 2. Free disk block can be added.
+	 *    Same behaviour as described above for Data block.
+	 * 3. Dabtree blocks.
+	 *    XFS_DA_NODE_MAXDEPTH blocks can be added. Each of these
+	 *    can be new extents. Hence extent count can increase by
+	 *    XFS_DA_NODE_MAXDEPTH.
+	 * Total = XFS_DA_NODE_MAXDEPTH + 1 + 1;
+	 */
+	error = xfs_trans_resv_ext_cnt(dp, XFS_DATA_FORK,
+			(XFS_DA_NODE_MAXDEPTH + 1 + 1) *
+				mp->m_dir_geo->fsbcount);
+	if (error)
+		goto out_trans_cancel;
+
 	/*
 	 * Allocate an inode for the symlink.
 	 */
-- 
2.20.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-01  8:28 [PATCH RESEND 0/2] Bail out if transaction can cause extent count to overflow Chandan Babu R
2020-08-01  8:28 ` [PATCH RESEND 1/2] xfs: Add helper for checking per-inode extent count overflow Chandan Babu R
2020-08-01  8:28 ` [PATCH RESEND 2/2] xfs: Bail out if transaction can cause extent count to overflow Chandan Babu R

Linux-XFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-xfs/0 linux-xfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-xfs linux-xfs/ https://lore.kernel.org/linux-xfs \
		linux-xfs@vger.kernel.org
	public-inbox-index linux-xfs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-xfs


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git