Re: [PATCH 7/7] xfs: advance the findparent inode scan cursor while holding ILOCK

[Carlos Maiolino <cem@kernel.org>]

On Tue, Sep 08, 2026 at 11:06:08PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM pointed out a race condition in xrep_findparent_scan -- the
> directory live update hook holds the directory ILOCK when it calls the
> xchk_iscan_want_live_update predicate to figure out if it needs to
> remember the live update, but xrep_findparent_scan drops the directory
> ILOCK before advancing the cursor.  Therefore, it's possible for a live
> update to check the scan cursor after the scan drops the ILOCK but
> before the scan updates its cursor.  If this happens, we'll fail to
> record the live update.  Fix this by moving the cursor update logic
> inside xrep_findparent_walk_directory.
> 
> Note that for non-directories it's ok to advance the cursor without
> holding any ILOCK because the findparent scan only cares about directory
> parents, not the children.
> 
> Cc: <stable@vger.kernel.org> # v6.10
> Fixes: a07b45576264e7 ("xfs: scan the filesystem to repair a directory dotdot entry")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> Assisted-by: LOLLM # finding obvious bugs
> ---

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>


>  fs/xfs/scrub/findparent.c |   54 ++++++++++++++++++++++++++++++---------------
>  1 file changed, 36 insertions(+), 18 deletions(-)
> 
> 
> diff --git a/fs/xfs/scrub/findparent.c b/fs/xfs/scrub/findparent.c
> index 04b6b96b0a30ed..eab3ac2704befe 100644
> --- a/fs/xfs/scrub/findparent.c
> +++ b/fs/xfs/scrub/findparent.c
> @@ -139,32 +139,52 @@ xrep_findparent_dirent(
>  	return 0;
>  }
>  
> -/*
> - * If this is a directory, walk the dirents looking for any that point to the
> - * scrub target inode.
> - */
> -STATIC int
> -xrep_findparent_walk_directory(
> -	struct xrep_findparent_info	*fpi)
> +static inline bool
> +xrep_findparent_want_scan_file(
> +	const struct xrep_findparent_info	*fpi)
>  {
> -	struct xfs_scrub		*sc = fpi->sc;
> -	struct xfs_inode		*dp = fpi->dp;
> -	unsigned int			lock_mode;
> -	int				error = 0;
> +	const struct xfs_scrub			*sc = fpi->sc;
> +	const struct xfs_inode			*dp = fpi->dp;
> +
> +	/* Only directories can be parents */
> +	if (!S_ISDIR(VFS_IC(dp)->i_mode))
> +		return false;
>  
>  	/*
>  	 * The inode being scanned cannot be its own parent, nor can any
>  	 * temporary directory we created to stage this repair.
>  	 */
>  	if (dp == sc->ip || dp == sc->tempip)
> -		return 0;
> +		return false;
>  
>  	/*
>  	 * Similarly, temporary files created to stage a repair cannot be the
>  	 * parent of this inode.
>  	 */
>  	if (xrep_is_tempfile(dp))
> +		return false;
> +
> +	return true;
> +}
> +
> +/*
> + * If this is a directory, walk the dirents looking for any that point to the
> + * scrub target inode.
> + */
> +STATIC int
> +xrep_findparent_walk_file(
> +	struct xrep_findparent_info	*fpi)
> +{
> +	struct xfs_scrub		*sc = fpi->sc;
> +	struct xfs_inode		*dp = fpi->dp;
> +	unsigned int			lock_mode;
> +	int				error = 0;
> +
> +	if (!xrep_findparent_want_scan_file(fpi)) {
> +		if (fpi->parent_scan)
> +			xchk_iscan_mark_visited(&fpi->parent_scan->iscan, dp);
>  		return 0;
> +	}
>  
>  	/*
>  	 * Scan the directory to see if there it contains an entry pointing to
> @@ -201,6 +221,8 @@ xrep_findparent_walk_directory(
>  		goto out_unlock;
>  
>  out_unlock:
> +	if (fpi->parent_scan)
> +		xchk_iscan_mark_visited(&fpi->parent_scan->iscan, dp);
>  	xfs_iunlock(dp, lock_mode);
>  	return error;
>  }
> @@ -308,11 +330,7 @@ xrep_findparent_scan(
>  	ASSERT(S_ISDIR(VFS_IC(sc->ip)->i_mode));
>  
>  	while ((ret = xchk_iscan_iter(&pscan->iscan, &fpi.dp)) == 1) {
> -		if (S_ISDIR(VFS_I(fpi.dp)->i_mode))
> -			ret = xrep_findparent_walk_directory(&fpi);
> -		else
> -			ret = 0;
> -		xchk_iscan_mark_visited(&pscan->iscan, fpi.dp);
> +		ret = xrep_findparent_walk_file(&fpi);
>  		xchk_irele(sc, fpi.dp);
>  		if (ret)
>  			break;
> @@ -401,7 +419,7 @@ xrep_findparent_confirm(
>  		goto out_rele;
>  	}
>  
> -	error = xrep_findparent_walk_directory(&fpi);
> +	error = xrep_findparent_walk_file(&fpi);
>  	if (error)
>  		goto out_rele;
>  
> 
>