Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Zi Yan" <ziy@nvidia.com>
To: "Kiryl Shutsemau" <kirill@shutemov.name>,
	"Hao Zhang" <hao_zhang_kdev@163.com>
Cc: "Andrew Morton" <akpm@linux-foundation.org>,
	"David Hildenbrand" <david@kernel.org>,
	"Lorenzo Stoakes" <ljs@kernel.org>, <linux-mm@kvack.org>,
	<owner-linux-mm@kvack.org>
Subject: Re: [PATCH] mm: thp: unlock i_mmap before releasing split folios
Date: Mon, 13 Jul 2026 22:12:52 -0400	[thread overview]
Message-ID: <DJXXIBZJUYX7.BUUBV4PUAVKX@nvidia.com> (raw)
In-Reply-To: <alEg6ZPc-ao7s6o6@thinkstation>

On Fri Jul 10, 2026 at 1:17 PM EDT, Kiryl Shutsemau wrote:
> On Fri, Jul 10, 2026 at 11:56:00PM +0800, Hao Zhang wrote:
>> On Fri, Jul 10, 2026, Kiryl Shutsemau wrote:
>> > On Fri, Jul 10, 2026 at 03:13:44PM +0800, Hao Zhang wrote:
>> > > From: Hao Zhang <zhanghao1@kylinos.cn>
>> > > Date: Thu, 9 Jul 2026 16:30:00 +0800
>> > > 
>> > > __folio_split() takes mapping->i_mmap_rwsem while unmapping and
>> > > splitting a file-backed large folio. The lock is currently released
>> > > only after the split folios that are not returned locked to the caller
>> > > have been unlocked and put.
>> > > 
>> > > That leaves a lifetime hole. Once the split folios are unlocked and
>> > > their references are dropped, inode eviction can make progress through
>> > > the final page-cache truncation path. After the folios are removed from
>> > > the page cache and the last inode reference is dropped, the inode that
>> > > embeds the address_space can be freed after an RCU grace period. A later
>> > > i_mmap_unlock_read(mapping) then dereferences mapping->i_mmap_rwsem
>> > > from freed memory.
>> > > 
>> > > A possible race is:
>> > > 
>> > >   CPU0                                    CPU1
>> > >   __folio_split()
>> > >     i_mmap_lock_read(mapping)
>> > >     ...
>> > >     remap_page()
>> > >     folio_unlock(new_folio)
>> > >     free_folio_and_swap_cache(new_folio)
>> > >                                           evict inode
>> > >                                           truncate_inode_pages_final()
>> > >                                           destroy_inode()
>> > >                                           call_rcu()
>> > >                                           RCU callback frees inode
>> > >     i_mmap_unlock_read(mapping)
>> > > 
>> > > Release mapping->i_mmap_rwsem after the page-cache and reverse-mapping
>> > > work has completed, but before unlocking and putting any of the split
>> > > folios. Clear the local mapping pointer after the early unlock so the
>> > > common exit path does not unlock it a second time.
>> > 
>> > I don't buy this analysis. We still have the lock_at page which is locked
>> > and still in the page cache. Inode eviction cannot complete past
>> > truncate_inode_pages_final() until it is unlocked, which only happens
>> > after __folio_split() has released i_mmap_rwsem.
>> > 
>> > One possible path how this can be hit is if lock_at ends up in an
>> > after-split folio beyond EOF and __folio_freeze_and_split_unmapped()
>> > removes it from the page cache. Note the drop loop starts at
>> > folio_next(folio), so this requires splitting at a tail page (e.g.
>> > memory-failure) racing with truncate.
>> > 
>> > But that's not what your analysis claims. Please share the actual crash
>> > reports. Let's get to the bottom of the situation before changing the
>> > code.
>> > 
>> > -- 
>> >   Kiryl Shutsemau / Kirill A. Shutemov
>> >
>> 
>> Memory failure: 0x5a6a7: recovery action for clean LRU page: Recovered
>> Injecting memory failure for pfn 0x81bfe at process virtual address 0x200000ffe000
>> ==================================================================
>> BUG: KASAN: slab-use-after-free in __up_read+0x634/0x790 data/linux/kernel/locking/rwsem.c:1353
>> Read of size 8 at addr ffff88800b81c5e8 by task syz.2.11034/45874
>> 
>> CPU: 0 UID: 0 PID: 45874 Comm: syz.2.11034 Not tainted 7.0.0-rc3base+ #62 PREEMPT(full)
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.2-0-gea1b7a073390-prebuilt.qemu.org 04/01/2014
>> Call Trace:
>>  <TASK>
>>  __dump_stack data/linux/lib/dump_stack.c:94 [inline]
>>  dump_stack_lvl+0xbe/0x130 data/linux/lib/dump_stack.c:120
>>  print_address_description data/linux/mm/kasan/report.c:378 [inline]
>>  print_report+0xd1/0x660 data/linux/mm/kasan/report.c:482
>>  kasan_report+0xec/0x130 data/linux/mm/kasan/report.c:595
>>  __asan_report_load8_noabort+0x14/0x30 data/linux/mm/kasan/report_generic.c:381
>>  __up_read+0x634/0x790 data/linux/kernel/locking/rwsem.c:1353
>>  up_read+0x22/0x30 data/linux/kernel/locking/rwsem.c:1633
>>  i_mmap_unlock_read data/linux/include/linux/fs.h:537 [inline]
>>  __folio_split+0x732/0x1640 data/linux/mm/huge_memory.c:4100
>>  __split_huge_page_to_list_to_order+0x7b/0x140 data/linux/mm/huge_memory.c:4203
>>  split_huge_page_to_list_to_order data/linux/include/linux/huge_mm.h:385 [inline]
>>  split_huge_page_to_order data/linux/include/linux/huge_mm.h:389 [inline]
>>  try_to_split_thp_page+0xab/0x390 data/linux/mm/memory-failure.c:1675
>>  memory_failure+0x1394/0x26e0 data/linux/mm/memory-failure.c:2470
>>  madvise_inject_error data/linux/mm/madvise.c:1487 [inline]
>>  madvise_do_behavior+0x4ae/0x8a0 data/linux/mm/madvise.c:1925
>>  do_madvise+0x162/0x230 data/linux/mm/madvise.c:2028
>>  __do_sys_madvise data/linux/mm/madvise.c:2037 [inline]
>>  __se_sys_madvise data/linux/mm/madvise.c:2035 [inline]
>>  __x64_sys_madvise+0xae/0x120 data/linux/mm/madvise.c:2035
>>  x64_sys_call+0x1bed/0x25e0 data/linux/arch/x86/include/generated/asm/syscalls_64.h:29
>>  do_syscall_x64 data/linux/arch/x86/entry/syscall_64.c:63 [inline]
>>  do_syscall_64+0xe2/0x14e0 data/linux/arch/x86/entry/syscall_64.c:94
>>  entry_SYSCALL_64_after_hwframe+0x76/0x7e
>
> ...
>
>> Memory failure: 0x81bfe: recovery action for already truncated LRU page: Ignored
>
> This log line confirms my suspicion.
>
> The UAF fires at i_mmap_unlock_read() while try_to_split_thp_page()
> still holds the poisoned page locked. For the inode to be freed at that
> point, eviction must have completed truncation, which is impossible
> while a locked folio remains in the page cache. So the after-split folio
> containing lock_at was no longer in the page cache — and the only way to
> remove a locked folio is the split itself: the new_folio->index >= end
> drop in __folio_freeze_and_split_unmapped(). It requires lock_at to be a
> tail page, which is what memory-failure passes.
>
>         CPU0                                    CPU1
>   i_mmap_lock_read(mapping)
>   __folio_freeze_and_split_unmapped()
>     __filemap_remove_folio(lock_at)   <-- beyond EOF: leaves cache, stays LOCKED
>
>   /* no locked folio pins the inode */
>                                         evict()
>                                           truncate_inode_pages_final()  /* nothing to block on */
>                                           call_rcu() -> free inode
>
>   i_mmap_unlock_read(mapping)          <-- UAF: rwsem in freed inode
>
> Your fix moves i_mmap_unlock_read() out of the window, but I don't think
> it is complete: shmem_uncharge(mapping->host) a few lines up also touches
> the inode, and its nr_shmem_dropped guard is exactly the beyond-EOF drop
> that triggers this. So it is in the same freed window; the patch just
> relocates the one dereference KASAN caught.

