From patchwork Wed Jun 5 09:17:10 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Hildenbrand X-Patchwork-Id: 13686445 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 B298AC25B76 for ; Wed, 5 Jun 2024 09:17:21 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 450AF6B008C; Wed, 5 Jun 2024 05:17:21 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 401426B0092; Wed, 5 Jun 2024 05:17:21 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2EEB36B0093; Wed, 5 Jun 2024 05:17:21 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 12A5C6B008C for ; Wed, 5 Jun 2024 05:17:21 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id B5012C0CC8 for ; Wed, 5 Jun 2024 09:17:20 +0000 (UTC) X-FDA: 82196281440.24.3F6AF4D Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf30.hostedemail.com (Postfix) with ESMTP id 2C3228000D for ; Wed, 5 Jun 2024 09:17:17 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=Ki+9sMSz; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf30.hostedemail.com: domain of david@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=david@redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1717579038; 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-transfer-encoding:content-transfer-encoding: in-reply-to:references:dkim-signature; bh=rYWKKIbN+7uLwqmZYu4Gu6iwgKUOkP1ippA/hZaEuBI=; b=B1Dh7P5wGMi639wO9jkrwdgeLibO9lHicrv6P1m9DJumnESA3PhAnuwx9xN4NIYU3i+wx2 P005ZpQxZX65edU8oTH/7N2vPQ6hTiDxr90ZFONHLp+crQV+hPkqMGHm/PRKLTlXu055H9 n0JzWATqIuKbctJt1lBYVM6V7GzGc3g= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=Ki+9sMSz; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf30.hostedemail.com: domain of david@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=david@redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1717579038; a=rsa-sha256; cv=none; b=cMsK/7apBf2vg6cRmgE34fgql2e+k6pv6WZBDuze4suwivbYsw0QAmSLJ0VoFd3LNN30k6 Rqj9FYJfbZSeDGoZ+4FFgIStoLLK9UkU3A7T5NFNcc6Cn3hJqBc5ichWTRJCFDBBOfOQph 5MQCXVtfLKY+F6X0gOZYTcKDNlsY1Gg= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1717579037; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=rYWKKIbN+7uLwqmZYu4Gu6iwgKUOkP1ippA/hZaEuBI=; b=Ki+9sMSzN54VIVcEMEWsQgHsKlf/gxzwzzXUjHHDxdg3PoM5KoRLzmSwQydKwYji5bqjqV TRGoLzVM9yLCH1WdkAhSZf5e3AKgG6YsJTm4Tf6jmEtQ0ZUqHN2gmXIDwQVumFEZRg2QN9 E88v1ENw9z/UzIH5fFFk5IGFMR3xJRg= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-689-2ulr3NoHNjGfnrVgFBOrdQ-1; Wed, 05 Jun 2024 05:17:14 -0400 X-MC-Unique: 2ulr3NoHNjGfnrVgFBOrdQ-1 Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.rdu2.redhat.com [10.11.54.10]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id A6A19101A531; Wed, 5 Jun 2024 09:17:13 +0000 (UTC) Received: from t14s.redhat.com (unknown [10.39.194.76]) by smtp.corp.redhat.com (Postfix) with ESMTP id 63C59492BF6; Wed, 5 Jun 2024 09:17:11 +0000 (UTC) From: David Hildenbrand To: linux-kernel@vger.kernel.org Cc: linux-mm@kvack.org, David Hildenbrand , David Wang <00107082@163.com>, Lance Yang , Andrew Morton , York Jasper Niebuhr , Matthew Wilcox , Kees Cook Subject: [PATCH v1] Revert "mm: init_mlocked_on_free_v3" Date: Wed, 5 Jun 2024 11:17:10 +0200 Message-ID: <20240605091710.38961-1-david@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.10 X-Rspamd-Queue-Id: 2C3228000D X-Stat-Signature: 7gcok3zdjt7xpp159tkyocemw4m7msa6 X-Rspam-User: X-Rspamd-Server: rspam04 X-HE-Tag: 1717579037-843690 X-HE-Meta: U2FsdGVkX1+84hdwx62B3D+Ovu0b1XEaitFV02uMIYlVIA+S+lmSL8+XwbTQG7sI2+hyMW+R8pNN3CufgEucZm3JOoHQ5URgIgMMCTOyPw7AEnqQUJG87JbEpBIzjpTkeC4sPKmhDtCD8TbVzOQnaSrMNyY47rrJcGgcgBMCvXTCB6Dqm5v0qUU85bm/IlY69PFV5Pb5KsYiOVgLjkFt+lNzZ+Z0ZbHbLJ32vDerxRVlWxTe4+UOF4PVw1oRMf19l/NF8e/uDw6U5/J+tdaPmtKAu9BVCH7/MJFe01Cs9XO6DK+AEbg0Oey0ItP4tVJV7g9IQJ3v8suVjMzCjnY9xh4ze01TlvetfU63jZSmllitW0XUaKNtPT4jygYOm3GfUXIrvemSmKaef9vGkDFLHLBNCTpMCBB0Mz1OBScvWoCpgKUwFFuGFG406acVHOpgiNmR1YVVL0YR1l7g0u3lcVm7lJCRDP73bFjwxbtz47uzMIBhjmENSG3SYhp8zOdt2PNATg0oWOv2Hfbi74l3zy3U083hrkrd8WqkJExi0dXL1d0mDS992YKZhb4jx1aFM3G7gYSYJdNwMBKYFbwXnaws4rTwb+HcYp3HQ1O0Y1ybWbZeObGUP8w3RkcQ7UIijMuMqu4u+cXyrdPmUx+ry2+DGw7pk82wYTyF+GBeLT0HhODGUwCa0tstiFTko7GDbwQq0iju9UXZlEq5jHv67gAn/7Og2Gs3aPPq9lX+fheoL8Y0mMrU+k7bUqaHgont2BaYvSK3ACKYT280M6+umen8gRTBAvjK9nFI8lnnPzny+2Y7tc16rEJTeJF1xBn9xG0Nab+AEOgB/8jkssuDR8pxURHw2gBIX2yDHGHvmhnXtj7FO7UwcxZAejj5ZnVlgjZCGcwzKnLiIfUJbiyoDqNflAttdzYwMBqpMrJ9LeQdGjLw4qTAK/acvA4v8XqcINfXu7X+l9WmHvzvZ/3 T4YT+sjx qm5txP8K7sXdu6azGpbq7H00fnLjQDwn09OWD1kNo9mid1c+HoGORLexxuetgSFB8X0dby//QFQDlmBzcAcd4MNqU7M5P6kawH+OtjI5HSAZvg6FTd0m37x3EWUbArNStaIIk2elY05Ua5BhdIlPah1xyNpJcgFLe3VYe22lO3AZi4ga696UxOspop54+KOrj8UEe32fsGqmYGKi3rMtGEpNsEG31L+SfEbmv3fir90ZJ4Tg/vN7S4vySkdApaKTWVrRW3+vme8PxOL1Xt5ZxMgR6kffrpPhnP141FZeeMcUy3VqWAt4aVH8Yf6xhs07dcWCYJP60EST4IgEIydQxeMNthzdFTdugFIImMSJ150gw5ckeFHN4FkzpdS5rbREvx0OFS4SqvBwcCVwO3J+bkymMfPX4g76qrluBYtIvUkmX56g+H/pbMAwhCLexHZO+eiaSJGCXIURNjiIbvAgdGT8oMHl3AgvDlw6R8Vt9pS/QDZfDs1nP/WLF1oP9oa4EtDjDcxVwvn3T9TB0J0LciXVl0AC4mcWoucaJY+E8oCNT36cxJVdlSli+KuQUfkKi5DpdkECpkNxTVVvY9W1JJH3QyBmD2mDq0CXN1C0lDai6Nik= 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: 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 because 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.com/ 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 Acked-by: Lance Yang --- .../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(-) base-commit: 32f88d65f01bf6f45476d7edbe675e44fb9e1d58 diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/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= [MM] Fill freed userspace memory with zeroes if - it was mlock'ed and not explicitly munlock'ed - afterwards. - Format: 0 | 1 - Default set by CONFIG_INIT_MLOCKED_ON_FREE_DEFAULT_ON - init_pkru= [X86] Specify the default memory protection keys 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_DEFAULT_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_mlocked_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, if 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 = true; *force_break = 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_mlocked_on_free); -EXPORT_SYMBOL(init_mlocked_on_free); - static bool _init_on_alloc_enabled_early __read_mostly = IS_ENABLED(CONFIG_INIT_ON_ALLOC_DEFAULT_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 - = IS_ENABLED(CONFIG_INIT_MLOCKED_ON_FREE_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_init(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 = false; _init_on_free_enabled_early = false; - _init_mlocked_on_free_enabled_early = 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 = false; } if (_init_on_alloc_enabled_early) { @@ -2609,17 +2589,9 @@ static void __init mem_debugging_and_hardening_init(void) static_branch_disable(&init_on_free); } - if (_init_mlocked_on_free_enabled_early) { - want_check_pages = 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, init_on_free and " - "init_mlocked_on_free are disabled when running KMSAN\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 and 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 = "off"; - pr_info("mem auto-init: stack:%s, heap alloc:%s, heap free:%s, mlocked 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 some 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) == 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=1" - on the kernel command line. If it is enabled, all mlocked process - 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 exposures - to other processes, as well as live forensics and cold boot attacks. - Any non-mlocked memory is not cleared before it is reassigned. This - configuration can be overwritten by setting "init_mlocked_on_free=0" - 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=used-gpr) # https://github.com/ClangBuiltLinux/linux/issues/1766