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 03B3BEB64DA for ; Tue, 18 Jul 2023 12:40:23 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8B6906B0071; Tue, 18 Jul 2023 08:40:22 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8659F8D0002; Tue, 18 Jul 2023 08:40:22 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 72E3B8D0001; Tue, 18 Jul 2023 08:40:22 -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 656B56B0071 for ; Tue, 18 Jul 2023 08:40:22 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 3BED312011E for ; Tue, 18 Jul 2023 12:40:22 +0000 (UTC) X-FDA: 81024690684.05.C919655 Received: from mail-wm1-f54.google.com (mail-wm1-f54.google.com [209.85.128.54]) by imf10.hostedemail.com (Postfix) with ESMTP id 3ED7EC0006 for ; Tue, 18 Jul 2023 12:40:19 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=iPdReQBN; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf10.hostedemail.com: domain of elver@google.com designates 209.85.128.54 as permitted sender) smtp.mailfrom=elver@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1689684020; 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=ikv3c5UKmNpyTHlS5fwGJGbwI2S8WZSWuI+ewsXdmvo=; b=kgvaAzL08SO6HZmJq0ezn9JFO2Avp/44IjZmRNFOC7un08zRnjS9Cbowa/Kzq8CDzw7nv4 I/pzF8Fl08bdGYTAH4VuvgFWHXwAgmKT/EXx4N+PgGy7vnhlvK493DvqpB07lofjbFZ+cK JaTNYdjZo7NyBPwzwrDXTRGffVsxWjY= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=iPdReQBN; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf10.hostedemail.com: domain of elver@google.com designates 209.85.128.54 as permitted sender) smtp.mailfrom=elver@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1689684020; a=rsa-sha256; cv=none; b=fLGUulzNY/WaCIaehegE1qYRUQtk9HurUSuUjN8Dq8k1q+bkKzIdPWp/ctrt6d4OotNmD3 WvV2279z8p4TMadiI1opDp3lXmrKmU7jJqlcyYqI1mo2A2CrneLmMnRCFmJltg9Hpv6kA/ XUMpFj84TU2EUQKTnkrAvtr+FaSwe1E= Received: by mail-wm1-f54.google.com with SMTP id 5b1f17b1804b1-3fbc244d307so57117545e9.1 for ; Tue, 18 Jul 2023 05:40:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1689684018; x=1692276018; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=ikv3c5UKmNpyTHlS5fwGJGbwI2S8WZSWuI+ewsXdmvo=; b=iPdReQBNazY9OQXhuoZsLAPbpAGbQ9Ro4LpfVnH39BW3Z6RqsLJ8o2UDwsfJJyLyYK GGgGArNqz+jWFv1W7CHn/OEAF7aYEncVWkhE22fOJYGYDZ/K7ZRugbLzSviQwRV0/aI+ vI+wTD8rAFdnSqxwFw7Dd1epq9W4QNLRfmU33s592m44UrZnDGJTWoWO41gs+6lWx/2x Yxlz1q6tXcv61bQ6q0eUXT27tJlgrTh+YHR+9NFA/a0eGnO6lnhTqfiacrrSMTAA394L G1DdevYTPDxEZJ/SegVYoN6S9yMBh1rlu3mVKHoukMLKhROEyonHGG4KA7Tr7HgbOOBm OlOQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689684018; x=1692276018; 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=ikv3c5UKmNpyTHlS5fwGJGbwI2S8WZSWuI+ewsXdmvo=; b=I5fcRuMMCdUTUtdP5kaAFsIv/wuJRy4I4XVgISyfV4zIscieU1EcsNjzcob8iWFS6E fLBDrOHkF6bPF4+POiF3abwPMJIKeZlq3OkutGXQ3H3+SCeqElmLzgPyqL3Bp7XBz/d/ QkG4Z9/46w91KeeeOiQLVoIlmEjeDJUtwx6Q7WQxNDebA1e7EsRnDTzMInL9KqMMuKrQ C91Zj2+2IH0VAEKNFsfdt7sRYfNHRr07ZCbeSIl+pOQsNzlvqYfG8yzJaYi0Qc5UbQQ8 RwB8jxg1WYPXUksbxXHQJ1mZkgqLfIMOKnZFvOVsG5d0SD4D9NPDDG+FIBlEkankTWQ6 pvzg== X-Gm-Message-State: ABy/qLabCwC2emAbuAKyWJizJm1hWxrItNRpv3p+YVOBCpXpUgwqSiLx yCEr3KuIzp9zwriOb3+SNYpHT63e0VC+l1DGdmnKGg== X-Google-Smtp-Source: APBJJlHrZz/Cq7fkx1TjUUPkf1UaAo/58hjI1pBUORe6tJ3gjOw4Cm/O0FT/XmEHA68W/xgHC3q9CESzqjvaQP0XEAE= X-Received: by 2002:a05:600c:204b:b0:3fa:9561:3016 with SMTP id p11-20020a05600c204b00b003fa95613016mr1714538wmg.30.1689684018077; Tue, 18 Jul 2023 05:40:18 -0700 (PDT) MIME-Version: 1.0 References: <20230718073019.52513-1-zhangpeng.00@bytedance.com> In-Reply-To: <20230718073019.52513-1-zhangpeng.00@bytedance.com> From: Marco Elver Date: Tue, 18 Jul 2023 14:39:41 +0200 Message-ID: Subject: Re: [PATCH v3] mm: kfence: allocate kfence_metadata at runtime To: Peng Zhang Cc: glider@google.com, dvyukov@google.com, akpm@linux-foundation.org, kasan-dev@googlegroups.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org, muchun.song@linux.dev, Kefeng Wang Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 3ED7EC0006 X-Stat-Signature: 3sjte3848i8aqukmtcmndy7aupb9sexq X-Rspam-User: X-HE-Tag: 1689684019-531456 X-HE-Meta: U2FsdGVkX19aozYSs1DkMlR8Ql5NRqZq8UURMuN8ydoegiKP6kUxotZdZvvNCJ5J8ocpgF5+pkqCya24H5JXc2MCmp7c3yR1rz40dj8tF3nztxcfAIkdILGk+/CaoFWm0oz2IWBpkPLd6dtzQhRpsBCQ32KG6a7in6L9re4jpVf+HCD24ZGQ6dRsMw2bt7yf9s3HK7FVbOfXNn8oZMReJrEJLdgejREfEBLR7Jo+/4J9wHuItIn1+4K1IyMKIQR4XahB99Md+octwqNnrn/KFcAbxek+4xcVOgz8zC1qEWqhxEu4K44gxgLDHoSyVnbKOIx7nkDea04YOivTdnOYTRI3krNL1Pc2yTJqfw3L2D5HJTDxgubT1jYpsV1/n3Opa5ubZ3NmFjJfsZd1Xky+FQEQ+1h/Mt3AfgoeMqeHkLeb+3JGa3dLDw52GLJKU7EQncg6qu89xxBCtPyrPoG51MRI4kB8sLApa/mOss6wtqM7Y1pkLRLOkqaPQ/Gh37iAaYgoRPmDUG7HdZerUOq595T+oav/H4BIsWrzava4ZJ8htnj+kJkxH9ZWVuI9f13EGfyHrNmHVWEjmsOTvGrPYHtCUR3ZTIipVA6ev+DqqWglSOYql3rpYIRVb0P2Fss9A/XrAkgqltIm83ItZ1ZyNdD6lW+6ZO2IDA6a5eZzoC97vUWQqgNNfcFN/FdgWlLphulU2lYbHt/PoRTl9lLdE/mw/3zA9F/YaS+0Sagay0bXzAOYDUKA67VSXPNJO9EN6uhU4OF4T46SI0aklII1ehYoKMyhwaQ1LeFPrt7cAa8N1Wgp6Otfdc9AD/XIkKbZ7A2MAEMtisN5qBZIt4VZUUDYp4/XzA/buU+euHlr/fL/NWzrYAa/bX2RsvqDmx4p2CvvEmfBdTh91pqaCTF66ZySK/yTRqQhmNt4ERhltLMGIyoIcirNbAB+gaKOzrnbw8g7QDthKacRq/z69Rb +47xDRDf Jc0VFfLgITjxY7dmu4kGy8u/BcFUnYuR6gM4Ja/bKmCk7Doqmj95TiG9MsOOfwlaSf8pP//qOdQ9HJe+tUO2Yl+aF3kN1ZkFowEAoH4QmoVCI5ktkyyR2HEpZWIWgWCxYq4kkJJDcTX0fjmwcit+bwAxBhWvA41GNF8YJAghSyppugUVjdb5OO/wi1bezAIPLeesQK1E+7By9VEMqE4a/5aQr5AW/5aCmc54j/jPOlRPfhbnLcQgLkYWAEOMglaOV5uVXcRVQ/hEOiBpZ453AuINXBbOpFunxn2XIw7BwvNlFauOOboPiPHaI8c6BkLmUpksSWwgYdAxZaXOBSLVmISmWOiNJ/qIkK21mOwNPzckkiqCLdR4yHh7LzWHzFkRkNZblx03SVSqHpP3bZtPnIHfrVatQAKoQnaXz 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, 18 Jul 2023 at 09:30, Peng Zhang wrote: > > kfence_metadata is currently a static array. For the purpose of allocating > scalable __kfence_pool, we first change it to runtime allocation of > metadata. Since the size of an object of kfence_metadata is 1160 bytes, we > can save at least 72 pages (with default 256 objects) without enabling > kfence. > > Signed-off-by: Peng Zhang This looks good (minor nit below). Reviewed-by: Marco Elver Thanks! > --- > Changes since v2: > - Fix missing renaming of kfence_alloc_pool. > - Add __read_mostly for kfence_metadata and kfence_metadata_init. > - Use smp_store_release() and smp_load_acquire() to access kfence_metadata. > - Some tweaks to comments and git log. > > v1: https://lore.kernel.org/lkml/20230710032714.26200-1-zhangpeng.00@bytedance.com/ > v2: https://lore.kernel.org/lkml/20230712081616.45177-1-zhangpeng.00@bytedance.com/ > > include/linux/kfence.h | 11 ++-- > mm/kfence/core.c | 124 ++++++++++++++++++++++++++++------------- > mm/kfence/kfence.h | 5 +- > mm/mm_init.c | 2 +- > 4 files changed, 97 insertions(+), 45 deletions(-) > > diff --git a/include/linux/kfence.h b/include/linux/kfence.h > index 726857a4b680..401af4757514 100644 > --- a/include/linux/kfence.h > +++ b/include/linux/kfence.h > @@ -59,15 +59,16 @@ static __always_inline bool is_kfence_address(const void *addr) > } > > /** > - * kfence_alloc_pool() - allocate the KFENCE pool via memblock > + * kfence_alloc_pool_and_metadata() - allocate the KFENCE pool and KFENCE > + * metadata via memblock > */ > -void __init kfence_alloc_pool(void); > +void __init kfence_alloc_pool_and_metadata(void); > > /** > * kfence_init() - perform KFENCE initialization at boot time > * > - * Requires that kfence_alloc_pool() was called before. This sets up the > - * allocation gate timer, and requires that workqueues are available. > + * Requires that kfence_alloc_pool_and_metadata() was called before. This sets > + * up the allocation gate timer, and requires that workqueues are available. > */ > void __init kfence_init(void); > > @@ -223,7 +224,7 @@ bool __kfence_obj_info(struct kmem_obj_info *kpp, void *object, struct slab *sla > #else /* CONFIG_KFENCE */ > > static inline bool is_kfence_address(const void *addr) { return false; } > -static inline void kfence_alloc_pool(void) { } > +static inline void kfence_alloc_pool_and_metadata(void) { } > 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 dad3c0eb70a0..6b526435886c 100644 > --- a/mm/kfence/core.c > +++ b/mm/kfence/core.c > @@ -116,7 +116,15 @@ EXPORT_SYMBOL(__kfence_pool); /* Export for test modules. */ > * backing pages (in __kfence_pool). > */ > static_assert(CONFIG_KFENCE_NUM_OBJECTS > 0); > -struct kfence_metadata kfence_metadata[CONFIG_KFENCE_NUM_OBJECTS]; > +struct kfence_metadata *kfence_metadata __read_mostly; > + > +/* > + * If kfence_metadata is not NULL, it may be accessed by kfence_shutdown_cache(). > + * So introduce kfence_metadata_init to initialize metadata, and then make > + * kfence_metadata visible after initialization is successful. This prevents > + * potential UAF or access to uninitialized metadata. > + */ > +static struct kfence_metadata *kfence_metadata_init __read_mostly; > > /* Freelist with available objects. */ > static struct list_head kfence_freelist = LIST_HEAD_INIT(kfence_freelist); > @@ -591,7 +599,7 @@ static unsigned long kfence_init_pool(void) > > __folio_set_slab(slab_folio(slab)); > #ifdef CONFIG_MEMCG > - slab->memcg_data = (unsigned long)&kfence_metadata[i / 2 - 1].objcg | > + slab->memcg_data = (unsigned long)&kfence_metadata_init[i / 2 - 1].objcg | > MEMCG_DATA_OBJCGS; > #endif > } > @@ -610,7 +618,7 @@ static unsigned long kfence_init_pool(void) > } > > for (i = 0; i < CONFIG_KFENCE_NUM_OBJECTS; i++) { > - struct kfence_metadata *meta = &kfence_metadata[i]; > + struct kfence_metadata *meta = &kfence_metadata_init[i]; > > /* Initialize metadata. */ > INIT_LIST_HEAD(&meta->list); > @@ -626,6 +634,12 @@ static unsigned long kfence_init_pool(void) > addr += 2 * PAGE_SIZE; > } > > + /* > + * Make kfence_metadata visible only when initialization is successful. > + * Otherwise, if the initialization fails and kfence_metadata is freed, > + * it may cause UAF in kfence_shutdown_cache(). > + */ > + smp_store_release(&kfence_metadata, kfence_metadata_init); > return 0; > > reset_slab: > @@ -672,26 +686,10 @@ static bool __init kfence_init_pool_early(void) > */ > memblock_free_late(__pa(addr), KFENCE_POOL_SIZE - (addr - (unsigned long)__kfence_pool)); > __kfence_pool = NULL; > - return false; > -} > - > -static bool kfence_init_pool_late(void) > -{ > - unsigned long addr, free_size; > > - addr = kfence_init_pool(); > - > - if (!addr) > - return true; > + memblock_free_late(__pa(kfence_metadata_init), KFENCE_METADATA_SIZE); > + kfence_metadata_init = NULL; > > - /* Same as above. */ > - free_size = KFENCE_POOL_SIZE - (addr - (unsigned long)__kfence_pool); > -#ifdef CONFIG_CONTIG_ALLOC > - free_contig_range(page_to_pfn(virt_to_page((void *)addr)), free_size / PAGE_SIZE); > -#else > - free_pages_exact((void *)addr, free_size); > -#endif > - __kfence_pool = NULL; > return false; > } > > @@ -841,19 +839,30 @@ static void toggle_allocation_gate(struct work_struct *work) > > /* === Public interface ===================================================== */ > > -void __init kfence_alloc_pool(void) > +void __init kfence_alloc_pool_and_metadata(void) > { > if (!kfence_sample_interval) > return; > > - /* if the pool has already been initialized by arch, skip the below. */ > - if (__kfence_pool) > - return; > - > - __kfence_pool = memblock_alloc(KFENCE_POOL_SIZE, PAGE_SIZE); > - > + /* > + * If the pool has already been initialized by arch, there is no need to > + * re-allocate the memory pool. > + */ > if (!__kfence_pool) > + __kfence_pool = memblock_alloc(KFENCE_POOL_SIZE, PAGE_SIZE); > + > + if (!__kfence_pool) { > pr_err("failed to allocate pool\n"); > + return; > + } > + > + /* The memory allocated by memblock has been zeroed out. */ > + kfence_metadata_init = memblock_alloc(KFENCE_METADATA_SIZE, PAGE_SIZE); > + if (!kfence_metadata_init) { > + pr_err("failed to allocate metadata\n"); > + memblock_free(__kfence_pool, KFENCE_POOL_SIZE); > + __kfence_pool = NULL; > + } > } > > static void kfence_init_enable(void) > @@ -895,33 +904,68 @@ void __init kfence_init(void) > > static int kfence_init_late(void) > { > - const unsigned long nr_pages = KFENCE_POOL_SIZE / PAGE_SIZE; > + const unsigned long nr_pages_pool = KFENCE_POOL_SIZE / PAGE_SIZE; > + const unsigned long nr_pages_meta = KFENCE_METADATA_SIZE / PAGE_SIZE; > + unsigned long addr = (unsigned long)__kfence_pool; > + unsigned long free_size = KFENCE_POOL_SIZE; > + int err = -ENOMEM; > + > #ifdef CONFIG_CONTIG_ALLOC > struct page *pages; > - Unnecessary blank line removal (it looks worse now). > - pages = alloc_contig_pages(nr_pages, GFP_KERNEL, first_online_node, NULL); > + pages = alloc_contig_pages(nr_pages_pool, GFP_KERNEL, first_online_node, > + NULL); > if (!pages) > return -ENOMEM; > + > __kfence_pool = page_to_virt(pages); > + pages = alloc_contig_pages(nr_pages_meta, GFP_KERNEL, first_online_node, > + NULL); > + if (pages) > + kfence_metadata_init = page_to_virt(pages); > #else > - if (nr_pages > MAX_ORDER_NR_PAGES) { > + if (nr_pages_pool > MAX_ORDER_NR_PAGES || > + nr_pages_meta > MAX_ORDER_NR_PAGES) { > pr_warn("KFENCE_NUM_OBJECTS too large for buddy allocator\n"); > return -EINVAL; > } > + > __kfence_pool = alloc_pages_exact(KFENCE_POOL_SIZE, GFP_KERNEL); > if (!__kfence_pool) > return -ENOMEM; > + > + kfence_metadata_init = alloc_pages_exact(KFENCE_METADATA_SIZE, GFP_KERNEL); > #endif > > - if (!kfence_init_pool_late()) { > - pr_err("%s failed\n", __func__); > - return -EBUSY; > + if (!kfence_metadata_init) > + goto free_pool; > + > + memzero_explicit(kfence_metadata_init, KFENCE_METADATA_SIZE); > + addr = kfence_init_pool(); > + if (!addr) { > + kfence_init_enable(); > + kfence_debugfs_init(); > + return 0; > } > > - kfence_init_enable(); > - kfence_debugfs_init(); > + pr_err("%s failed\n", __func__); > + free_size = KFENCE_POOL_SIZE - (addr - (unsigned long)__kfence_pool); > + err = -EBUSY; > > - return 0; > +#ifdef CONFIG_CONTIG_ALLOC > + free_contig_range(page_to_pfn(virt_to_page((void *)kfence_metadata_init)), > + nr_pages_meta); > +free_pool: > + free_contig_range(page_to_pfn(virt_to_page((void *)addr)), > + free_size / PAGE_SIZE); > +#else > + free_pages_exact((void *)kfence_metadata_init, KFENCE_METADATA_SIZE); > +free_pool: > + free_pages_exact((void *)addr, free_size); > +#endif > + > + kfence_metadata_init = NULL; > + __kfence_pool = NULL; > + return err; > } > > static int kfence_enable_late(void) > @@ -941,6 +985,10 @@ void kfence_shutdown_cache(struct kmem_cache *s) > struct kfence_metadata *meta; > int i; > > + /* Pairs with release in kfence_init_pool(). */ > + if (!smp_load_acquire(&kfence_metadata)) > + return; > + > for (i = 0; i < CONFIG_KFENCE_NUM_OBJECTS; i++) { > bool in_use; > > diff --git a/mm/kfence/kfence.h b/mm/kfence/kfence.h > index 392fb273e7bd..f46fbb03062b 100644 > --- a/mm/kfence/kfence.h > +++ b/mm/kfence/kfence.h > @@ -102,7 +102,10 @@ struct kfence_metadata { > #endif > }; > > -extern struct kfence_metadata kfence_metadata[CONFIG_KFENCE_NUM_OBJECTS]; > +#define KFENCE_METADATA_SIZE PAGE_ALIGN(sizeof(struct kfence_metadata) * \ > + CONFIG_KFENCE_NUM_OBJECTS) > + > +extern struct kfence_metadata *kfence_metadata; > > static inline struct kfence_metadata *addr_to_metadata(unsigned long addr) > { > diff --git a/mm/mm_init.c b/mm/mm_init.c > index 7f7f9c677854..3d0a63c75829 100644 > --- a/mm/mm_init.c > +++ b/mm/mm_init.c > @@ -2721,7 +2721,7 @@ void __init mm_core_init(void) > */ > page_ext_init_flatmem(); > mem_debugging_and_hardening_init(); > - kfence_alloc_pool(); > + kfence_alloc_pool_and_metadata(); > report_meminit(); > kmsan_init_shadow(); > stack_depot_early_init(); > -- > 2.20.1 >