[Devel] [PATCH RHEL9 COMMIT] ms/xfs: fix perag reference leak on iteration race with growfs

[Konstantin Khorenko]

The commit is pushed to "branch-rh9-5.14.vz9.1.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh9-5.14.0-4.vz9.10.33
------>
commit 58d502cdcffad1bd5ca8b785616b99e67aa43e87
Author: Brian Foster <bfoster at redhat.com>
Date:   Tue Dec 7 18:14:47 2021 +0300

    ms/xfs: fix perag reference leak on iteration race with growfs
    
    ms commit 892a666fafa1
    
    The for_each_perag*() set of macros are hacky in that some (i.e.
    those based on sb_agcount) rely on the assumption that perag
    iteration terminates naturally with a NULL perag at the specified
    end_agno. Others allow for the final AG to have a valid perag and
    require the calling function to clean up any potential leftover
    xfs_perag reference on termination of the loop.
    
    Aside from providing a subtly inconsistent interface, the former
    variant is racy with growfs because growfs can create discoverable
    post-eofs perags before the final superblock update that completes
    the grow operation and increases sb_agcount. This leads to the
    following assert failure (reproduced by xfs/104) in the perag free
    path during unmount:
    
    XFS: Assertion failed: atomic_read(&pag->pag_ref) == 0, file: fs/xfs/libxfs/xfs_ag.c, line: 195
    
    This occurs because one of the many for_each_perag() loops in the
    code that is expected to terminate with a NULL pag (and thus has no
    post-loop xfs_perag_put() check) raced with a growfs and found a
    non-NULL post-EOFS perag, but terminated naturally based on the
    end_agno check without releasing the post-EOFS perag.
    
    Rework the iteration logic to lift the agno check from the main for
    loop conditional to the iteration helper function. The for loop now
    purely terminates on a NULL pag and xfs_perag_next() avoids taking a
    reference to any perag beyond end_agno in the first place.
    
    https://jira.sw.ru/browse/PSBM-136723
    
    Fixes: f250eedcf762 ("xfs: make for_each_perag... a first class citizen")
    Signed-off-by: Brian Foster <bfoster at redhat.com>
    Reviewed-by: Dave Chinner <dchinner at redhat.com>
    Reviewed-by: Darrick J. Wong <djwong at kernel.org>
    Signed-off-by: Darrick J. Wong <djwong at kernel.org>
    
    Feature: fix ms/xfs
    Signed-off-by: Kirill Tkhai <ktkhai at virtuozzo.com>
---
 fs/xfs/libxfs/xfs_ag.h | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h
index 4585ebb3f450..3f597cad2c33 100644
--- a/fs/xfs/libxfs/xfs_ag.h
+++ b/fs/xfs/libxfs/xfs_ag.h
@@ -116,30 +116,26 @@ void xfs_perag_put(struct xfs_perag *pag);
 
 /*
  * Perag iteration APIs
- *
- * XXX: for_each_perag_range() usage really needs an iterator to clean up when
- * we terminate at end_agno because we may have taken a reference to the perag
- * beyond end_agno. Right now callers have to be careful to catch and clean that
- * up themselves. This is not necessary for the callers of for_each_perag() and
- * for_each_perag_from() because they terminate at sb_agcount where there are
- * no perag structures in tree beyond end_agno.
  */
 static inline struct xfs_perag *
 xfs_perag_next(
 	struct xfs_perag	*pag,
-	xfs_agnumber_t		*agno)
+	xfs_agnumber_t		*agno,
+	xfs_agnumber_t		end_agno)
 {
 	struct xfs_mount	*mp = pag->pag_mount;
 
 	*agno = pag->pag_agno + 1;
 	xfs_perag_put(pag);
+	if (*agno > end_agno)
+		return NULL;
 	return xfs_perag_get(mp, *agno);
 }
 
 #define for_each_perag_range(mp, agno, end_agno, pag) \
 	for ((pag) = xfs_perag_get((mp), (agno)); \
-		(pag) != NULL && (agno) <= (end_agno); \
-		(pag) = xfs_perag_next((pag), &(agno)))
+		(pag) != NULL; \
+		(pag) = xfs_perag_next((pag), &(agno), (end_agno)))
 
 #define for_each_perag_from(mp, agno, pag) \
 	for_each_perag_range((mp), (agno), (mp)->m_sb.sb_agcount - 1, (pag))