Re: [PATCH] mm: thp: unlock i_mmap before releasing split folios

[Kiryl Shutsemau <kirill@shutemov.name>]

On Mon, Jul 13, 2026 at 10:15:40PM -0400, Zi Yan wrote:
> > 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.

I agree that Hao's fix works, but I don't think "all after-split folios
are locked" is the right rule: the beyond-EOF folios are locked, yet no
longer in the page cache, so they don't block eviction. What actually
pins the inode is @folio itself: the drop loop in
__folio_freeze_and_split_unmapped() starts at folio_next(folio), so
@folio always stays in the page cache -- even if it is entirely beyond
EOF -- and stays locked until the unlock loop.

But maybe we should sidestep the problem altogether and refuse the
split with -EBUSY if @lock_at is beyond @end. Then the caller's locked
folio is guaranteed to stay in the page cache and pins the inode for
the whole function. No ordering rules needed.

It is normally only reachable as a race with truncate (or eviction),
where failing the split is harmless. The exception is a folio left
straddling EOF by a failed split in truncate_inode_partial_folio():
memory_failure() on a beyond-EOF subpage of it would now fail to split
instead of dropping the poisoned page. That range is zeroed and
unreachable, so I think it is acceptable.

Any opinions?

-- 
  Kiryl Shutsemau / Kirill A. Shutemov