From patchwork Fri Aug 4 09:02:58 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marco Elver X-Patchwork-Id: 13341547 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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 5AE7FC001DE for ; Fri, 4 Aug 2023 09:07:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:From:Subject:Message-ID: References:Mime-Version:In-Reply-To:Date:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Owner; bh=VFO+VyVSOtpL0PwL7hO63QdZVACQ7dnoG1pasQBla0o=; b=oS9EZ6fOBtCx7bTPHkDPI0MQTJ GCrRNbJARR8hlfjrzJfPoiuAZ3McTQrdAyIttN6gM+hys+AbfzRots7foXZVOS4lRTyMRrhZKfQ/H wOjYj9tbTJp3FviCyLx98AzHx71iV6JH0G7zYwuubfKMV1hzFlBrRAf/dQ33dcL6QNerBcDG4YGxK 2+gJIxO2yffIfXfK+8QWBHU1afDToLoAYjbjoXdmgb55gDtVSxNNphORCebcOX+Xoh+qUh8dudqh4 Hj1pOZ8qIWrPw9pOUlaTtuab4BiEqUJtBQwdoIFOL37iGapGceD23tkMZ1pkHIVpD4R+G/Y4CAa7b 33kUEBIA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qRqlz-00By3w-38; Fri, 04 Aug 2023 09:07:03 +0000 Received: from mail-yw1-x1149.google.com ([2607:f8b0:4864:20::1149]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qRqlw-00By14-1i for linux-arm-kernel@lists.infradead.org; Fri, 04 Aug 2023 09:07:02 +0000 Received: by mail-yw1-x1149.google.com with SMTP id 00721157ae682-5847479b559so22190287b3.1 for ; Fri, 04 Aug 2023 02:06:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1691140015; x=1691744815; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=MzWCD4HwKY1r1A6RBU3efg4gsiTlnVjc3j6YbCfBqUM=; b=X1qQHJBN0yCJmiiCOnUz6+pj65LuFmL9UZz8FfdlIEiWrVdg46nOF6JPHUsBf46tyT gu/cU7tVPjn5xggOa6K89RQzk9Xs2SR+GfyZp18F5E0spipr3dg5dVvX7DslvcvYCFRD 2gvOVVugZtDQlWRnRsapExEDo9j1EfKY2zqVkvdakxjxG7T1dXj/JefW2y72bQ2yBMMe weUy74bSRSwjfV9CAKw48V+vgo+KAzLIPbCDKTWEXh9SxkSClJWIQsafkhKswHn4G16r tvs0P0LXkXbfRVlSTUqEog1H3rU0OIcvaDaBHXz9cepHqF4vObfSkJ5xxrTUelL8NE+l pocA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1691140015; x=1691744815; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=MzWCD4HwKY1r1A6RBU3efg4gsiTlnVjc3j6YbCfBqUM=; b=K/l3Qs8R4MbTUE3WHxj7438Yrar1MPkRf2ymkVpDHT3Kpxh1OgB7JqLpn6sr4OAw7H m4mvP8gw+xv6TiSx/l8qZHtROkGNi7Uof4Qeg6UKBogOhUkgAOc4UwIhIRM8E/xtQ8zg b5Ralc+wRTKqH+yAk/P3k8QHIcdWLLUNxnVP8Ej8Gdn1cNu0qKT3DS/c7Q/tHbk9YhmU w0Kt+UCCQ+UR3YrO2nUpDEcaHjuXY70r81SMK3S+W0XAgzJ5+HtXJQUYydlOd4zQ5oTa tyksXACvm/ILfGUh/+snlaSHzh39ccNZYOtzICvWO5thYC9aEG9BuJdOVCG9otNrGYhd D/eQ== X-Gm-Message-State: AOJu0YyBP8vSA2BvoDGcHbhb3NzuJeNVwBFQLQPtVMcMxD6EnTNe+yhI P/lhcmDILBZPEk5e3/HWuN7C5fzgzA== X-Google-Smtp-Source: AGHT+IHb47om6RdVmuoXy9g0+Rz+FIp330BgZAdxPKXZUIucVRKYEj76dp9OQ4SsePP+7+HTjAkoWKdCsg== X-Received: from elver.muc.corp.google.com ([2a00:79e0:9c:201:2ebf:f3ea:4841:53b6]) (user=elver job=sendgmr) by 2002:a81:af41:0:b0:573:8316:8d04 with SMTP id x1-20020a81af41000000b0057383168d04mr7391ywj.4.1691140015704; Fri, 04 Aug 2023 02:06:55 -0700 (PDT) Date: Fri, 4 Aug 2023 11:02:58 +0200 In-Reply-To: <20230804090621.400-1-elver@google.com> Mime-Version: 1.0 References: <20230804090621.400-1-elver@google.com> X-Mailer: git-send-email 2.41.0.640.ga95def55d0-goog Message-ID: <20230804090621.400-3-elver@google.com> Subject: [PATCH v2 3/3] list_debug: Introduce CONFIG_DEBUG_LIST_MINIMAL From: Marco Elver To: elver@google.com, Andrew Morton , Kees Cook Cc: Guenter Roeck , Peter Zijlstra , Mark Rutland , Steven Rostedt , Marc Zyngier , Oliver Upton , James Morse , Suzuki K Poulose , Zenghui Yu , Catalin Marinas , Will Deacon , Nathan Chancellor , Nick Desaulniers , Tom Rix , Miguel Ojeda , linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org, llvm@lists.linux.dev, Dmitry Vyukov , Alexander Potapenko , kasan-dev@googlegroups.com, linux-toolchains@vger.kernel.org X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230804_020700_572416_69AA9BEB X-CRM114-Status: GOOD ( 23.98 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Numerous production kernel configs (see [1, 2]) are choosing to enable CONFIG_DEBUG_LIST, which is also being recommended by KSPP for hardened configs [3]. The feature has never been designed with performance in mind, yet common list manipulation is happening across hot paths all over the kernel. Introduce CONFIG_DEBUG_LIST_MINIMAL, which performs list pointer checking inline, and only upon list corruption delegates to the reporting slow path. To generate optimal machine code with CONFIG_DEBUG_LIST_MINIMAL: 1. Elide checking for pointer values which upon dereference would result in an immediate access fault -- therefore "minimal" checks. The trade-off is lower-quality error reports. 2. Use the newly introduced __preserve_most function attribute (available with Clang, but not yet with GCC) to minimize the code footprint for calling the reporting slow path. As a result, function size of callers is reduced by avoiding saving registers before calling the rarely called reporting slow path. Note that all TUs in lib/Makefile already disable function tracing, including list_debug.c, and __preserve_most's implied notrace has no effect in this case. 3. Because the inline checks are a subset of the full set of checks in ___list_*_valid(), always return false if the inline checks failed. This avoids redundant compare and conditional branch right after return from the slow path. As a side-effect of the checks being inline, if the compiler can prove some condition to always be true, it can completely elide some checks. Running netperf with CONFIG_DEBUG_LIST_MINIMAL (using a Clang compiler with "preserve_most") shows throughput improvements, in my case of ~7% on average (up to 20-30% on some test cases). Link: https://r.android.com/1266735 [1] Link: https://gitlab.archlinux.org/archlinux/packaging/packages/linux/-/blob/main/config [2] Link: https://kernsec.org/wiki/index.php/Kernel_Self_Protection_Project/Recommended_Settings [3] Signed-off-by: Marco Elver --- v2: * Note that lib/Makefile disables function tracing for everything and __preserve_most's implied notrace is a noop here. --- arch/arm64/kvm/hyp/nvhe/list_debug.c | 2 + include/linux/list.h | 56 +++++++++++++++++++++++++--- lib/Kconfig.debug | 15 ++++++++ lib/list_debug.c | 2 + 4 files changed, 69 insertions(+), 6 deletions(-) diff --git a/arch/arm64/kvm/hyp/nvhe/list_debug.c b/arch/arm64/kvm/hyp/nvhe/list_debug.c index 589284496ac5..df718e29f6d4 100644 --- a/arch/arm64/kvm/hyp/nvhe/list_debug.c +++ b/arch/arm64/kvm/hyp/nvhe/list_debug.c @@ -26,6 +26,7 @@ static inline __must_check bool nvhe_check_data_corruption(bool v) /* The predicates checked here are taken from lib/list_debug.c. */ +__list_valid_slowpath bool ___list_add_valid(struct list_head *new, struct list_head *prev, struct list_head *next) { @@ -37,6 +38,7 @@ bool ___list_add_valid(struct list_head *new, struct list_head *prev, return true; } +__list_valid_slowpath bool ___list_del_entry_valid(struct list_head *entry) { struct list_head *prev, *next; diff --git a/include/linux/list.h b/include/linux/list.h index e0b2cf904409..a28a215a3eb1 100644 --- a/include/linux/list.h +++ b/include/linux/list.h @@ -39,20 +39,64 @@ static inline void INIT_LIST_HEAD(struct list_head *list) } #ifdef CONFIG_DEBUG_LIST -extern bool ___list_add_valid(struct list_head *new, - struct list_head *prev, - struct list_head *next); + +#ifdef CONFIG_DEBUG_LIST_MINIMAL +# define __list_valid_slowpath __cold __preserve_most +#else +# define __list_valid_slowpath +#endif + +extern bool __list_valid_slowpath ___list_add_valid(struct list_head *new, + struct list_head *prev, + struct list_head *next); static __always_inline bool __list_add_valid(struct list_head *new, struct list_head *prev, struct list_head *next) { - return ___list_add_valid(new, prev, next); + bool ret = true; + + if (IS_ENABLED(CONFIG_DEBUG_LIST_MINIMAL)) { + /* + * In the minimal config, elide checking if next and prev are + * NULL, since the immediate dereference of them below would + * result in a fault if NULL. + * + * With the minimal config we can afford to inline the checks, + * which also gives the compiler a chance to elide some of them + * completely if they can be proven at compile-time. If one of + * the pre-conditions does not hold, the slow-path will show a + * report which pre-condition failed. + */ + if (likely(next->prev == prev && prev->next == next && new != prev && new != next)) + return true; + ret = false; + } + + ret &= ___list_add_valid(new, prev, next); + return ret; } -extern bool ___list_del_entry_valid(struct list_head *entry); +extern bool __list_valid_slowpath ___list_del_entry_valid(struct list_head *entry); static __always_inline bool __list_del_entry_valid(struct list_head *entry) { - return ___list_del_entry_valid(entry); + bool ret = true; + + if (IS_ENABLED(CONFIG_DEBUG_LIST_MINIMAL)) { + struct list_head *prev = entry->prev; + struct list_head *next = entry->next; + + /* + * In the minimal config, elide checking if next and prev are + * NULL, LIST_POISON1 or LIST_POISON2, since the immediate + * dereference of them below would result in a fault. + */ + if (likely(prev->next == entry && next->prev == entry)) + return true; + ret = false; + } + + ret &= ___list_del_entry_valid(entry); + return ret; } #else static inline bool __list_add_valid(struct list_head *new, diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index fbc89baf7de6..e72cf08af0fa 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -1680,6 +1680,21 @@ config DEBUG_LIST If unsure, say N. +config DEBUG_LIST_MINIMAL + bool "Minimal linked list debug checks" + default !DEBUG_KERNEL + depends on DEBUG_LIST + help + Only perform the minimal set of checks in the linked-list walking + routines to catch corruptions that are not guaranteed to result in an + immediate access fault. + + This trades lower quality error reports for improved performance: the + generated code should be more optimal and provide trade-offs that may + better serve safety- and performance- critical environments. + + If unsure, say Y. + config DEBUG_PLIST bool "Debug priority linked list manipulation" depends on DEBUG_KERNEL diff --git a/lib/list_debug.c b/lib/list_debug.c index fd69009cc696..daad32855f0d 100644 --- a/lib/list_debug.c +++ b/lib/list_debug.c @@ -17,6 +17,7 @@ * attempt). */ +__list_valid_slowpath bool ___list_add_valid(struct list_head *new, struct list_head *prev, struct list_head *next) { @@ -39,6 +40,7 @@ bool ___list_add_valid(struct list_head *new, struct list_head *prev, } EXPORT_SYMBOL(___list_add_valid); +__list_valid_slowpath bool ___list_del_entry_valid(struct list_head *entry) { struct list_head *prev, *next;