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 1D9C7C77B7A for ; Wed, 17 May 2023 02:13:55 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 738A8900004; Tue, 16 May 2023 22:13:55 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6E8E1900003; Tue, 16 May 2023 22:13:55 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5B17E900004; Tue, 16 May 2023 22:13:55 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 4607D900003 for ; Tue, 16 May 2023 22:13:55 -0400 (EDT) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 1107A140239 for ; Wed, 17 May 2023 02:13:55 +0000 (UTC) X-FDA: 80798126430.14.5739AE1 Received: from mail-il1-f171.google.com (mail-il1-f171.google.com [209.85.166.171]) by imf05.hostedemail.com (Postfix) with ESMTP id 528BA100008 for ; Wed, 17 May 2023 02:13:52 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=rshD+IoT; spf=pass (imf05.hostedemail.com: domain of pcc@google.com designates 209.85.166.171 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=1684289632; 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=J9H9xxY0IRCExY9H0nSI5ZkiuSsdqps+Roz6yPP7WfA=; b=8avGEP8sZAk5lvnD40q6QjkK0f3kuD03nC2munuSRaFcsGtXWC3rvOS/M9gtElOYgAoTHi r2Iv4FNPxKD7x+PoOnoWHBhL4b1OxIG75FfLhzphA6WaccSyNu6Isr5pFfB81/3quQRz0g ujSuZ2C1nbwLWvU1qpxRZH4z753OX0k= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1684289632; a=rsa-sha256; cv=none; b=oA/h6+SExlvZZGvDiPwaZ8U8lHOjU+xqgg0lLzwRJmRMeuugMedWKP7XMaGF6Yi4dfvkA5 TEz1NuGQkNvG6FupnB2S7ocSSf3eJ0B9Suw4JH5olxmuv1xD+dFW4vyRSpAbVLpXfkfnBr E46nloq5sMtyXGesFvWfLHjfgosEAYM= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=rshD+IoT; spf=pass (imf05.hostedemail.com: domain of pcc@google.com designates 209.85.166.171 as permitted sender) smtp.mailfrom=pcc@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-il1-f171.google.com with SMTP id e9e14a558f8ab-3330afe3d2fso30145ab.1 for ; Tue, 16 May 2023 19:13:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1684289631; x=1686881631; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=J9H9xxY0IRCExY9H0nSI5ZkiuSsdqps+Roz6yPP7WfA=; b=rshD+IoTAeqt1ylgFUS6t+KhyG6Irx7kcgC3HeFkd7U21A71Qmq1wGP4Heqs1TyrUC RD58Q1x2mJ2EErmTXTA56YQmQJAYBDFoOLQgmt/E4R9VXFA8Br8/S/SvkbJOTbyYYz8b elnk0vUY5RroWJ5sW36rpHNpM99N3Z4GjcZ37ctYATRLVKC8ZSWkXQW8m++XxR7EV6No bMEoKVr+H1tTtWfxcmtj9rjIyiKXgBlyVd5G0H1F5BT85VVKdPQJY6ogi9hoYm9eQ/5Z YZA849oauL0LAwAoxKgEbZSwauNLnCppPcaiOCQeEnFOj3iAOd8Fn0ZIrtawBjcq6fg0 eNbQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684289631; x=1686881631; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=J9H9xxY0IRCExY9H0nSI5ZkiuSsdqps+Roz6yPP7WfA=; b=bMPnoWcpMZAwTYp5YM5jvmKy2OxvNZH0J1z/hetVvq7noAtuRwHl8M8OXc0w/WRkTl rU3EOHcM5xW/29klNz1vyLLC9AQLJaVz86VWmDIEvnZ7Jqr2VWvjgI1iu2Wy/y4Bm7kc vp3geYwH7f2Qxm3gZt7Y2i40/FPlQCvOAKwuTXmDSRzFFAId5JmywZzlpBbQkw3pn7yc ff8LUfKgDKaPLYuRb/nhiVNBlaXuvPWEKsXXdaoCOarExeaOkWWbJMVuDQqmrpVsjlIQ uIhGdKM4CRf5MctOKpzZruIBDeFHYedBqhe1axu16f1+4iPIhNHbwuPG93VVn0SQpvhW EWGA== X-Gm-Message-State: AC+VfDzsV7sOV646ejT758Svn3715Jgf5vcxZ76qAwpBKCeF47NExWjA NG17D8B/zmdHjLOJA9CxB/7UXWeMIFtE2QLFpu0aOQ== X-Google-Smtp-Source: ACHHUZ6nP3vNUbS8Y+1y5SYoY+bj5/YtxVVRjlTpt6WH7z8whziPYI19nahZr4YBtm3V4uBd0oNOkf1nFEcsG+h7VUU= X-Received: by 2002:a05:6e02:1523:b0:335:a48:f668 with SMTP id i3-20020a056e02152300b003350a48f668mr79515ilu.15.1684289631318; Tue, 16 May 2023 19:13:51 -0700 (PDT) MIME-Version: 1.0 References: <20230512235755.1589034-1-pcc@google.com> <20230512235755.1589034-2-pcc@google.com> <7471013e-4afb-e445-5985-2441155fc82c@redhat.com> In-Reply-To: From: Peter Collingbourne Date: Tue, 16 May 2023 19:13:40 -0700 Message-ID: Subject: Re: [PATCH 1/3] mm: Move arch_do_swap_page() call to before swap_free() To: David Hildenbrand Cc: Catalin Marinas , =?UTF-8?B?UXVuLXdlaSBMaW4gKOael+e+pOW0tCk=?= , linux-arm-kernel@lists.infradead.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, "surenb@google.com" , =?UTF-8?B?Q2hpbndlbiBDaGFuZyAo5by16Yym5paHKQ==?= , "kasan-dev@googlegroups.com" , =?UTF-8?B?S3Vhbi1ZaW5nIExlZSAo5p2O5Yag56mOKQ==?= , =?UTF-8?B?Q2FzcGVyIExpICjmnY7kuK3mpq4p?= , "gregkh@linuxfoundation.org" , vincenzo.frascino@arm.com, Alexandru Elisei , will@kernel.org, eugenis@google.com, Steven Price , stable@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: t6ban1zm8jrqqdsugwiakknesdf141ie X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 528BA100008 X-Rspam-User: X-HE-Tag: 1684289632-826206 X-HE-Meta: U2FsdGVkX1/XLLBPkrDYYGwtdZeZozD5ZfmHIL5GRqnWhj1hPO7VRhGNi3ujDQxn31c0a9BqLHx2OGM+67QRCW29/3GTwDjDNR4lGCD5OIihIdn/D0InBoZvSreZBAFLbu9NviIQ04b7vzx/ruTMiSAFMQhXa+ZPOg+eHWz58wEcvrZoe2VxEOz0rEM6gupgLv1gv6ZVKlSe4NYSjaJwEWoNoFqSDJ3Sin5BfkiH+ERwXPcwp6dOEUnzozebBKkOrbqRRcbaauQaVkQz4ljKGn4H6EK8lLkLni0RwHsJJiTB9WLLJyRKjpXt9glnjNGHXBCCY8DVgGTTcb5S1PqNr6vAAwQ4QD3Lg2BN8Ri2CzLJS5h/LPCu9BJSVwSt2U71CwsR+WozDrKxy8Rvmfj9LADGpXCEaOZbZJxWxBw1w3wEtPQdbPn1RSSDDRpy3qU7WU0yndZqEaUEOKluo1OHyAyXePoJw1DU9lkW/wZu2xkssla0bKkHk2mKb6wlHkWg/xUvFs4XSYeLOjslMGcc7fauMZELIw9BlC2NutwRNoOpqADYiFJ++34OrtMnmvsO9rWDfq9upKoK/7wT/XhO3UeTywJ4+JqJDh6LkyeLVlxcmUzfWLRJ7J9dBUuBy8s0DMOABrbS9YkDz+xRykwPfFng/f93lwhPOtPcVKIiryMV6fxB/31I9uIghZ9yL3Bl63hncRaI8EW0VOQIDJei/vPtOcp4FIYiRKCLRLHiUUf989AbvKR1MljYrzAGW4ZFLvpRSm30BtdvJf/69R+ioj+97AzDrD2n797a1QD5MO05Vl1pkZ2F1EUIFUTvvJQJv7/W5SGKqx2C9inxcC7xygIuwsokacCI8BzD+kxBNbZcnhnBYJRHFAP9kQkc6GOCXjRF0gPEG7hPo8IaJk7CGIbqFy64fd9nBXaLLRSuQrPtYfJp8cQ1mQu7wInfy+r8M8jSsbH6dDj0I8MGNBq KbijKtZ+ lYdd+jXZADTntYVOnCoc/5Lux5aTMfeq2P2IoZ9bnlCvU1GT3N65DSaTL/n8zzj796BMhHKqxcXqJIP+09POCPHvD6WFA+O1EqtgUIrWXAKa1VGahkI4HOP6+wGhWRROsgTXVytaVLN+nYAWx33WF/VGueqV01BI+EmuJEIdVW1K/fkP81JlAci4LthTUczzubKJU5JLhILQs1F2vrQKVLoWV14oG9CWQ41SyDwzS00MxnWgFuy1p5HLNyixnkcDf/1OtY4HCI1o2zBdeRQKEgkRQtbVJCkgZwyqUQhFFq4amhE032sg/p7DMO36kFnq0VnW1pLPvytnKe9c1XnqECialI4IPZ+MnfN6QvqnLAUZIIoreZclbAxUkdTNwY+6cwsOJgcYI+/iV21D75ZCsXj2+Zvktc5QUukJauMGwsSLPjMJzVKMztFxitgGC28z/sXJHobJdbEhV3IjtIgWWtosOK/uFGESAK5a38td9RUFd4BMoBW1D2eryWjBc/lWT96w//djIqOr/fE68lxDefg8yvRSjOUcNHrwC 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 Tue, May 16, 2023 at 5:40=E2=80=AFAM David Hildenbrand wrote: > > On 16.05.23 02:16, 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()") mo= ved > >>> 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 chanc= e > >>> 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 w= as a > >>> racy window where uninitialized metadata may be exposed to usersp= ace. > >>> 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 th= e > >>> oldpte argument, preventing the hook from knowing the swap index. > >>> > >>> Fix all of these problems by moving the arch_do_swap_page() call befo= re > >>> the call to free_page(), and ensuring that we do not set orig_pte unt= il > >>> after the call. > >>> > >>> Signed-off-by: Peter Collingbourne > >>> Suggested-by: Catalin Marinas > >>> Link: https://linux-review.googlesource.com/id/I6470efa669e8bd2f84104= 9b8c61020c510678965 > >>> Cc: # 6.1 > >>> Fixes: ca827d55ebaa ("mm, swap: Add infrastructure for saving page me= tadata 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. > > > > Oh. That's indeed confusing. Maybe that should all be squashed to have > one logical fix for the overall problem. It's especially confusing > because this patch here fixes the other two issues touches code moved by > c145e0b47c77. Maybe. It can sometimes be hard to reconcile "one logical change per patch" with "bug requires more than one logical change to fix" though. Fortunately in this case I think we have an approach that fixes the bug in one logical change, with some followup patches to clean things up. > > - 6.1 is the first stable version in which all 3 commits in my Fixes: t= ags > > 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 ker= nel > > versions, but it wouldn't fix any problems that we are facing with M= TE > > (because it only fixes problems relating to the arch_do_swap_page() > > hook, which older kernel versions don't hook with MTE), and there ar= e > > some merge conflicts if we go back further anyway. If the SPARC folk= s > > (the previous only user of this hook) want to fix these issues with = ADI, > > they can propose their own backport. > > Sometimes, it's a good idea to not specify a stable version and rather > let the Fixes: tags imply that. Yeah, but sometimes it's hard to say which way would be more efficient. Either we spend time discussing why the version is necessary or Greg spends time trying to apply patches to the wrong trees because I wasn't more explicit... Peter