> > 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