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 X-Spam-Level: X-Spam-Status: No, score=-11.4 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E7A47C47423 for ; Fri, 2 Oct 2020 14:23:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A628B207DE for ; Fri, 2 Oct 2020 14:23:13 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="RKfZWi+R" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388087AbgJBOXM (ORCPT ); Fri, 2 Oct 2020 10:23:12 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53514 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2387688AbgJBOXM (ORCPT ); Fri, 2 Oct 2020 10:23:12 -0400 Received: from mail-qt1-x841.google.com (mail-qt1-x841.google.com [IPv6:2607:f8b0:4864:20::841]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 09569C0613E2 for ; Fri, 2 Oct 2020 07:23:12 -0700 (PDT) Received: by mail-qt1-x841.google.com with SMTP id j22so1310888qtj.8 for ; Fri, 02 Oct 2020 07:23:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=CAgelpYpJihRsjmlp4EjmhmaunlYd8C4mVKtF8jDi3w=; b=RKfZWi+RUXInTmQ3BLHhXRQmW4buor7BLC4IxwXfU0Vqf9mByJNzip/dve4hOmvzk6 EaPUIEdyK1G1ywRnCXUSs6nvcfZgwNa9fxlrgjhNf6Lx1tOsZrRqjN9/KaNIfA3BeviP YAVkCBrNlInA7HUts3canKehhdEDbAXhYkd/3n/HlYQAaC0a6m+AKB7qz+rs84ZfK7A3 ea3Shr+TAS/DiGKVASkTi14XB182x/nbUVybsej3xjWJgec9CX3W/cxbq374DFY9JlTT 3dnQFEJNiGdMLbJxX3RuO43mIUOn+sDon5zqHf5EDY4guonk1AnckfPwU4PkR4lsqbpu n4Uw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=CAgelpYpJihRsjmlp4EjmhmaunlYd8C4mVKtF8jDi3w=; b=JEwgZ2Wh/qqvzyWBvVsju4jJq0DFH8wDVVRlQTRtHhRH8o3GSEOZl8dWGPB9EKY6eB 88mSAUPZxNR5xs6gAMb6DaLYf7vsgm/VQSDtQn0IaJcrObc4vLjx7P5fyG6HNrPcf+bn 5Bk9dOGy4INVMXKchlauHBe6aC0Crmfv2JCdnaY02RGUWZo9OagdvbWtrzIkTpuXWAXE x+fx+HaQfNkkcMX+nwSGRU8r0tL/+f9Q4HxTmx/cRmHCgw1VmppeEt6vcxMPAASy38hs AD1tKIdoKwbXaHg/Yu+i6KqwfCF96GMKUWR5hGZJyrlBDi6BE6w10mbTH8VUmm6O6xF+ f3Aw== X-Gm-Message-State: AOAM5334M3h4AK3WmwbpyK6u1Jpelmu0J7TWoeGoM+5jwu0RZwo3gOUB Dgt7+FDgB2VElngVWVSd6X6A1df4WhRdrtHOzNV20A== X-Google-Smtp-Source: ABdhPJxhXvszZ3go98nq7QHOuHf3kr4I/tA2aQd/4OmU/YkV5N0hrqi5M/eM2IoMPwR8MVkfDwfA94y9edqNJufG7i4= X-Received: by 2002:ac8:4806:: with SMTP id g6mr2529805qtq.380.1601648590888; Fri, 02 Oct 2020 07:23:10 -0700 (PDT) MIME-Version: 1.0 References: <20200929133814.2834621-1-elver@google.com> <20200929133814.2834621-2-elver@google.com> In-Reply-To: From: Dmitry Vyukov Date: Fri, 2 Oct 2020 16:22:59 +0200 Message-ID: Subject: Re: [PATCH v4 01/11] mm: add Kernel Electric-Fence infrastructure To: Jann Horn Cc: Marco Elver , Andrew Morton , Alexander Potapenko , "H . Peter Anvin" , "Paul E . McKenney" , Andrey Konovalov , Andrey Ryabinin , Andy Lutomirski , Borislav Petkov , Catalin Marinas , Christoph Lameter , Dave Hansen , David Rientjes , Eric Dumazet , Greg Kroah-Hartman , Hillf Danton , Ingo Molnar , Jonathan.Cameron@huawei.com, Jonathan Corbet , Joonsoo Kim , Kees Cook , Mark Rutland , Pekka Enberg , Peter Zijlstra , sjpark@amazon.com, Thomas Gleixner , Vlastimil Babka , Will Deacon , "the arch/x86 maintainers" , "open list:DOCUMENTATION" , kernel list , kasan-dev , Linux ARM , Linux-MM , SeongJae Park Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Oct 2, 2020 at 9:54 AM Jann Horn wrote: > > On Fri, Oct 2, 2020 at 8:33 AM Jann Horn wrote: > > On Tue, Sep 29, 2020 at 3:38 PM Marco Elver wrote: > > > This adds the Kernel Electric-Fence (KFENCE) infrastructure. KFENCE is a > > > low-overhead sampling-based memory safety error detector of heap > > > use-after-free, invalid-free, and out-of-bounds access errors. > > > > > > KFENCE is designed to be enabled in production kernels, and has near > > > zero performance overhead. Compared to KASAN, KFENCE trades performance > > > for precision. The main motivation behind KFENCE's design, is that with > > > enough total uptime KFENCE will detect bugs in code paths not typically > > > exercised by non-production test workloads. One way to quickly achieve a > > > large enough total uptime is when the tool is deployed across a large > > > fleet of machines. > [...] > > > +/* > > > + * The pool of pages used for guard pages and objects. If supported, allocated > > > + * statically, so that is_kfence_address() avoids a pointer load, and simply > > > + * compares against a constant address. Assume that if KFENCE is compiled into > > > + * the kernel, it is usually enabled, and the space is to be allocated one way > > > + * or another. > > > + */ > > > > If this actually brings a performance win, the proper way to do this > > would probably be to implement this as generic kernel infrastructure > > that makes the compiler emit large-offset relocations (either through > > compiler support or using inline asm statements that move an immediate > > into a register output and register the location in a special section, > > kinda like how e.g. static keys work) and patches them at boot time, > > or something like that - there are other places in the kernel where > > very hot code uses global pointers that are only ever written once > > during boot, e.g. the dentry cache of the VFS and the futex hash > > table. Those are probably far hotter than the kfence code. > > > > While I understand that that goes beyond the scope of this project, it > > might be something to work on going forward - this kind of > > special-case logic that turns the kernel data section into heap memory > > would not be needed if we had that kind of infrastructure. > > After thinking about it a bit more, I'm not even convinced that this > is a net positive in terms of overall performance - while it allows > you to avoid one level of indirection in some parts of kfence, that > kfence code by design only runs pretty infrequently. And to enable > this indirection avoidance, your x86 arch_kfence_initialize_pool() is > shattering potentially unrelated hugepages in the kernel data section, > which might increase the TLB pressure (and therefore the number of > memory loads that have to fall back to slow page walks) in code that > is much hotter than yours. > > And if this indirection is a real performance problem, that problem > would be many times worse in the VFS and the futex subsystem, so > developing a more generic framework for doing this cleanly would be > far more important than designing special-case code to allow kfence to > do this. > > And from what I've seen, a non-trivial chunk of the code in this > series, especially the arch/ parts, is only necessary to enable this > microoptimization. > > Do you have performance numbers or a description of why you believe > that this part of kfence is exceptionally performance-sensitive? If > not, it might be a good idea to remove this optimization, at least for > the initial version of this code. (And even if the optimization is > worthwhile, it might be a better idea to go for the generic version > immediately.) This check is very hot, it happens on every free. For every freed object we need to understand if it belongs to KFENCE or not. The generic framework for this already exists -- you simply create a global variable ;) KFENCE needs the range to be covered by struct page's and that's what creates problems for arm64. But I would assume most other users don't need that. 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 X-Spam-Level: X-Spam-Status: No, score=-11.4 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A49DAC47425 for ; Fri, 2 Oct 2020 14:23:14 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 1DCED2193E for ; Fri, 2 Oct 2020 14:23:13 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="RKfZWi+R" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1DCED2193E Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 843F26B0071; Fri, 2 Oct 2020 10:23:13 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7CE7F6B0072; Fri, 2 Oct 2020 10:23:13 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 66D256B0073; Fri, 2 Oct 2020 10:23:13 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 2F9CC6B0071 for ; Fri, 2 Oct 2020 10:23:13 -0400 (EDT) Received: from smtpin07.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id BDA82362C for ; Fri, 2 Oct 2020 14:23:12 +0000 (UTC) X-FDA: 77327202624.07.print24_490ade4271a4 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin07.hostedemail.com (Postfix) with ESMTP id 9B82C1803F9AB for ; Fri, 2 Oct 2020 14:23:12 +0000 (UTC) X-HE-Tag: print24_490ade4271a4 X-Filterd-Recvd-Size: 8249 Received: from mail-qt1-f193.google.com (mail-qt1-f193.google.com [209.85.160.193]) by imf02.hostedemail.com (Postfix) with ESMTP for ; Fri, 2 Oct 2020 14:23:11 +0000 (UTC) Received: by mail-qt1-f193.google.com with SMTP id n10so1337680qtv.3 for ; Fri, 02 Oct 2020 07:23:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=CAgelpYpJihRsjmlp4EjmhmaunlYd8C4mVKtF8jDi3w=; b=RKfZWi+RUXInTmQ3BLHhXRQmW4buor7BLC4IxwXfU0Vqf9mByJNzip/dve4hOmvzk6 EaPUIEdyK1G1ywRnCXUSs6nvcfZgwNa9fxlrgjhNf6Lx1tOsZrRqjN9/KaNIfA3BeviP YAVkCBrNlInA7HUts3canKehhdEDbAXhYkd/3n/HlYQAaC0a6m+AKB7qz+rs84ZfK7A3 ea3Shr+TAS/DiGKVASkTi14XB182x/nbUVybsej3xjWJgec9CX3W/cxbq374DFY9JlTT 3dnQFEJNiGdMLbJxX3RuO43mIUOn+sDon5zqHf5EDY4guonk1AnckfPwU4PkR4lsqbpu n4Uw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=CAgelpYpJihRsjmlp4EjmhmaunlYd8C4mVKtF8jDi3w=; b=FhVmNmiZBWFMlVXSNyFdrs4EsINMIFV9DwcycVmTeb9j6H7qk6yj8FN25vIRcObZB8 WJTMMCrj3zRuaYoVeQKDhc+0bbHcNikafCam4A5qAujBw0QblKnXAQqWmVkCn7QElyKt Y5p2wxCWIrTPE2EcyVwVZ0T78z9Htpj8SSrXwzW/HGOeDWuShRLniTojuY/jtYjSdQf/ 2xLsrfVR8q99fqnnwd8MwQK5P18qxug9n/F4hqhaUkgvFAB4PXmv64FHO3SPUEZRHJhm QD1fBzMsD42JyuJ1ykYYZxqbdLHhUZaDRcPK+GJGc1IkuMOw6SnTcJ9k4RheobDgcNd6 RTQg== X-Gm-Message-State: AOAM532iEFtlHcHFll+VeaOxPBCIkPPtUQkDZjD5STpHiI0m8or832SO yt3h4UdcAyAg2WEcsDhZ7zimPP/KaBwrrCtinyWA4A== X-Google-Smtp-Source: ABdhPJxhXvszZ3go98nq7QHOuHf3kr4I/tA2aQd/4OmU/YkV5N0hrqi5M/eM2IoMPwR8MVkfDwfA94y9edqNJufG7i4= X-Received: by 2002:ac8:4806:: with SMTP id g6mr2529805qtq.380.1601648590888; Fri, 02 Oct 2020 07:23:10 -0700 (PDT) MIME-Version: 1.0 References: <20200929133814.2834621-1-elver@google.com> <20200929133814.2834621-2-elver@google.com> In-Reply-To: From: Dmitry Vyukov Date: Fri, 2 Oct 2020 16:22:59 +0200 Message-ID: Subject: Re: [PATCH v4 01/11] mm: add Kernel Electric-Fence infrastructure To: Jann Horn Cc: Marco Elver , Andrew Morton , Alexander Potapenko , "H . Peter Anvin" , "Paul E . McKenney" , Andrey Konovalov , Andrey Ryabinin , Andy Lutomirski , Borislav Petkov , Catalin Marinas , Christoph Lameter , Dave Hansen , David Rientjes , Eric Dumazet , Greg Kroah-Hartman , Hillf Danton , Ingo Molnar , Jonathan.Cameron@huawei.com, Jonathan Corbet , Joonsoo Kim , Kees Cook , Mark Rutland , Pekka Enberg , Peter Zijlstra , sjpark@amazon.com, Thomas Gleixner , Vlastimil Babka , Will Deacon , "the arch/x86 maintainers" , "open list:DOCUMENTATION" , kernel list , kasan-dev , Linux ARM , Linux-MM , SeongJae Park Content-Type: text/plain; charset="UTF-8" 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 Fri, Oct 2, 2020 at 9:54 AM Jann Horn wrote: > > On Fri, Oct 2, 2020 at 8:33 AM Jann Horn wrote: > > On Tue, Sep 29, 2020 at 3:38 PM Marco Elver wrote: > > > This adds the Kernel Electric-Fence (KFENCE) infrastructure. KFENCE is a > > > low-overhead sampling-based memory safety error detector of heap > > > use-after-free, invalid-free, and out-of-bounds access errors. > > > > > > KFENCE is designed to be enabled in production kernels, and has near > > > zero performance overhead. Compared to KASAN, KFENCE trades performance > > > for precision. The main motivation behind KFENCE's design, is that with > > > enough total uptime KFENCE will detect bugs in code paths not typically > > > exercised by non-production test workloads. One way to quickly achieve a > > > large enough total uptime is when the tool is deployed across a large > > > fleet of machines. > [...] > > > +/* > > > + * The pool of pages used for guard pages and objects. If supported, allocated > > > + * statically, so that is_kfence_address() avoids a pointer load, and simply > > > + * compares against a constant address. Assume that if KFENCE is compiled into > > > + * the kernel, it is usually enabled, and the space is to be allocated one way > > > + * or another. > > > + */ > > > > If this actually brings a performance win, the proper way to do this > > would probably be to implement this as generic kernel infrastructure > > that makes the compiler emit large-offset relocations (either through > > compiler support or using inline asm statements that move an immediate > > into a register output and register the location in a special section, > > kinda like how e.g. static keys work) and patches them at boot time, > > or something like that - there are other places in the kernel where > > very hot code uses global pointers that are only ever written once > > during boot, e.g. the dentry cache of the VFS and the futex hash > > table. Those are probably far hotter than the kfence code. > > > > While I understand that that goes beyond the scope of this project, it > > might be something to work on going forward - this kind of > > special-case logic that turns the kernel data section into heap memory > > would not be needed if we had that kind of infrastructure. > > After thinking about it a bit more, I'm not even convinced that this > is a net positive in terms of overall performance - while it allows > you to avoid one level of indirection in some parts of kfence, that > kfence code by design only runs pretty infrequently. And to enable > this indirection avoidance, your x86 arch_kfence_initialize_pool() is > shattering potentially unrelated hugepages in the kernel data section, > which might increase the TLB pressure (and therefore the number of > memory loads that have to fall back to slow page walks) in code that > is much hotter than yours. > > And if this indirection is a real performance problem, that problem > would be many times worse in the VFS and the futex subsystem, so > developing a more generic framework for doing this cleanly would be > far more important than designing special-case code to allow kfence to > do this. > > And from what I've seen, a non-trivial chunk of the code in this > series, especially the arch/ parts, is only necessary to enable this > microoptimization. > > Do you have performance numbers or a description of why you believe > that this part of kfence is exceptionally performance-sensitive? If > not, it might be a good idea to remove this optimization, at least for > the initial version of this code. (And even if the optimization is > worthwhile, it might be a better idea to go for the generic version > immediately.) This check is very hot, it happens on every free. For every freed object we need to understand if it belongs to KFENCE or not. The generic framework for this already exists -- you simply create a global variable ;) KFENCE needs the range to be covered by struct page's and that's what creates problems for arm64. But I would assume most other users don't need that. 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 X-Spam-Level: X-Spam-Status: No, score=-3.7 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_ADSP_CUSTOM_MED,DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8FA57C47423 for ; Fri, 2 Oct 2020 14:24:41 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 14B01206DB for ; Fri, 2 Oct 2020 14:24:41 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="PS9VGt8t"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=google.com header.i=@google.com header.b="RKfZWi+R" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 14B01206DB Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:To:Subject:Message-ID:Date:From:In-Reply-To: References:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=X62Bxh/E2lTzm6D6gTApnCw9ETMk8xIZNtzKp41OUs0=; b=PS9VGt8tQYlge69s0C6wKk27n i8nqjQ2PbBJ6qLZoMSCjs9wUguzHhZNGgYnuRWRS7QIF6gBvg+K3lhcMwER/N8ZUWC1nA1TQCnYyU GJQ1s1Cs3bV68MggonV09B4w619X14VHaAeKue1HmseOKonXwknE/gO7juxe3VQFdz4kFAJdkFiZq c1XKdMSWdeO+6obySv9CPx3fxpSpS3ADdyPtMH1c3o8FTjy5ZZGcr3TmlC2oIbDRayuWBhxzQO7P5 KJw+p3k1LAt/8onQJbSw0y1VbPEvl2OHMgaD/CL/AGcoEaDQRHIF+HzM8Bai4Sk9tlHcpA+4TE6lk ZuV475erQ==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kOLxt-0000CC-IJ; Fri, 02 Oct 2020 14:23:17 +0000 Received: from mail-qt1-x843.google.com ([2607:f8b0:4864:20::843]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kOLxq-0000AG-RJ for linux-arm-kernel@lists.infradead.org; Fri, 02 Oct 2020 14:23:15 +0000 Received: by mail-qt1-x843.google.com with SMTP id d1so1316349qtr.6 for ; Fri, 02 Oct 2020 07:23:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=CAgelpYpJihRsjmlp4EjmhmaunlYd8C4mVKtF8jDi3w=; b=RKfZWi+RUXInTmQ3BLHhXRQmW4buor7BLC4IxwXfU0Vqf9mByJNzip/dve4hOmvzk6 EaPUIEdyK1G1ywRnCXUSs6nvcfZgwNa9fxlrgjhNf6Lx1tOsZrRqjN9/KaNIfA3BeviP YAVkCBrNlInA7HUts3canKehhdEDbAXhYkd/3n/HlYQAaC0a6m+AKB7qz+rs84ZfK7A3 ea3Shr+TAS/DiGKVASkTi14XB182x/nbUVybsej3xjWJgec9CX3W/cxbq374DFY9JlTT 3dnQFEJNiGdMLbJxX3RuO43mIUOn+sDon5zqHf5EDY4guonk1AnckfPwU4PkR4lsqbpu n4Uw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=CAgelpYpJihRsjmlp4EjmhmaunlYd8C4mVKtF8jDi3w=; b=n/uP5fR9G7rR49/Sb7lQpXqMoAZe6IpPQICeq2y5WAQ9t59r8yvI0oZx9/Cm8DX3Ld bop3zstSFRMnhlB2f16r2ILjSpciGXuVl71LMoiJcUr+3gDLwSpPzADDvwR2WKVN9Db5 SwvusbJJoShi1VmX4c4a2SyewUSZU/tRi4+kG2Bw694XgnpQvfv188BMm99mZEHomraO fr9/YRPscl9qk3gqmuR9I+xfGLGnT9+EzeCTd2CCVJbBwNVcLSjPn1klTT0LY6ZTC4oV lKcOmA+RMSGZf3IQNrVP9STEP7Sa58ClcVwTN9GEJMqNkSs/6XqnsX49W/fgscaMjtuw XD9Q== X-Gm-Message-State: AOAM533WwL7jy9RiUw6Cjt5dNZoR6POaC3dfz/4vqPndsGio0QM2aFX8 gWqGBrXf4VVZ725HwaQKqFHtsz42DplhNKbGRMcFeQ== X-Google-Smtp-Source: ABdhPJxhXvszZ3go98nq7QHOuHf3kr4I/tA2aQd/4OmU/YkV5N0hrqi5M/eM2IoMPwR8MVkfDwfA94y9edqNJufG7i4= X-Received: by 2002:ac8:4806:: with SMTP id g6mr2529805qtq.380.1601648590888; Fri, 02 Oct 2020 07:23:10 -0700 (PDT) MIME-Version: 1.0 References: <20200929133814.2834621-1-elver@google.com> <20200929133814.2834621-2-elver@google.com> In-Reply-To: From: Dmitry Vyukov Date: Fri, 2 Oct 2020 16:22:59 +0200 Message-ID: Subject: Re: [PATCH v4 01/11] mm: add Kernel Electric-Fence infrastructure To: Jann Horn X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201002_102314_907963_A4DC83DB X-CRM114-Status: GOOD ( 38.43 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , Hillf Danton , "open list:DOCUMENTATION" , Peter Zijlstra , Catalin Marinas , Dave Hansen , SeongJae Park , Linux-MM , Eric Dumazet , Alexander Potapenko , "H . Peter Anvin" , Christoph Lameter , Will Deacon , sjpark@amazon.com, Jonathan Corbet , the arch/x86 maintainers , kasan-dev , Ingo Molnar , Linux ARM , David Rientjes , Andrey Ryabinin , Marco Elver , Kees Cook , "Paul E . McKenney" , Andrey Konovalov , Borislav Petkov , Andy Lutomirski , Jonathan.Cameron@huawei.com, Thomas Gleixner , Joonsoo Kim , Vlastimil Babka , Greg Kroah-Hartman , kernel list , Pekka Enberg , Andrew Morton Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Oct 2, 2020 at 9:54 AM Jann Horn wrote: > > On Fri, Oct 2, 2020 at 8:33 AM Jann Horn wrote: > > On Tue, Sep 29, 2020 at 3:38 PM Marco Elver wrote: > > > This adds the Kernel Electric-Fence (KFENCE) infrastructure. KFENCE is a > > > low-overhead sampling-based memory safety error detector of heap > > > use-after-free, invalid-free, and out-of-bounds access errors. > > > > > > KFENCE is designed to be enabled in production kernels, and has near > > > zero performance overhead. Compared to KASAN, KFENCE trades performance > > > for precision. The main motivation behind KFENCE's design, is that with > > > enough total uptime KFENCE will detect bugs in code paths not typically > > > exercised by non-production test workloads. One way to quickly achieve a > > > large enough total uptime is when the tool is deployed across a large > > > fleet of machines. > [...] > > > +/* > > > + * The pool of pages used for guard pages and objects. If supported, allocated > > > + * statically, so that is_kfence_address() avoids a pointer load, and simply > > > + * compares against a constant address. Assume that if KFENCE is compiled into > > > + * the kernel, it is usually enabled, and the space is to be allocated one way > > > + * or another. > > > + */ > > > > If this actually brings a performance win, the proper way to do this > > would probably be to implement this as generic kernel infrastructure > > that makes the compiler emit large-offset relocations (either through > > compiler support or using inline asm statements that move an immediate > > into a register output and register the location in a special section, > > kinda like how e.g. static keys work) and patches them at boot time, > > or something like that - there are other places in the kernel where > > very hot code uses global pointers that are only ever written once > > during boot, e.g. the dentry cache of the VFS and the futex hash > > table. Those are probably far hotter than the kfence code. > > > > While I understand that that goes beyond the scope of this project, it > > might be something to work on going forward - this kind of > > special-case logic that turns the kernel data section into heap memory > > would not be needed if we had that kind of infrastructure. > > After thinking about it a bit more, I'm not even convinced that this > is a net positive in terms of overall performance - while it allows > you to avoid one level of indirection in some parts of kfence, that > kfence code by design only runs pretty infrequently. And to enable > this indirection avoidance, your x86 arch_kfence_initialize_pool() is > shattering potentially unrelated hugepages in the kernel data section, > which might increase the TLB pressure (and therefore the number of > memory loads that have to fall back to slow page walks) in code that > is much hotter than yours. > > And if this indirection is a real performance problem, that problem > would be many times worse in the VFS and the futex subsystem, so > developing a more generic framework for doing this cleanly would be > far more important than designing special-case code to allow kfence to > do this. > > And from what I've seen, a non-trivial chunk of the code in this > series, especially the arch/ parts, is only necessary to enable this > microoptimization. > > Do you have performance numbers or a description of why you believe > that this part of kfence is exceptionally performance-sensitive? If > not, it might be a good idea to remove this optimization, at least for > the initial version of this code. (And even if the optimization is > worthwhile, it might be a better idea to go for the generic version > immediately.) This check is very hot, it happens on every free. For every freed object we need to understand if it belongs to KFENCE or not. The generic framework for this already exists -- you simply create a global variable ;) KFENCE needs the range to be covered by struct page's and that's what creates problems for arm64. But I would assume most other users don't need that. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel