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 57E44C4321E for ; Tue, 29 Nov 2022 14:22:01 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E06B56B0074; Tue, 29 Nov 2022 09:22:00 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id DB70C6B0075; Tue, 29 Nov 2022 09:22:00 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C58256B0078; Tue, 29 Nov 2022 09:22:00 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id B74396B0074 for ; Tue, 29 Nov 2022 09:22:00 -0500 (EST) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id A6EBC40F51 for ; Tue, 29 Nov 2022 09:32:33 +0000 (UTC) X-FDA: 80185964586.19.32686D6 Received: from mail-yb1-f169.google.com (mail-yb1-f169.google.com [209.85.219.169]) by imf25.hostedemail.com (Postfix) with ESMTP id 4585DA0015 for ; Tue, 29 Nov 2022 09:32:32 +0000 (UTC) Received: by mail-yb1-f169.google.com with SMTP id 205so16682493ybf.6 for ; Tue, 29 Nov 2022 01:32:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=AYIdy/RYAfzcFTrw7/WTVi/Lx6QQB74ydKBftJOdtrA=; b=qvQ+QtukWl6Kk9VSamQNhFmiHSyCPTA+puKBPu7ffKeh19HI2jkHkmylfZpF6uFeXG 0SzK8R3C3Dnbv2XlVD0Uh07Nf6yXb+fDzrVt92TuLk2eFFuaB+vZoo1HZx/QZWMLFvqd UZeXoJNU3007Mv3R4nApk5suKrpXnWlMSqaTGGfKlm1eH45/nYxDNFr7SCWIolXLRsel NgKa553g6LuyVmowhY0lauSJFy1UpIPVs74JmZ4UVChAgLdKetaC2KzYr/T4EpAAv0s3 Fd4o8bcvyxmyTM0L6Dv2989Nioyv0tgzMegbOBdZsFWyFPytibIypqgs7oxNiJZ7RGFP zPcA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=AYIdy/RYAfzcFTrw7/WTVi/Lx6QQB74ydKBftJOdtrA=; b=Bvstnss965eeY8hDfTJ503M9x1w1/9rtZaAV7YwHkzc8WnF0Fy00JhmT2G8Cvb3PQP 8qzV+U1UClUF50Lp/sBiuPvdVt/35JdGcIE8AsAlbYigmB4yQB9ywTTbIVZq1ZzHDAms aVByUlNKa2tpyWv3mdOlEn3YDvzNiet/nI0XKHYWVlyLJAW2kdwOwMjVa7aUBzjeXYbw L//mAdUDmRT6GUtybtjB9oQDfJ6L1gMIaaf9sfvhhXAo9/sT1hrSxKzZn4S4awP1iOB7 agqUDcpvBNML9fekCq+7YXoE+F/byX5zCsnQEkSBNMTm1iwWkLQhtVzULnDxXQ9A+7nB B4Ng== X-Gm-Message-State: ANoB5pn/txcrk5G0fiGYIyAtsyLYKfkduC1VddSeXK5Fe6nImWmeAzm9 9gB2wA4ihF3Hd+iLWrmZbYnECqSyWeUyQ8WCG3Pg2g== X-Google-Smtp-Source: AA0mqf7joEDlOeJDMEl6F1uWJdSxRY7OaY4zhHB63BnOBVQjtU7i9SZUm1WZFTvc18BEq0Y2DNzc1kF2EvlYSdzIETE= X-Received: by 2002:a25:3851:0:b0:6f0:6175:2cc7 with SMTP id f78-20020a253851000000b006f061752cc7mr27731021yba.93.1669714351353; Tue, 29 Nov 2022 01:32:31 -0800 (PST) MIME-Version: 1.0 References: <20221129063358.3012362-1-feng.tang@intel.com> <20221129063358.3012362-2-feng.tang@intel.com> In-Reply-To: <20221129063358.3012362-2-feng.tang@intel.com> From: Marco Elver Date: Tue, 29 Nov 2022 10:31:55 +0100 Message-ID: Subject: Re: [PATCH v2 2/2] mm/slub, kunit: Add a test case for kmalloc redzone check To: Feng Tang Cc: Vlastimil Babka , Andrew Morton , Oliver Glitta , Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Roman Gushchin , Hyeonggon Yoo <42.hyeyoo@gmail.com>, linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1669714352; 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=AYIdy/RYAfzcFTrw7/WTVi/Lx6QQB74ydKBftJOdtrA=; b=gbsJ1HKpCUPViwdvBBXcJZBOhLMWtLu/AyG3TdcdkyqnWrlM7Y2WjUfNvnGfSD68N0kQNS AVRh4ew63XONT8XXcbV5oYFA1J5ym+5xm2PT8MUqBSwli2OOp8DsbGy02oP2dfvf1Z/Y+N 4fYhgIydSys0ZhfFywgDm0C07/gobLc= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=qvQ+Qtuk; spf=pass (imf25.hostedemail.com: domain of elver@google.com designates 209.85.219.169 as permitted sender) smtp.mailfrom=elver@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1669714352; a=rsa-sha256; cv=none; b=u0t7qz24EMsZ1aKbQBWKShwIrw/xmNHhJavX+2YeiF/TPKcFhv/jIjfb9NvyEQkxVI7O0j qAT63qQwg9QaRcalbJWLFoJwAyDzpQdqs8LIsPL1oC5CgZAw0XnwER9Zciwi/11exeeLYO 9AxDOP9RsHgYwj+Ir85cNsdZvXaVgl8= X-Stat-Signature: 9oyii94qs1764wpabe699wgunqubkndi X-Rspam-User: X-Rspamd-Queue-Id: 4585DA0015 X-Rspamd-Server: rspam11 Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=qvQ+Qtuk; spf=pass (imf25.hostedemail.com: domain of elver@google.com designates 209.85.219.169 as permitted sender) smtp.mailfrom=elver@google.com; dmarc=pass (policy=reject) header.from=google.com X-HE-Tag: 1669714352-614533 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, 29 Nov 2022 at 07:37, Feng Tang wrote: > > kmalloc redzone check for slub has been merged, and it's better to add > a kunit case for it, which is inspired by a real-world case as described > in commit 120ee599b5bf ("staging: octeon-usb: prevent memory corruption"): > > " > octeon-hcd will crash the kernel when SLOB is used. This usually happens > after the 18-byte control transfer when a device descriptor is read. > The DMA engine is always transferring full 32-bit words and if the > transfer is shorter, some random garbage appears after the buffer. > The problem is not visible with SLUB since it rounds up the allocations > to word boundary, and the extra bytes will go undetected. > " > > To avoid interrupting the normal functioning of kmalloc caches, a > kmem_cache mimicing kmalloc cache is created with similar and all > necessary flags to have kmalloc-redzone enabled, and kmalloc_trace() > is used to really test the orig_size and redzone setup. > > Suggested-by: Vlastimil Babka > Signed-off-by: Feng Tang > --- > Changelog: > > since v1: > * create a new cache mimicing kmalloc cache, reduce dependency > over global slub_debug setting (Vlastimil Babka) > > lib/slub_kunit.c | 23 +++++++++++++++++++++++ > mm/slab.h | 3 ++- > 2 files changed, 25 insertions(+), 1 deletion(-) > > diff --git a/lib/slub_kunit.c b/lib/slub_kunit.c > index a303adf8f11c..dbdd656624d0 100644 > --- a/lib/slub_kunit.c > +++ b/lib/slub_kunit.c > @@ -122,6 +122,28 @@ static void test_clobber_redzone_free(struct kunit *test) > kmem_cache_destroy(s); > } > > +static void test_kmalloc_redzone_access(struct kunit *test) > +{ > + struct kmem_cache *s = kmem_cache_create("TestSlub_RZ_kmalloc", 32, 0, > + SLAB_KMALLOC|SLAB_STORE_USER|SLAB_RED_ZONE|DEFAULT_FLAGS, > + NULL); > + u8 *p = kmalloc_trace(s, GFP_KERNEL, 18); > + > + kasan_disable_current(); > + > + /* Suppress the -Warray-bounds warning */ > + OPTIMIZER_HIDE_VAR(p); > + p[18] = 0xab; > + p[19] = 0xab; > + > + kmem_cache_free(s, p); > + validate_slab_cache(s); > + KUNIT_EXPECT_EQ(test, 2, slab_errors); > + > + kasan_enable_current(); > + kmem_cache_destroy(s); > +} > + > static int test_init(struct kunit *test) > { > slab_errors = 0; > @@ -141,6 +163,7 @@ static struct kunit_case test_cases[] = { > #endif > > KUNIT_CASE(test_clobber_redzone_free), > + KUNIT_CASE(test_kmalloc_redzone_access), > {} > }; > > diff --git a/mm/slab.h b/mm/slab.h > index c71590f3a22b..b6cd98b16ba7 100644 > --- a/mm/slab.h > +++ b/mm/slab.h > @@ -327,7 +327,8 @@ static inline slab_flags_t kmem_cache_flags(unsigned int object_size, > /* Legal flag mask for kmem_cache_create(), for various configurations */ > #define SLAB_CORE_FLAGS (SLAB_HWCACHE_ALIGN | SLAB_CACHE_DMA | \ > SLAB_CACHE_DMA32 | SLAB_PANIC | \ > - SLAB_TYPESAFE_BY_RCU | SLAB_DEBUG_OBJECTS ) > + SLAB_KMALLOC | SLAB_SKIP_KFENCE | \ > + SLAB_TYPESAFE_BY_RCU | SLAB_DEBUG_OBJECTS) Shouldn't this hunk be in the previous patch, otherwise that patch alone will fail? This will also make SLAB_SKIP_KFENCE generally available to be used for cache creation. This is a significant change, and before it wasn't possible. Perhaps add a brief note to the commit message (or have a separate patch). We were trying to avoid making this possible, as it might be abused - however, given it's required for tests like these, I suppose there's no way around it. Thanks, -- Marco