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 7334FC25B76 for ; Wed, 5 Jun 2024 09:22:32 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 082706B0093; Wed, 5 Jun 2024 05:22:32 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 00B9C6B0095; Wed, 5 Jun 2024 05:22:31 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DC8BF6B0096; Wed, 5 Jun 2024 05:22:31 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id BAF4D6B0093 for ; Wed, 5 Jun 2024 05:22:31 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 6892A80E29 for ; Wed, 5 Jun 2024 09:22:31 +0000 (UTC) X-FDA: 82196294502.07.3D36086 Received: from mail-lf1-f43.google.com (mail-lf1-f43.google.com [209.85.167.43]) by imf16.hostedemail.com (Postfix) with ESMTP id 79CE5180004 for ; Wed, 5 Jun 2024 09:22:29 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=edP0SzYA; spf=pass (imf16.hostedemail.com: domain of ioworker0@gmail.com designates 209.85.167.43 as permitted sender) smtp.mailfrom=ioworker0@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1717579349; 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=7VMsO98g6T+rFoaq3isAVnjASq1OTBZpOaxNXgac48g=; b=0+ID4i9FA6pIvYgrwaLpB0nI2N4zZ+0/XGvW5kHolpesTbCYaAiTt8cGTEz42dFc5OfM+k 3tzf0RorzEHW2DjVKL2+LnIhdH1lo3u5KK1Qdrnrf1upJ6ybxCoRlFLIWIDHfP11aTKBHQ /xRuniH6sAztjJogmddEjDC6wtx8Xgw= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1717579349; a=rsa-sha256; cv=none; b=Y4MpmM4PHu45xmdNWETREZYBlHQq+QnRtU+YG6BgiFMSLrrc9A81u+XUNVaO/Il+u+FRy1 tWrA38x12dc9qjhDocrQDmY1cfGSkQp/FOJE4EIWEp+8wCKdjWUZ5Eqow/IHpXySwO2vdE oAqDF1gF3J4qOQaXP43JZbxaMTUlXg4= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=edP0SzYA; spf=pass (imf16.hostedemail.com: domain of ioworker0@gmail.com designates 209.85.167.43 as permitted sender) smtp.mailfrom=ioworker0@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-lf1-f43.google.com with SMTP id 2adb3069b0e04-5295e488248so6847759e87.2 for ; Wed, 05 Jun 2024 02:22:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1717579348; x=1718184148; darn=kvack.org; 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=7VMsO98g6T+rFoaq3isAVnjASq1OTBZpOaxNXgac48g=; b=edP0SzYAJfGLMiF1HxcYiaqs/ywNtXVp2O/70aX1uU06w0PeIbKISG9VpRiQjV1aFh Evhzd81W9+6i+oYwS/VCCrqSsuTwlfBgPiYFg8KF7wUJ7nWuPGfY33aYvJtYCFsrJqkz inFYFnK31ndWZP//Iy96yXLYxcgnjsGo+2sQZDpUQ3ZzG+RRFfwC6Lj6QbQHT5Jgs35m d8R/FIjRfqEiybcXbAQk8nTlJGpFUexmheg99jwJv9aYdgue8qoRn/i3colW3ro8SeU8 MdCeCfkuqTobKStQqZufDeUhz9pNrzNbwQoqPb/o9qZsuItmVqaLdKBQmjZkTCZXb2qV BkiA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717579348; x=1718184148; 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=7VMsO98g6T+rFoaq3isAVnjASq1OTBZpOaxNXgac48g=; b=HE2Bw1H1qBAOHXXP3qfsn1gUeFmEu/hHksO+nXzWEUJIiK1ywa7liQRRWShSj/i3dy UBt/0PiXdlEqkmaz049CZ80Ug+zUkO/vd2ALZaqewpI7aTUhW3Q89ZKwxpFdHhWRCP4C xcVAc0rzrwbRaCIKQVNarYi/hOQpIvgiMPCxBOgH8x7c22cYnbK00VRXhZQ3htBbpqTW lH7bwcKZOT3kIvy/t2qBxFXqJgmHFL9pG/UICNCt7Gcj8mfQXa4q1N+bsZMBuUUlxhTr ko1PFf55JjjRT8ira+RSLQu90LSFxpHspos7AJsdL0Oa7a+2Lx0TLA1F5xjkuIaBsI41 HBEQ== X-Forwarded-Encrypted: i=1; AJvYcCUIi+EJtm521CNwbCrRWDF3TwT9qw85tE7g0DDTvuEl4RZHIwtvkXE6clrP1fSbjnLtHGxqXdOJACuL6KvWjS2Ih8E= X-Gm-Message-State: AOJu0YyxFg4oioaimi2aGc9nX2bSVzx+J1SafCTPXUcuqNdv8mmLqRRg 0Yce9UUZ/eR5ZjVjHIF710nI3g7NK61i3UYw8pQIsiTWNLrOJZ3hSoxzgM2IFbJ/M1vCIK8dbg1 Tl3yKrSiFHa37M479pP/YvH4TWeU= X-Google-Smtp-Source: AGHT+IHqHTdbgQjlDQRgE5s43tJfb+esbUo0eviuUGiP6JNHiOuTWS7nads1ksv3fHGAiGsZ3qOkVo7oebCk06UJtww= X-Received: by 2002:ac2:42cf:0:b0:52b:798e:74e8 with SMTP id 2adb3069b0e04-52bab4e5540mr995815e87.39.1717579347414; Wed, 05 Jun 2024 02:22:27 -0700 (PDT) MIME-Version: 1.0 References: <20240605091710.38961-1-david@redhat.com> In-Reply-To: <20240605091710.38961-1-david@redhat.com> From: Lance Yang Date: Wed, 5 Jun 2024 17:22:13 +0800 Message-ID: Subject: Re: [PATCH v1] Revert "mm: init_mlocked_on_free_v3" To: David Hildenbrand Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, David Wang <00107082@163.com>, Andrew Morton , York Jasper Niebuhr , Matthew Wilcox , Kees Cook Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Stat-Signature: wyfz7fai7qmm1gtyk5s8txermochqyiy X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 79CE5180004 X-HE-Tag: 1717579349-273938 X-HE-Meta: U2FsdGVkX1/mETv/aijdg4kASOnDLoOhZ6GF/XrHGy7OwAQZwEK8HBhQZ+pYE3GsPKssj/JScP9z3E6ZyGT9E6sRTYXH80Hrlt3xdq9/qm4xr0xXNBH3FxdOX7gbAwuHSSC702YIi5Ma+BKJNhtY8VY62sjGJUOIFFSJkL61ZclRKgP3DoUC98y1F10dK0NgKqacx6D10UVTPAPmx+QjRaJgK+GDNsD+qt61MThhlkqJM7DDO5wbIv4M9Vo8K6GydBsWsJYwdDHfTN4bofq7w6f03d1cSh5prdGAEMRukGBSOTP98aADQeT0xn1PawoCxU+w+nqTIxhTuRKgscnENpXY9Q+qrwxfLeEnK37umewO8s39QH/l0I/5Y4mn8Rm3GhxDkAMM+1336x1Zp1oDlIXbIaNVfEQHXAzqsI8Wfcrvy03ZbgyaIIpcS07mdFqDodl1cRE4NFvhcJNwiYtyMMfBttrUsNvJGyRk6ggc1z5utxO0kXKjhLtFoGRi7LAYp1Gmpa6fE6oOrtTxvO1MGyi0YwIomMpYhoEn0zO7PYyGXgaHvunTd+HDEyfsgZdOC1j90TNT6tpfjMpJ7j3MXTsSTpNZBU4l2VVuZ7orXHuaE3kizA1VJvQFCzefvH2UK3h4nWYbD4hnS0+uEdx6I7TQlHEBB5mCySKed/BLK3ss5BI0lmM1+HEK8yh0+MeyzS5uqPMYRCUxjHl94kmTjU/kds8eHlni8I6K78bLE4+WcFKIZ2ODI+2zrB40/NKq4VdEmug99laFWvBZd85GIZAF2+HtfZgS45h4kzDSPffnBifx1YcsGiC+WJVOv5Lvjlx7gsD3HtYACTZ6VqYTBW9kSiJ6DYiJGloYrdi/khw0++uYnGTVwXKkd1f602ATKW3nZvbynamCOVre9WFqjlPd65NdZQYkaR7P4yVenqfSqpFMEQwim7SKD7eEcDt7yUvDNSPDWnjTU1J7LVS Fs2SpcbT lY+2rR9QWlwbCLqeY1KzlkkXejrzp22yoPPX9vb9hlgxP5dVxX/q3JWHQ/eHm8qkmfr0pEw/6IIYDBxYYbcLA1fyDGN/T46Z5diuDXRA4Z4x4RS1d3Em3Zk/v/ZA4dRY2/Cjkiu8jlgBRMt01B0dfWr0M6P9poGubO5+V+zzegZ6ar+/mTdFJP7LJaZSmvky62Ux/KHhISZiCQXVP/Ks63hM/BUYkV32eov/h40TljeaR/y+7sceQzTehRP1gGYD4xDYWUMGGdSy6nNyPpfC8vm1UzOSlzi/36uPyIcVaXxQMzuzKyylItkJHhtZq1S+gfu+9Edknz9x80e/FpMNrSq2iodjEZe357A+KbTUt+LpBrtwQUSvI+M0vB20znp9ojsxFHb2KL7VqpyruIz8PwQQOLLRWxw5q3i/qOg/wZIBiGfE4H5pp53MVc3x8LMRqZrrAtt0k1qM9ZHF2TkoGrbFsjafMnZJy3oOblXefVqvhp1/ugc3SoivrEb5s9HFPLV8d3FupiVolgL6MfYruVvU5SqzxuuO+QGjbK6S0YmN/3uKs44zgdPlfuGS/1ts7ats6Z1zL8Ds4GIA8rcXhg/nNBY0epnhuQ0B4E38Y8ruUgK1j1P2noYbbscRHnmuiN7y18OTyYW1jyk60qkNbfvoC1Ow9csq55D4ACCVLZ3Ly979kkBfAyErgRn7HdgJsE79SZINRU2BCG3VQBrnCToYe79rtZUOh2MgHEw9lGwXdmYVV0Gvo86K5GcRE14hWGQmaoTRlYZwcNaeQUPJcEG8YGw== 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 Wed, Jun 5, 2024 at 5:17=E2=80=AFPM David Hildenbrand = wrote: > > There was insufficient review and no agreement that this is the right > approach. > > There are serious flaws with the implementation that make processes using > mlock() not even work with simple fork() [1] and we get reliable crashes > when rebooting. > > Further, simply because we might be unmapping a single PTE of a large > mlocked folio, we shouldn't zero out the whole folio. > > ... especially because the code can also *corrupt* urelated memory becaus= e > kernel_init_pages(page, folio_nr_pages(folio)); > > Could end up writing outside of the actual folio if we work with a > tail page. > > Let's revert it. Once there is agreement that this is the right approach, > the issues were fixed and there was reasonable review and proper > testing, we can consider it again. > > [1] https://lkml.kernel.org/r/4da9da2f-73e4-45fd-b62f-a8a513314057@redhat= .com > > Fixes: ba42b524a040 ("mm: init_mlocked_on_free_v3") > Reported-by: David Wang <00107082@163.com> > Closes: https://lore.kernel.org/lkml/20240528151340.4282-1-00107082@163.c= om/ > Reported-by: Lance Yang > Closes: https://lkml.kernel.org/r/20240601140917.43562-1-ioworker0@gmail.= com > Cc: Andrew Morton > Cc: York Jasper Niebuhr > Cc: Matthew Wilcox (Oracle) > Cc: Kees Cook > Signed-off-by: David Hildenbrand LGTM. Acked-by: Lance Yang Thanks, Lance > --- > .../admin-guide/kernel-parameters.txt | 6 --- > include/linux/mm.h | 9 +--- > mm/internal.h | 1 - > mm/memory.c | 6 --- > mm/mm_init.c | 43 +++---------------- > mm/page_alloc.c | 2 +- > security/Kconfig.hardening | 15 ------- > 7 files changed, 9 insertions(+), 73 deletions(-) > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentat= ion/admin-guide/kernel-parameters.txt > index b600df82669db..11e57ba2985cc 100644 > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -2192,12 +2192,6 @@ > Format: 0 | 1 > Default set by CONFIG_INIT_ON_FREE_DEFAULT_ON. > > - init_mlocked_on_free=3D [MM] Fill freed userspace memory with z= eroes if > - it was mlock'ed and not explicitly munloc= k'ed > - afterwards. > - Format: 0 | 1 > - Default set by CONFIG_INIT_MLOCKED_ON_FRE= E_DEFAULT_ON > - > init_pkru=3D [X86] Specify the default memory protection key= s rights > register contents for all processes. 0x55555554 = by > default (disallow access to all but pkey 0). Can > diff --git a/include/linux/mm.h b/include/linux/mm.h > index 9849dfda44d43..9a5652c5fadd5 100644 > --- a/include/linux/mm.h > +++ b/include/linux/mm.h > @@ -3776,14 +3776,7 @@ DECLARE_STATIC_KEY_MAYBE(CONFIG_INIT_ON_FREE_DEFAU= LT_ON, init_on_free); > static inline bool want_init_on_free(void) > { > return static_branch_maybe(CONFIG_INIT_ON_FREE_DEFAULT_ON, > - &init_on_free); > -} > - > -DECLARE_STATIC_KEY_MAYBE(CONFIG_INIT_MLOCKED_ON_FREE_DEFAULT_ON, init_ml= ocked_on_free); > -static inline bool want_init_mlocked_on_free(void) > -{ > - return static_branch_maybe(CONFIG_INIT_MLOCKED_ON_FREE_DEFAULT_ON= , > - &init_mlocked_on_free); > + &init_on_free); > } > > extern bool _debug_pagealloc_enabled_early; > diff --git a/mm/internal.h b/mm/internal.h > index b2c75b12014e7..c72c306761a48 100644 > --- a/mm/internal.h > +++ b/mm/internal.h > @@ -588,7 +588,6 @@ extern void __putback_isolated_page(struct page *page= , unsigned int order, > extern void memblock_free_pages(struct page *page, unsigned long pfn, > unsigned int order); > extern void __free_pages_core(struct page *page, unsigned int order); > -extern void kernel_init_pages(struct page *page, int numpages); > > /* > * This will have no effect, other than possibly generating a warning, i= f the > diff --git a/mm/memory.c b/mm/memory.c > index 0f47a533014e4..2bc8032a30a2f 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -1507,12 +1507,6 @@ static __always_inline void zap_present_folio_ptes= (struct mmu_gather *tlb, > if (unlikely(folio_mapcount(folio) < 0)) > print_bad_pte(vma, addr, ptent, page); > } > - > - if (want_init_mlocked_on_free() && folio_test_mlocked(folio) && > - !delay_rmap && folio_test_anon(folio)) { > - kernel_init_pages(page, folio_nr_pages(folio)); > - } > - > if (unlikely(__tlb_remove_folio_pages(tlb, page, nr, delay_rmap))= ) { > *force_flush =3D true; > *force_break =3D true; > diff --git a/mm/mm_init.c b/mm/mm_init.c > index f72b852bd5b8e..3ec04933f7fd8 100644 > --- a/mm/mm_init.c > +++ b/mm/mm_init.c > @@ -2523,9 +2523,6 @@ EXPORT_SYMBOL(init_on_alloc); > DEFINE_STATIC_KEY_MAYBE(CONFIG_INIT_ON_FREE_DEFAULT_ON, init_on_free); > EXPORT_SYMBOL(init_on_free); > > -DEFINE_STATIC_KEY_MAYBE(CONFIG_INIT_MLOCKED_ON_FREE_DEFAULT_ON, init_mlo= cked_on_free); > -EXPORT_SYMBOL(init_mlocked_on_free); > - > static bool _init_on_alloc_enabled_early __read_mostly > =3D IS_ENABLED(CONFIG_INIT_ON_ALLOC_DEFAU= LT_ON); > static int __init early_init_on_alloc(char *buf) > @@ -2543,14 +2540,6 @@ static int __init early_init_on_free(char *buf) > } > early_param("init_on_free", early_init_on_free); > > -static bool _init_mlocked_on_free_enabled_early __read_mostly > - =3D IS_ENABLED(CONFIG_INIT_MLOCKED_ON_FRE= E_DEFAULT_ON); > -static int __init early_init_mlocked_on_free(char *buf) > -{ > - return kstrtobool(buf, &_init_mlocked_on_free_enabled_early); > -} > -early_param("init_mlocked_on_free", early_init_mlocked_on_free); > - > DEFINE_STATIC_KEY_MAYBE(CONFIG_DEBUG_VM, check_pages_enabled); > > /* > @@ -2578,21 +2567,12 @@ static void __init mem_debugging_and_hardening_in= it(void) > } > #endif > > - if ((_init_on_alloc_enabled_early || _init_on_free_enabled_early = || > - _init_mlocked_on_free_enabled_early) && > + if ((_init_on_alloc_enabled_early || _init_on_free_enabled_early)= && > page_poisoning_requested) { > pr_info("mem auto-init: CONFIG_PAGE_POISONING is on, " > - "will take precedence over init_on_alloc, init_on= _free " > - "and init_mlocked_on_free\n"); > + "will take precedence over init_on_alloc and init= _on_free\n"); > _init_on_alloc_enabled_early =3D false; > _init_on_free_enabled_early =3D false; > - _init_mlocked_on_free_enabled_early =3D false; > - } > - > - if (_init_mlocked_on_free_enabled_early && _init_on_free_enabled_= early) { > - pr_info("mem auto-init: init_on_free is on, " > - "will take precedence over init_mlocked_on_free\n= "); > - _init_mlocked_on_free_enabled_early =3D false; > } > > if (_init_on_alloc_enabled_early) { > @@ -2609,17 +2589,9 @@ static void __init mem_debugging_and_hardening_ini= t(void) > static_branch_disable(&init_on_free); > } > > - if (_init_mlocked_on_free_enabled_early) { > - want_check_pages =3D true; > - static_branch_enable(&init_mlocked_on_free); > - } else { > - static_branch_disable(&init_mlocked_on_free); > - } > - > - if (IS_ENABLED(CONFIG_KMSAN) && (_init_on_alloc_enabled_early || > - _init_on_free_enabled_early || _init_mlocked_on_free_enabled_= early)) > - pr_info("mem auto-init: please make sure init_on_alloc, i= nit_on_free and " > - "init_mlocked_on_free are disabled when running K= MSAN\n"); > + if (IS_ENABLED(CONFIG_KMSAN) && > + (_init_on_alloc_enabled_early || _init_on_free_enabled_early)= ) > + pr_info("mem auto-init: please make sure init_on_alloc an= d init_on_free are disabled when running KMSAN\n"); > > #ifdef CONFIG_DEBUG_PAGEALLOC > if (debug_pagealloc_enabled()) { > @@ -2658,10 +2630,9 @@ static void __init report_meminit(void) > else > stack =3D "off"; > > - pr_info("mem auto-init: stack:%s, heap alloc:%s, heap free:%s, ml= ocked free:%s\n", > + pr_info("mem auto-init: stack:%s, heap alloc:%s, heap free:%s\n", > stack, want_init_on_alloc(GFP_KERNEL) ? "on" : "off", > - want_init_on_free() ? "on" : "off", > - want_init_mlocked_on_free() ? "on" : "off"); > + want_init_on_free() ? "on" : "off"); > if (want_init_on_free()) > pr_info("mem auto-init: clearing system memory may take s= ome time...\n"); > } > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 2e22ce5675ca1..a264eac20d1de 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -1016,7 +1016,7 @@ static inline bool should_skip_kasan_poison(struct = page *page) > return page_kasan_tag(page) =3D=3D KASAN_TAG_KERNEL; > } > > -void kernel_init_pages(struct page *page, int numpages) > +static void kernel_init_pages(struct page *page, int numpages) > { > int i; > > diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening > index effbf5982be10..2cff851ebfd7e 100644 > --- a/security/Kconfig.hardening > +++ b/security/Kconfig.hardening > @@ -255,21 +255,6 @@ config INIT_ON_FREE_DEFAULT_ON > touching "cold" memory areas. Most cases see 3-5% impact. Some > synthetic workloads have measured as high as 8%. > > -config INIT_MLOCKED_ON_FREE_DEFAULT_ON > - bool "Enable mlocked memory zeroing on free" > - depends on !KMSAN > - help > - This config has the effect of setting "init_mlocked_on_free=3D1= " > - on the kernel command line. If it is enabled, all mlocked proce= ss > - memory is zeroed when freed. This restriction to mlocked memory > - improves performance over "init_on_free" but can still be used = to > - protect confidential data like key material from content exposu= res > - to other processes, as well as live forensics and cold boot att= acks. > - Any non-mlocked memory is not cleared before it is reassigned. = This > - configuration can be overwritten by setting "init_mlocked_on_fr= ee=3D0" > - on the command line. The "init_on_free" boot option takes > - precedence over "init_mlocked_on_free". > - > config CC_HAS_ZERO_CALL_USED_REGS > def_bool $(cc-option,-fzero-call-used-regs=3Dused-gpr) > # https://github.com/ClangBuiltLinux/linux/issues/1766 > > base-commit: 32f88d65f01bf6f45476d7edbe675e44fb9e1d58 > -- > 2.45.1 >