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

["David Hildenbrand (Arm)" <david@kernel.org>]

On 7/14/26 12:26, Kiryl Shutsemau wrote:
> 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?

I guess a deferred iput through some workqueue is not an option?

I'd prefer if we could keep folio splitting working even when the passed page is
EOF.

-- 
Cheers,

David