Re: [PATCH v2 1/3] xfs: fix possible null pointer dereference in xfs_attri_recover_work

["Darrick J. Wong" <djwong@kernel.org>]

On Thu, Mar 19, 2026 at 09:06:16AM +0800, Long Li wrote:
> When xlog_recover_iget() or xlog_recover_iget_handle() fails, ip is
> not guaranteed to be initialized. Calling xfs_irele(ip) unconditionally
> in the error path may dereference a null pointer.
> 
> Cc: <stable@vger.kernel.org> # v6.9
> Fixes: ae673f534a30 ("xfs: record inode generation in xattr update log intent items")
> Signed-off-by: Long Li <leo.lilong@huawei.com>
> ---
>  fs/xfs/xfs_attr_item.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/xfs_attr_item.c b/fs/xfs/xfs_attr_item.c
> index 354472bf45f1..8ebdd0926b89 100644
> --- a/fs/xfs/xfs_attr_item.c
> +++ b/fs/xfs/xfs_attr_item.c
> @@ -633,7 +633,7 @@ xfs_attri_recover_work(
>  {
>  	struct xfs_attr_intent		*attr;
>  	struct xfs_da_args		*args;
> -	struct xfs_inode		*ip;
> +	struct xfs_inode		*ip = NULL;
>  	int				local;
>  	int				error;
>  
> @@ -653,7 +653,8 @@ xfs_attri_recover_work(
>  		break;
>  	}
>  	if (error) {
> -		xfs_irele(ip);
> +		if (ip)
> +			xfs_irele(ip);

Hrmm.  On second thought, there's a much more severe UAF bug here:

int
xlog_recover_iget(
	struct xfs_mount	*mp,
	xfs_ino_t		ino,
	struct xfs_inode	**ipp)
{
	int			error;

	error = xfs_iget(mp, NULL, ino, 0, 0, ipp);
	if (error)
		return error;

	error = xfs_qm_dqattach(*ipp);
	if (error) {
		xfs_irele(*ipp);
		return error;

^^^^^ here we return a nonzero error, having previously set @ipp.
The xfs_irele in xfs_attri_recover_work is, in this case, the wrong
thing to do.

	}

	if (VFS_I(*ipp)->i_nlink == 0)
		xfs_iflags_set(*ipp, XFS_IRECOVERY);

	return 0;
}

With that fixed, the xfs_irele call in xfs_attri_recover_work becomes
incorrect because the xlog*iget functions never return nonzero *and* set
*ipp.  If you found this via static checker, I wonder if that's what is
tripping it up?

--D

>  		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, attrp,
>  				sizeof(*attrp));
>  		return ERR_PTR(-EFSCORRUPTED);
> -- 
> 2.39.2
> 
>