diff mbox series

[3/3] xfs: sb_spino_align is not verified

Message ID 20241024025142.4082218-4-david@fromorbit.com (mailing list archive)
State New
Headers show
Series xfs: sparse inodes overlap end of filesystem | expand

Commit Message

Dave Chinner Oct. 24, 2024, 2:51 a.m. UTC 
From: Dave Chinner <dchinner@redhat.com>

It's just read in from the superblock and used without doing any
validity checks at all on the value.

Fixes: fb4f2b4e5a82 ("xfs: add sparse inode chunk alignment superblock field")
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/libxfs/xfs_sb.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Darrick J. Wong Oct. 24, 2024, 4:55 p.m. UTC | #1 
On Thu, Oct 24, 2024 at 01:51:05PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> It's just read in from the superblock and used without doing any
> validity checks at all on the value.
> 
> Fixes: fb4f2b4e5a82 ("xfs: add sparse inode chunk alignment superblock field")
> Signed-off-by: Dave Chinner <dchinner@redhat.com>

Cc: <stable@vger.kernel.org> # v4.2

Oof yeah that's quite a gap!
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/xfs/libxfs/xfs_sb.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> index d95409f3cba6..0d181bc140f0 100644
> --- a/fs/xfs/libxfs/xfs_sb.c
> +++ b/fs/xfs/libxfs/xfs_sb.c
> @@ -398,6 +398,20 @@ xfs_validate_sb_common(
>  					 sbp->sb_inoalignmt, align);
>  				return -EINVAL;
>  			}
> +
> +			if (!sbp->sb_spino_align ||
> +			    sbp->sb_spino_align > sbp->sb_inoalignmt ||
> +			    (sbp->sb_inoalignmt % sbp->sb_spino_align) != 0) {
> +				xfs_warn(mp,
> +				"Sparse inode alignment (%u) is invalid.",
> +					sbp->sb_spino_align);
> +				return -EINVAL;
> +			}
> +		} else if (sbp->sb_spino_align) {
> +			xfs_warn(mp,
> +				"Sparse inode alignment (%u) should be zero.",
> +				sbp->sb_spino_align);
> +			return -EINVAL;
>  		}
>  	} else if (sbp->sb_qflags & (XFS_PQUOTA_ENFD | XFS_GQUOTA_ENFD |
>  				XFS_PQUOTA_CHKD | XFS_GQUOTA_CHKD)) {
> -- 
> 2.45.2
> 
>
Dave Chinner Oct. 25, 2024, 6:33 a.m. UTC | #2 
On Thu, Oct 24, 2024 at 09:55:44AM -0700, Darrick J. Wong wrote:
> On Thu, Oct 24, 2024 at 01:51:05PM +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > It's just read in from the superblock and used without doing any
> > validity checks at all on the value.
> > 
> > Fixes: fb4f2b4e5a82 ("xfs: add sparse inode chunk alignment superblock field")
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> 
> Cc: <stable@vger.kernel.org> # v4.2

Yeah. And probably what ever fix we decide on, too.

> Oof yeah that's quite a gap!

*nod*

What surprises me is that syzbot hasn't found this - it's exactly
the sort of thing that randomised structure fuzzing is supposed to
find..... 

> Reviewed-by: Darrick J. Wong <djwong@kernel.org>

Thanks!

-Dave.
diff mbox series

Patch

diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index d95409f3cba6..0d181bc140f0 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -398,6 +398,20 @@  xfs_validate_sb_common(
 					 sbp->sb_inoalignmt, align);
 				return -EINVAL;
 			}
+
+			if (!sbp->sb_spino_align ||
+			    sbp->sb_spino_align > sbp->sb_inoalignmt ||
+			    (sbp->sb_inoalignmt % sbp->sb_spino_align) != 0) {
+				xfs_warn(mp,
+				"Sparse inode alignment (%u) is invalid.",
+					sbp->sb_spino_align);
+				return -EINVAL;
+			}
+		} else if (sbp->sb_spino_align) {
+			xfs_warn(mp,
+				"Sparse inode alignment (%u) should be zero.",
+				sbp->sb_spino_align);
+			return -EINVAL;
 		}
 	} else if (sbp->sb_qflags & (XFS_PQUOTA_ENFD | XFS_GQUOTA_ENFD |
 				XFS_PQUOTA_CHKD | XFS_GQUOTA_CHKD)) {