xfs: nlink scrub must take IOLOCK before determining ILOCK state

In xchk_nlinks_ilock_dir, take the IOLOCK before accessing internal
inode state to figure out if we need to take ILOCK shared or exclusive.
That way we can't race with directory updates.  LOLLM pointed out that
the code was initially correct w.r.t. the IOLOCK, but then I broke it.

Cc: stable@vger.kernel.org # v6.18
Fixes: f477af0cfa0487 ("xfs: fix locking in xchk_nlinks_collect_dir")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>

diff --git a/fs/xfs/scrub/nlinks.c b/fs/xfs/scrub/nlinks.c
index 355ab6de23eaba8db5931e852d591a76e9a3f602..bcedb8c3e4e608bc9813d528c8d91c19634f26c1 100644 (file)
--- a/fs/xfs/scrub/nlinks.c
+++ b/fs/xfs/scrub/nlinks.c
@@ -382,6 +382,12 @@ xchk_nlinks_ilock_dir(
 {
        uint                    lock_mode = XFS_ILOCK_SHARED;
 
+       /*
+        * Take the IOLOCK so that other threads cannot start a directory
+        * update while we're scanning.
+        */
+       xfs_ilock(ip, XFS_IOLOCK_SHARED);
+
        /*
         * We're going to scan the directory entries, so we must be ready to
         * pull the data fork mappings into memory if they aren't already.
@@ -397,13 +403,8 @@ xchk_nlinks_ilock_dir(
            xfs_need_iread_extents(&ip->i_af))
                lock_mode = XFS_ILOCK_EXCL;
 
-       /*
-        * Take the IOLOCK so that other threads cannot start a directory
-        * update while we're scanning.
-        */
-       lock_mode |= XFS_IOLOCK_SHARED;
        xfs_ilock(ip, lock_mode);
-       return lock_mode;
+       return lock_mode | XFS_IOLOCK_SHARED;
 }
 
 /* Walk a directory to bump the observed link counts of the children. */