All of 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.