[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <5ce77812-8bef-4199-a951-71ff616dee5b@redhat.com>
Date: Thu, 22 May 2025 09:11:52 +0200
From: David Hildenbrand <david@...hat.com>
To: Shivank Garg <shivankg@....com>,
syzbot <syzbot+2b99589e33edbe9475ca@...kaller.appspotmail.com>,
Liam.Howlett@...cle.com, akpm@...ux-foundation.org,
baolin.wang@...ux.alibaba.com, dev.jain@....com,
linux-kernel@...r.kernel.org, linux-mm@...ck.org,
lorenzo.stoakes@...cle.com, npache@...hat.com, ryan.roberts@....com,
syzkaller-bugs@...glegroups.com, ziy@...dia.com,
Matthew Wilcox <willy@...radead.org>
Subject: Re: [syzbot] [mm?] WARNING in folio_large_mapcount
On 22.05.25 06:57, Shivank Garg wrote:
> On 5/20/2025 7:35 PM, David Hildenbrand wrote:
>> On 20.05.25 07:45, Shivank Garg wrote:
>>> On 5/19/2025 6:56 PM, David Hildenbrand wrote:
>>>> On 17.05.25 10:21, syzbot wrote:
>>>>> Hello,
>>>>>
>>>>> syzbot found the following issue on:
>>>>>
>>>>> HEAD commit: 627277ba7c23 Merge tag 'arm64_cbpf_mitigation_2025_05_08' ..
>>>>> git tree: upstream
>>>>> console output: https://syzkaller.appspot.com/x/log.txt?x=1150f670580000
>>>>> kernel config: https://syzkaller.appspot.com/x/.config?x=5929ac65be9baf3c
>>>>> dashboard link: https://syzkaller.appspot.com/bug?extid=2b99589e33edbe9475ca
>>>>> compiler: Debian clang version 20.1.2 (++20250402124445+58df0ef89dd6-1~exp1~20250402004600.97), Debian LLD 20.1.2
>>>>>
>>>>> Unfortunately, I don't have any reproducer for this issue yet.
>>>>>
>>>>> Downloadable assets:
>>>>> disk image: https://storage.googleapis.com/syzbot-assets/0a42ae72fe0e/disk-627277ba.raw.xz
>>>>> vmlinux: https://storage.googleapis.com/syzbot-assets/0be88297bb66/vmlinux-627277ba.xz
>>>>> kernel image: https://storage.googleapis.com/syzbot-assets/31808a4b1210/bzImage-627277ba.xz
>>>>>
>>>>> IMPORTANT: if you fix the issue, please add the following tag to the commit:
>>>>> Reported-by: syzbot+2b99589e33edbe9475ca@...kaller.appspotmail.com
>>>>>
>>>>> ------------[ cut here ]------------
>>>>> WARNING: CPU: 1 PID: 38 at ./include/linux/mm.h:1335 folio_large_mapcount+0xd0/0x110 include/linux/mm.h:1335
>>>>
>>>> This should be
>>>>
>>>> VM_WARN_ON_FOLIO(!folio_test_large(folio), folio);
>>>>
>>>>> Modules linked in:
>>>>> CPU: 1 UID: 0 PID: 38 Comm: khugepaged Not tainted 6.15.0-rc6-syzkaller-00025-g627277ba7c23 #0 PREEMPT(full)
>>>>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 05/07/2025
>>>>> RIP: 0010:folio_large_mapcount+0xd0/0x110 include/linux/mm.h:1335
>>>>> Code: 04 38 84 c0 75 29 8b 03 ff c0 5b 41 5e 41 5f e9 96 d2 2b 09 cc e8 d0 cb 99 ff 48 89 df 48 c7 c6 20 de 77 8b e8 a1 dc de ff 90 <0f> 0b 90 eb b6 89 d9 80 e1 07 80 c1 03 38 c1 7c cb 48 89 df e8 87
>>>>> RSP: 0018:ffffc90000af77e0 EFLAGS: 00010246
>>>>> RAX: e1fcb38c0ff8ce00 RBX: ffffea00014c8000 RCX: e1fcb38c0ff8ce00
>>>>> RDX: 0000000000000001 RSI: ffffffff8d9226df RDI: ffff88801e2fbc00
>>>>> RBP: ffffc90000af7b50 R08: ffff8880b8923e93 R09: 1ffff110171247d2
>>>>> R10: dffffc0000000000 R11: ffffed10171247d3 R12: 1ffffd4000299000
>>>>> R13: dffffc0000000000 R14: 0000000000000000 R15: dffffc0000000000
>>>>> FS: 0000000000000000(0000) GS:ffff8881261fb000(0000) knlGS:0000000000000000
>>>>> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>>> CR2: 00007ffe58f12dc0 CR3: 0000000030e04000 CR4: 00000000003526f0
>>>>> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>>>>> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>>>>> Call Trace:
>>>>> <TASK>
>>>>> folio_mapcount include/linux/mm.h:1369 [inline]
>>>>
>>>> And here we come through
>>>>
>>>> if (likely(!folio_test_large(folio))) {
>>>> ...
>>>> }
>>>> return folio_large_mapcount(folio);
>>>>
>>>>
>>>> So the folio is split concurrently. And I think there is nothing stopping it from getting freed.
>>>>
>>>> We do a xas_for_each() under RCU. So yes, this is racy.
>>>>
>>>> In collapse_file(), we re-validate everything.
>>>>
>>>> We could
>>>>
>>>> (A) Take proper pagecache locks
>>>>
>>>> (B) Try grabbing a temporary folio reference
>>>>
>>>> (C) Try snapshotting the folio
>>>>
>>>> Probably, in this code, (B) might be cleanest for now? Handling it just like other code in mm/filemap.c.
>>>>
>>>
>>> Hi,
>>
>> Hi,
>>
>>>
>>> I've implemented your suggestion (B) using folio_try_get().
>>> Could you please review if my patch looks correct?
>>
>> You should probably drop both comments, the code merely mimics what filemap.c does.
>>
>> Apart from that, nothing jumped at me.
>>
>
> Thank you. I have attached revised patch.
LGTM, please send it as a proper patch.
Not sure about which Fixes: tag.
I added that VM_WARN_ON_FOLIO in 05c5323b2a34 ("mm: track mapcount of
large folios in single value"), but the real problem is rather the
raciness of the code that probably dates back a bit further.
Maybe that code was always assumed to be okay when racing? Possibly.
Maybe let's just use 05c5323b2a34 unless we know that the raciness
resulted in some other problems earlier.
--
Cheers,
David / dhildenb
Powered by blists - more mailing lists