Message ID | 20230107040203.never.112-kees@kernel.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:4e01:0:0:0:0:0 with SMTP id p1csp1157890wrt; Fri, 6 Jan 2023 20:42:40 -0800 (PST) X-Google-Smtp-Source: AMrXdXtWIMHtRm1wCT1pN0bo9zyu9GFmk4m3+szdrVpWX23R5Co6IScTO5wnZ5xyJprtAzGiOkMx X-Received: by 2002:a17:90a:6443:b0:226:f85c:2333 with SMTP id y3-20020a17090a644300b00226f85c2333mr249437pjm.20.1673066560409; Fri, 06 Jan 2023 20:42:40 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1673066560; cv=none; d=google.com; s=arc-20160816; b=VpQf/MwUY8XXo6FXPzsCCm1k3EftvbMfZbHw2EcUXbfByufmhwl4lFodILIycjzdSf b+CZ+gU3Fs6VUuJSXto3KXCcXWEDPqGgQoExxdjhkb3P2MUlUgXeoseDbh5mz7Oljq4/ 6ND8H4wbiJMVbde3y7H4w+Cah0Xd5Ppx7Dm+mYQhK4TQuveBaUAuRNWeyGwSFRLL10+q KITeMcCgu9HR0JPI5Vs4GvwXiCUlnyQGpxrR5nk4Yt4Y+YvFbsVfWJganO/aOdvWiLKZ Tx/fVV9AoTqr/ik2JT11t6hHXfLDb5UsXLAQB/04IharBNEkGWJLcjC86GeSye+hDZng yscA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=EWXbynFoW/2e/uZz9M4AEeGGJkY04HNDA5rxUAIqcKE=; b=O6q7ZPcjBUz6d12sk1E7aaiaeHT4ceHlbX0ND9bYkFtrYAAm4Q6kmnssPwFN5bc3/s ECU1+mElXRqWli8md5S4zd+N4pdWrxmeXGfksYaMyr2tzz3GgApPd/YizcgJ8oxIdIqk NopcmDrrvWcl1zWNtoqCgOBSjT1az1TJCnaJkInwqapa4O4YdTPdi1gYD5oexOIWvBIy OdMg0U5DIjX08OzxNAsiATR/6bj+lLBimoFpdmDjCEliYvEACjI8Uh0A5v0ffutlTa4k RkTA4EdHlNEE+ngLDwZFF09e4fSMwRmnPJ21mo0fNTH8+L3kSWJKGnflM38NPXe1XJt0 lzvg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=TwZFAwNJ; 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=NONE sp=NONE dis=NONE) header.from=chromium.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id k69-20020a638448000000b00476e63cfd0fsi3369183pgd.466.2023.01.06.20.42.27; Fri, 06 Jan 2023 20:42:40 -0800 (PST) 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=@chromium.org header.s=google header.b=TwZFAwNJ; 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=NONE sp=NONE dis=NONE) header.from=chromium.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236839AbjAGECZ (ORCPT <rfc822;dolce.eric@gmail.com> + 99 others); Fri, 6 Jan 2023 23:02:25 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48978 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231335AbjAGECM (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 6 Jan 2023 23:02:12 -0500 Received: from mail-pl1-x633.google.com (mail-pl1-x633.google.com [IPv6:2607:f8b0:4864:20::633]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2F1B36339B for <linux-kernel@vger.kernel.org>; Fri, 6 Jan 2023 20:02:11 -0800 (PST) Received: by mail-pl1-x633.google.com with SMTP id c4so3758472plc.5 for <linux-kernel@vger.kernel.org>; Fri, 06 Jan 2023 20:02:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=EWXbynFoW/2e/uZz9M4AEeGGJkY04HNDA5rxUAIqcKE=; b=TwZFAwNJybUz0yonjGsE9z7kl9WbSslHemaDeV4wAmKEyvxKyNPZnKiYcA1kPZilAh +9x+3FmJkmrUtoE9n3M7ThHaNit+NMws+70Z+m4mbJAXDt+qtzeg5Laa87yMWP89zVJb AUnGtbbWNK5dyEI8uyKt4jmLaYIw6PZHfANG0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=EWXbynFoW/2e/uZz9M4AEeGGJkY04HNDA5rxUAIqcKE=; b=kugmpIxbOTZ32+ZWI8Gwh9ecgS5+NjAga0wkMGagYEkk9yMXNLxf4VsCrdi5ztrI5l Qk1/ciF/OHxf0h9uFwAuZhytqeAvwJkJ9G7E3eWBpD2w8m9cKSyKnkI4DyJKcQ+23KLs 74FcxAVFIJ5ROw92b2qZjE/NYeUVsbwpDnyN4NVIhFcnuAakj1P4EVcCBwpCEvDC0oFG iuC1uq+2nqqnL86cfcBuV6hwWrc8zxkscBf1/r+80X45yU4QK/H1ha4SFc2fsPmpMRCn RVcqSLNxoAut9qs80sSlBYsE6Qz43Mjef81BjLgJN8o3c3LLxvsyvEwGFsfJEVojE2Ch XUPQ== X-Gm-Message-State: AFqh2ko3+IXzF9XKnIu/dGrSf07vpG6WF6SMTv58EnaXKOsnxmFepC1e nSOlrv6bs6Aj1m6iHVbq+r/waA== X-Received: by 2002:a17:90a:7607:b0:226:c72b:5dc with SMTP id s7-20020a17090a760700b00226c72b05dcmr8955271pjk.2.1673064130735; Fri, 06 Jan 2023 20:02:10 -0800 (PST) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id d5-20020a17090a2a4500b0021e1c8ef788sm3461813pjg.51.2023.01.06.20.02.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 06 Jan 2023 20:02:10 -0800 (PST) From: Kees Cook <keescook@chromium.org> To: Guenter Roeck <linux@roeck-us.net> Cc: Kees Cook <keescook@chromium.org>, Andrew Morton <akpm@linux-foundation.org>, Nathan Chancellor <nathan@kernel.org>, linux-hardening@vger.kernel.org, David Gow <davidgow@google.com>, Nick Desaulniers <ndesaulniers@google.com>, Josh Poimboeuf <jpoimboe@kernel.org>, Geert Uytterhoeven <geert+renesas@glider.be>, Miguel Ojeda <ojeda@kernel.org>, Isabella Basso <isabbasso@riseup.net>, Vlastimil Babka <vbabka@suse.cz>, Dan Williams <dan.j.williams@intel.com>, Rasmus Villemoes <linux@rasmusvillemoes.dk>, linux-kernel@vger.kernel.org Subject: [PATCH] kunit: memcpy: Split slow memcpy tests into MEMCPY_SLOW_KUNIT_TEST Date: Fri, 6 Jan 2023 20:02:07 -0800 Message-Id: <20230107040203.never.112-kees@kernel.org> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=3285; h=from:subject:message-id; bh=V/i0uXy5BP8TRGxR82kJZeDEVMC9HYARmsaavbYd0Ks=; b=owEBbQKS/ZANAwAKAYly9N/cbcAmAcsmYgBjuO6/MQqpabc6XOqu8BvbSrSXd/OVVfg0mqDmIQpV i6WHskWJAjMEAAEKAB0WIQSlw/aPIp3WD3I+bhOJcvTf3G3AJgUCY7juvwAKCRCJcvTf3G3AJqkCD/ 9MP7gCFdhzhO5HM8a7dcjygkv3Ilst8zdtCxDAYXyJY5zXRFmw33rLgAHwO/+aV9x1VvHBExT6nnGh h0V4IaKXvLXJkIoeSiyopzfIjcoQqEY6W86yYyTIWtMsX13u+FU6Pr6TX/NjOo4kcnrCFjVAdDF9y4 L4aDjNymqFEJFXJklLDAgDL7ElJTznVdUC1HD39XTl+E2dFBMdbr8d5rRAFhcKvz215mo+Ra9MNJvf h8Q0i4gBIGOlS2DfZB8bIVpzZrtbrrbixdiBLG/h5D7/TekRIcG1ObVsvT8xOYbiy9/eYL/5oNvBSz 6l6tQYOTGB6kAk7JMIyJJYWFmr1bUOZIib1VSmZdHUZyPpdCDaYlXX5haWRqZ6CaLpRROSXHn95WoV iljizBHb/6+8S/BZmaKfVgCEqj+14XsGS6RA4aClBK2MLWGE7nyMZQvJhm7i8mpu7yzo6T0vsmMEc+ IYkwlE7i3BvE4IhC2QWlI99DGUeyVy7lhVAOSE/kRmopEtAR5Q26ej6MLaT1he5Zac6sX7n0GWojmu Kd/fyBpOjUZ0nwkfl41yrCSLl4JSC7EBwZ2NXkdMGEPReptAhMZPeCveY5Roi4gijxChig7P6RHzYQ zn5dLzKRrjOppOBPL2M7VWroKG6QYwaJk/Bt8omyY43mjID2NjSUJXoE64pw== X-Developer-Key: i=keescook@chromium.org; a=openpgp; fpr=A5C3F68F229DD60F723E6E138972F4DFDC6DC026 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS autolearn=ham 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: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1754337441745352690?= X-GMAIL-MSGID: =?utf-8?q?1754337441745352690?= |
Series |
kunit: memcpy: Split slow memcpy tests into MEMCPY_SLOW_KUNIT_TEST
|
|
Commit Message
Kees Cook
Jan. 7, 2023, 4:02 a.m. UTC
Since the long memcpy tests may stall a system for tens of seconds
in virtualized architecture environments, split those tests off under
CONFIG_MEMCPY_SLOW_KUNIT_TEST so they can be separately disabled.
Reported-by: Guenter Roeck <linux@roeck-us.net>
Link: https://lore.kernel.org/lkml/20221226195206.GA2626419@roeck-us.net
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: linux-hardening@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
Guenter, does this give you the needed flexibility to turn on the memcpy
kunit tests again in your slower environments?
---
lib/Kconfig.debug | 9 +++++++++
lib/memcpy_kunit.c | 17 +++++++++++++----
2 files changed, 22 insertions(+), 4 deletions(-)
Comments
On Fri, Jan 06, 2023 at 08:02:07PM -0800, Kees Cook wrote: > Since the long memcpy tests may stall a system for tens of seconds > in virtualized architecture environments, split those tests off under > CONFIG_MEMCPY_SLOW_KUNIT_TEST so they can be separately disabled. > > Reported-by: Guenter Roeck <linux@roeck-us.net> > Link: https://lore.kernel.org/lkml/20221226195206.GA2626419@roeck-us.net > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Nathan Chancellor <nathan@kernel.org> > Cc: linux-hardening@vger.kernel.org > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > Guenter, does this give you the needed flexibility to turn on the memcpy > kunit tests again in your slower environments? I'll be traveling this weekend. Give me until early next week to give it a try. Thanks, Guenter > --- > lib/Kconfig.debug | 9 +++++++++ > lib/memcpy_kunit.c | 17 +++++++++++++---- > 2 files changed, 22 insertions(+), 4 deletions(-) > > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index c2c78d0e761c..b5e94807f41c 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -2621,6 +2621,15 @@ config MEMCPY_KUNIT_TEST > > If unsure, say N. > > +config MEMCPY_SLOW_KUNIT_TEST > + tristate "Include exhaustive memcpy tests" if !KUNIT_ALL_TESTS > + depends on MEMCPY_KUNIT_TEST > + default KUNIT_ALL_TESTS > + help > + Some memcpy tests are quite exhaustive in checking for overlaps > + and bit ranges. These can be very slow, so they are split out > + as a separate config. > + > config IS_SIGNED_TYPE_KUNIT_TEST > tristate "Test is_signed_type() macro" if !KUNIT_ALL_TESTS > depends on KUNIT > diff --git a/lib/memcpy_kunit.c b/lib/memcpy_kunit.c > index 89128551448d..cc1f36335a9b 100644 > --- a/lib/memcpy_kunit.c > +++ b/lib/memcpy_kunit.c > @@ -307,8 +307,12 @@ static void set_random_nonzero(struct kunit *test, u8 *byte) > } > } > > -static void init_large(struct kunit *test) > +static int init_large(struct kunit *test) > { > + if (!IS_ENABLED(CONFIG_MEMCPY_SLOW_KUNIT_TEST)) { > + kunit_skip(test, "Slow test skipped. Enable with CONFIG_MEMCPY_SLOW_KUNIT_TEST=y"); > + return -EBUSY; > + } > > /* Get many bit patterns. */ > get_random_bytes(large_src, ARRAY_SIZE(large_src)); > @@ -319,6 +323,8 @@ static void init_large(struct kunit *test) > > /* Explicitly zero the entire destination. */ > memset(large_dst, 0, ARRAY_SIZE(large_dst)); > + > + return 0; > } > > /* > @@ -327,7 +333,9 @@ static void init_large(struct kunit *test) > */ > static void copy_large_test(struct kunit *test, bool use_memmove) > { > - init_large(test); > + > + if (init_large(test)) > + return; > > /* Copy a growing number of non-overlapping bytes ... */ > for (int bytes = 1; bytes <= ARRAY_SIZE(large_src); bytes++) { > @@ -472,7 +480,8 @@ static void memmove_overlap_test(struct kunit *test) > static const int bytes_start = 1; > static const int bytes_end = ARRAY_SIZE(large_src) + 1; > > - init_large(test); > + if (init_large(test)) > + return; > > /* Copy a growing number of overlapping bytes ... */ > for (int bytes = bytes_start; bytes < bytes_end; > @@ -549,8 +558,8 @@ static void strtomem_test(struct kunit *test) > static struct kunit_case memcpy_test_cases[] = { > KUNIT_CASE(memset_test), > KUNIT_CASE(memcpy_test), > - KUNIT_CASE(memcpy_large_test), > KUNIT_CASE(memmove_test), > + KUNIT_CASE(memcpy_large_test), > KUNIT_CASE(memmove_large_test), > KUNIT_CASE(memmove_overlap_test), > KUNIT_CASE(strtomem_test), > -- > 2.34.1 >
Hi Kees, On Sat, Jan 7, 2023 at 5:02 AM Kees Cook <keescook@chromium.org> wrote: > Since the long memcpy tests may stall a system for tens of seconds > in virtualized architecture environments, split those tests off under > CONFIG_MEMCPY_SLOW_KUNIT_TEST so they can be separately disabled. > > Reported-by: Guenter Roeck <linux@roeck-us.net> > Link: https://lore.kernel.org/lkml/20221226195206.GA2626419@roeck-us.net > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Nathan Chancellor <nathan@kernel.org> > Cc: linux-hardening@vger.kernel.org > Signed-off-by: Kees Cook <keescook@chromium.org> Thanks for your patch! > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -2621,6 +2621,15 @@ config MEMCPY_KUNIT_TEST > > If unsure, say N. > > +config MEMCPY_SLOW_KUNIT_TEST > + tristate "Include exhaustive memcpy tests" if !KUNIT_ALL_TESTS Why the tristate? > + depends on MEMCPY_KUNIT_TEST > + default KUNIT_ALL_TESTS > + help > + Some memcpy tests are quite exhaustive in checking for overlaps > + and bit ranges. These can be very slow, so they are split out > + as a separate config. > + > config IS_SIGNED_TYPE_KUNIT_TEST > tristate "Test is_signed_type() macro" if !KUNIT_ALL_TESTS > depends on KUNIT > diff --git a/lib/memcpy_kunit.c b/lib/memcpy_kunit.c > index 89128551448d..cc1f36335a9b 100644 > --- a/lib/memcpy_kunit.c > +++ b/lib/memcpy_kunit.c > @@ -307,8 +307,12 @@ static void set_random_nonzero(struct kunit *test, u8 *byte) > } > } > > -static void init_large(struct kunit *test) > +static int init_large(struct kunit *test) > { > + if (!IS_ENABLED(CONFIG_MEMCPY_SLOW_KUNIT_TEST)) { > + kunit_skip(test, "Slow test skipped. Enable with CONFIG_MEMCPY_SLOW_KUNIT_TEST=y"); So I can't make the slower tests available for when I need them, but not run them by default? I guess that's why you made MEMCPY_SLOW_KUNIT_TEST tristate originally, to have a separate module with the slow tests? > + return -EBUSY; > + } > > /* Get many bit patterns. */ > get_random_bytes(large_src, ARRAY_SIZE(large_src)); Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Fri, Jan 6, 2023 at 8:02 PM Kees Cook <keescook@chromium.org> wrote: > > Since the long memcpy tests may stall a system for tens of seconds > in virtualized architecture environments, split those tests off under > CONFIG_MEMCPY_SLOW_KUNIT_TEST so they can be separately disabled. > > Reported-by: Guenter Roeck <linux@roeck-us.net> > Link: https://lore.kernel.org/lkml/20221226195206.GA2626419@roeck-us.net > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Nathan Chancellor <nathan@kernel.org> > Cc: linux-hardening@vger.kernel.org > Signed-off-by: Kees Cook <keescook@chromium.org> Thanks for the patch! Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > --- > Guenter, does this give you the needed flexibility to turn on the memcpy > kunit tests again in your slower environments? > --- > lib/Kconfig.debug | 9 +++++++++ > lib/memcpy_kunit.c | 17 +++++++++++++---- > 2 files changed, 22 insertions(+), 4 deletions(-) > > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index c2c78d0e761c..b5e94807f41c 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -2621,6 +2621,15 @@ config MEMCPY_KUNIT_TEST > > If unsure, say N. > > +config MEMCPY_SLOW_KUNIT_TEST > + tristate "Include exhaustive memcpy tests" if !KUNIT_ALL_TESTS > + depends on MEMCPY_KUNIT_TEST > + default KUNIT_ALL_TESTS > + help > + Some memcpy tests are quite exhaustive in checking for overlaps > + and bit ranges. These can be very slow, so they are split out > + as a separate config. > + > config IS_SIGNED_TYPE_KUNIT_TEST > tristate "Test is_signed_type() macro" if !KUNIT_ALL_TESTS > depends on KUNIT > diff --git a/lib/memcpy_kunit.c b/lib/memcpy_kunit.c > index 89128551448d..cc1f36335a9b 100644 > --- a/lib/memcpy_kunit.c > +++ b/lib/memcpy_kunit.c > @@ -307,8 +307,12 @@ static void set_random_nonzero(struct kunit *test, u8 *byte) > } > } > > -static void init_large(struct kunit *test) > +static int init_large(struct kunit *test) > { > + if (!IS_ENABLED(CONFIG_MEMCPY_SLOW_KUNIT_TEST)) { > + kunit_skip(test, "Slow test skipped. Enable with CONFIG_MEMCPY_SLOW_KUNIT_TEST=y"); > + return -EBUSY; > + } > > /* Get many bit patterns. */ > get_random_bytes(large_src, ARRAY_SIZE(large_src)); > @@ -319,6 +323,8 @@ static void init_large(struct kunit *test) > > /* Explicitly zero the entire destination. */ > memset(large_dst, 0, ARRAY_SIZE(large_dst)); > + > + return 0; > } > > /* > @@ -327,7 +333,9 @@ static void init_large(struct kunit *test) > */ > static void copy_large_test(struct kunit *test, bool use_memmove) > { > - init_large(test); > + > + if (init_large(test)) > + return; > > /* Copy a growing number of non-overlapping bytes ... */ > for (int bytes = 1; bytes <= ARRAY_SIZE(large_src); bytes++) { > @@ -472,7 +480,8 @@ static void memmove_overlap_test(struct kunit *test) > static const int bytes_start = 1; > static const int bytes_end = ARRAY_SIZE(large_src) + 1; > > - init_large(test); > + if (init_large(test)) > + return; > > /* Copy a growing number of overlapping bytes ... */ > for (int bytes = bytes_start; bytes < bytes_end; > @@ -549,8 +558,8 @@ static void strtomem_test(struct kunit *test) > static struct kunit_case memcpy_test_cases[] = { > KUNIT_CASE(memset_test), > KUNIT_CASE(memcpy_test), > - KUNIT_CASE(memcpy_large_test), > KUNIT_CASE(memmove_test), > + KUNIT_CASE(memcpy_large_test), > KUNIT_CASE(memmove_large_test), > KUNIT_CASE(memmove_overlap_test), > KUNIT_CASE(strtomem_test), > -- > 2.34.1 >
On Fri, Jan 06, 2023 at 08:02:07PM -0800, Kees Cook wrote: > Since the long memcpy tests may stall a system for tens of seconds > in virtualized architecture environments, split those tests off under > CONFIG_MEMCPY_SLOW_KUNIT_TEST so they can be separately disabled. > > Reported-by: Guenter Roeck <linux@roeck-us.net> > Link: https://lore.kernel.org/lkml/20221226195206.GA2626419@roeck-us.net > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Nathan Chancellor <nathan@kernel.org> > Cc: linux-hardening@vger.kernel.org > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > Guenter, does this give you the needed flexibility to turn on the memcpy > kunit tests again in your slower environments? Yes, it does, and it works. Reviewed-and-tested-by: Guenter Roeck <linux@roeck-us.net> > --- > lib/Kconfig.debug | 9 +++++++++ > lib/memcpy_kunit.c | 17 +++++++++++++---- > 2 files changed, 22 insertions(+), 4 deletions(-) > > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index c2c78d0e761c..b5e94807f41c 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -2621,6 +2621,15 @@ config MEMCPY_KUNIT_TEST > > If unsure, say N. > > +config MEMCPY_SLOW_KUNIT_TEST > + tristate "Include exhaustive memcpy tests" if !KUNIT_ALL_TESTS As Geert noticed, bool should be sufficient here since this is not a separate test. Thanks, Guenter > + depends on MEMCPY_KUNIT_TEST > + default KUNIT_ALL_TESTS > + help > + Some memcpy tests are quite exhaustive in checking for overlaps > + and bit ranges. These can be very slow, so they are split out > + as a separate config. > + > config IS_SIGNED_TYPE_KUNIT_TEST > tristate "Test is_signed_type() macro" if !KUNIT_ALL_TESTS > depends on KUNIT > diff --git a/lib/memcpy_kunit.c b/lib/memcpy_kunit.c > index 89128551448d..cc1f36335a9b 100644 > --- a/lib/memcpy_kunit.c > +++ b/lib/memcpy_kunit.c > @@ -307,8 +307,12 @@ static void set_random_nonzero(struct kunit *test, u8 *byte) > } > } > > -static void init_large(struct kunit *test) > +static int init_large(struct kunit *test) > { > + if (!IS_ENABLED(CONFIG_MEMCPY_SLOW_KUNIT_TEST)) { > + kunit_skip(test, "Slow test skipped. Enable with CONFIG_MEMCPY_SLOW_KUNIT_TEST=y"); > + return -EBUSY; > + } > > /* Get many bit patterns. */ > get_random_bytes(large_src, ARRAY_SIZE(large_src)); > @@ -319,6 +323,8 @@ static void init_large(struct kunit *test) > > /* Explicitly zero the entire destination. */ > memset(large_dst, 0, ARRAY_SIZE(large_dst)); > + > + return 0; > } > > /* > @@ -327,7 +333,9 @@ static void init_large(struct kunit *test) > */ > static void copy_large_test(struct kunit *test, bool use_memmove) > { > - init_large(test); > + > + if (init_large(test)) > + return; > > /* Copy a growing number of non-overlapping bytes ... */ > for (int bytes = 1; bytes <= ARRAY_SIZE(large_src); bytes++) { > @@ -472,7 +480,8 @@ static void memmove_overlap_test(struct kunit *test) > static const int bytes_start = 1; > static const int bytes_end = ARRAY_SIZE(large_src) + 1; > > - init_large(test); > + if (init_large(test)) > + return; > > /* Copy a growing number of overlapping bytes ... */ > for (int bytes = bytes_start; bytes < bytes_end; > @@ -549,8 +558,8 @@ static void strtomem_test(struct kunit *test) > static struct kunit_case memcpy_test_cases[] = { > KUNIT_CASE(memset_test), > KUNIT_CASE(memcpy_test), > - KUNIT_CASE(memcpy_large_test), > KUNIT_CASE(memmove_test), > + KUNIT_CASE(memcpy_large_test), > KUNIT_CASE(memmove_large_test), > KUNIT_CASE(memmove_overlap_test), > KUNIT_CASE(strtomem_test), > -- > 2.34.1 >
+Cc rest of kunit from MAINTAINERS On 1/7/23 11:55, Geert Uytterhoeven wrote: > Hi Kees, > > On Sat, Jan 7, 2023 at 5:02 AM Kees Cook <keescook@chromium.org> wrote: >> Since the long memcpy tests may stall a system for tens of seconds >> in virtualized architecture environments, split those tests off under >> CONFIG_MEMCPY_SLOW_KUNIT_TEST so they can be separately disabled. >> >> Reported-by: Guenter Roeck <linux@roeck-us.net> >> Link: https://lore.kernel.org/lkml/20221226195206.GA2626419@roeck-us.net >> Cc: Andrew Morton <akpm@linux-foundation.org> >> Cc: Nathan Chancellor <nathan@kernel.org> >> Cc: linux-hardening@vger.kernel.org >> Signed-off-by: Kees Cook <keescook@chromium.org> > > Thanks for your patch! > >> --- a/lib/Kconfig.debug >> +++ b/lib/Kconfig.debug >> @@ -2621,6 +2621,15 @@ config MEMCPY_KUNIT_TEST >> >> If unsure, say N. >> >> +config MEMCPY_SLOW_KUNIT_TEST >> + tristate "Include exhaustive memcpy tests" if !KUNIT_ALL_TESTS > > Why the tristate? > >> + depends on MEMCPY_KUNIT_TEST >> + default KUNIT_ALL_TESTS >> + help >> + Some memcpy tests are quite exhaustive in checking for overlaps >> + and bit ranges. These can be very slow, so they are split out >> + as a separate config. >> + >> config IS_SIGNED_TYPE_KUNIT_TEST >> tristate "Test is_signed_type() macro" if !KUNIT_ALL_TESTS >> depends on KUNIT >> diff --git a/lib/memcpy_kunit.c b/lib/memcpy_kunit.c >> index 89128551448d..cc1f36335a9b 100644 >> --- a/lib/memcpy_kunit.c >> +++ b/lib/memcpy_kunit.c >> @@ -307,8 +307,12 @@ static void set_random_nonzero(struct kunit *test, u8 *byte) >> } >> } >> >> -static void init_large(struct kunit *test) >> +static int init_large(struct kunit *test) >> { >> + if (!IS_ENABLED(CONFIG_MEMCPY_SLOW_KUNIT_TEST)) { >> + kunit_skip(test, "Slow test skipped. Enable with CONFIG_MEMCPY_SLOW_KUNIT_TEST=y"); > > So I can't make the slower tests available for when I need them, > but not run them by default? Indeed it seems weird to tie this to a config option without runtime override. > I guess that's why you made MEMCPY_SLOW_KUNIT_TEST tristate originally, > to have a separate module with the slow tests? On the other hand I can imagine requiring a separate module for slow tests would lead to more churn - IIUC there would need to be two files instead of memcpy_kunit.c, possibly a duplicated boilerplate code (or another shared .c file). So the idea is to have a generic way to mark some tests as slow and a way to opt-in/opt-out for those when running the tests. Maybe KUnit folks already have such mechanism or have an idea how to implement that. >> + return -EBUSY; >> + } >> >> /* Get many bit patterns. */ >> get_random_bytes(large_src, ARRAY_SIZE(large_src)); > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
On Mon, Jan 9, 2023 at 11:51 PM Vlastimil Babka <vbabka@suse.cz> wrote: > > +Cc rest of kunit from MAINTAINERS > > On 1/7/23 11:55, Geert Uytterhoeven wrote: > > Hi Kees, > > > > On Sat, Jan 7, 2023 at 5:02 AM Kees Cook <keescook@chromium.org> wrote: > >> Since the long memcpy tests may stall a system for tens of seconds > >> in virtualized architecture environments, split those tests off under > >> CONFIG_MEMCPY_SLOW_KUNIT_TEST so they can be separately disabled. <snip> > >> > >> -static void init_large(struct kunit *test) > >> +static int init_large(struct kunit *test) > >> { > >> + if (!IS_ENABLED(CONFIG_MEMCPY_SLOW_KUNIT_TEST)) { > >> + kunit_skip(test, "Slow test skipped. Enable with CONFIG_MEMCPY_SLOW_KUNIT_TEST=y"); > > > > So I can't make the slower tests available for when I need them, > > but not run them by default? > > Indeed it seems weird to tie this to a config option without runtime override. > > > I guess that's why you made MEMCPY_SLOW_KUNIT_TEST tristate originally, > > to have a separate module with the slow tests? > > On the other hand I can imagine requiring a separate module for slow tests > would lead to more churn - IIUC there would need to be two files instead of > memcpy_kunit.c, possibly a duplicated boilerplate code (or another shared .c > file). > > So the idea is to have a generic way to mark some tests as slow and a way to > opt-in/opt-out for those when running the tests. Maybe KUnit folks already > have such mechanism or have an idea how to implement that. There is no mechanism atm, and we'd still need to figure it out so it'll be a while. So I think a patch like this makes sense in the short-term. This is definitely something we've always thought would be useful eventually. See this TODO which has been there since day 1 ;) https://elixir.bootlin.com/linux/latest/source/lib/kunit/try-catch.c#L36 It just felt like it would be premature to come up with something when basically all the tests up until now ran ~instantly. Throwing out some rough implementation ideas: I was thinking the granularity for these timeout annotations would be at the suite-level. If we go with that, then I guess the intended flow is to group slow tests into their own suite and mark them as such. Then maybe we'd have some runtime way of disabling/enabling "long" tests, like a cmdline opt. E.g. you'd pass `kunit.max_test_size=30` to exclude tests longer than 30 seconds. Daniel
On Sat, 7 Jan 2023 at 12:02, Kees Cook <keescook@chromium.org> wrote: > > Since the long memcpy tests may stall a system for tens of seconds > in virtualized architecture environments, split those tests off under > CONFIG_MEMCPY_SLOW_KUNIT_TEST so they can be separately disabled. > > Reported-by: Guenter Roeck <linux@roeck-us.net> > Link: https://lore.kernel.org/lkml/20221226195206.GA2626419@roeck-us.net > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Nathan Chancellor <nathan@kernel.org> > Cc: linux-hardening@vger.kernel.org > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > Guenter, does this give you the needed flexibility to turn on the memcpy > kunit tests again in your slower environments? > --- Sorry for the delay: I've just got back from holidays. This looks good to me (modulo the tristate/bool issue mentioned). Having a specific way to tag tests as "slow" is something we've considered having in KUnit for a while, but (at least initially), there weren't any tests slow enough for it to be worthwhile. It looks like that's changing (alongside this, there are a few DRM tests which are quite slow, as well as some of the KCSAN tests. Even the time64_to_tm_test_date_range test, while very quick under UML on a modern system, is noticeably slow under qemu and times out on something like an old 486...). Daniel's comments elsewhere in this thread have some good ideas. That being said, it'll probably take long enough to work out and implement a good way of giving tests "properties" and requesting only fast tests run that just putting tests which are slow enough to cause problems behind a kconfig entry seems a pretty solid intermediate solution. So this is: Reviewed-by: David Gow <davidgow@google.com> (assuming the tristate goes to bool), and I'll look into getting a way of marking tests as "slow" and enabling/disabling them at runtime. Cheers, -- David > lib/Kconfig.debug | 9 +++++++++ > lib/memcpy_kunit.c | 17 +++++++++++++---- > 2 files changed, 22 insertions(+), 4 deletions(-) > > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index c2c78d0e761c..b5e94807f41c 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -2621,6 +2621,15 @@ config MEMCPY_KUNIT_TEST > > If unsure, say N. > > +config MEMCPY_SLOW_KUNIT_TEST > + tristate "Include exhaustive memcpy tests" if !KUNIT_ALL_TESTS As mentioned, if this is just going to be a toggle, it can be a "bool". > + depends on MEMCPY_KUNIT_TEST > + default KUNIT_ALL_TESTS > + help > + Some memcpy tests are quite exhaustive in checking for overlaps > + and bit ranges. These can be very slow, so they are split out > + as a separate config. > + > config IS_SIGNED_TYPE_KUNIT_TEST > tristate "Test is_signed_type() macro" if !KUNIT_ALL_TESTS > depends on KUNIT > diff --git a/lib/memcpy_kunit.c b/lib/memcpy_kunit.c > index 89128551448d..cc1f36335a9b 100644 > --- a/lib/memcpy_kunit.c > +++ b/lib/memcpy_kunit.c > @@ -307,8 +307,12 @@ static void set_random_nonzero(struct kunit *test, u8 *byte) > } > } > > -static void init_large(struct kunit *test) > +static int init_large(struct kunit *test) > { > + if (!IS_ENABLED(CONFIG_MEMCPY_SLOW_KUNIT_TEST)) { > + kunit_skip(test, "Slow test skipped. Enable with CONFIG_MEMCPY_SLOW_KUNIT_TEST=y"); > + return -EBUSY; > + } > > /* Get many bit patterns. */ > get_random_bytes(large_src, ARRAY_SIZE(large_src)); > @@ -319,6 +323,8 @@ static void init_large(struct kunit *test) > > /* Explicitly zero the entire destination. */ > memset(large_dst, 0, ARRAY_SIZE(large_dst)); > + > + return 0; > } > > /* > @@ -327,7 +333,9 @@ static void init_large(struct kunit *test) > */ > static void copy_large_test(struct kunit *test, bool use_memmove) > { > - init_large(test); > + > + if (init_large(test)) > + return; > > /* Copy a growing number of non-overlapping bytes ... */ > for (int bytes = 1; bytes <= ARRAY_SIZE(large_src); bytes++) { > @@ -472,7 +480,8 @@ static void memmove_overlap_test(struct kunit *test) > static const int bytes_start = 1; > static const int bytes_end = ARRAY_SIZE(large_src) + 1; > > - init_large(test); > + if (init_large(test)) > + return; > > /* Copy a growing number of overlapping bytes ... */ > for (int bytes = bytes_start; bytes < bytes_end; > @@ -549,8 +558,8 @@ static void strtomem_test(struct kunit *test) > static struct kunit_case memcpy_test_cases[] = { > KUNIT_CASE(memset_test), > KUNIT_CASE(memcpy_test), > - KUNIT_CASE(memcpy_large_test), > KUNIT_CASE(memmove_test), > + KUNIT_CASE(memcpy_large_test), > KUNIT_CASE(memmove_large_test), > KUNIT_CASE(memmove_overlap_test), > KUNIT_CASE(strtomem_test), > -- > 2.34.1 >
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index c2c78d0e761c..b5e94807f41c 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -2621,6 +2621,15 @@ config MEMCPY_KUNIT_TEST If unsure, say N. +config MEMCPY_SLOW_KUNIT_TEST + tristate "Include exhaustive memcpy tests" if !KUNIT_ALL_TESTS + depends on MEMCPY_KUNIT_TEST + default KUNIT_ALL_TESTS + help + Some memcpy tests are quite exhaustive in checking for overlaps + and bit ranges. These can be very slow, so they are split out + as a separate config. + config IS_SIGNED_TYPE_KUNIT_TEST tristate "Test is_signed_type() macro" if !KUNIT_ALL_TESTS depends on KUNIT diff --git a/lib/memcpy_kunit.c b/lib/memcpy_kunit.c index 89128551448d..cc1f36335a9b 100644 --- a/lib/memcpy_kunit.c +++ b/lib/memcpy_kunit.c @@ -307,8 +307,12 @@ static void set_random_nonzero(struct kunit *test, u8 *byte) } } -static void init_large(struct kunit *test) +static int init_large(struct kunit *test) { + if (!IS_ENABLED(CONFIG_MEMCPY_SLOW_KUNIT_TEST)) { + kunit_skip(test, "Slow test skipped. Enable with CONFIG_MEMCPY_SLOW_KUNIT_TEST=y"); + return -EBUSY; + } /* Get many bit patterns. */ get_random_bytes(large_src, ARRAY_SIZE(large_src)); @@ -319,6 +323,8 @@ static void init_large(struct kunit *test) /* Explicitly zero the entire destination. */ memset(large_dst, 0, ARRAY_SIZE(large_dst)); + + return 0; } /* @@ -327,7 +333,9 @@ static void init_large(struct kunit *test) */ static void copy_large_test(struct kunit *test, bool use_memmove) { - init_large(test); + + if (init_large(test)) + return; /* Copy a growing number of non-overlapping bytes ... */ for (int bytes = 1; bytes <= ARRAY_SIZE(large_src); bytes++) { @@ -472,7 +480,8 @@ static void memmove_overlap_test(struct kunit *test) static const int bytes_start = 1; static const int bytes_end = ARRAY_SIZE(large_src) + 1; - init_large(test); + if (init_large(test)) + return; /* Copy a growing number of overlapping bytes ... */ for (int bytes = bytes_start; bytes < bytes_end; @@ -549,8 +558,8 @@ static void strtomem_test(struct kunit *test) static struct kunit_case memcpy_test_cases[] = { KUNIT_CASE(memset_test), KUNIT_CASE(memcpy_test), - KUNIT_CASE(memcpy_large_test), KUNIT_CASE(memmove_test), + KUNIT_CASE(memcpy_large_test), KUNIT_CASE(memmove_large_test), KUNIT_CASE(memmove_overlap_test), KUNIT_CASE(strtomem_test),