Commit 6c30af0b authored by DaeMyung Kang's avatar DaeMyung Kang Committed by Namjae Jeon
Browse files

ntfs: avoid use-after-free of index inode in ntfs_inode_sync_filename()



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: af0db57d ("ntfs: update inode operations")
Signed-off-by: default avatarDaeMyung Kang <charsyam@gmail.com>
Reviewed-by: default avatarHyunchul Lee <hyc.lee@gmail.com>
Signed-off-by: default avatarNamjae Jeon <linkinjeon@kernel.org>
parent f3c8cd8a
Loading
Loading
Loading
Loading
+3 −3
Original line number Diff line number Diff line
@@ -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. */