> > I was thinking of possibility of an overflow but at the moment the
> > blocklog is capped at 16 (65536 bytes) right? mkfs refuses any block
> > sizes more than 64k. And we have check for this in xfs_validate_sb_common()
> > in the kernel, which will catch it before this happens?
>
> The sb_blocklog is checked in the superblock verifier when we first read in the
> superblock:
>
> sbp->sb_blocksize < XFS_MIN_BLOCKSIZE ||
> sbp->sb_blocksize > XFS_MAX_BLOCKSIZE ||
> sbp->sb_blocklog < XFS_MIN_BLOCKSIZE_LOG ||
> sbp->sb_blocklog > XFS_MAX_BLOCKSIZE_LOG ||
> sbp->sb_blocksize != (1 << sbp->sb_blocklog) ||
>
> #define XFS_MAX_BLOCKSIZE_LOG 16
>
> However, we pass mp->m_sb.sb_dblocks or m_sb.sb_rblocks to this
> function, and they are validated by the same verifier as invalid
> if:
>
> sbp->sb_dblocks > XFS_MAX_DBLOCKS(sbp)
>
> #define XFS_MAX_DBLOCKS(s) ((xfs_rfsblock_t)(s)->sb_agcount *
> (s)->sb_agblocks)
>
> Which means as long as someone can corrupt some combination of
> sb_dblocks, sb_agcount and sb_agblocks that allows sb_dblocks to be
> greater than 2^48 on a 64kB fsb fs, then that the above code:
>
> uint64_t bytes = nblocks << sbp->sb_blocklog;
>
> will overflow.
>
> I also suspect that we can feed a huge rtdev to this new code
> and have it overflow without needing to corrupt the superblock in
> any way....
So we could use the check_mul_overflow to detect these cases:
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 596aa2cdefbc..23faa993fb80 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -132,8 +132,12 @@ xfs_sb_validate_fsb_count(
uint64_t nblocks)
{
ASSERT(sbp->sb_blocklog >= BBSHIFT);
- unsigned long mapping_count;
- uint64_t bytes = nblocks << sbp->sb_blocklog;
+ uint64_t mapping_count;
+ uint64_t bytes;
+
+ if (check_mul_overflow(nblocks, (1 << sbp->sb_blocklog), &bytes))
+ return -EFBIG;
if (!IS_ENABLED(CONFIG_XFS_LBS))
ASSERT(PAGE_SHIFT >= sbp->sb_blocklog);
>
> -Dave.
> --
> Dave Chinner
> david@xxxxxxxxxxxxx
