[PATCH 0/3] xfs: fix a few realtime bugs

[PATCH 0/3] xfs: fix a few realtime bugs

[Darrick J. Wong]

Hi all,

While running fstests with realtime enabled and rtinherit=1, the kernel
tripped over a bunch of bugs related to the realtime support.  The three
patches in this series fix some problems with inode flags being
propagated incorrectly and some math errors in the rt allocator and the
fallocate code that cause filesystem shutdowns.

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=realtime-bugs
---
 fs/xfs/xfs_bmap_util.c |   16 ++++++++++++++++
 fs/xfs/xfs_inode.c     |    3 ++-
 fs/xfs/xfs_rtalloc.c   |   11 +++++++++++
 3 files changed, 29 insertions(+), 1 deletion(-)

[PATCH 1/3] xfs: don't propagate RTINHERIT -> REALTIME when there is no rtdev

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

While running generic/042 with -drtinherit=1 set in MKFS_OPTIONS, I
observed that the kernel will gladly set the realtime flag on any file
created on the loopback filesystem even though that filesystem doesn't
actually have a realtime device attached.  This leads to verifier
failures and doesn't make any sense, so be smarter about this.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_inode.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index c06129cffba9..10312e1ae60c 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -870,7 +870,8 @@ xfs_ialloc(
 				if (pip->i_d.di_flags & XFS_DIFLAG_PROJINHERIT)
 					di_flags |= XFS_DIFLAG_PROJINHERIT;
 			} else if (S_ISREG(mode)) {
-				if (pip->i_d.di_flags & XFS_DIFLAG_RTINHERIT)
+				if ((pip->i_d.di_flags & XFS_DIFLAG_RTINHERIT) &&
+				    xfs_sb_version_hasrealtime(&mp->m_sb))
 					di_flags |= XFS_DIFLAG_REALTIME;
 				if (pip->i_d.di_flags & XFS_DIFLAG_EXTSZINHERIT) {
 					di_flags |= XFS_DIFLAG_EXTSIZE;

Re: [PATCH 1/3] xfs: don't propagate RTINHERIT -> REALTIME when there is no rtdev

[Christoph Hellwig]

On Mon, Sep 07, 2020 at 11:01:08AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> While running generic/042 with -drtinherit=1 set in MKFS_OPTIONS, I
> observed that the kernel will gladly set the realtime flag on any file
> created on the loopback filesystem even though that filesystem doesn't
> actually have a realtime device attached.  This leads to verifier
> failures and doesn't make any sense, so be smarter about this.

Looks good, but add an overly long line.  Which just suggested that
the whole flag inheritance really needs to be split into a helper..

[PATCH 2/3] xfs: make sure the rt allocator doesn't run off the end

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

There's an overflow bug in the realtime allocator.  If the rt volume is
large enough to handle a single allocation request that is larger than
the maximum bmap extent length and the rt bitmap ends exactly on a
bitmap block boundary, it's possible that the near allocator will try to
check the freeness of a range that extends past the end of the bitmap.
This fails with a corruption error and shuts down the fs.

Therefore, constrain maxlen so that the range scan cannot run off the
end of the rt bitmap.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_rtalloc.c |   11 +++++++++++
 1 file changed, 11 insertions(+)


diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
index 6209e7b6b895..86994d7f7cba 100644
--- a/fs/xfs/xfs_rtalloc.c
+++ b/fs/xfs/xfs_rtalloc.c
@@ -247,6 +247,9 @@ xfs_rtallocate_extent_block(
 		end = XFS_BLOCKTOBIT(mp, bbno + 1) - 1;
 	     i <= end;
 	     i++) {
+		/* Make sure we don't scan off the end of the rt volume. */
+		maxlen = min(mp->m_sb.sb_rextents, i + maxlen) - i;
+
 		/*
 		 * See if there's a free extent of maxlen starting at i.
 		 * If it's not so then next will contain the first non-free.
@@ -442,6 +445,14 @@ xfs_rtallocate_extent_near(
 	 */
 	if (bno >= mp->m_sb.sb_rextents)
 		bno = mp->m_sb.sb_rextents - 1;
+
+	/* Make sure we don't run off the end of the rt volume. */
+	maxlen = min(mp->m_sb.sb_rextents, bno + maxlen) - bno;
+	if (maxlen < minlen) {
+		*rtblock = NULLRTBLOCK;
+		return 0;
+	}
+
 	/*
 	 * Try the exact allocation first.
 	 */

Re: [PATCH 2/3] xfs: make sure the rt allocator doesn't run off the end

[Christoph Hellwig]

On Mon, Sep 07, 2020 at 11:01:14AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> There's an overflow bug in the realtime allocator.  If the rt volume is
> large enough to handle a single allocation request that is larger than
> the maximum bmap extent length and the rt bitmap ends exactly on a
> bitmap block boundary, it's possible that the near allocator will try to
> check the freeness of a range that extends past the end of the bitmap.
> This fails with a corruption error and shuts down the fs.
> 
> Therefore, constrain maxlen so that the range scan cannot run off the
> end of the rt bitmap.

Looks good,

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

[PATCH 3/3] xfs: ensure that fpunch, fcollapse, and finsert operations are aligned to rt extent size

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Make sure that any fallocate operation that requires the range to be
block-aligned also checks that the range is aligned to the realtime
extent size.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_bmap_util.c |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)


diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 5123f82f2477..f2a8a0e75e1f 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -946,6 +946,14 @@ xfs_free_file_space(
 	startoffset_fsb = XFS_B_TO_FSB(mp, offset);
 	endoffset_fsb = XFS_B_TO_FSBT(mp, offset + len);
 
+	/* We can only free complete realtime extents. */
+	if (XFS_IS_REALTIME_INODE(ip)) {
+		xfs_extlen_t	extsz = xfs_get_extsz_hint(ip);
+
+		if ((startoffset_fsb | endoffset_fsb) & (extsz - 1))
+			return -EINVAL;
+	}
+
 	/*
 	 * Need to zero the stuff we're not freeing, on disk.
 	 */
@@ -1139,6 +1147,14 @@ xfs_insert_file_space(
 
 	trace_xfs_insert_file_space(ip);
 
+	/* We can only insert complete realtime extents. */
+	if (XFS_IS_REALTIME_INODE(ip)) {
+		xfs_extlen_t	extsz = xfs_get_extsz_hint(ip);
+
+		if ((stop_fsb | shift_fsb) & (extsz - 1))
+			return -EINVAL;
+	}
+
 	error = xfs_bmap_can_insert_extents(ip, stop_fsb, shift_fsb);
 	if (error)
 		return error;

Re: [PATCH 3/3] xfs: ensure that fpunch, fcollapse, and finsert operations are aligned to rt extent size

[Christoph Hellwig]

On Mon, Sep 07, 2020 at 11:01:20AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Make sure that any fallocate operation that requires the range to be
> block-aligned also checks that the range is aligned to the realtime
> extent size.

Looks good,

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