From 83a21c18441f75aec64548692b52d34582b98a6a Mon Sep 17 00:00:00 2001 From: Chandan Babu R Date: Tue, 29 Mar 2022 06:14:00 +0000 Subject: [PATCH] xfs: Directory's data fork extent counter can never overflow The maximum file size that can be represented by the data fork extent counter in the worst case occurs when all extents are 1 block in length and each block is 1KB in size. With XFS_MAX_EXTCNT_DATA_FORK_SMALL representing maximum extent count and with 1KB sized blocks, a file can reach upto, (2^31) * 1KB = 2TB This is much larger than the theoretical maximum size of a directory i.e. XFS_DIR2_SPACE_SIZE * 3 = ~96GB. Since a directory's inode can never overflow its data fork extent counter, this commit removes all the overflow checks associated with it. xfs_dinode_verify() now performs a rough check to verify if a diretory's data fork is larger than 96GB. Reviewed-by: Dave Chinner Reviewed-by: Darrick J. Wong Signed-off-by: Chandan Babu R --- fs/xfs/libxfs/xfs_bmap.c | 20 --------------- fs/xfs/libxfs/xfs_da_btree.h | 1 + fs/xfs/libxfs/xfs_da_format.h | 1 + fs/xfs/libxfs/xfs_dir2.c | 8 ++++++ fs/xfs/libxfs/xfs_format.h | 13 ++++++++++ fs/xfs/libxfs/xfs_inode_buf.c | 3 +++ fs/xfs/libxfs/xfs_inode_fork.h | 13 ---------- fs/xfs/xfs_inode.c | 55 ++---------------------------------------- fs/xfs/xfs_symlink.c | 5 ---- 9 files changed, 28 insertions(+), 91 deletions(-) diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c index 1254d4d4821e..4fab0c92ab70 100644 --- a/fs/xfs/libxfs/xfs_bmap.c +++ b/fs/xfs/libxfs/xfs_bmap.c @@ -5147,26 +5147,6 @@ xfs_bmap_del_extent_real( * Deleting the middle of the extent. */ - /* - * For directories, -ENOSPC is returned since a directory entry - * remove operation must not fail due to low extent count - * availability. -ENOSPC will be handled by higher layers of XFS - * by letting the corresponding empty Data/Free blocks to linger - * until a future remove operation. Dabtree blocks would be - * swapped with the last block in the leaf space and then the - * new last block will be unmapped. - * - * The above logic also applies to the source directory entry of - * a rename operation. - */ - error = xfs_iext_count_may_overflow(ip, whichfork, 1); - if (error) { - ASSERT(S_ISDIR(VFS_I(ip)->i_mode) && - whichfork == XFS_DATA_FORK); - error = -ENOSPC; - goto done; - } - old = got; got.br_blockcount = del->br_startoff - got.br_startoff; diff --git a/fs/xfs/libxfs/xfs_da_btree.h b/fs/xfs/libxfs/xfs_da_btree.h index 0faf7d9ac241..7f08f6de48bf 100644 --- a/fs/xfs/libxfs/xfs_da_btree.h +++ b/fs/xfs/libxfs/xfs_da_btree.h @@ -30,6 +30,7 @@ struct xfs_da_geometry { unsigned int free_hdr_size; /* dir2 free header size */ unsigned int free_max_bests; /* # of bests entries in dir2 free */ xfs_dablk_t freeblk; /* blockno of free data v2 */ + xfs_extnum_t max_extents; /* Max. extents in corresponding fork */ xfs_dir2_data_aoff_t data_first_offset; size_t data_entry_offset; diff --git a/fs/xfs/libxfs/xfs_da_format.h b/fs/xfs/libxfs/xfs_da_format.h index 5a49caa5c9df..95354b7ab7f5 100644 --- a/fs/xfs/libxfs/xfs_da_format.h +++ b/fs/xfs/libxfs/xfs_da_format.h @@ -277,6 +277,7 @@ xfs_dir2_sf_firstentry(struct xfs_dir2_sf_hdr *hdr) * Directory address space divided into sections, * spaces separated by 32GB. */ +#define XFS_DIR2_MAX_SPACES 3 #define XFS_DIR2_SPACE_SIZE (1ULL << (32 + XFS_DIR2_DATA_ALIGN_LOG)) #define XFS_DIR2_DATA_SPACE 0 #define XFS_DIR2_DATA_OFFSET (XFS_DIR2_DATA_SPACE * XFS_DIR2_SPACE_SIZE) diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c index 5f1e4799e8fa..3cd51fa3837b 100644 --- a/fs/xfs/libxfs/xfs_dir2.c +++ b/fs/xfs/libxfs/xfs_dir2.c @@ -150,6 +150,8 @@ xfs_da_mount( dageo->freeblk = xfs_dir2_byte_to_da(dageo, XFS_DIR2_FREE_OFFSET); dageo->node_ents = (dageo->blksize - dageo->node_hdr_size) / (uint)sizeof(xfs_da_node_entry_t); + dageo->max_extents = (XFS_DIR2_MAX_SPACES * XFS_DIR2_SPACE_SIZE) >> + mp->m_sb.sb_blocklog; dageo->magicpct = (dageo->blksize * 37) / 100; /* set up attribute geometry - single fsb only */ @@ -161,6 +163,12 @@ xfs_da_mount( dageo->node_hdr_size = mp->m_dir_geo->node_hdr_size; dageo->node_ents = (dageo->blksize - dageo->node_hdr_size) / (uint)sizeof(xfs_da_node_entry_t); + + if (xfs_has_large_extent_counts(mp)) + dageo->max_extents = XFS_MAX_EXTCNT_ATTR_FORK_LARGE; + else + dageo->max_extents = XFS_MAX_EXTCNT_ATTR_FORK_SMALL; + dageo->magicpct = (dageo->blksize * 37) / 100; return 0; } diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h index 82b404c99b80..43de892d0305 100644 --- a/fs/xfs/libxfs/xfs_format.h +++ b/fs/xfs/libxfs/xfs_format.h @@ -915,6 +915,19 @@ enum xfs_dinode_fmt { * * Rounding up 47 to the nearest multiple of bits-per-byte results in 48. Hence * 2^48 was chosen as the maximum data fork extent count. + * + * The maximum file size that can be represented by the data fork extent counter + * in the worst case occurs when all extents are 1 block in length and each + * block is 1KB in size. + * + * With XFS_MAX_EXTCNT_DATA_FORK_SMALL representing maximum extent count and + * with 1KB sized blocks, a file can reach upto, + * 1KB * (2^31) = 2TB + * + * This is much larger than the theoretical maximum size of a directory + * i.e. XFS_DIR2_SPACE_SIZE * XFS_DIR2_MAX_SPACES = ~96GB. + * + * Hence, a directory inode can never overflow its data fork extent counter. */ #define XFS_MAX_EXTCNT_DATA_FORK_LARGE ((xfs_extnum_t)((1ULL << 48) - 1)) #define XFS_MAX_EXTCNT_ATTR_FORK_LARGE ((xfs_extnum_t)((1ULL << 32) - 1)) diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c index ee8d4eb7d048..74b82ec80f8e 100644 --- a/fs/xfs/libxfs/xfs_inode_buf.c +++ b/fs/xfs/libxfs/xfs_inode_buf.c @@ -491,6 +491,9 @@ xfs_dinode_verify( if (mode && nextents + naextents > nblocks) return __this_address; + if (S_ISDIR(mode) && nextents > mp->m_dir_geo->max_extents) + return __this_address; + if (mode && XFS_DFORK_BOFF(dip) > mp->m_sb.sb_inodesize) return __this_address; diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h index fd5c3c2d77e0..6f9d69f8896e 100644 --- a/fs/xfs/libxfs/xfs_inode_fork.h +++ b/fs/xfs/libxfs/xfs_inode_fork.h @@ -40,19 +40,6 @@ struct xfs_ifork { #define XFS_IEXT_PUNCH_HOLE_CNT (1) /* - * Directory entry addition can cause the following, - * 1. Data block can be added/removed. - * A new extent can cause extent count to increase by 1. - * 2. Free disk block can be added/removed. - * 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. - */ -#define XFS_IEXT_DIR_MANIP_CNT(mp) \ - ((XFS_DA_NODE_MAXDEPTH + 1 + 1) * (mp)->m_dir_geo->fsbcount) - -/* * Adding/removing an xattr can cause XFS_DA_NODE_MAXDEPTH extents to * be added. One extra extent for dabtree in case a local attr is * large enough to cause a double split. It can also cause extent diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c index adc1355ce853..20f15a0393e1 100644 --- a/fs/xfs/xfs_inode.c +++ b/fs/xfs/xfs_inode.c @@ -1024,11 +1024,6 @@ xfs_create( xfs_ilock(dp, XFS_ILOCK_EXCL | XFS_ILOCK_PARENT); unlock_dp_on_error = true; - error = xfs_iext_count_may_overflow(dp, XFS_DATA_FORK, - XFS_IEXT_DIR_MANIP_CNT(mp)); - 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 @@ -1242,11 +1237,6 @@ xfs_link( if (error) goto std_return; - error = xfs_iext_count_may_overflow(tdp, XFS_DATA_FORK, - XFS_IEXT_DIR_MANIP_CNT(mp)); - 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 @@ -3210,35 +3200,6 @@ retry: /* * Check for expected errors before we dirty the transaction * so we can return an error without a transaction abort. - * - * Extent count overflow check: - * - * From the perspective of src_dp, a rename operation is essentially a - * directory entry remove operation. Hence the only place where we check - * for extent count overflow for src_dp is in - * xfs_bmap_del_extent_real(). xfs_bmap_del_extent_real() returns - * -ENOSPC when it detects a possible extent count overflow and in - * response, the higher layers of directory handling code do the - * following: - * 1. Data/Free blocks: XFS lets these blocks linger until a - * future remove operation removes them. - * 2. Dabtree blocks: XFS swaps the blocks with the last block in the - * Leaf space and unmaps the last block. - * - * For target_dp, there are two cases depending on whether the - * destination directory entry exists or not. - * - * When destination directory entry does not exist (i.e. target_ip == - * NULL), extent count overflow check is performed only when transaction - * has a non-zero sized space reservation associated with it. With a - * zero-sized space reservation, XFS allows a rename operation to - * continue only when the directory has sufficient free space in its - * data/leaf/free space blocks to hold the new entry. - * - * When destination directory entry exists (i.e. target_ip != NULL), all - * we need to do is change the inode number associated with the already - * existing entry. Hence there is no need to perform an extent count - * overflow check. */ if (target_ip == NULL) { /* @@ -3249,12 +3210,6 @@ retry: error = xfs_dir_canenter(tp, target_dp, target_name); if (error) goto out_trans_cancel; - } else { - error = xfs_iext_count_may_overflow(target_dp, - XFS_DATA_FORK, - XFS_IEXT_DIR_MANIP_CNT(mp)); - if (error) - goto out_trans_cancel; } } else { /* @@ -3422,18 +3377,12 @@ retry: * inode number of the whiteout inode rather than removing it * altogether. */ - if (wip) { + if (wip) error = xfs_dir_replace(tp, src_dp, src_name, wip->i_ino, spaceres); - } else { - /* - * NOTE: We don't need to check for extent count overflow here - * because the dir remove name code will leave the dir block in - * place if the extent count would overflow. - */ + else error = xfs_dir_removename(tp, src_dp, src_name, src_ip->i_ino, spaceres); - } if (error) goto out_trans_cancel; diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c index affbedf78160..4145ba872547 100644 --- a/fs/xfs/xfs_symlink.c +++ b/fs/xfs/xfs_symlink.c @@ -226,11 +226,6 @@ xfs_symlink( goto out_trans_cancel; } - error = xfs_iext_count_may_overflow(dp, XFS_DATA_FORK, - XFS_IEXT_DIR_MANIP_CNT(mp)); - if (error) - goto out_trans_cancel; - /* * Allocate an inode for the symlink. */ -- 2.11.0