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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2B011ECAAA3 for ; Fri, 26 Aug 2022 14:39:14 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8786A6B0074; Fri, 26 Aug 2022 10:39:13 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 827816B0075; Fri, 26 Aug 2022 10:39:13 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6C7F5940007; Fri, 26 Aug 2022 10:39:13 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 5B2A36B0074 for ; Fri, 26 Aug 2022 10:39:13 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 2CB39C0E20 for ; Fri, 26 Aug 2022 14:39:13 +0000 (UTC) X-FDA: 79842001386.20.D2E8C7E Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf07.hostedemail.com (Postfix) with ESMTP id BCC5A40011 for ; Fri, 26 Aug 2022 14:39:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1661524752; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Ft0lQYVoEV6jBNoLlY/vGG48ZPsyM0mxyqwdiyDO9u0=; b=dcwGiRGbGSRAanFnVzgiQ4iesOcWmsWNGcLBixhLCFxtZUWtAzUJy02C54lyMP8MWLm7Fy hMsQogDXyR+/EDfW6jNn0zk0l9pxfdZAQi1/ZybKy3hgDG246Ro4f2F4ixy5mfOE7euq8h wWRxg8x+H87sF8uUEMy3bvs7NYgfDuo= Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-1-rAvknvxRNDKjVdVRfzMtqw-1; Fri, 26 Aug 2022 10:39:10 -0400 X-MC-Unique: rAvknvxRNDKjVdVRfzMtqw-1 Received: by mail-wr1-f70.google.com with SMTP id o3-20020adfa103000000b0022514e8e99bso190409wro.19 for ; Fri, 26 Aug 2022 07:39:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:subject:organization:from :references:cc:to:content-language:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc; bh=Ft0lQYVoEV6jBNoLlY/vGG48ZPsyM0mxyqwdiyDO9u0=; b=5sOKftlv8x/0VybD7LunrVeEKm7xinJ6xpvlI//UIMsleepqtncgw/YROwY6NUdtRt BUj7T+5cByp/WAXqrOmLfDnNTkVDvYIUPSVQjkZb9KVfs7piPuRVceJFrj7rmv8ODk6a SzM408CzIh/hbeCSu72lKGnReOkQ13g1XLVVdm3ox666Ez4v34ce3h+YyQHMT1WR9zKX 5G7JDN26KOu7qthBNrGROavm79F2UI6PUKC5xSn5ecrBDHlrQzKjB71rmc5FNUDPJC54 JO1anky3e0H/0otuu3/CPd4IV+rlBaDzFYq/icaygh5Gz/ogFiiKEraZmRVFWBYCMCGM RH9g== X-Gm-Message-State: ACgBeo1oIOB/iJheYmvzMsKUdwS+jEvEBBR+hlF0hGmP24+bRjXkjQX+ /2WifCcMwGwA3/HMkwFX9vvtrnJmzidqjdMYg91b1P+iSAo3tVQkcMLt57vsrYx1ol7xpDPYEBw 4R6ouMS4qbr0= X-Received: by 2002:a05:600c:1f1a:b0:3a6:2569:496b with SMTP id bd26-20020a05600c1f1a00b003a62569496bmr5450956wmb.176.1661524749691; Fri, 26 Aug 2022 07:39:09 -0700 (PDT) X-Google-Smtp-Source: AA6agR78RH87xC4FNL/JmgMsnjs3esusw3W4DDiJGKyeEE33BMY6II+9QepZiEzqEmrMzkzxyftCIg== X-Received: by 2002:a05:600c:1f1a:b0:3a6:2569:496b with SMTP id bd26-20020a05600c1f1a00b003a62569496bmr5450936wmb.176.1661524749334; Fri, 26 Aug 2022 07:39:09 -0700 (PDT) Received: from ?IPV6:2003:cb:c708:f600:abad:360:c840:33fa? (p200300cbc708f600abad0360c84033fa.dip0.t-ipconnect.de. [2003:cb:c708:f600:abad:360:c840:33fa]) by smtp.gmail.com with ESMTPSA id q12-20020adfcd8c000000b00225206dd595sm2100027wrj.86.2022.08.26.07.39.08 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 26 Aug 2022 07:39:08 -0700 (PDT) Message-ID: Date: Fri, 26 Aug 2022 16:39:08 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.12.0 To: Peter Xu Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, Andrew Morton , Huang Ying , stable@vger.kernel.org, Yu Zhao References: <20220823221138.45602-1-peterx@redhat.com> <411d7b8c-f914-875e-b397-856e6a894366@redhat.com> From: David Hildenbrand Organization: Red Hat Subject: Re: [PATCH] mm/mprotect: Only reference swap pfn page if type match In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1661524752; a=rsa-sha256; cv=none; b=gPeAmiW8Gk49obCGc/h/0DwjgfBM6HxBbbwo6IeZNtSX8MD1SDDO8AEnOEXWcZGpY2NGUA t5WmUuOazZT8NQ/CYgIHVjdS5sXKb89drbl6pLX+I56TQgZdkdsC4sBv+oWJhwIB3buFno t6zwcZJAoEWOZr5rtEBH1efGkm73vnc= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=dcwGiRGb; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf07.hostedemail.com: domain of david@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=david@redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1661524752; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Ft0lQYVoEV6jBNoLlY/vGG48ZPsyM0mxyqwdiyDO9u0=; b=2WQIJlcYF9dzwI3bZuVytOsJfEq00vUflbJjDI6uMxQeC8zcTJ/0V1XVTdicJdDp+WQY7c M8t4xxp3AtP2X/11Q8/O9JxoNg/Uo5jbt099zzjlwsIDuAHEdroLa99ymWqiq44NBiMomL 9SAfTojuwpSn61x7Pk69/pt3o28GP6w= X-Rspamd-Queue-Id: BCC5A40011 Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=dcwGiRGb; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf07.hostedemail.com: domain of david@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=david@redhat.com X-Rspam-User: X-Rspamd-Server: rspam01 X-Stat-Signature: z391n9twmpmijeetkku69swjsjna798d X-HE-Tag: 1661524752-457080 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 26.08.22 16:25, Peter Xu wrote: > On Fri, Aug 26, 2022 at 12:49:37PM +0200, David Hildenbrand wrote: >> On 24.08.22 00:11, Peter Xu wrote: >>> Yu Zhao reported a bug after the commit "mm/swap: Add swp_offset_pfn() to >>> fetch PFN from swap entry" added a check in swp_offset_pfn() for swap type [1]: >>> >>> kernel BUG at include/linux/swapops.h:117! >>> CPU: 46 PID: 5245 Comm: EventManager_De Tainted: G S O L 6.0.0-dbg-DEV #2 >>> RIP: 0010:pfn_swap_entry_to_page+0x72/0xf0 >>> Code: c6 48 8b 36 48 83 fe ff 74 53 48 01 d1 48 83 c1 08 48 8b 09 f6 >>> c1 01 75 7b 66 90 48 89 c1 48 8b 09 f6 c1 01 74 74 5d c3 eb 9e <0f> 0b >>> 48 ba ff ff ff ff 03 00 00 00 eb ae a9 ff 0f 00 00 75 13 48 >>> RSP: 0018:ffffa59e73fabb80 EFLAGS: 00010282 >>> RAX: 00000000ffffffe8 RBX: 0c00000000000000 RCX: ffffcd5440000000 >>> RDX: 1ffffffffff7a80a RSI: 0000000000000000 RDI: 0c0000000000042b >>> RBP: ffffa59e73fabb80 R08: ffff9965ca6e8bb8 R09: 0000000000000000 >>> R10: ffffffffa5a2f62d R11: 0000030b372e9fff R12: ffff997b79db5738 >>> R13: 000000000000042b R14: 0c0000000000042b R15: 1ffffffffff7a80a >>> FS: 00007f549d1bb700(0000) GS:ffff99d3cf680000(0000) knlGS:0000000000000000 >>> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >>> CR2: 0000440d035b3180 CR3: 0000002243176004 CR4: 00000000003706e0 >>> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 >>> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 >>> Call Trace: >>> >>> change_pte_range+0x36e/0x880 >>> change_p4d_range+0x2e8/0x670 >>> change_protection_range+0x14e/0x2c0 >>> mprotect_fixup+0x1ee/0x330 >>> do_mprotect_pkey+0x34c/0x440 >>> __x64_sys_mprotect+0x1d/0x30 >>> >>> It triggers because pfn_swap_entry_to_page() could be called upon e.g. a >>> genuine swap entry. >>> >>> Fix it by only calling it when it's a write migration entry where the page* >>> is used. >>> >>> [1] https://lore.kernel.org/lkml/CAOUHufaVC2Za-p8m0aiHw6YkheDcrO-C3wRGixwDS32VTS+k1w@mail.gmail.com/ >>> >>> Fixes: 6c287605fd56 ("mm: remember exclusively mapped anonymous pages with PG_anon_exclusive") >>> Cc: David Hildenbrand >>> Cc: >>> Reported-by: Yu Zhao >>> Signed-off-by: Peter Xu >>> --- >>> mm/mprotect.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/mm/mprotect.c b/mm/mprotect.c >>> index f2b9b1da9083..4549f5945ebe 100644 >>> --- a/mm/mprotect.c >>> +++ b/mm/mprotect.c >>> @@ -203,10 +203,11 @@ static unsigned long change_pte_range(struct mmu_gather *tlb, >>> pages++; >>> } else if (is_swap_pte(oldpte)) { >>> swp_entry_t entry = pte_to_swp_entry(oldpte); >>> - struct page *page = pfn_swap_entry_to_page(entry); >>> pte_t newpte; >>> >>> if (is_writable_migration_entry(entry)) { >>> + struct page *page = pfn_swap_entry_to_page(entry); >>> + >>> /* >>> * A protection check is difficult so >>> * just be safe and disable write >> >> >> Stumbling over the THP code, I was wondering if we also want to adjust change_huge_pmd() >> and hugetlb_change_protection. There are no actual swap entries, so I assume we're fine. >> >> >> diff --git a/mm/huge_memory.c b/mm/huge_memory.c >> index 482c1826e723..466364e7fc5f 100644 >> --- a/mm/huge_memory.c >> +++ b/mm/huge_memory.c >> @@ -1798,10 +1798,10 @@ int change_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma, >> #ifdef CONFIG_ARCH_ENABLE_THP_MIGRATION >> if (is_swap_pmd(*pmd)) { >> swp_entry_t entry = pmd_to_swp_entry(*pmd); >> - struct page *page = pfn_swap_entry_to_page(entry); >> >> VM_BUG_ON(!is_pmd_migration_entry(*pmd)); >> if (is_writable_migration_entry(entry)) { >> + struct page *page = pfn_swap_entry_to_page(entry); >> pmd_t newpmd; >> /* >> * A protection check is difficult so >> diff --git a/mm/hugetlb.c b/mm/hugetlb.c >> index 2480ba627aa5..559465fae5cd 100644 >> --- a/mm/hugetlb.c >> +++ b/mm/hugetlb.c >> @@ -6370,9 +6370,9 @@ unsigned long hugetlb_change_protection(struct vm_area_struct *vma, >> } >> if (unlikely(is_hugetlb_entry_migration(pte))) { >> swp_entry_t entry = pte_to_swp_entry(pte); >> - struct page *page = pfn_swap_entry_to_page(entry); >> >> if (!is_readable_migration_entry(entry)) { >> + struct page *page = pfn_swap_entry_to_page(entry); >> pte_t newpte; >> >> if (PageAnon(page)) >> >> >> @Peter, what's your thought? > > IMHO they're not needed? > > The rule is simple in my mind: we should only pass in a pfn-typed swap > entry into pfn_swap_entry_to_page() (or the new swp_offset_pfn()), or it's > a violation of the API. In these two cases they do not violate the API and > they're always safe because they're guaranteed to be pfn swap entries when > calling. I was wondering about extreme corner cases regarding the struct page. Assume we have a hwpoison_entry that pointed at a valid struct page. We can succeed in offlining+removing the section it's located on (I was recently challenging if we want to keep that behavior as it's really shaky already), freeing the relevant memmap entry and the memory section. pfn_swap_entry_to_page() -> pfn_to_page() would be problematic if there is no memmap anymore. I assume it's ok to always call it for is_pfn_swap_entry(), but in the PMD case we only check for is_swap_pmd()? Isn't that problematic? I was confused by the hugetlb case, it's indeed fine as we check for is_hugetlb_entry_migration(). -- Thanks, David / dhildenb