On 4/12/20 6:03 PM, Longpeng(Mike) wrote: > From: Longpeng <longpeng2@xxxxxxxxxx> > > Our machine encountered a panic(addressing exception) after run > for a long time and the calltrace is: > RIP: 0010:[<ffffffff9dff0587>] [<ffffffff9dff0587>] hugetlb_fault+0x307/0xbe0 > RSP: 0018:ffff9567fc27f808 EFLAGS: 00010286 > RAX: e800c03ff1258d48 RBX: ffffd3bb003b69c0 RCX: e800c03ff1258d48 > RDX: 17ff3fc00eda72b7 RSI: 00003ffffffff000 RDI: e800c03ff1258d48 > RBP: ffff9567fc27f8c8 R08: e800c03ff1258d48 R09: 0000000000000080 > R10: ffffaba0704c22a8 R11: 0000000000000001 R12: ffff95c87b4b60d8 > R13: 00005fff00000000 R14: 0000000000000000 R15: ffff9567face8074 > FS: 00007fe2d9ffb700(0000) GS:ffff956900e40000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: ffffd3bb003b69c0 CR3: 000000be67374000 CR4: 00000000003627e0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > Call Trace: > [<ffffffff9df9b71b>] ? unlock_page+0x2b/0x30 > [<ffffffff9dff04a2>] ? hugetlb_fault+0x222/0xbe0 > [<ffffffff9dff1405>] follow_hugetlb_page+0x175/0x540 > [<ffffffff9e15b825>] ? cpumask_next_and+0x35/0x50 > [<ffffffff9dfc7230>] __get_user_pages+0x2a0/0x7e0 > [<ffffffff9dfc648d>] __get_user_pages_unlocked+0x15d/0x210 > [<ffffffffc068cfc5>] __gfn_to_pfn_memslot+0x3c5/0x460 [kvm] > [<ffffffffc06b28be>] try_async_pf+0x6e/0x2a0 [kvm] > [<ffffffffc06b4b41>] tdp_page_fault+0x151/0x2d0 [kvm] > ... > [<ffffffffc06a6f90>] kvm_arch_vcpu_ioctl_run+0x330/0x490 [kvm] > [<ffffffffc068d919>] kvm_vcpu_ioctl+0x309/0x6d0 [kvm] > [<ffffffff9deaa8c2>] ? dequeue_signal+0x32/0x180 > [<ffffffff9deae34d>] ? do_sigtimedwait+0xcd/0x230 > [<ffffffff9e03aed0>] do_vfs_ioctl+0x3f0/0x540 > [<ffffffff9e03b0c1>] SyS_ioctl+0xa1/0xc0 > [<ffffffff9e53879b>] system_call_fastpath+0x22/0x27 > > For 1G hugepages, huge_pte_offset() wants to return NULL or pudp, but it > may return a wrong 'pmdp' if there is a race. Please look at the following > code snippet: > ... > pud = pud_offset(p4d, addr); > if (sz != PUD_SIZE && pud_none(*pud)) > return NULL; > /* hugepage or swap? */ > if (pud_huge(*pud) || !pud_present(*pud)) > return (pte_t *)pud; > > pmd = pmd_offset(pud, addr); > if (sz != PMD_SIZE && pmd_none(*pmd)) > return NULL; > /* hugepage or swap? */ > if (pmd_huge(*pmd) || !pmd_present(*pmd)) > return (pte_t *)pmd; > ... > > The following sequence would trigger this bug: > 1. CPU0: sz = PUD_SIZE and *pud = 0 , continue > 1. CPU0: "pud_huge(*pud)" is false > 2. CPU1: calling hugetlb_no_page and set *pud to xxxx8e7(PRESENT) > 3. CPU0: "!pud_present(*pud)" is false, continue > 4. CPU0: pmd = pmd_offset(pud, addr) and maybe return a wrong pmdp > However, we want CPU0 to return NULL or pudp in this case. > > We must make sure there is exactly one dereference of pud and pmd. > > Cc: Mike Kravetz <mike.kravetz@xxxxxxxxxx> > Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> > Cc: Jason Gunthorpe <jgg@xxxxxxxx> > Cc: Matthew Wilcox <willy@xxxxxxxxxxxxx> > Cc: Sean Christopherson <sean.j.christopherson@xxxxxxxxx> > Cc: stable@xxxxxxxxxxxxxxx > Signed-off-by: Longpeng <longpeng2@xxxxxxxxxx> Thank you for continuing to work this! Reviewed-by: Mike Kravetz <mike.kravetz@xxxxxxxxxx> -- Mike Kravetz
