Re: [PATCH] ntfs: avoid use-after-free of index inode in ntfs_inode_sync_filename()

[Hyunchul Lee <hyc.lee@gmail.com>]

On Sat, May 02, 2026 at 09:49:16AM +0900, DaeMyung Kang wrote:
> ntfs_inode_sync_filename() walks every FILE_NAME attribute and, for
> each one that points at a different parent, opens the parent index
> inode with ntfs_iget() and locks index_ni->mrec_lock.  All three error
> branches (NInoBeingDeleted, ntfs_index_ctx_get failure, ntfs_index_lookup
> failure) drop the parent reference before unlocking:
> 
> 	iput(index_vi);
> 	mutex_unlock(&index_ni->mrec_lock);
> 	continue;
> 
> index_ni is NTFS_I(index_vi), so the ntfs_inode (and its mrec_lock) is
> embedded in the inode allocation.  If the parent directory is not held
> outside the icache - no open dentry, recently evicted from dcache, no
> other concurrent lookup - ntfs_iget() returns with i_count == 1 and
> our iput() drops the last reference.  evict_inode() then runs and
> destroy_inode() schedules the slab object for RCU free, while
> mutex_unlock() on the next line is still touching index_ni->mrec_lock.
> 
> Swap the order so the mutex is dropped while index_vi is still alive,
> matching the success path at the bottom of the loop which already
> unlocks before iput().
> 
> Reproduced under KASAN with a debug build that forces
> ntfs_index_ctx_get() to fail when the parent index inode has been
> opened with i_count == 1.  KASAN reports a slab-use-after-free read
> on the parent's mrec_lock from mutex_unlock() on the writeback worker:
> 
>   BUG: KASAN: slab-use-after-free in __mutex_unlock_slowpath+0xb5/0x970
>   Read of size 8 at addr ffff8880014b7598 by task kworker/u8:0/12
>   Workqueue: writeback wb_workfn (flush-253:0)
>   Call Trace:
>    mutex_unlock
>    ntfs_inode_sync_filename
>    __ntfs_write_inode
>    ntfs_write_inode
>    __writeback_single_inode
> 
>   Allocated by task 103:
>    ntfs_alloc_big_inode
>    ntfs_iget
>    ntfs_lookup
>    __x64_sys_mkdir
> 
>   Freed by task 12:
>    ntfs_free_big_inode
>    i_callback
>    rcu_do_batch
> 
>   Last potentially related work creation:
>    call_rcu
>    destroy_inode
>    evict
>    dispose_list
>    evict_inodes
>    ntfs_inode_sync_filename
>    __ntfs_write_inode
> 
>   The buggy address belongs to the object at ffff8880014b7440
>    which belongs to the cache ntfs_big_inode_cache of size 1800
> 
> The freed object is the parent directory inode itself: allocated by
> mkdir(2) via ntfs_iget(), then released through call_rcu(i_callback)
> that destroy_inode() scheduled when evict_inodes() ran from inside
> ntfs_inode_sync_filename().  Re-running the same workload with
> mutex_unlock() moved before iput() runs cleanly under KASAN.
> 
> Fixes: af0db57d4293 ("ntfs: update inode operations")
> Signed-off-by: DaeMyung Kang <charsyam@gmail.com>

Looks good to me.

Reviewed-by: Hyunchul Lee <hyc.lee@gmail.com>

> ---
>  fs/ntfs/inode.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ntfs/inode.c b/fs/ntfs/inode.c
> index 16890d411194..360bebd1ee3f 100644
> --- a/fs/ntfs/inode.c
> +++ b/fs/ntfs/inode.c
> @@ -2582,8 +2582,8 @@ int ntfs_inode_sync_filename(struct ntfs_inode *ni)
>  
>  		mutex_lock_nested(&index_ni->mrec_lock, NTFS_INODE_MUTEX_PARENT);
>  		if (NInoBeingDeleted(ni)) {
> -			iput(index_vi);
>  			mutex_unlock(&index_ni->mrec_lock);
> +			iput(index_vi);
>  			continue;
>  		}
>  
> @@ -2591,8 +2591,8 @@ int ntfs_inode_sync_filename(struct ntfs_inode *ni)
>  		if (!ictx) {
>  			ntfs_error(sb, "Failed to get index ctx, inode %llu",
>  					index_ni->mft_no);
> -			iput(index_vi);
>  			mutex_unlock(&index_ni->mrec_lock);
> +			iput(index_vi);
>  			continue;
>  		}
>  
> @@ -2601,8 +2601,8 @@ int ntfs_inode_sync_filename(struct ntfs_inode *ni)
>  			ntfs_debug("Index lookup failed, inode %llu",
>  					index_ni->mft_no);
>  			ntfs_index_ctx_put(ictx);
> -			iput(index_vi);
>  			mutex_unlock(&index_ni->mrec_lock);
> +			iput(index_vi);
>  			continue;
>  		}
>  		/* Update flags and file size. */
> -- 
> 2.43.0
> 

-- 
Thanks,
Hyunchul