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

[Long Li <leo.lilong@huawei.com>]

On Thu, Mar 19, 2026 at 09:52:00AM -0700, Darrick J. Wong wrote:
> 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?
> 

Yes, we found this through static analysis. Obviously, I didn't look
at this code carefully enough. Thank you for pointing it out and fixing
it.

Thanks,
Long Li