Re: [PATCH] xfs: fix use-after-free in xfs_inode_item_push()

[Darrick J. Wong]

On Wed, Mar 04, 2026 at 04:24:06PM +0000, Yuto Ohnuki wrote:
> Since commit 90c60e164012 ("xfs: xfs_iflush() is no longer necessary"),
> xfs_inode_item_push() no longer holds the inode locked (ILOCK_SHARED)
> while flushing, so the inode and its log item can be freed via
> RCU callback (xfs_inode_free_callback) while the AIL lock is
> temporarily dropped.
> 
> This results in a use-after-free when the function reacquires the AIL
> lock by dereferencing the freed log item's li_ailp pointer at offset 48.
> 
> Fix this by saving the ailp pointer in a local variable while the AIL
> lock is held and the log item is guaranteed to be valid.
> 
> Also move trace_xfs_ail_push() before xfsaild_push_item() because the
> log item may be freed during the push.
> 
> Reported-by: syzbot+652af2b3c5569c4ab63c@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=652af2b3c5569c4ab63c
> Fixes: 90c60e164012 ("xfs: xfs_iflush() is no longer necessary")
> Signed-off-by: Yuto Ohnuki <ytohnuki@xxxxxxxxxx>

Cc: <stable@xxxxxxxxxxxxxxx> # v5.9

> ---
>  fs/xfs/xfs_inode_item.c | 5 +++--
>  fs/xfs/xfs_trans_ail.c  | 8 +++++++-
>  2 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
> index 8913036b8024..0a8957f9c72f 100644
> --- a/fs/xfs/xfs_inode_item.c
> +++ b/fs/xfs/xfs_inode_item.c
> @@ -746,6 +746,7 @@ xfs_inode_item_push(
>  	struct xfs_inode_log_item *iip = INODE_ITEM(lip);
>  	struct xfs_inode	*ip = iip->ili_inode;
>  	struct xfs_buf		*bp = lip->li_buf;
> +	struct xfs_ail		*ailp = lip->li_ailp;
>  	uint			rval = XFS_ITEM_SUCCESS;
>  	int			error;
>  
> @@ -771,7 +772,7 @@ xfs_inode_item_push(
>  	if (!xfs_buf_trylock(bp))
>  		return XFS_ITEM_LOCKED;
>  
> -	spin_unlock(&lip->li_ailp->ail_lock);
> +	spin_unlock(&ailp->ail_lock);
>  
>  	/*
>  	 * We need to hold a reference for flushing the cluster buffer as it may
> @@ -795,7 +796,7 @@ xfs_inode_item_push(
>  		rval = XFS_ITEM_LOCKED;
>  	}
>  
> -	spin_lock(&lip->li_ailp->ail_lock);
> +	spin_lock(&ailp->ail_lock);

Hmm, so the @lip UAF is here, where we try to re-acquire the AIL lock?

>  	return rval;
>  }
>  
> diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
> index 923729af4206..e34d8a7e341d 100644
> --- a/fs/xfs/xfs_trans_ail.c
> +++ b/fs/xfs/xfs_trans_ail.c
> @@ -510,6 +510,13 @@ xfsaild_push(
>  		if (test_bit(XFS_LI_FLUSHING, &lip->li_flags))
>  			goto next_item;
>  
> +		/*
> +		 * The log item may be freed after the push if the AIL lock is
> +		 * temporarily dropped and the RCU grace period expires,
> +		 * so trace it before pushing.
> +		 */
> +		trace_xfs_ail_push(lip);
> +
>  		/*
>  		 * Note that iop_push may unlock and reacquire the AIL lock.  We
>  		 * rely on the AIL cursor implementation to be able to deal with
> @@ -519,7 +526,6 @@ xfsaild_push(
>  		switch (lock_result) {
>  		case XFS_ITEM_SUCCESS:
>  			XFS_STATS_INC(mp, xs_push_ail_success);
> -			trace_xfs_ail_push(lip);

Do the tracepoints in the other legs of the switch statement have a
similar UAF problem because they dereference @lip?

--D

>  
>  			ailp->ail_last_pushed_lsn = lsn;
>  			break;
> -- 
> 2.50.1
> 
> 
> 
> 
> Amazon Web Services EMEA SARL, 38 avenue John F. Kennedy, L-1855 Luxembourg, R.C.S. Luxembourg B186284
> 
> Amazon Web Services EMEA SARL, Irish Branch, One Burlington Plaza, Burlington Road, Dublin 4, Ireland, branch registration number 908705
> 
> 
> 
>