Re: [PATCH] xfs: Fix error pointer dereference

[Nirjhar Roy (IBM)]

On Wed, 2026-02-18 at 13:51 -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.

Nit:In the subject, maybe just add the function name where the error pointer dereference is being
fixed?
> 
> Detected by Smatch:
> fs/xfs/scrub/orphanage.c:449 xrep_adoption_check_dcache() error: 
> 'd_child' dereferencing possible ERR_PTR()
> 
> fs/xfs/scrub/orphanage.c:485 xrep_adoption_zap_dcache() error: 
> 'd_child' dereferencing possible ERR_PTR()
> 
> Fixes: 06c567403ae5a ("Use try_lookup_noperm() instead of d_hash_and_lookup() outside of VFS")
> Signed-off-by: Ethan Tidmore <ethantidmore06@xxxxxxxxx>
> ---
>  fs/xfs/scrub/orphanage.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/scrub/orphanage.c b/fs/xfs/scrub/orphanage.c
> index 52a108f6d5f4..cdb0f486f50c 100644
> --- a/fs/xfs/scrub/orphanage.c
> +++ b/fs/xfs/scrub/orphanage.c
> @@ -442,7 +442,7 @@ xrep_adoption_check_dcache(
>  		return 0;
>  
>  	d_child = try_lookup_noperm(&qname, d_orphanage);
> -	if (d_child) {
> +	if (!IS_ERR_OR_NULL(d_child)) {
>  		trace_xrep_adoption_check_child(sc->mp, d_child);
>  
>  		if (d_is_positive(d_child)) {
> @@ -479,7 +479,7 @@ xrep_adoption_zap_dcache(
>  		return;
>  
>  	d_child = try_lookup_noperm(&qname, d_orphanage);
> -	while (d_child != NULL) {
> +	while (!IS_ERR_OR_NULL(d_child)) {
>  		trace_xrep_adoption_invalidate_child(sc->mp, d_child);

Based on my limited knowledge of this change looks okay to me. I looked into the return values of
try_lookup_noperm() and it does return error pointer which is not NULL. I also checked the other
call sites of try_lookup_noperm() but I do see a mixed handling i.e, some places just checks for
!ptr and some for IS_ERR_OR_NULL. For example in fs/autofs it checks with IS_ERR_OR_NULL whereas in
fs/proc/base.c it just checks for !child. However, IMO, it is better to check for both NULL and
error pointer if there is a possibility for both.
--NR
>  
>  		ASSERT(d_is_negative(d_child));