[PATCH v2] xfs: Fix error pointer dereference

Ethan Tidmore posted 1 patch 1 month, 1 week ago
There is a newer version of this series
fs/xfs/scrub/orphanage.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
[PATCH v2] xfs: Fix error pointer dereference
Posted by Ethan Tidmore 1 month, 1 week ago
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@vger.kernel.org> # v6.16
Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
---
 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);
+
 	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);
+
 	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
Re: [PATCH v2] xfs: Fix error pointer dereference
Posted by Darrick J. Wong 1 month, 1 week ago
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@vger.kernel.org> # v6.16
> Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
> ---
>  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
>