public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ntfs: avoid use-after-free of index inode in ntfs_inode_sync_filename()
@ 2026-05-02  0:49 DaeMyung Kang
  2026-05-03 23:38 ` Hyunchul Lee
  2026-05-04 10:59 ` Namjae Jeon
  0 siblings, 2 replies; 3+ messages in thread
From: DaeMyung Kang @ 2026-05-02  0:49 UTC (permalink / raw)
  To: Namjae Jeon, Hyunchul Lee; +Cc: linux-fsdevel, linux-kernel, DaeMyung Kang

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>
---
 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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] ntfs: avoid use-after-free of index inode in ntfs_inode_sync_filename()
  2026-05-02  0:49 [PATCH] ntfs: avoid use-after-free of index inode in ntfs_inode_sync_filename() DaeMyung Kang
@ 2026-05-03 23:38 ` Hyunchul Lee
  2026-05-04 10:59 ` Namjae Jeon
  1 sibling, 0 replies; 3+ messages in thread
From: Hyunchul Lee @ 2026-05-03 23:38 UTC (permalink / raw)
  To: DaeMyung Kang; +Cc: Namjae Jeon, linux-fsdevel, linux-kernel

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ntfs: avoid use-after-free of index inode in ntfs_inode_sync_filename()
  2026-05-02  0:49 [PATCH] ntfs: avoid use-after-free of index inode in ntfs_inode_sync_filename() DaeMyung Kang
  2026-05-03 23:38 ` Hyunchul Lee
@ 2026-05-04 10:59 ` Namjae Jeon
  1 sibling, 0 replies; 3+ messages in thread
From: Namjae Jeon @ 2026-05-04 10:59 UTC (permalink / raw)
  To: DaeMyung Kang; +Cc: Hyunchul Lee, linux-fsdevel, linux-kernel

On Sat, May 2, 2026 at 9:49 AM DaeMyung Kang <charsyam@gmail.com> 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>
Applied it to #ntfs-next
Thanks!

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-05-04 10:59 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-02  0:49 [PATCH] ntfs: avoid use-after-free of index inode in ntfs_inode_sync_filename() DaeMyung Kang
2026-05-03 23:38 ` Hyunchul Lee
2026-05-04 10:59 ` Namjae Jeon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox