diff mbox series

[3/3] xfs: don't allow overly small or large realtime volumes

Message ID 170162990673.3038044.6698602496725473343.stgit@frogsfrogsfrogs (mailing list archive)
State Superseded
Headers show
Series xfs: fix realtime geometry integer overflows | expand

Commit Message

Darrick J. Wong Dec. 3, 2023, 7:05 p.m. UTC
From: Darrick J. Wong <djwong@kernel.org>

Don't allow realtime volumes that are less than one rt extent long.
This has been broken across 4 LTS kernels with nobody noticing, so let's
just disable it.

Per the previous patch, I also observed integer overflows in calculating
rextslog (the number of rt summary levels) when the rtextent count
exceeds 2^32.  If you're lucky, this means that mkfs will fail to format
the filesystem; if not, then the fs will go down due to corruption
errors.  Prohibit those too; larger volume support will return with
rtgroups.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/libxfs/xfs_rtbitmap.h |   12 ++++++++++++
 fs/xfs/libxfs/xfs_sb.c       |    3 ++-
 fs/xfs/xfs_rtalloc.c         |    2 ++
 3 files changed, 16 insertions(+), 1 deletion(-)

Comments

Darrick J. Wong Dec. 3, 2023, 7:09 p.m. UTC | #1
On Sun, Dec 03, 2023 at 11:05:50AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Don't allow realtime volumes that are less than one rt extent long.
> This has been broken across 4 LTS kernels with nobody noticing, so let's
> just disable it.
> 
> Per the previous patch, I also observed integer overflows in calculating
> rextslog (the number of rt summary levels) when the rtextent count
> exceeds 2^32.  If you're lucky, this means that mkfs will fail to format
> the filesystem; if not, then the fs will go down due to corruption
> errors.  Prohibit those too; larger volume support will return with
> rtgroups.

....aaand I forgot to update the commit message on this one, sorry.
Strike the previous paragraph, please.

--D

> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  fs/xfs/libxfs/xfs_rtbitmap.h |   12 ++++++++++++
>  fs/xfs/libxfs/xfs_sb.c       |    3 ++-
>  fs/xfs/xfs_rtalloc.c         |    2 ++
>  3 files changed, 16 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_rtbitmap.h b/fs/xfs/libxfs/xfs_rtbitmap.h
> index 1610d0e4a04c..411de3b889ae 100644
> --- a/fs/xfs/libxfs/xfs_rtbitmap.h
> +++ b/fs/xfs/libxfs/xfs_rtbitmap.h
> @@ -353,6 +353,18 @@ int xfs_rtfree_blocks(struct xfs_trans *tp, xfs_fsblock_t rtbno,
>  
>  uint8_t xfs_compute_rextslog(xfs_rtbxlen_t rtextents);
>  
> +/* Do we support an rt volume having this number of rtextents? */
> +static inline bool
> +xfs_validate_rtextents(
> +	xfs_rtbxlen_t		rtextents)
> +{
> +	/* No runt rt volumes */
> +	if (rtextents == 0)
> +		return false;
> +
> +	return true;
> +}
> +
>  xfs_filblks_t xfs_rtbitmap_blockcount(struct xfs_mount *mp, xfs_rtbxlen_t
>  		rtextents);
>  unsigned long long xfs_rtbitmap_wordcount(struct xfs_mount *mp,
> diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> index df12bf82ed18..4a9e8588f4c9 100644
> --- a/fs/xfs/libxfs/xfs_sb.c
> +++ b/fs/xfs/libxfs/xfs_sb.c
> @@ -509,7 +509,8 @@ xfs_validate_sb_common(
>  		rbmblocks = howmany_64(sbp->sb_rextents,
>  				       NBBY * sbp->sb_blocksize);
>  
> -		if (sbp->sb_rextents != rexts ||
> +		if (!xfs_validate_rtextents(rexts) ||
> +		    sbp->sb_rextents != rexts ||
>  		    sbp->sb_rextslog != xfs_compute_rextslog(rexts) ||
>  		    sbp->sb_rbmblocks != rbmblocks) {
>  			xfs_notice(mp,
> diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> index 7c5a50163d2d..8feb58c6241c 100644
> --- a/fs/xfs/xfs_rtalloc.c
> +++ b/fs/xfs/xfs_rtalloc.c
> @@ -963,6 +963,8 @@ xfs_growfs_rt(
>  	 */
>  	nrextents = nrblocks;
>  	do_div(nrextents, in->extsize);
> +	if (!xfs_validate_rtextents(nrextents))
> +		return -EINVAL;
>  	nrbmblocks = xfs_rtbitmap_blockcount(mp, nrextents);
>  	nrextslog = xfs_compute_rextslog(nrextents);
>  	nrsumlevels = nrextslog + 1;
> 
>
Christoph Hellwig Dec. 4, 2023, 4:56 a.m. UTC | #2
With the updated commit message:

Reviewed-by: Christoph Hellwig <hch@lst.de>
diff mbox series

Patch

diff --git a/fs/xfs/libxfs/xfs_rtbitmap.h b/fs/xfs/libxfs/xfs_rtbitmap.h
index 1610d0e4a04c..411de3b889ae 100644
--- a/fs/xfs/libxfs/xfs_rtbitmap.h
+++ b/fs/xfs/libxfs/xfs_rtbitmap.h
@@ -353,6 +353,18 @@  int xfs_rtfree_blocks(struct xfs_trans *tp, xfs_fsblock_t rtbno,
 
 uint8_t xfs_compute_rextslog(xfs_rtbxlen_t rtextents);
 
+/* Do we support an rt volume having this number of rtextents? */
+static inline bool
+xfs_validate_rtextents(
+	xfs_rtbxlen_t		rtextents)
+{
+	/* No runt rt volumes */
+	if (rtextents == 0)
+		return false;
+
+	return true;
+}
+
 xfs_filblks_t xfs_rtbitmap_blockcount(struct xfs_mount *mp, xfs_rtbxlen_t
 		rtextents);
 unsigned long long xfs_rtbitmap_wordcount(struct xfs_mount *mp,
diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index df12bf82ed18..4a9e8588f4c9 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -509,7 +509,8 @@  xfs_validate_sb_common(
 		rbmblocks = howmany_64(sbp->sb_rextents,
 				       NBBY * sbp->sb_blocksize);
 
-		if (sbp->sb_rextents != rexts ||
+		if (!xfs_validate_rtextents(rexts) ||
+		    sbp->sb_rextents != rexts ||
 		    sbp->sb_rextslog != xfs_compute_rextslog(rexts) ||
 		    sbp->sb_rbmblocks != rbmblocks) {
 			xfs_notice(mp,
diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
index 7c5a50163d2d..8feb58c6241c 100644
--- a/fs/xfs/xfs_rtalloc.c
+++ b/fs/xfs/xfs_rtalloc.c
@@ -963,6 +963,8 @@  xfs_growfs_rt(
 	 */
 	nrextents = nrblocks;
 	do_div(nrextents, in->extsize);
+	if (!xfs_validate_rtextents(nrextents))
+		return -EINVAL;
 	nrbmblocks = xfs_rtbitmap_blockcount(mp, nrextents);
 	nrextslog = xfs_compute_rextslog(nrextents);
 	nrsumlevels = nrextslog + 1;