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 4E76DC7EE22 for ; Tue, 16 May 2023 00:16:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 95584900003; Mon, 15 May 2023 20:16:18 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 905ED900002; Mon, 15 May 2023 20:16:18 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7A6BA900003; Mon, 15 May 2023 20:16:18 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 6A67F900002 for ; Mon, 15 May 2023 20:16:18 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 356CA121326 for ; Tue, 16 May 2023 00:16:18 +0000 (UTC) X-FDA: 80794201236.28.A8120E1 Received: from mail-pl1-f178.google.com (mail-pl1-f178.google.com [209.85.214.178]) by imf22.hostedemail.com (Postfix) with ESMTP id 55706C000C for ; Tue, 16 May 2023 00:16:16 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=4DT4IrQP; spf=pass (imf22.hostedemail.com: domain of pcc@google.com designates 209.85.214.178 as permitted sender) smtp.mailfrom=pcc@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1684196176; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=T6gh2qYVut9aSylVwiYmVdq2jPzWcix3qnUjMW+9u1Y=; b=1+CNHqL2ucDknHWZ1jourEw2FMd59KVWGCTYR8Fm2VIbpzwEexqtBm9PtFbhOuelhxxiHV 2iS0RzD9TGQDQ2jWAqcTji9Vg0NJcNT9uHzpwOHOplLjzMVuwXABplZ6MTdzlx4Kul0wRK j7hyVnOjGQxidmsNp3apTR4POYVdUc8= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=4DT4IrQP; spf=pass (imf22.hostedemail.com: domain of pcc@google.com designates 209.85.214.178 as permitted sender) smtp.mailfrom=pcc@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1684196176; a=rsa-sha256; cv=none; b=TR+7TdxQAgFkWNvCGJaJYDFSzsMfHMgMLAoW8o9c70EKPDa70F3CHZZ0aiMbCrm00u9qnx FOvK9kPZUS9tOYOG9ox5pSWWtaigfNBnWyLJsqoCbq47Zvrg720JdcO+m6OCk1e8bpCbUs ikJ8Ic3iBTu1+gfZkNf4/6PdptLr9ZI= Received: by mail-pl1-f178.google.com with SMTP id d9443c01a7336-1ac65ab7432so784985ad.0 for ; Mon, 15 May 2023 17:16:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1684196175; x=1686788175; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=T6gh2qYVut9aSylVwiYmVdq2jPzWcix3qnUjMW+9u1Y=; b=4DT4IrQPyQsz4h+dcRk/i2LEM+ilSfmH36PEG55MInjjTBGxQELkAy2DYSBQDQwIaO 4X9iQxrb0bD6WqOfmbVD1TRc+OSr1/fhNSat4Lh1WFRxQfsuAIolN8800kDElXhNXelP LlqIpSwDtJPlkSMJL3mCFlcx7Mpde4o4iDk7xUzqFS4y8VdInEFgNXsvhb5OVozPBqPC P5nDQrkneFxH9jJ+OustYYE3W7BHQQzUaU7WXrV/bo2+IfYJL0t8d7cDO2VZ7sGKINON QPt9EIBsy+JHePYNsII5qP0NAJTzYMjlpMKd/qeQ7nDio77QEz76JhMEYmYceqmrZ+oQ Ji3Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684196175; x=1686788175; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=T6gh2qYVut9aSylVwiYmVdq2jPzWcix3qnUjMW+9u1Y=; b=AMZ0HLd1bSh187VjnEJpi1pM4JhD3pmTZNxxYmNq5VYgyW9Vl9jiBI2yhsSNXzxlj+ ScVvED4OUysciZT/nhI3kQ+hW8d1gW2b6/Z86CawjBsyVcdZ4NTYgWWAqoxlzYcJX8JL /v4H5/yzOYsQtpl+R/SwZ41ksrn1LXuwlaK7MPojDSox1y9QRZCX02WOZL+oJHd378lt vFl7Ze2GBX4OrmTp+DsfDbtiHhVoRDSn0f6iZTXVeuNDRFUClBncmQCrD5U9eVdkR6kW TiG+f2UWOKvvXKeC/BDy4PBQapIl/7iz5TptlOs/0z0h1DsQyli3gGxt9viyc5szsSye W5oQ== X-Gm-Message-State: AC+VfDw7cemvYDuNzXKQO9uD1aHti2LBDpUTSyR7gDMuzyI4jz/1s6y+ 3O6uDhQS/gx7YCSeAoaqM0OKfw== X-Google-Smtp-Source: ACHHUZ6F8d7WPH/3QMv++9pQyJNemJ8pH/5jv3HUHQFDdhHoClEdG/sj9VEr8QKBhfu3MpNJ0Ldv3Q== X-Received: by 2002:a17:903:2281:b0:1aa:dfdf:9232 with SMTP id b1-20020a170903228100b001aadfdf9232mr30882plh.16.1684196174906; Mon, 15 May 2023 17:16:14 -0700 (PDT) Received: from google.com ([2620:15c:2d3:205:c825:9c0b:b4be:8ee4]) by smtp.gmail.com with ESMTPSA id z21-20020aa791d5000000b006260526cf0csm12286564pfa.116.2023.05.15.17.16.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 15 May 2023 17:16:14 -0700 (PDT) Date: Mon, 15 May 2023 17:16:09 -0700 From: Peter Collingbourne To: David Hildenbrand Cc: Catalin Marinas , Qun-wei Lin =?utf-8?B?KOael+e+pOW0tCk=?= , linux-arm-kernel@lists.infradead.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, "surenb@google.com" , Chinwen Chang =?utf-8?B?KOW8temMpuaWhyk=?= , "kasan-dev@googlegroups.com" , Kuan-Ying Lee =?utf-8?B?KOadjuWGoOepjik=?= , Casper Li =?utf-8?B?KOadjuS4reamrik=?= , "gregkh@linuxfoundation.org" , vincenzo.frascino@arm.com, Alexandru Elisei , will@kernel.org, eugenis@google.com, Steven Price , stable@vger.kernel.org Subject: Re: [PATCH 1/3] mm: Move arch_do_swap_page() call to before swap_free() Message-ID: References: <20230512235755.1589034-1-pcc@google.com> <20230512235755.1589034-2-pcc@google.com> <7471013e-4afb-e445-5985-2441155fc82c@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7471013e-4afb-e445-5985-2441155fc82c@redhat.com> X-Rspam-User: X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: 55706C000C X-Stat-Signature: parrhk3iz8qkfnkpsznjcehzjkofcpq6 X-HE-Tag: 1684196176-9387 X-HE-Meta: U2FsdGVkX1/PJQcFTyz0K6aBQJNxjDuJBXHd1+/qXx/OPJQwteUuhuACGzOvGpd/CdYZUrjVp5YBI9b4kICJqiqrdTyahDg0cfqGOxQ39akJsTlcfhdfBnyoNvkMGHozwgajhPTp/sAvxluSwT2aMkJZZQzwvV2mu2j0C+ZGvFg+/uJXG4AAAtRlvZDWoQjXmxerqkD4Uof2YRcsRmd+VgUUOudI55+JYMy5oxl2If7XzURGNp8eiSbH0Sg2e+OdCQFOtn4gCRyVALK/rbYiin85kcybs549ifkNauYU9gYlfYvGduzGQabo1zQ9kLoK2SVn+g8dn4And3rHIT5UtE6mQUDOwBPumeWGGPCoKLaiJZKHMb/vUuiRpipasnAjtOaX6/oLkeZOSPtbZG/m4PNpgl3HT7RS4odJ+d+h/Wv+f7yvGMqoOC3cZLOFYKDlZWsa3H+q1MGGcYR24kYKs79q3L+W9BumzS4nqB8nRYHoWKX55TdsxQMNn1Nmr7nvQvxpMJ7uG/jKEk35BvtvkkhOtCWPjHldRU8sY+HpB0s7E+yzD2g/ZdKV9YHIOZT0JiprtRHgV6kV/2Z/ys6aoSzV0Z2I3kf7ky/JKoyXimOIPqcYk+uhqb6MvMrPbUfG6C9twRbcsEnWezyIO3a7l7LkDFZtuBjZsZlBJyaV+DMY+0UbElpGj/26yc1M92FE7nPywN7F4wiofoHMDYJnsK/w1ycEXLziaWsLoNs8EILud1rRFIG1JrZy7jnAQ4qr9t+SY0nWMmSTEhMSnVK0Vf9/6bxcG9LzqhvWc0zqJxkNHYoAa9Lf9UOSBM9bG3UlCT9dPBE/FiUwtq5RO7iADCl1nuuCAYk0VB7qL4T1zkNr1C+vjzKcPRjXfw/fd7ZEs9Vt3t+fNhxSKRkK+8GRWhatiKhQEPJUArIDTud4JORBoqMaG7LcXEvF+v2B5gcvwdudn8e6tM6F3EuIxVN N8aTyzr2 Dm+UucDVgWxOqAKZdpB3SG7bKtuBefMru8ngtGzv4SJO8e+8oqYnl+BwBwOncS+c57GTRKAWfMeF9gg4N75ysUfq6E37sWujB+rvWcHVGam9GdleWmjRFpRo0wEMfXpXI+qlStbceI2ZP6WwKbfX37bee1o+50E/LZdII+eaXPNW5j3NSG25lv5HQim8r9/UJlTQsnyGnYUjkOXpTRdFzy+5yAVb2MR1VlbOJ0VPDSehguTnq5USOibJMRNVcuxG1gkm0WenZ7g4Y8rQ5QRmxCZvxYRX1aVr6FYO+EmDFHJeqAMnRy2N4qcTEgel7FuY5tQ38uTvbjiEfuEm+VTKONFB/migndDTB+yYVbH0L/emnc0ukR7utRWNWQrblKzxeGju4oP6RfqYY7mFaAOGu2++9N08Jp+7rmZcEfUfFZ03QjrhDQxnTVoy6yu4NsXOqYIry6NKu0XZNpV0gTwqrqoUMnXe1pu4Uxl7z38dkZlgnv4Ckg8La7aBSS+0yCnC1rNwPtqFBgHjxUqv09eEmBMBSA765t2XBRjh5XEDKBAHiIUqpQ6h9XF5IhputKZ0k5X7GZjZiWeLbbPkD6dP8aP1JlL0zfQobrfHdeaUgNkJVOZvopC7XrBmKeq4toKz5LD01VbEO6JRmPX1NjgbyI2un6BeaYLTc1Z4I 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 Sat, May 13, 2023 at 05:29:53AM +0200, David Hildenbrand wrote: > On 13.05.23 01:57, Peter Collingbourne wrote: > > Commit c145e0b47c77 ("mm: streamline COW logic in do_swap_page()") moved > > the call to swap_free() before the call to set_pte_at(), which meant that > > the MTE tags could end up being freed before set_pte_at() had a chance > > to restore them. One other possibility was to hook arch_do_swap_page(), > > but this had a number of problems: > > > > - The call to the hook was also after swap_free(). > > > > - The call to the hook was after the call to set_pte_at(), so there was a > > racy window where uninitialized metadata may be exposed to userspace. > > This likely also affects SPARC ADI, which implements this hook to > > restore tags. > > > > - As a result of commit 1eba86c096e3 ("mm: change page type prior to > > adding page table entry"), we were also passing the new PTE as the > > oldpte argument, preventing the hook from knowing the swap index. > > > > Fix all of these problems by moving the arch_do_swap_page() call before > > the call to free_page(), and ensuring that we do not set orig_pte until > > after the call. > > > > Signed-off-by: Peter Collingbourne > > Suggested-by: Catalin Marinas > > Link: https://linux-review.googlesource.com/id/I6470efa669e8bd2f841049b8c61020c510678965 > > Cc: # 6.1 > > Fixes: ca827d55ebaa ("mm, swap: Add infrastructure for saving page metadata on swap") > > Fixes: 1eba86c096e3 ("mm: change page type prior to adding page table entry") > > I'm confused. You say c145e0b47c77 changed something (which was after above > commits), indicate that it fixes two other commits, and indicate "6.1" as > stable which does not apply to any of these commits. Sorry, the situation is indeed a bit confusing. - In order to make the arch_do_swap_page() hook suitable for fixing the bug introduced by c145e0b47c77, patch 1 addresses a number of issues, including fixing bugs introduced by ca827d55ebaa and 1eba86c096e3, but we haven't fixed the c145e0b47c77 bug yet, so there's no Fixes: tag for it yet. - Patch 2, relying on the fixes in patch 1, makes MTE install an arch_do_swap_page() hook (indirectly, by making arch_swap_restore() also hook arch_do_swap_page()), thereby fixing the c145e0b47c77 bug. - 6.1 is the first stable version in which all 3 commits in my Fixes: tags are present, so that is the version that I've indicated in my stable tag for this series. In theory patch 1 could be applied to older kernel versions, but it wouldn't fix any problems that we are facing with MTE (because it only fixes problems relating to the arch_do_swap_page() hook, which older kernel versions don't hook with MTE), and there are some merge conflicts if we go back further anyway. If the SPARC folks (the previous only user of this hook) want to fix these issues with ADI, they can propose their own backport. > > @@ -3959,7 +3960,6 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) > > VM_BUG_ON(!folio_test_anon(folio) || > > (pte_write(pte) && !PageAnonExclusive(page))); > > set_pte_at(vma->vm_mm, vmf->address, vmf->pte, pte); > > - arch_do_swap_page(vma->vm_mm, vma, vmf->address, pte, vmf->orig_pte); > > folio_unlock(folio); > > if (folio != swapcache && swapcache) { > > > You are moving the folio_free_swap() call after the folio_ref_count(folio) > == 1 check, which means that such (previously) swapped pages that are > exclusive cannot be detected as exclusive. Ack. I will fix this in v2. Peter