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 07BF5C64EC4 for ; Thu, 9 Mar 2023 15:48:52 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 67E606B0071; Thu, 9 Mar 2023 10:48:51 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 62E896B0072; Thu, 9 Mar 2023 10:48:51 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 51D6C280001; Thu, 9 Mar 2023 10:48:51 -0500 (EST) 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 415F76B0071 for ; Thu, 9 Mar 2023 10:48:51 -0500 (EST) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 036781A110F for ; Thu, 9 Mar 2023 15:48:50 +0000 (UTC) X-FDA: 80549792862.23.5AE635D Received: from mail-ua1-f48.google.com (mail-ua1-f48.google.com [209.85.222.48]) by imf19.hostedemail.com (Postfix) with ESMTP id 34C9B1A0003 for ; Thu, 9 Mar 2023 15:48:48 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=fE1nYEKP; spf=pass (imf19.hostedemail.com: domain of elver@google.com designates 209.85.222.48 as permitted sender) smtp.mailfrom=elver@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=1678376929; 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=bdBTcFq4uhvEA5oTaEAsvRAaPMfrbecdresU0Hqq2NY=; b=ZHkyoo+QCTynMsPly/HuSZU2DAU0psdS7oG+eimDL1NhP4kxZiehBRNiLq8yRUATjjSSsx VgWgMBFBTqZ7vt9EB3bhDVvnnBXOvu/T5bYtlfW6MJiwKMrz29Huf6fDEJQtg9gvLb3D7N t8Wd9IfmTnQcDgBkBfxxs0/48aiXKG8= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=fE1nYEKP; spf=pass (imf19.hostedemail.com: domain of elver@google.com designates 209.85.222.48 as permitted sender) smtp.mailfrom=elver@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1678376929; a=rsa-sha256; cv=none; b=zyuQ5FPB9WvJjlKOkudx2WW42mD91S9FMq5kFPv0RC7m5HMG0bqN3C4lrRBwu9KwF9aNoC rkDdAcyySRHgasCR9DMTgfCIgV2RqZ5zUvzCtVcjqFmrK1kbM8GiQp5T36E8qSp9d48mox KM7gkIB0OqioA5fJ72wNKSse1bCrS+A= Received: by mail-ua1-f48.google.com with SMTP id f17so1456014uax.7 for ; Thu, 09 Mar 2023 07:48:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; t=1678376928; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=bdBTcFq4uhvEA5oTaEAsvRAaPMfrbecdresU0Hqq2NY=; b=fE1nYEKPUH44xSn/cu3jKitarc57GTeJOICw8TVNnbggdG/tjrs3dVLV8uq9zj7wIF hCEH8WqnmArZwhg++i9Aat7FyinlqDw5TURySIrjQ9yfZ0CgZlf4XYzrsVy4MIjjCthB 4p89uZYaKYnkKKgMeZot1TTV89pPN6BSD8s55yFvcX901TV9ungzx1yZ1XiLnuJH9cWt ny4dp1NxIdcO6+Mj4/vcG03yrTmO8hajhz5GaoZ5gTgm9kNnLl5D9OYt09C7Pu3rcoV9 MkBVfZP37dowkWfw/GP3iABHrSgOeellUDZEr4CE28lcK7vEKOZC5h+SuXUa1UKb7w58 vQxA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1678376928; h=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=bdBTcFq4uhvEA5oTaEAsvRAaPMfrbecdresU0Hqq2NY=; b=GP+jOmOR3AN0yLebuy1g4OnYsRCdxOOJ7+NvYZCZ9aSuvLmhvQuWKYZeP6GFM4P6ES 3Glm6IF8Wx43i1RWAkz8u4gkMrVy18++iuPeoKNvFFC+95SIK5p1Hmcf/GlIO4nRpQjT 927rVymR+D5cgxA4y5N+Nxy7ElswNnDDwHbuYGZU/o4rCWIaHq/6fn3BC7JOV/i8/y/m kakLBKAq5dvKVxj3UoZezCvxhF3aQhylhl3fdQDsCUmVxDjqgIPFInaxLA7zv+SMpDDL WP2kbsXbhebGvpU8/sgfz61JgOTmd1gna7zpjXQryb4v9RLwfBzWkwpJ4+wk8DPjVjAq TmBg== X-Gm-Message-State: AO0yUKWJ5xUZZt/puCBxLvf93VqxFKNCRO0XuJ7PR2GiWnwZwyvAGQVb A3mF+K4nK1/pBau6bjWOgGXoYSmYFv31sKBV0Sr0tA== X-Google-Smtp-Source: AK7set/joUUbBLQBrBrRTobBnbTf6vzDHtbZpsjmq0zU65BQsoJQnAzbF9VZdngEaXqn49sTJbKR60bNh7Y/oduqQhM= X-Received: by 2002:a1f:b292:0:b0:42d:18f9:d0b6 with SMTP id b140-20020a1fb292000000b0042d18f9d0b6mr4216436vkf.2.1678376927991; Thu, 09 Mar 2023 07:48:47 -0800 (PST) MIME-Version: 1.0 References: <1678376273-7030-1-git-send-email-quic_zhenhuah@quicinc.com> In-Reply-To: <1678376273-7030-1-git-send-email-quic_zhenhuah@quicinc.com> From: Marco Elver Date: Thu, 9 Mar 2023 16:48:07 +0100 Message-ID: Subject: Re: [PATCH v2] mm,kfence: decouple kfence from page granularity mapping judgement To: Zhenhua Huang Cc: catalin.marinas@arm.com, will@kernel.org, glider@google.com, dvyukov@google.com, akpm@linux-foundation.org, robin.murphy@arm.com, mark.rutland@arm.com, jianyong.wu@arm.com, james.morse@arm.com, linux-arm-kernel@lists.infradead.org, kasan-dev@googlegroups.com, linux-mm@kvack.org, quic_pkondeti@quicinc.com, quic_guptap@quicinc.com, quic_tingweiz@quicinc.com Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 34C9B1A0003 X-Stat-Signature: k5ey9tq4t14eza83mqjkbjm43usftkqq X-Rspam-User: X-HE-Tag: 1678376928-819549 X-HE-Meta: U2FsdGVkX19bn7o0lljveyNPyuyYqfNtciTy/fWAZwji/1SdoztfGNmIaH5krTrrNddqivDHzf/WMyCYyUgLc4034ZdjHGsAH++LwvtphX7Xeq0ge16wQaKai1CKYWwYn8U0m7rnkklpsLYvXSJNC/XWTwx5CiZE+a+7MM6IUw+J2poiVO45Pe4ncBNlYcjxz5qewLYIBF1zJtd8sr5Q1yV6TDwQ5az/EMVJP/ouwdN9djP+eip0j6vDDVJRGy4f9yojnrF5992KvDoWqDPoV7zWVT6Wlgq8FGlh2qJ+8xlGwb+kus4QB4zN884cJn7K+/PzC/k9gTl8GpfUrUmrn4Vbz0F+cTa87NjJ+uHNsByj7X9Rb5ClaW3X7iCLejBELk5ZRjT6AB2A8GpFUE4GIKHd5KioJkTDjyLuI1Pl4UOqVg6OnI1XmuW4BU+9jW7KkMVESTYAanTamqNAcqvkfi/oZWopJVkwQgmVy7RtvOrA0Hxo1ooo2z+SypyMok+5LCjP5q2opGI46NOyNIIlwV3Bd+jVXrI7CgbTpZOZJdDEoR8mZYVYyARo+k5wj5Et2KgINOiSGGPH/7GBBVzC9dT1mXinpA7lgvhtZAs+tdkr1lp7Cql8iZQeLo2QvWemFMYdUqCYT7oPUow1g4sabKTWDwy87C2u5Y1Py6lPCjoQV/98T6SJN7yhIVBdL/3QM6cL9Jpi6F0tHKvVcRWifzwest9Al9vo0POjw2/+rCQPaFDHBTPw6rEiA4myYCqvql7F4Q+qHgy3ZDxC3wEoAvLVyQk2OvvtrBsPLrZ3u+7X4SlrXaYsxHatWAqNbNsSAoWLBzcs/bXM9S6o/0PAu1kGlT1XZa837JSKdfiKzAOWqODFNaiAzC2i6LFohw/nxLSVIpiZLAFzzpxqJ3rQWw8YPQGdvXWsveX2ey8/NPuiUkspkRed1LtHMEKG1+ZpHVAjVzYYx9pJpqOyyGX 43senA6O /YCRc4ebzKY/S3Mk53QsNSfSCvRKFIeKcRgyWNP2Hc6GjnQwhe5aTtrRc2VYhCJETpUJrKuc0op2iMjgd2H/9QiiNOPpC26lj+9Rpt6B/vC7Sr7gOJmghLaEsBz8Hv1Pwod+J97s/wzPs4VKF+UFVJz62kjxJtMV4oLy/5dIMqxg8yb66PL1IQz6q4Xf+0fvJJC+x3joWSTHfki3V5hmCN4TfiNRcCk8tfyrW6x7iU5tjnAIw4OrroozPmoOZVhg7UOSFKailNzux0BxRmsOjcIk5zkgbUHJRsLX8J2WHeuyLFCclFX2+tdx+DQqHWktw+Q9koYBPx1kwGHDvlgJj41IOF7vc+jwK/03UNnd9s33dYo0oYBI34rb2Zvzq+D39a/ugDK6NVqViCCWH6XpIl8TMVucYQIFfnkirmxvQPzbpc9w= 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 Thu, 9 Mar 2023 at 16:38, Zhenhua Huang wrote: > > Kfence only needs its pool to be mapped as page granularity, previous > judgement was a bit over protected. Decouple it from judgement and do > page granularity mapping for kfence pool only [1]. > > To implement this, also relocate the kfence pool allocation before the > linear mapping setting up, arm64_kfence_alloc_pool is to allocate phys > addr, __kfence_pool is to be set after linear mapping set up. > > LINK: [1] https://lore.kernel.org/linux-arm-kernel/1675750519-1064-1-git-send-email-quic_zhenhuah@quicinc.com/T/ > Suggested-by: Mark Rutland > Signed-off-by: Zhenhua Huang > --- > arch/arm64/mm/mmu.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ > arch/arm64/mm/pageattr.c | 5 ++--- > include/linux/kfence.h | 7 +++++++ > mm/kfence/core.c | 9 +++++++++ > 4 files changed, 62 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c > index 6f9d889..46afe3f 100644 > --- a/arch/arm64/mm/mmu.c > +++ b/arch/arm64/mm/mmu.c > @@ -24,6 +24,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -525,6 +526,33 @@ static int __init enable_crash_mem_map(char *arg) > } > early_param("crashkernel", enable_crash_mem_map); > > +#ifdef CONFIG_KFENCE > + > +static phys_addr_t arm64_kfence_alloc_pool(void) > +{ > + phys_addr_t kfence_pool = 0; > + > + if (!kfence_sample_interval) > + return 0; > + > + kfence_pool = memblock_phys_alloc(KFENCE_POOL_SIZE, PAGE_SIZE); > + if (!kfence_pool) { > + pr_err("failed to allocate kfence pool\n"); > + return 0; > + } > + > + return kfence_pool; > +} > + > +#else > + > +static phys_addr_t arm64_kfence_alloc_pool(void) > +{ > + return (phys_addr_t)NULL; Just return "0" - which the above function does as well on error. Or the above function should also do (phys_addr_t)NULL for consistency. > +} > + > +#endif > + > static void __init map_mem(pgd_t *pgdp) > { > static const u64 direct_map_end = _PAGE_END(VA_BITS_MIN); > @@ -532,6 +560,7 @@ static void __init map_mem(pgd_t *pgdp) > phys_addr_t kernel_end = __pa_symbol(__init_begin); > phys_addr_t start, end; > int flags = NO_EXEC_MAPPINGS; > + phys_addr_t kfence_pool = 0; > u64 i; > > /* > @@ -564,6 +593,10 @@ static void __init map_mem(pgd_t *pgdp) > } > #endif > > + kfence_pool = arm64_kfence_alloc_pool(); > + if (kfence_pool) > + memblock_mark_nomap(kfence_pool, KFENCE_POOL_SIZE); > + > /* map all the memory banks */ > for_each_mem_range(i, &start, &end) { > if (start >= end) > @@ -608,6 +641,17 @@ static void __init map_mem(pgd_t *pgdp) > } > } > #endif > + > + /* Kfence pool needs page-level mapping */ > + if (kfence_pool) { > + __map_memblock(pgdp, kfence_pool, > + kfence_pool + KFENCE_POOL_SIZE, > + pgprot_tagged(PAGE_KERNEL), > + NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS); > + memblock_clear_nomap(kfence_pool, KFENCE_POOL_SIZE); > + /* kfence_pool really mapped now */ > + kfence_set_pool(kfence_pool); > + } > } > > void mark_rodata_ro(void) > diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c > index 79dd201..61156d0 100644 > --- a/arch/arm64/mm/pageattr.c > +++ b/arch/arm64/mm/pageattr.c > @@ -22,12 +22,11 @@ bool rodata_full __ro_after_init = IS_ENABLED(CONFIG_RODATA_FULL_DEFAULT_ENABLED > bool can_set_direct_map(void) > { > /* > - * rodata_full, DEBUG_PAGEALLOC and KFENCE require linear map to be > + * rodata_full and DEBUG_PAGEALLOC require linear map to be > * mapped at page granularity, so that it is possible to > * protect/unprotect single pages. > */ > - return (rodata_enabled && rodata_full) || debug_pagealloc_enabled() || > - IS_ENABLED(CONFIG_KFENCE); > + return (rodata_enabled && rodata_full) || debug_pagealloc_enabled(); > } > > static int change_page_range(pte_t *ptep, unsigned long addr, void *data) > diff --git a/include/linux/kfence.h b/include/linux/kfence.h > index 726857a..d982ac2 100644 > --- a/include/linux/kfence.h > +++ b/include/linux/kfence.h > @@ -64,6 +64,11 @@ static __always_inline bool is_kfence_address(const void *addr) > void __init kfence_alloc_pool(void); > > /** > + * kfence_set_pool() - KFENCE pool mapped and can be used I don't understand the comment. Maybe just "allows an arch to set the KFENCE pool during early init" > + */ > +void __init kfence_set_pool(phys_addr_t addr); > + > +/** > * kfence_init() - perform KFENCE initialization at boot time > * > * Requires that kfence_alloc_pool() was called before. This sets up the > @@ -222,8 +227,10 @@ bool __kfence_obj_info(struct kmem_obj_info *kpp, void *object, struct slab *sla > > #else /* CONFIG_KFENCE */ > > +#define KFENCE_POOL_SIZE 0 > static inline bool is_kfence_address(const void *addr) { return false; } > static inline void kfence_alloc_pool(void) { } > +static inline void kfence_set_pool(phys_addr_t addr) { } > static inline void kfence_init(void) { } > static inline void kfence_shutdown_cache(struct kmem_cache *s) { } > static inline void *kfence_alloc(struct kmem_cache *s, size_t size, gfp_t flags) { return NULL; } > diff --git a/mm/kfence/core.c b/mm/kfence/core.c > index 5349c37..a17c20c2 100644 > --- a/mm/kfence/core.c > +++ b/mm/kfence/core.c > @@ -814,12 +814,21 @@ void __init kfence_alloc_pool(void) > if (!kfence_sample_interval) > return; > > + /* if __kfence_pool already initialized in some arch, abort */ Abort sounds like it's a failure condition, but it's actually ok. Maybe just write: /* Check if the pool has already been initialized by arch; if so, skip the below. */ > + if (__kfence_pool) > + return; > + > __kfence_pool = memblock_alloc(KFENCE_POOL_SIZE, PAGE_SIZE); > > if (!__kfence_pool) > pr_err("failed to allocate pool\n"); > } > > +void __init kfence_set_pool(phys_addr_t addr) > +{ > + __kfence_pool = phys_to_virt(addr); > +} > + The rest looks good.