Re: [syzbot] [mm?] kernel BUG in const_folio_flags (2)

[David Hildenbrand <david@redhat.com>]

On 28.11.24 13:02, David Hildenbrand wrote:
> On 28.11.24 12:42, Hillf Danton wrote:
>> On Thu, 28 Nov 2024 11:52:42 +0100 David Hildenbrand <david@redhat.com>
>>> On 23.11.24 08:31, syzbot wrote:
>>>> Hello,
>>>>
>>>> syzbot found the following issue on:
>>>>
>>>> HEAD commit:    9fb2cfa4635a Merge tag 'pull-ufs' of git://git.kernel.org/..
>>>> git tree:       upstream
>>>> console output: https://syzkaller.appspot.com/x/log.txt?x=10042930580000
>>>> kernel config:  https://syzkaller.appspot.com/x/.config?x=c4515f1b6a4e50b7
>>>> dashboard link: https://syzkaller.appspot.com/bug?extid=9f9a7f73fb079b2387a6
>>>> compiler:       gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
>>>> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=105ff2e8580000
>>>>
>>>> Downloadable assets:
>>>> disk image: https://storage.googleapis.com/syzbot-assets/7c0c61a15f60/disk-9fb2cfa4.raw.xz
>>>> vmlinux: https://storage.googleapis.com/syzbot-assets/3363d84eeb74/vmlinux-9fb2cfa4.xz
>>>> kernel image: https://storage.googleapis.com/syzbot-assets/2b1a270af550/bzImage-9fb2cfa4.xz
>>>>
>>>> IMPORTANT: if you fix the issue, please add the following tag to the commit:
>>>> Reported-by: syzbot+9f9a7f73fb079b2387a6@syzkaller.appspotmail.com
>>>>
>>>
>>> Staring at the console output:
>>>
>>> [  520.222112][ T7269] page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x1403 pfn:0x125be
>>
>> ->mapping is cleared for a order9 page
>   > >> [  520.362213][ T7269] head: order:9 mapcount:0 entire_mapcount:0
> nr_pages_mapped:0 pincount:0
>>> [  520.411963][ T7269] memcg:ffff88807c73c000
>>> [  520.492069][ T7269] flags: 0xfff00000000040(head|node=0|zone=1|lastcpupid=0x7ff)
>>> [  520.499844][ T7269] raw: 00fff00000000000 ffffea0000490001 dead000000000122 dead000000000400
>>> [  520.551982][ T7269] raw: 00000000000014d0 0000000000000000 00000000ffffffff 0000000000000000
>>> [  520.560912][ T7269] head: 00fff00000000040 0000000000000000 dead000000000122 0000000000000000
>>> [  520.672020][ T7269] head: 0000000000001245 0000000000000000 00000001ffffffff ffff88807c73c000
>>> [  520.735699][ T7269] head: 00fff00000000209 ffffea0000490001 ffffffffffffffff 0000000000000000
>>> [  520.901989][ T7269] head: 0000000000000200 0000000000000000 00000000ffffffff 0000000000000000
>>> [  520.991952][ T7269] page dumped because: VM_BUG_ON_PAGE(PageTail(page))
>>> [  521.086487][ T7269] page_owner tracks the page as allocated
>>> [  521.132208][ T7269] page last allocated via order 0, migratetype Movable, gfp_mask 0x3d24ca(GFP_TRANSHUGE|__GFP_NORETRY|
>>>
>>> ^order 0 looks wrong, but let;s not get distracted.
>>>
>>> __GFP_THISNODE), pid 7321, tgid 7321 (syz.1.194), ts 520201520231, free_ts 520193076092
>>> [  521.272012][ T7269]  post_alloc_hook+0x2d1/0x350
>>> [  521.276977][ T7269]  __alloc_pages_direct_compact+0x20e/0x590
>>> [  521.314428][ T7269]  __alloc_pages_noprof+0x182b/0x25a0
>>> [  521.319975][ T7269]  alloc_pages_mpol_noprof+0x282/0x610
>>> [  521.420092][ T7269]  folio_alloc_mpol_noprof+0x36/0xd0
>>> [  521.483167][ T7269]  vma_alloc_folio_noprof+0xee/0x1b0
>>> [  521.539677][ T7269]  do_huge_pmd_anonymous_page+0x258/0x2ae0
>>> ...
>>> [  521.851719][ T7269] page last free pid 7323 tgid 7321 stack trace:
>>> [  521.972611][ T7269]  free_unref_folios+0xa87/0x14f0
>>> [  521.977735][ T7269]  folios_put_refs+0x587/0x7b0
>>> [  522.072508][ T7269]  folio_batch_move_lru+0x2c4/0x3b0
>>> [  522.077794][ T7269]  __folio_batch_add_and_move+0x35b/0xc60
>>> [  522.191992][ T7269]  reclaim_folio_list+0x205/0x3a0
>>> [  522.197131][ T7269]  reclaim_pages+0x481/0x650
>>> [  522.201760][ T7269]  madvise_cold_or_pageout_pte_range+0x163b/0x20d0
>>> ...
>>>
>>>
>>> So we allocated a order-9 anonymous folio, but suddenly find it via shmem in the pagecache?
>>>
>>> Is this some crazy use-after-free / double-free, where we end up freeing a shmem folio
>>> that is still in the pagecache? Once freed, it gets merged in the buddy, and we then re-allocate
>>> it as part of a PMD THP; but shmem still finds it in the pagecache, and as the it's now suddenly
>>
>> It is not in the pagecache.
> 
> next_uptodate_folio() finds it there? Which is .. via the pagecache
> xas_next_entry()?
> 
> But good point on the mapping. If we would have freed a folio while
> still in the pagecache (before truncation), we'd likely have gotten an
> error from free_page_is_bad().
> 
> Well, unless check_pages_enabled() is false.

Ah, now I get it; at the point int time we check it actually isn't in 
the pagecache anymore. We perform a folio_test_locked() check before the 
folio_try_get(), which is wrong as the folio can get freed+reallocated 
in the meantime.

The easy fix would be:

diff --git a/mm/filemap.c b/mm/filemap.c
index 7c76a123ba18b..f61cf51c22389 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -3501,10 +3501,10 @@ static struct folio *next_uptodate_folio(struct 
xa_state *xas,
                         continue;
                 if (xa_is_value(folio))
                         continue;
-               if (folio_test_locked(folio))
-                       continue;
                 if (!folio_try_get(folio))
                         continue;
+               if (folio_test_locked(folio))
+                       goto skip;
                 /* Has the page moved or been split? */
                 if (unlikely(folio != xas_reload(xas)))
                         goto skip;


-- 
Cheers,

David / dhildenb