From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4C434C1975A for ; Mon, 23 Mar 2020 02:04:05 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id D9B1020714 for ; Mon, 23 Mar 2020 02:04:04 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D9B1020714 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=huawei.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 6D1EE6B0005; Sun, 22 Mar 2020 22:04:04 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6828C8E0003; Sun, 22 Mar 2020 22:04:04 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 597F46B0007; Sun, 22 Mar 2020 22:04:04 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0167.hostedemail.com [216.40.44.167]) by kanga.kvack.org (Postfix) with ESMTP id 3E2E56B0005 for ; Sun, 22 Mar 2020 22:04:04 -0400 (EDT) Received: from smtpin07.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 4C0C3824805A for ; Mon, 23 Mar 2020 02:04:04 +0000 (UTC) X-FDA: 76624981608.07.lace65_64c5721c484e X-HE-Tag: lace65_64c5721c484e X-Filterd-Recvd-Size: 7807 Received: from huawei.com (szxga06-in.huawei.com [45.249.212.32]) by imf21.hostedemail.com (Postfix) with ESMTP for ; Mon, 23 Mar 2020 02:04:03 +0000 (UTC) Received: from DGGEMS413-HUB.china.huawei.com (unknown [172.30.72.59]) by Forcepoint Email with ESMTP id 968B961BB91CCDCFEC8E; Mon, 23 Mar 2020 10:03:39 +0800 (CST) Received: from [10.173.228.124] (10.173.228.124) by smtp.huawei.com (10.3.19.213) with Microsoft SMTP Server (TLS) id 14.3.487.0; Mon, 23 Mar 2020 10:03:33 +0800 Subject: Re: [PATCH v2] mm/hugetlb: fix a addressing exception caused by huge_pte_offset() To: Mike Kravetz CC: , , , , , , , , Matthew Wilcox , "Sean Christopherson" , References: <1582342427-230392-1-git-send-email-longpeng2@huawei.com> <51a25d55-de49-4c0a-c994-bf1a8cfc8638@oracle.com> From: "Longpeng (Mike, Cloud Infrastructure Service Product Dept.)" Message-ID: <5700f44e-9df9-1b12-bc29-68e0463c2860@huawei.com> Date: Mon, 23 Mar 2020 10:03:33 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <51a25d55-de49-4c0a-c994-bf1a8cfc8638@oracle.com> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.173.228.124] X-CFilter-Loop: Reflected X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On 2020/3/22 7:38, Mike Kravetz wrote: > On 2/21/20 7:33 PM, Longpeng(Mike) wrote: >> From: Longpeng >> >> Our machine encountered a panic(addressing exception) after run >> for a long time and the calltrace is: >> RIP: 0010:[] [] 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: >> [] ? unlock_page+0x2b/0x30 >> [] ? hugetlb_fault+0x222/0xbe0 >> [] follow_hugetlb_page+0x175/0x540 >> [] ? cpumask_next_and+0x35/0x50 >> [] __get_user_pages+0x2a0/0x7e0 >> [] __get_user_pages_unlocked+0x15d/0x210 >> [] __gfn_to_pfn_memslot+0x3c5/0x460 [kvm] >> [] try_async_pf+0x6e/0x2a0 [kvm] >> [] tdp_page_fault+0x151/0x2d0 [kvm] >> [] ? vmx_vcpu_run+0x2ec/0xc80 [kvm_intel] >> [] ? vmx_vcpu_run+0x2f8/0xc80 [kvm_intel] >> [] kvm_mmu_page_fault+0x31/0x140 [kvm] >> [] handle_ept_violation+0x9e/0x170 [kvm_intel] >> [] vmx_handle_exit+0x2bc/0xc70 [kvm_intel] >> [] ? __vmx_complete_interrupts.part.73+0x80/0xd0 [kvm_intel] >> [] ? vmx_vcpu_run+0x490/0xc80 [kvm_intel] >> [] vcpu_enter_guest+0x7be/0x13a0 [kvm] >> [] ? kvm_check_async_pf_completion+0x8e/0xb0 [kvm] >> [] kvm_arch_vcpu_ioctl_run+0x330/0x490 [kvm] >> [] kvm_vcpu_ioctl+0x309/0x6d0 [kvm] >> [] ? dequeue_signal+0x32/0x180 >> [] ? do_sigtimedwait+0xcd/0x230 >> [] do_vfs_ioctl+0x3f0/0x540 >> [] SyS_ioctl+0xa1/0xc0 >> [] system_call_fastpath+0x22/0x27 >> >> ( The kernel we used is older, but we think the latest kernel also has this >> bug after dig into this problem. ) >> >> 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. >> >> We can avoid this race by read the pud only once. What's more, we also use >> READ_ONCE to access the entries for safe(e.g. avoid the compilier mischief) >> >> Cc: Matthew Wilcox >> Cc: Sean Christopherson >> Cc: Mike Kravetz >> Cc: stable@vger.kernel.org >> Signed-off-by: Longpeng > > Andrew dropped this patch from his tree which caused me to go back and > look at the status of this patch/issue. > > It is pretty obvious that code in the current huge_pte_offset routine > is racy. I checked out the assembly code produced by my compiler and > verified that the line, > > if (pud_huge(*pud) || !pud_present(*pud)) > > does actually dereference *pud twice. So, the value could change between > those two dereferences. Longpeng (Mike) could easlily recreate the issue > if he put a delay between the two dereferences. I believe the only > reservations/concerns about the patch below was the use of READ_ONCE(). > Is that correct? > Hi Mike, It seems I've missed your another mail in my client, I found it here (https://lkml.org/lkml/2020/2/27/1927) just now. I think we have reached an agreement that the pud/pmd need READ_ONCE in huge_pte_offset() and disagreement is whether the pgd/p4d also need READ_ONCE, right ? > Are there any objections to the patch if the READ_ONCE() calls are removed? > Because the pgd/p4g are only accessed and dereferenced once here, so some guys want to remove it. But we must make sure they are *really* accessed once, in other words, this makes we need to care about both the implementation of pgd_present/p4d_present and the behavior of any compiler, for example: ''' static inline int func(int val) { return subfunc1(val) & subfunc2(val); } func(*p); // int *p ''' We must make sure there's no strange compiler to generate an assemble code that access and dereference 'p' more than once. I've not found any backwards with READ_ONCE here. However, if you also agree to remove READ_ONCE around pgd/p4d, I'll do. > Longpeng (Mike), can you recreate the issue by adding the delay and removing > the READ_ONCE() calls? > I think remove the READ_ONCE around pgd/p4d won't cause any fucntional change. --- Regards, Longpeng(Mike)