Re: [PATCH v2] mm/memory-failure: fix folio refcount leak and min_order_for_split() locking

[Tal Zussman]

On 10/30/26 6:29 PM, David Hildenbrand (Arm) wrote:
> hwpoison code can end up calling min_order_for_split() without holding
> the folio lock. There isn't really something that would prevent
> concurrent folio split. Consequently folio->mapping can get set to
> NULL after checking for "!folio->mapping", and if the compiler
> reloads folio->mapping, mapping_min_folio_order() would try to
> dereference NULL.
> 
> While very unlikely to happen in practice, let's just enforce that
> min_order_for_split() is called with the folio lock held. We can
> significantly cleanup the calling hwpoison code, and just get rid
> of try_to_split_thp_page() to hold the folio lock for a bit longer.
> 
> Just work on folios now, which further cleans up the code. We just
> have to be careful about doing the page_folio() after splitting, which
> we have to do already either way. Do not change the way we split for
> now, this needs more thought and should be done separately.
> 
> Cleaning this up we fix another issue: in soft_offline_in_use_page(), we
> would currently have leaked a folio reference.
> 
> In min_order_for_split(), document and assert that we need the folio lock.
> Drop the questionable VM_BUG_ON_PAGE(!page_count(p), p) check entirely.
> 
> While updating the comment in soft_offline_in_use_page(), just drop the
> "NOTE" part that does not add any value to current code.
> 
> The folio->mapping problem was identified by Sashiko, and Li Youhong
> reported it by sending a proposal fix.
> 
> This likely does not really warrant CCing stable, but I expect little
> conflicts when doing the backport, so let's just CC stable because of
> the refcount leak.
> 
> Reported-by: Li Youhong <liyouhong@xxxxxxxxxx>
> Closes: https://lore.kernel.org/r/20260804035828.2684059-1-dayou5941@xxxxxxx
> Fixes: 689b8986776c ("mm/memory-failure: improve large block size folio handling")
> Acked-by: Usama Arif <usama.arif@xxxxxxxxx>
> Reviewed-by: Lorenzo Stoakes (ARM) <ljs@xxxxxxxxxx>
> Reviewed-by: Zi Yan <ziy@xxxxxxxxxx>
> Reviewed-by: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>
> Acked-by: Miaohe Lin <linmiaohe@xxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>

I managed to trigger a UAF via folio->mapping due to the race with truncation
that this patch fixes (KASAN output below). Soft offline is preempted inside
min_order_for_split() after it's loaded folio->mapping. The file is then
unlinked and closed, the inode is freed after an RCU grace period, and the
subsequent mapping->flags read hits the freed shmem inode.

As noted, the potential folio->mapping NULL dereference is dependent on
compiler codegen. x86 GCC and Clang only load folio->mapping once on the
versions I tested, so I couldn't trigger it.

The above UAF was very, very hard to trigger, but it's less theoretical
than the NULL deref issue, so I think the Cc: stable is justified :)

Reviewed-by: Tal Zussman <tz2294@xxxxxxxxxxxx>
Tested-by: Tal Zussman <tz2294@xxxxxxxxxxxx>

KASAN splat:

BUG: KASAN: slab-use-after-free in min_order_for_split+0x78/0x80
Read of size 8 at addr ffff888114bce3d8 by task mfsplit/230

CPU: 0 UID: 0 PID: 230 Comm: mfsplit Not tainted 7.3.0-virtme #1 PREEMPT(lazy)
Call Trace:
 <TASK>
 dump_stack_lvl+0x66/0xa0
 print_report+0xd0/0x630
 kasan_report+0xe4/0x120
 min_order_for_split+0x78/0x80
 soft_offline_page+0x900/0xc70
 soft_offline_page_store+0xa0/0x100
 kernfs_fop_write_iter+0x2f5/0x4a0
 vfs_write+0x60f/0x1100
 __x64_sys_pwrite64+0x18a/0x1f0
 do_syscall_64+0xfa/0x5e0
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
 </TASK>

Allocated by task 229:
 kmem_cache_alloc_lru_noprof+0x1e0/0x5b0
 shmem_alloc_inode+0x21/0x40
 alloc_inode+0x5f/0x1f0
 new_inode+0x18/0x180
 __shmem_get_inode+0x173/0xdf0
 shmem_mknod+0xcb/0x240
 lookup_open+0xbb7/0x1760
 path_openat+0xc8b/0x1dd0
 do_file_open+0x1d2/0x400
 do_sys_openat2+0x126/0x170
 __x64_sys_openat+0x122/0x1e0

Freed by task 231:
 kmem_cache_free+0x29c/0x610
 rcu_core+0x691/0x1de0
 handle_softirqs+0x204/0x8e0
 __irq_exit_rcu+0x153/0x1e0
 irq_exit_rcu+0xe/0x20
 sysvec_apic_timer_interrupt+0x3e/0x80

Last potentially related work creation:
 __call_rcu_common.constprop.0+0x80/0x270
 evict+0x4da/0x910
 dentry_kill+0x1f1/0xa90
 dput.part.0+0xa0/0x180
 __fput+0x42a/0xaa0
 fput_close_sync+0xe7/0x1a0
 __x64_sys_close+0x7d/0xd0

The buggy address belongs to the object at ffff888114bcdf80
 which belongs to the cache shmem_inode_cache of size 1400
The buggy address is located 1112 bytes inside of
 freed 1400-byte region [ffff888114bcdf80, ffff888114bce4f8)