Received: by 2002:a05:6359:6284:b0:131:369:b2a3 with SMTP id se4csp4667821rwb; Tue, 8 Aug 2023 11:48:18 -0700 (PDT) X-Google-Smtp-Source: AGHT+IG3xeeYCPuQAsQiCXhVzJvui1d51QuX46wL4A2GRgbKvVm+DFxOSD2J1qe3YzM+iJNYZ9+3 X-Received: by 2002:a17:90a:6389:b0:262:f99b:a530 with SMTP id f9-20020a17090a638900b00262f99ba530mr389055pjj.34.1691520498628; Tue, 08 Aug 2023 11:48:18 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1691520498; cv=none; d=google.com; s=arc-20160816; b=vfynTP2cnan+Aeyoj9Xl+MfSlY5DWrXN2PdA198l6rBKwtO7H9tSgT13EF/uIypXWX AxzXEaFVsIayyYPiaDGxKlY8EX+D6o69wwp8LB7c6JF1JD9NZodKRSkyB/kXOzIX3seQ fluskv47bdCIFYyVWWv/Af629V4ZBXgUKkZxu8rsSpIggKeNy2Zp+1kOfZ1M11DIL0wd Fv2aapcDbh0cWq3HoN1BryDPBZ1eoXkOHGrSZcM7ObJIS+ypT1G5h7WnaRMI0vpuihki KGeWwlh57Jr6zVyYvDAaN2mBmn6EAerlh9nuo25VGlHgGrOpFIHJ3OEBEFX6XWMHO5cr qZBA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:dkim-signature; bh=8CDlzc3vfWuiyArBAiXe26CLSydTdANPU8OF+/ikf0k=; fh=21lXPMSK2yK/sEXDyQrJ/fRnyJ9/L86+gRgt0otc1Cc=; b=gBSql2mpd7HdkgSudYqnIh+NyNYGjcZJ4mqMF3XecAndg1QOlkFnRMSU6ResIfzuOq +JgZvS17RVAaPCMqOSXwn9Co04+KVWe1wfxSkLa4pil1PoFeLeT8UHNsSCmOUbPrd3Um 7u8iqX6gIqC5FDbaizK+hBSCY9rFWiZWVa8xXh/ihgOy6C3+y77gB3A/SwS0ujdfdbGL KOKyd12IB9AwDOjiINOnx3aQbCPSl9xImxhRbPxRdpXOVYq7ruWMDNtNf8NJpz9NIRrD V0W9PVUjAtCMWJWfHsj+3/iBwSMZYDiLWcbg1IckmrOUvoy4Y2H+ut6B2519rwwwkRbO QeoQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20221208 header.b=KRd1GAMl; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id v21-20020a17090abb9500b0026925bf4059si6927084pjr.106.2023.08.08.11.48.07; Tue, 08 Aug 2023 11:48:18 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20221208 header.b=KRd1GAMl; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234517AbjHHR1b (ORCPT + 99 others); Tue, 8 Aug 2023 13:27:31 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42344 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234719AbjHHR1I (ORCPT ); Tue, 8 Aug 2023 13:27:08 -0400 Received: from mail-ed1-x549.google.com (mail-ed1-x549.google.com [IPv6:2a00:1450:4864:20::549]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 85AD8209B0 for ; Tue, 8 Aug 2023 09:11:39 -0700 (PDT) Received: by mail-ed1-x549.google.com with SMTP id 4fb4d7f45d1cf-5219ceead33so4311408a12.2 for ; Tue, 08 Aug 2023 09:11:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1691511028; x=1692115828; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=8CDlzc3vfWuiyArBAiXe26CLSydTdANPU8OF+/ikf0k=; b=KRd1GAMl3k6ikf13quRuzRHx1WXoxu3WPLdcEgXviiDKdETC6P9nUHIMIjV486c7p4 iGoUz/E95AxcdwiqA3ca86eEi4wgP7C6VCD8dWdz1PhQFluVs6m9UAvx2yRxScI2dCXj 7lb86dsrshzS9puJ8F2Gi7/w/S9maTYBMUcyihKPVoNGpG1sDqMYyNhMY6RlQuKbdw27 RYC6/7OH1A1BxCqvTmTk1Ez83XUHRh4xXpEo23e7pZXoTxwSEw2EFwy8d1lbhXUJAWcI O2QCgsQsS0F7E0IQt45ldpa8vPYm/U4ceibrlxfZfPJHluP7VDGeJFzLlXFMOfjBKGx2 WxEQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1691511028; x=1692115828; 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=8CDlzc3vfWuiyArBAiXe26CLSydTdANPU8OF+/ikf0k=; b=AmJHLKLCLR6glqIjiedRl9sUeoQ/K950pUy/5E0CxSgTl8J1Plk55E45LG941gRjS3 C+/NvTNBYna1Fcfnzc5VaKh4vlqQlXZ4yK/BtPKxv9J5zam6onv4vG7ja564UTfscbhp xsjrP0Es1rShElXrq1VOHcNAxoWpvsXczBjIfcvxJcuNf3Tu+vsuinVzNaiAEHvIjAe+ Fk4fXTn+nPeqE1VGRRRrLqwyBvLU7Mg3iABVVW0yOc8SKVzWQqfy+ebAdeuoe5cLjYNk 1Jh01iMKZEKsoSzsWHamMXEHAid/jlAR7A4ctIcbJ86LY5iod6WqfoWuZqscgxtXBeJM 7C7A== X-Gm-Message-State: AOJu0YyJRjjwF2vKHWIJeaFL7/UvJvb0+a3hPyrfgG/02pft2zlzWC6K HA8EnqoLcHmuVmhDTELZeIt2gtl8tg== X-Received: from elver.muc.corp.google.com ([2a00:79e0:9c:201:39c0:833d:c267:7f64]) (user=elver job=sendgmr) by 2002:a5d:66c6:0:b0:317:cc33:106 with SMTP id k6-20020a5d66c6000000b00317cc330106mr78663wrw.11.1691490091926; Tue, 08 Aug 2023 03:21:31 -0700 (PDT) Date: Tue, 8 Aug 2023 12:17:27 +0200 In-Reply-To: <20230808102049.465864-1-elver@google.com> Mime-Version: 1.0 References: <20230808102049.465864-1-elver@google.com> X-Mailer: git-send-email 2.41.0.640.ga95def55d0-goog Message-ID: <20230808102049.465864-3-elver@google.com> Subject: [PATCH v3 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 , Sami Tolvanen , 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 Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-8.0 required=5.0 tests=BAYES_00,DATE_IN_PAST_03_06, DKIMWL_WL_MED,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF, RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.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 --- v3: * Rename ___list_*_valid() to __list_*_valid_or_report(). * More comments. 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 | 64 +++++++++++++++++++++++++--- lib/Kconfig.debug | 15 +++++++ lib/list_debug.c | 2 + 4 files changed, 77 insertions(+), 6 deletions(-) diff --git a/arch/arm64/kvm/hyp/nvhe/list_debug.c b/arch/arm64/kvm/hyp/nvhe/list_debug.c index 16266a939a4c..46a2d4f2b3c6 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_or_report(struct list_head *new, struct list_head *prev, struct list_head *next) { @@ -37,6 +38,7 @@ bool __list_add_valid_or_report(struct list_head *new, struct list_head *prev, return true; } +__list_valid_slowpath bool __list_del_entry_valid_or_report(struct list_head *entry) { struct list_head *prev, *next; diff --git a/include/linux/list.h b/include/linux/list.h index 130c6a1bb45c..066fe33e99bf 100644 --- a/include/linux/list.h +++ b/include/linux/list.h @@ -39,38 +39,90 @@ static inline void INIT_LIST_HEAD(struct list_head *list) } #ifdef CONFIG_DEBUG_LIST + +#ifdef CONFIG_DEBUG_LIST_MINIMAL +# define __list_valid_slowpath __cold __preserve_most +#else +# define __list_valid_slowpath +#endif + /* * Performs the full set of list corruption checks before __list_add(). * On list corruption reports a warning, and returns false. */ -extern bool __list_add_valid_or_report(struct list_head *new, - struct list_head *prev, - struct list_head *next); +extern bool __list_valid_slowpath __list_add_valid_or_report(struct list_head *new, + struct list_head *prev, + struct list_head *next); /* * Performs list corruption checks before __list_add(). Returns false if a * corruption is detected, true otherwise. + * + * With CONFIG_DEBUG_LIST_MINIMAL set, performs minimal list integrity checking + * (that do not result in a fault) inline, and only if a corruption is detected + * calls the reporting function __list_add_valid_or_report(). */ static __always_inline bool __list_add_valid(struct list_head *new, struct list_head *prev, struct list_head *next) { - return __list_add_valid_or_report(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_or_report(new, prev, next); + return ret; } /* * Performs the full set of list corruption checks before __list_del_entry(). * On list corruption reports a warning, and returns false. */ -extern bool __list_del_entry_valid_or_report(struct list_head *entry); +extern bool __list_valid_slowpath __list_del_entry_valid_or_report(struct list_head *entry); /* * Performs list corruption checks before __list_del_entry(). Returns false if a * corruption is detected, true otherwise. + * + * With CONFIG_DEBUG_LIST_MINIMAL set, performs minimal list integrity checking + * (that do not result in a fault) inline, and only if a corruption is detected + * calls the reporting function __list_del_entry_valid_or_report(). */ static __always_inline bool __list_del_entry_valid(struct list_head *entry) { - return __list_del_entry_valid_or_report(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_or_report(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 2def33b1491f..0ff547910dd0 100644 --- a/lib/list_debug.c +++ b/lib/list_debug.c @@ -17,6 +17,7 @@ * attempt). */ +__list_valid_slowpath bool __list_add_valid_or_report(struct list_head *new, struct list_head *prev, struct list_head *next) { @@ -39,6 +40,7 @@ bool __list_add_valid_or_report(struct list_head *new, struct list_head *prev, } EXPORT_SYMBOL(__list_add_valid_or_report); +__list_valid_slowpath bool __list_del_entry_valid_or_report(struct list_head *entry) { struct list_head *prev, *next; -- 2.41.0.640.ga95def55d0-goog