On Sun, Nov 03, 2019 at 02:23:49PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> Each of the four functions that operate on shortform directories checks
> that the directory's di_size is at least as large as the shortform
> directory header. This is now checked by the inode fork verifiers
> (di_size is used to allocate if_bytes, and if_bytes is checked against
> the header structure size) so we can turn these checks into ASSERTions.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
Looks good:
Reviewed-by: Carlos Maiolino <cmaiolino@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_dir2_block.c | 8 +-------
> fs/xfs/libxfs/xfs_dir2_sf.c | 32 ++++----------------------------
> 2 files changed, 5 insertions(+), 35 deletions(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_dir2_block.c b/fs/xfs/libxfs/xfs_dir2_block.c
> index 49e4bc39e7bb..e1afa35141c5 100644
> --- a/fs/xfs/libxfs/xfs_dir2_block.c
> +++ b/fs/xfs/libxfs/xfs_dir2_block.c
> @@ -1073,13 +1073,7 @@ xfs_dir2_sf_to_block(
> mp = dp->i_mount;
> ifp = XFS_IFORK_PTR(dp, XFS_DATA_FORK);
> ASSERT(ifp->if_flags & XFS_IFINLINE);
> - /*
> - * Bomb out if the shortform directory is way too short.
> - */
> - if (dp->i_d.di_size < offsetof(xfs_dir2_sf_hdr_t, parent)) {
> - ASSERT(XFS_FORCED_SHUTDOWN(mp));
> - return -EIO;
> - }
> + ASSERT(dp->i_d.di_size >= offsetof(struct xfs_dir2_sf_hdr, parent));
>
> oldsfp = (xfs_dir2_sf_hdr_t *)ifp->if_u1.if_data;
>
> diff --git a/fs/xfs/libxfs/xfs_dir2_sf.c b/fs/xfs/libxfs/xfs_dir2_sf.c
> index ae16ca7c422a..d6b164a2fe57 100644
> --- a/fs/xfs/libxfs/xfs_dir2_sf.c
> +++ b/fs/xfs/libxfs/xfs_dir2_sf.c
> @@ -277,13 +277,7 @@ xfs_dir2_sf_addname(
> ASSERT(xfs_dir2_sf_lookup(args) == -ENOENT);
> dp = args->dp;
> ASSERT(dp->i_df.if_flags & XFS_IFINLINE);
> - /*
> - * Make sure the shortform value has some of its header.
> - */
> - if (dp->i_d.di_size < offsetof(xfs_dir2_sf_hdr_t, parent)) {
> - ASSERT(XFS_FORCED_SHUTDOWN(dp->i_mount));
> - return -EIO;
> - }
> + ASSERT(dp->i_d.di_size >= offsetof(struct xfs_dir2_sf_hdr, parent));
> ASSERT(dp->i_df.if_bytes == dp->i_d.di_size);
> ASSERT(dp->i_df.if_u1.if_data != NULL);
> sfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
> @@ -793,13 +787,7 @@ xfs_dir2_sf_lookup(
> dp = args->dp;
>
> ASSERT(dp->i_df.if_flags & XFS_IFINLINE);
> - /*
> - * Bail out if the directory is way too short.
> - */
> - if (dp->i_d.di_size < offsetof(xfs_dir2_sf_hdr_t, parent)) {
> - ASSERT(XFS_FORCED_SHUTDOWN(dp->i_mount));
> - return -EIO;
> - }
> + ASSERT(dp->i_d.di_size >= offsetof(struct xfs_dir2_sf_hdr, parent));
> ASSERT(dp->i_df.if_bytes == dp->i_d.di_size);
> ASSERT(dp->i_df.if_u1.if_data != NULL);
> sfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
> @@ -879,13 +867,7 @@ xfs_dir2_sf_removename(
>
> ASSERT(dp->i_df.if_flags & XFS_IFINLINE);
> oldsize = (int)dp->i_d.di_size;
> - /*
> - * Bail out if the directory is way too short.
> - */
> - if (oldsize < offsetof(xfs_dir2_sf_hdr_t, parent)) {
> - ASSERT(XFS_FORCED_SHUTDOWN(dp->i_mount));
> - return -EIO;
> - }
> + ASSERT(oldsize >= offsetof(struct xfs_dir2_sf_hdr, parent));
> ASSERT(dp->i_df.if_bytes == oldsize);
> ASSERT(dp->i_df.if_u1.if_data != NULL);
> sfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
> @@ -963,13 +945,7 @@ xfs_dir2_sf_replace(
> dp = args->dp;
>
> ASSERT(dp->i_df.if_flags & XFS_IFINLINE);
> - /*
> - * Bail out if the shortform directory is way too small.
> - */
> - if (dp->i_d.di_size < offsetof(xfs_dir2_sf_hdr_t, parent)) {
> - ASSERT(XFS_FORCED_SHUTDOWN(dp->i_mount));
> - return -EIO;
> - }
> + ASSERT(dp->i_d.di_size >= offsetof(struct xfs_dir2_sf_hdr, parent));
> ASSERT(dp->i_df.if_bytes == dp->i_d.di_size);
> ASSERT(dp->i_df.if_u1.if_data != NULL);
> sfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
>
--
Carlos