I was discussing this issue with Codex after I saw Kriyl's patch and the
iput() issue raised by Sashiko[1]. Hao's patch might be still valid for
shmem_uncharge(), since all after-split folios are locked at
shmem_uncharge() and prevents inode from going away. So the rule is no
mapping/inode dereferences after the after-split folios unlock loop
begins.

But let me know if I miss anything. Thanks.

[1]
https://sashiko.dev/#/patchset/20260713170915.239819-1-kirill@shutemov.name
>
> This is a lifetime problem, not a lock-ordering one: __folio_split()
> relies on the caller's locked folio in the page cache to keep the inode
> alive, then removes that very folio. I'd rather fix it by pinning the
> inode explicitly -- igrab(mapping->host) up front, so the inode is
> alive), iput() after i_mmap_unlock_read(). That covers shmem_uncharge()
> and the rwsem both, and won't regress if someone adds another mapping
> deref later.
>
> See the patch below. Untested.
>
> Any opinions?
>
> Hao, could you give it a try?
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 2bccb0a53a0a..9bfa3a879453 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -3982,6 +3982,7 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
>  	bool is_anon = folio_test_anon(folio);
>  	struct address_space *mapping = NULL;
>  	struct anon_vma *anon_vma = NULL;
> +	struct inode *inode = NULL;
>  	int old_order = folio_order(folio);
>  	struct folio *new_folio, *next;
>  	int nr_shmem_dropped = 0;
> @@ -4053,6 +4054,20 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
>  		}
>  
>  		anon_vma = NULL;
> +
> +		/*
> +		 * The locked @lock_at folio keeps the inode alive: eviction
> +		 * cannot remove it from the page cache while it is locked. But
> +		 * the split drops it if it lies beyond EOF, after which we
> +		 * still touch @mapping (shmem_uncharge(), i_mmap_unlock_read()).
> +		 * Hold an inode reference across the split to be safe.
> +		 */
> +		inode = igrab(mapping->host);
> +		if (!inode) {
> +			/* Inode is being evicted; nothing to split. */
> +			ret = -EBUSY;
> +			goto out;
> +		}
>  		i_mmap_lock_read(mapping);
>  
>  		/*
> @@ -4135,6 +4150,8 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
>  	}
>  	if (mapping)
>  		i_mmap_unlock_read(mapping);
> +	if (inode)
> +		iput(inode);
>  out:
>  	xas_destroy(&xas);
>  	if (is_pmd_order(old_order))


-- 
Best Regards,
Yan, Zi



  reply	other threads:[~2026-07-14  2:13 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-10  7:13 [PATCH] mm: thp: unlock i_mmap before releasing split folios Hao Zhang
2026-07-10 11:16 ` Kiryl Shutsemau
2026-07-10 15:56   ` Hao Zhang
2026-07-10 17:04     ` Lorenzo Stoakes
2026-07-10 17:17     ` Kiryl Shutsemau
2026-07-14  2:12       ` Zi Yan [this message]
2026-07-14  2:15       ` Zi Yan
2026-07-14  3:44         ` Baolin Wang
2026-07-14 10:26         ` Kiryl Shutsemau
2026-07-14 11:29           ` David Hildenbrand (Arm)
2026-07-14 12:17             ` Kiryl Shutsemau
2026-07-14 12:34               ` David Hildenbrand (Arm)
2026-07-14  9:33       ` Hao Zhang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DJXXIBZJUYX7.BUUBV4PUAVKX@nvidia.com \
    --to=ziy@nvidia.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@kernel.org \
    --cc=hao_zhang_kdev@163.com \
    --cc=kirill@shutemov.name \
    --cc=linux-mm@kvack.org \
    --cc=ljs@kernel.org \
    --cc=owner-linux-mm@kvack.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox