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 872CBC77B7D for ; Tue, 16 May 2023 02:35:34 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 21981900006; Mon, 15 May 2023 22:35:34 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 17C84900002; Mon, 15 May 2023 22:35:34 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 01CA8900006; Mon, 15 May 2023 22:35:33 -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 E68BC900002 for ; Mon, 15 May 2023 22:35:33 -0400 (EDT) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id C156CAF42C for ; Tue, 16 May 2023 02:35:33 +0000 (UTC) X-FDA: 80794552146.18.360F355 Received: from mail-pl1-f170.google.com (mail-pl1-f170.google.com [209.85.214.170]) by imf11.hostedemail.com (Postfix) with ESMTP id DFFCF4000C for ; Tue, 16 May 2023 02:35:31 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=Y9M5TADW; spf=pass (imf11.hostedemail.com: domain of pcc@google.com designates 209.85.214.170 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=1684204531; a=rsa-sha256; cv=none; b=f1JcW0zszU6hyxGDC+ULlEV0S1LE8VaLRjR8S0lVZ1zozA+1TGAi/hkQ4DZJSNe4XP8WTD YKrdIuHPZBDhbt1R0cN5+eAvFtQTxBg2QdFb1mze/3e1mrwm4iHmgPknij/dQ84kCg2BiP dpE5FDbN0+P3ZqzbtbG3omRO9qexa4Y= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=Y9M5TADW; spf=pass (imf11.hostedemail.com: domain of pcc@google.com designates 209.85.214.170 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=1684204531; 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=LMBjYQfNrE6XjpDNQgsEFZW5LIEYARmq7ri2dsjhSck=; b=RyWHi+2J6E2imV1YRkUxSXUWxDPy+EcE/l+zekuVwa3OhWG6zPZrPdqaQuyQvsFcqjoWtE kg+Od8muA/eJ0IU24XDT5r/NdpZIX7TxelVdE8VfWKU60olke9QxN+vMMhhwKagfMkJyDC B8mCFNPB9sSpPPuV0cvl8W0OhroaPFQ= Received: by mail-pl1-f170.google.com with SMTP id d9443c01a7336-1aae90f5ebcso772865ad.1 for ; Mon, 15 May 2023 19:35:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1684204531; x=1686796531; 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=LMBjYQfNrE6XjpDNQgsEFZW5LIEYARmq7ri2dsjhSck=; b=Y9M5TADWZmKtWP/pjJc/AL6uypfuLC3F175I1lHhKkjmCaaG0uqoZBHO8dNAmzCurF mSuWZAiDqzx99dwMuAC3x1Sm3in8ctWriOZNyqdyM7/l2GYAbTStuAUCVC/TGugUIbXB VIyiv88/gu3mEghvmAZsxlsqlEgqy5tEtiv5+5GzubDW94Sn9J2GxUJ22Olh+HZPtgGG O9YC/yXBcZOdAxFeBxjmQInlps6eDlKo0aZymWEcgDqLH/dXkhtGv5b5bRv4BUdiPtI3 ZrS7uKquQVk1kRRSUApyhR44WIY4nh6IJeRqRjIPyPBkGp/x0+5jbKTcgKFcbPEEePwf SGxQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684204531; x=1686796531; 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=LMBjYQfNrE6XjpDNQgsEFZW5LIEYARmq7ri2dsjhSck=; b=RbIe/8titqPDcAKB1/fLmcHBpeTiig7YWxCL5x00Mc1eWEmvggpqIClwALYvxaVd4t cuVQr6/Q/39QLZ4VEwlrLxpZy2SzPdL549vXf36QinEFzO+UKTNCr5XtZh38PT+qpIaU RKu5jWv+qrR6hwVwgO2wZnKFbZAKigyIcsIU8KZ2QsrxEZr5aL4dk/ovhQrCoUtmMxEW PG+Nh05RZGM+O4P7Wg6aGBYDDn/h6GiqEXL2i63Dnata7z84h+RtQTm6BDKznyqBEQtb xfXjcKx01KmEl4bFNHGNQmZSe1ZahE0Jtblgj92AF/tKgj1pFETHAZefxPWtX0qm/zQw DrOw== X-Gm-Message-State: AC+VfDyD9s7hRGv7UObbOiB0StuuamKfIm1Wosk3/b26UeRsz6BFqSPo AOn926He0aA7wu+ZirLomYtqAg== X-Google-Smtp-Source: ACHHUZ5JXRMgy2Sy3iucvOjzvE5J55mKVFuuRALHbThX2oA4vZEbQg9phtDXVle1ISQ5G6F2JuLD2g== X-Received: by 2002:a17:902:c1d3:b0:19c:c5d4:afd2 with SMTP id c19-20020a170902c1d300b0019cc5d4afd2mr9124plc.11.1684204530548; Mon, 15 May 2023 19:35:30 -0700 (PDT) Received: from google.com ([2620:15c:2d3:205:c825:9c0b:b4be:8ee4]) by smtp.gmail.com with ESMTPSA id t23-20020a634457000000b0051afa49e07asm12283006pgk.50.2023.05.15.19.35.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 15 May 2023 19:35:30 -0700 (PDT) Date: Mon, 15 May 2023 19:35:24 -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: X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: DFFCF4000C X-Stat-Signature: fhky93qhyegndm6qpx9bodb3m44xs6d7 X-HE-Tag: 1684204531-446837 X-HE-Meta: U2FsdGVkX1+kvGzSOOX355chn7ADaewu8QK7j8Lhluz/VLmm884cSP8nIYTtUEh4AUUaTFZDNWRgZ6OBFVpmU+id9FD48M2jZOleIiBDo7zYN3Rgzu/W0Vls1QokT9QUmcKIvlQljQ5E2rK8dc9urp+8xmJ273oWNSq4Ci1ig7x9VU40PsstaJFcEI50YoZq7hPax2oOMCKDW8S+WmJXg9THz5/HnJd+9Fi+cEus1N+Sh1u3IbDxz23F58g4qP21QhSdBhC+z3zh6kEEUIXc9YyrrfLLuNf5K7TEVvm1boW8enLQTinlDuTbYTR+ONZjU/ifZ7sqeY08z/ZGY7EZyyCpV92r0J2dq3AhQpSJgvOzrSSOLaBPUUJ4gF8UdnICREY9C3oM0YV1XGkaxBMUOlwk8UtOxoubrMQ0g27nc1E7RrlKSjdNIkRaQMfR6vjgqae+et7QPRmYzW3m7PZUI+zGMx2w4X/QBfry/TcqZ42h7bSMV0ITdqsGscl+FlNj0zzW8303FsurlsLCh5l/pL9Fp3f0ks96H3WrKlJCNo5KXezDXLLzFb6jYacCki+QxNedblVH9VxN2gpCMdVuL0P3v/k9nF+t2sp3RytpywbDxepYO0SGDvLuZ+SNgaTotVKMmhirHn4bJkA3g5mzHRUCMYVLUfbA8tloIOui2HkmyL9g8sXi/Tx8fHjc3RPxTc/GAyFMVCK8/h2udYHnU/nkn7mBj/eCBFCH3snbgXQpMjud8rr8T/VNW5oIDH9ZSyYOkaF/3wnXgdlUYiUAUzV3VWRstrQeBXq+lwyt2XGn1eT/HiVzWOCNq8r9t9O5IqV+nM/9+EztX2rsRc1tsmzQTuWHexdGBNjOCjWoOOTnpHpJsNsmp0st652GlYv7MuTi/DWg2I7vWgNtYLvBQtggyxPxvm9FGLkQwIU+oF1LkS4SxspWLUEjy2VlciC0DNh0wvcAmwTNZNtaM5A 4OkQ2k0S BIgLh3UyEsLJ70MzsTk4vyCAXSKf+c5u63sjGwCnVT36NX2gn6mWLP6rd3FzC1UOMGfeNj4L4ApYFa8cKlO5oKdcZ5ApRybNSKVhpFuj/x+0S0OmLd7wAPDHZYVKXOIV/oi/J11n5RnAkOAp0Th7O9TNMgFSYivdI5/qje7l/zvMoJ1ahUKV5sfQPu/XAYxHV2da58kpxFmKFceMP5hCEvlTVn/gXUeed45+6HoHZehPzkwj+PrdV/JWi9/TR+feF4bMW3CDSYGhFKHAOG/GI4czVLKYwrhZnxo9y4no9yjGpGUAfeauJ4ZP6oLXRXH5mIQps3eh+t7Ckfd7g/s7M4vbGkUZSEXLMyR4uwzfF4BvRl6TsaHayBlpctakTV7uB6RvaeAWiDe5MOo0f0oFXQelEOOTDa12Yv3JhA/UJ89rPxq0rJchKeCdtEqARCX+eaoGsX8kG9T6Pcm9zLEGDGccc9ZZ4UcsCpSY67boJEU+GumHU74KEVY2F5HIn6VYF+I4R+dmaoLDCVXStT4vfaGt1v2iKSmB5cUfHDiDTP5Mny0s92BfFfhnIzOgCd2LEZzXjqFNS0a7YgmWH6e2dFw6mgoLiGAQTE+/jzHJQJsRpO3OXAuijkrCBF6fOxF5avCYdGmug60s/lLKgD9VrY8L3PgiSA1m/1TsW 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 Mon, May 15, 2023 at 05:16:09PM -0700, Peter Collingbourne wrote: > 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. I gave this some thought and concluded that the added complexity needed to make this hook suitable for arm64 without breaking sparc probably isn't worth it in the end, and as I explained in patch 2, sparc ought to be moving away from this hook anyway. So in v2 I replaced patches 1 and 2 with a patch that adds a direct call to the arch_swap_restore() hook before the call to swap_free(). Peter