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 39650C369AB for ; Mon, 21 Apr 2025 14:48:14 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 145056B0005; Mon, 21 Apr 2025 10:48:13 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 0F5886B0007; Mon, 21 Apr 2025 10:48:13 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id F25326B0008; Mon, 21 Apr 2025 10:48:12 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id CE1F36B0005 for ; Mon, 21 Apr 2025 10:48:12 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id B549CB52F3 for ; Mon, 21 Apr 2025 14:48:11 +0000 (UTC) X-FDA: 83358331182.22.9B66428 Received: from fanzine2.igalia.com (fanzine.igalia.com [178.60.130.6]) by imf30.hostedemail.com (Postfix) with ESMTP id 908E68000D for ; Mon, 21 Apr 2025 14:48:09 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=igalia.com header.s=20170329 header.b=NvnrrWqB; dmarc=pass (policy=none) header.from=igalia.com; spf=pass (imf30.hostedemail.com: domain of gavinguo@igalia.com designates 178.60.130.6 as permitted sender) smtp.mailfrom=gavinguo@igalia.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1745246890; a=rsa-sha256; cv=none; b=GirNo7+HpzDe+w1p0GIMGdUtAj8HNoZYar5rLQiwijR8NEAs3/HCh4Xm4be+6buuyCkXMU NKkVwNNss0/nY5P4yneqt0obfHMEG4T/bZIrl9fVN/OBf3QeX0sssGzi7YgVIpoOXRjLpv itrFCOYWU/Bf0T3U6YRJgf1vLDgDd1U= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=igalia.com header.s=20170329 header.b=NvnrrWqB; dmarc=pass (policy=none) header.from=igalia.com; spf=pass (imf30.hostedemail.com: domain of gavinguo@igalia.com designates 178.60.130.6 as permitted sender) smtp.mailfrom=gavinguo@igalia.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1745246890; 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=kwVVw2CqIRRe0Myaq1Inek1FYVeiqCcyjpy3VoSZ7XY=; b=BHt0eZKSsxttj/5qqY2gIsKAKngJn6F4qR41ZF3eCFBl/rqFymFGpeMF9AezVHXSJEckZ4 O3765N+bfh8dhxOrIBNP9jPuPnqtPJ4kkUvJZi6/2fr4DV89rnLiSfS60CNDy5zQO6oAlE aeGpzdk/BFceXR4AkPSFakmqU4qt2zs= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:From: References:Cc:To:Subject:MIME-Version:Date:Message-ID:Sender:Reply-To: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=kwVVw2CqIRRe0Myaq1Inek1FYVeiqCcyjpy3VoSZ7XY=; b=NvnrrWqB8J2UENYzRduUt5rh6M HGubS1PYlulbA5D5rAiOfo5eEOVjSt0k0FezS2GHeKNRMQG+Q/qT36/9PFNAPYRGKgesNiduV+q8U dHbX9zEbmfh3b76m2sOvd9WaroSythbTkir+oi8DcbJBSA+djJb/IwbjJ7ZdUMgdcs89+/g03spFO op/X/nag1CXf9JVZYWjltWttrSKsRdhd6pukWfNDSrzTrzk8O+ukOtZwVOdJMfQX5wyCqZaUE5HLU JCp3piW6Z0xrJ7XnJjol2mYYwBqcAK193GBTIWr0eF50Znfrhs2NIbD8rV2ESfB379bx0AA2JSpWa N5u9F5PQ==; Received: from 114-44-233-154.dynamic-ip.hinet.net ([114.44.233.154] helo=[192.168.1.104]) by fanzine2.igalia.com with esmtpsa (Cipher TLS1.3:ECDHE_X25519__RSA_PSS_RSAE_SHA256__AES_128_GCM:128) (Exim) id 1u6sRA-0060Jw-Ve; Mon, 21 Apr 2025 16:47:57 +0200 Message-ID: Date: Mon, 21 Apr 2025 22:47:50 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2] mm/huge_memory: fix dereferencing invalid pmd migration entry To: Gavin Shan , David Hildenbrand , linux-mm@kvack.org, akpm@linux-foundation.org Cc: willy@infradead.org, ziy@nvidia.com, linmiaohe@huawei.com, hughd@google.com, revest@google.com, kernel-dev@igalia.com, linux-kernel@vger.kernel.org, stable@vger.kernel.org References: <20250418085802.2973519-1-gavinguo@igalia.com> Content-Language: en-US From: Gavin Guo In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspamd-Server: rspam11 X-Rspamd-Queue-Id: 908E68000D X-Stat-Signature: im9fcnzzjncua84b8uf5353rdbxugkni X-Rspam-User: X-HE-Tag: 1745246889-366172 X-HE-Meta: U2FsdGVkX1/ZSBMSm4mUDr/Qhj1G4YVoQedMremTccDzbYrDjp5hbdnAdAf7RrIyWxWyB6OqNRiuQmIpjOoxzE8mh5dZ2UoHNtcaeo3weOD7HYZ+PFLdD2Atp/PlQe7GDS8EB/Nfh24WlscuKuBfne5SXQz8Prh4/aRGbV5T6SFDWvBqHTb0a7dWoRtU5ZNDpnRObgTowlCun1DLg05eQx+Xx8cZaoeC+5cSrds1+RAh6NOVxccZeggibsv/CH1erNKUTaaDT4NSluM+MF85TJ7V3LYJBVjV4uZwyU+f+sjQgzzKfvIS6Hf6DozjUJTXpkHT7fQyeeuO4M5c4UphH5gVs2+EmjBlqNFDWS7QqhhNJY+J6uURm+cfeMjNIIMdXBo3am//CbHSj8ZDmEwqZCMFjEnbMz6sIh7P8qISn5sBCli4bLzPpsXKLFsumLMTJA/2F/YeKNXn+86Yylkb4PIG244Myvevrtkenpkn/75sldR9jJAjtISV1MlzSsOVgTZ5gphAadAuRBtrjPmjj/wuvP7IfmZIm2PwaxGX3W0l42xzntS0HKuNgXRbxykYZ0hAbKE0mDP1N7knTkUKKNG5ViABPTD4ohjp2EA1kMRQwdSaHms2vCVl5CBDAB5X+n0vebudO04y1Nb+6eKcLANOnp/rm01uvNKmiG/acrubENCaR5xXxMcKpne9g9b/fgPx6aoWVI77nXuFhp85BNdjlhzlzt7wLlax7CO5a/fwHuDMK7LEbW3dYWV6X1q0eOEex07tuIKZLQu3yPw1StCHDt0Izz7fQwaO86sJG0+r5b1NDpBzzxlRdJ81qtbIPFC/HHcZzzXIuqvolIhmZcirWUDQJhpKJkFI0ynYdWByo7vSA2osR3bd5D100wJGBwAlJNPK8olyLbwSl22fHczZ3cV4bgDa8wYWdetozGY9rdEgXAT7P2GUJJomB8A8bz6G5KUEVmmwa39Byzl rxPdBmxy hXd+b/kcItz7INpboGt25vPEmDZeOY8Pmif1JWChaam7HwiqqteFYnc760rp7X/lpoSoXyaCtxQFbMr1J1fTijgPUXrw/N9jrDMYFOgZoRFuKC3jqY+WDDO1YH60Ctmrcs+A+Q/gKWE71OrWhqun8liW4EpC8xhNNJsowc4ZEIJtwJDqifQFT82uSl8F7B7riOQCnPjktqcJtJspobCd92Ue9zlhsvZexwKALVl+L5XSup6PiJT3rrzppHpbUKm2QibWrq1XDzbcVUjQiCq8Emn3lzv1uHi+WOx9pp/7WSjkvUtjZD0fYyzlguQoKo5JURXXKH6+iRMohFSDga6/k5g5/45s+wTcf3Qq6Y1XahwWqAgyhUGnuC8DE1xS+D6wSEAcWw5L3OyQD04JaxobO7c+l5O/4bFJfeY4zQZkUwdDunwJAvxn7THlPZYf7R++JJWsNyVUkvuBXLDLqHQ30Y+CDKjAGdwLZi7Hmr5k8VcXW3/0= 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: List-Subscribe: List-Unsubscribe: On 4/19/25 07:53, Gavin Shan wrote: > Hi Gavin, > > On 4/18/25 8:42 PM, David Hildenbrand wrote: >> On 18.04.25 10:58, Gavin Guo wrote: >>> When migrating a THP, concurrent access to the PMD migration entry >>> during a deferred split scan can lead to a invalid address access, as >>> illustrated below. To prevent this page fault, it is necessary to check >>> the PMD migration entry and return early. In this context, there is no >>> need to use pmd_to_swp_entry and pfn_swap_entry_to_page to verify the >>> equality of the target folio. Since the PMD migration entry is locked, >>> it cannot be served as the target. >>> >>> Mailing list discussion and explanation from Hugh Dickins: >>> "An anon_vma lookup points to a location which may contain the folio of >>> interest, but might instead contain another folio: and weeding out those >>> other folios is precisely what the "folio != pmd_folio((*pmd)" check >>> (and the "risk of replacing the wrong folio" comment a few lines above >>> it) is for." >>> >>> BUG: unable to handle page fault for address: ffffea60001db008 >>> CPU: 0 UID: 0 PID: 2199114 Comm: tee Not tainted 6.14.0+ #4 NONE >>> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3- >>> debian-1.16.3-2 04/01/2014 >>> RIP: 0010:split_huge_pmd_locked+0x3b5/0x2b60 >>> Call Trace: >>> >>> try_to_migrate_one+0x28c/0x3730 >>> rmap_walk_anon+0x4f6/0x770 >>> unmap_folio+0x196/0x1f0 >>> split_huge_page_to_list_to_order+0x9f6/0x1560 >>> deferred_split_scan+0xac5/0x12a0 >>> shrinker_debugfs_scan_write+0x376/0x470 >>> full_proxy_write+0x15c/0x220 >>> vfs_write+0x2fc/0xcb0 >>> ksys_write+0x146/0x250 >>> do_syscall_64+0x6a/0x120 >>> entry_SYSCALL_64_after_hwframe+0x76/0x7e >>> >>> The bug is found by syzkaller on an internal kernel, then confirmed on >>> upstream. >>> >>> Fixes: 84c3fc4e9c56 ("mm: thp: check pmd migration entry in common >>> path") >>> Cc: stable@vger.kernel.org >>> Signed-off-by: Gavin Guo >>> Acked-by: David Hildenbrand >>> Acked-by: Hugh Dickins >>> Acked-by: Zi Yan >>> Link: https://lore.kernel.org/all/20250414072737.1698513-1- >>> gavinguo@igalia.com/ >>> --- >>> V1 -> V2: Add explanation from Hugh and correct the wording from page >>> fault to invalid address access. >>> >>>   mm/huge_memory.c | 18 ++++++++++++++---- >>>   1 file changed, 14 insertions(+), 4 deletions(-) >>> > > Reviewed-by: Gavin Shan > >>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c >>> index 2a47682d1ab7..0cb9547dcff2 100644 >>> --- a/mm/huge_memory.c >>> +++ b/mm/huge_memory.c >>> @@ -3075,6 +3075,8 @@ static void __split_huge_pmd_locked(struct >>> vm_area_struct *vma, pmd_t *pmd, >>>   void split_huge_pmd_locked(struct vm_area_struct *vma, unsigned >>> long address, >>>                  pmd_t *pmd, bool freeze, struct folio *folio) >>>   { >>> +    bool pmd_migration = is_pmd_migration_entry(*pmd); >>> + >>>       VM_WARN_ON_ONCE(folio && !folio_test_pmd_mappable(folio)); >>>       VM_WARN_ON_ONCE(!IS_ALIGNED(address, HPAGE_PMD_SIZE)); >>>       VM_WARN_ON_ONCE(folio && !folio_test_locked(folio)); >>> @@ -3085,10 +3087,18 @@ void split_huge_pmd_locked(struct >>> vm_area_struct *vma, unsigned long address, >>>        * require a folio to check the PMD against. Otherwise, there >>>        * is a risk of replacing the wrong folio. >>>        */ >>> -    if (pmd_trans_huge(*pmd) || pmd_devmap(*pmd) || >>> -        is_pmd_migration_entry(*pmd)) { >>> -        if (folio && folio != pmd_folio(*pmd)) >>> -            return; >>> +    if (pmd_trans_huge(*pmd) || pmd_devmap(*pmd) || pmd_migration) { >>> +        if (folio) { >>> +            /* >>> +             * Do not apply pmd_folio() to a migration entry; and >>> +             * folio lock guarantees that it must be of the wrong >>> +             * folio anyway. >>> +             */ >>> +            if (pmd_migration) >>> +                return; >>> +            if (folio != pmd_folio(*pmd)) >>> +                return; >> >> Nit: just re-reading, I would have simply done >> >> if (pmd_migration || folio != pmd_folio(*pmd) >>      return; >> >> Anyway, this will hopefully get cleaned up soon either way, so I don't >> particularly mind. :) >> > > If v3 is needed to fix Zi's comments (commit log improvement), it can be > improved > slightly based on David's suggestion, to avoid another nested if > statement. Otherwise, > it's fine since it needs to be cleaned up soon. > >     /* >      * Do not apply pmd_folio() to a migration entry, and folio lock >      * guarantees that it must be of the wrong folio anyway. >      */ >     if (folio && (pmd_migration || folio != pmd_filio(*pmd)) >         return; > > Thanks, > Gavin > > Gavin, thank you for the review as well. I submitted v3 including your suggestion with David's indentation idea and Zi's commit log fix.