Re: [PATCH v2] xfs: Fix error pointer dereference

[Darrick J. Wong]

On Wed, Feb 18, 2026 at 11:18:41PM -0600, Ethan Tidmore wrote:
> The function try_lookup_noperm() can return an error pointer and is not
> checked for one. Add checks for error pointer and propagate it.
> 
> Fixes: 73597e3e42b4 ("xfs: ensure dentry consistency when the orphanage adopts a file")
> Cc: <stable@xxxxxxxxxxxxxxx> # v6.16
> Signed-off-by: Ethan Tidmore <ethantidmore06@xxxxxxxxx>
> ---
>  fs/xfs/scrub/orphanage.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/xfs/scrub/orphanage.c b/fs/xfs/scrub/orphanage.c
> index 52a108f6d5f4..3269a0646e19 100644
> --- a/fs/xfs/scrub/orphanage.c
> +++ b/fs/xfs/scrub/orphanage.c
> @@ -442,6 +442,9 @@ xrep_adoption_check_dcache(
>  		return 0;
>  
>  	d_child = try_lookup_noperm(&qname, d_orphanage);
> +	if (IS_ERR(d_child))
> +		return PTR_ERR(d_child);

I think you still need to dput(d_orphanage) below if you're returning an
error code.

>  	if (d_child) {
>  		trace_xrep_adoption_check_child(sc->mp, d_child);
>  
> @@ -464,7 +467,7 @@ xrep_adoption_check_dcache(
>   * There should not be any positive entries for the name, since we've
>   * maintained our lock on the orphanage directory.
>   */
> -static void
> +static int
>  xrep_adoption_zap_dcache(
>  	struct xrep_adoption	*adopt)
>  {
> @@ -476,9 +479,12 @@ xrep_adoption_zap_dcache(
>  	/* Invalidate all dentries for the adoption name */
>  	d_orphanage = d_find_alias(VFS_I(sc->orphanage));
>  	if (!d_orphanage)
> -		return;
> +		return 0;
>  
>  	d_child = try_lookup_noperm(&qname, d_orphanage);
> +	if (IS_ERR(d_child))
> +		return PTR_ERR(d_child);

Don't we still want to perform the d_find_alias loop below?

The changes you made to xrep_adoption_zap_dcache in the first version
seemed reasonable to me since it only tried to invalidate a bunch of
dcache entries.  Also I think if this call to try_lookup_noperm()
returns an ERR_PTR, then the call in _check_dcache would have gotten the
same ERR_PTR return value and ended the entire repair attempt.

--D

> +
>  	while (d_child != NULL) {
>  		trace_xrep_adoption_invalidate_child(sc->mp, d_child);
>  
> @@ -497,6 +503,8 @@ xrep_adoption_zap_dcache(
>  		d_invalidate(d_child);
>  		dput(d_child);
>  	}
> +
> +	return 0;
>  }
>  
>  /*
> @@ -592,7 +600,10 @@ xrep_adoption_move(
>  	xfs_dir_update_hook(sc->orphanage, sc->ip, 1, adopt->xname);
>  
>  	/* Remove negative dentries from the lost+found's dcache */
> -	xrep_adoption_zap_dcache(adopt);
> +	error = xrep_adoption_zap_dcache(adopt);
> +	if (error)
> +		return error;
> +
>  	return 0;
>  }
>  
> -- 
> 2.53.0
>