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=-10.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS autolearn=ham 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 AE137C433B4 for ; Thu, 13 May 2021 03:40:28 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 2C1DF613FC for ; Thu, 13 May 2021 03:40:28 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2C1DF613FC Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=linux-foundation.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 9EB586B0070; Wed, 12 May 2021 23:40:27 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 99BC06B0071; Wed, 12 May 2021 23:40:27 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 83BCA6B0072; Wed, 12 May 2021 23:40:27 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0063.hostedemail.com [216.40.44.63]) by kanga.kvack.org (Postfix) with ESMTP id 5004F6B0070 for ; Wed, 12 May 2021 23:40:27 -0400 (EDT) Received: from smtpin35.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id CF8E098A9 for ; Thu, 13 May 2021 03:40:26 +0000 (UTC) X-FDA: 78134805252.35.0F6995B Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by imf14.hostedemail.com (Postfix) with ESMTP id E1BCAC0007F8 for ; Thu, 13 May 2021 03:40:00 +0000 (UTC) Received: by mail.kernel.org (Postfix) with ESMTPSA id 00019613F7; Thu, 13 May 2021 03:40:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linux-foundation.org; s=korg; t=1620877225; bh=1E/vgh/+WpmAsgp9m69RubUIpeZA4hdZiaw/+4uS6ds=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=MlwDtV0+RYKgENgM8ynfernEGP+3noACuYmPGIrZgS0pmx8aUQ9bf7TLhgFfjO9bu o3AGbazLHZaOaMqyWNvtC/ZjdfdwEBsNg35w560OsRNhPFMfjsZTyYQ/H8rE3fCjo6 mW6jgklaTqDtue/bF9ru7Tw2R3jN5yq/zzoaUvt4= Date: Wed, 12 May 2021 20:40:24 -0700 From: Andrew Morton To: Hyeonggon Yoo <42.hyeyoo@gmail.com> Cc: vbabka@suse.cz, iamjoonsoo.kim@lge.com, rientjes@google.com, penberg@kernel.org, cl@linux.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Marco Elver Subject: Re: [PATCH v3] mm, slub: change run-time assertion in kmalloc_index() to compile-time Message-Id: <20210512204024.401ff3de38649d7d0f5a45e8@linux-foundation.org> In-Reply-To: <20210513031220.GA133011@hyeyoo> References: <20210511173448.GA54466@hyeyoo> <20210512195227.245000695c9014242e9a00e5@linux-foundation.org> <20210513031220.GA133011@hyeyoo> X-Mailer: Sylpheed 3.5.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: E1BCAC0007F8 Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=korg header.b=MlwDtV0+; spf=pass (imf14.hostedemail.com: domain of akpm@linux-foundation.org designates 198.145.29.99 as permitted sender) smtp.mailfrom=akpm@linux-foundation.org; dmarc=none X-Rspamd-Server: rspam04 X-Stat-Signature: nxgc4c3yci9utbbip31rnr56xnkompkh Received-SPF: none (linux-foundation.org>: No applicable sender policy available) receiver=imf14; identity=mailfrom; envelope-from=""; helo=mail.kernel.org; client-ip=198.145.29.99 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1620877200-927921 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 Thu, 13 May 2021 12:12:20 +0900 Hyeonggon Yoo <42.hyeyoo@gmail.com> wrote: > On Wed, May 12, 2021 at 07:52:27PM -0700, Andrew Morton wrote: > > This explodes in mysterious ways. The patch as I have it is appended, > > for reference. > > > > gcc-10.3.0 allmodconfig. > > > > This patch suppresses the error: > > > > --- a/mm/kfence/kfence_test.c~a > > +++ a/mm/kfence/kfence_test.c > > @@ -318,13 +318,13 @@ static void test_out_of_bounds_read(stru > > > > /* Test both sides. */ > > > > - buf = test_alloc(test, size, GFP_KERNEL, ALLOCATE_LEFT); > > + buf = test_alloc(test, 32, GFP_KERNEL, ALLOCATE_LEFT); > > expect.addr = buf - 1; > > READ_ONCE(*expect.addr); > > KUNIT_EXPECT_TRUE(test, report_matches(&expect)); > > test_free(buf); > > > > - buf = test_alloc(test, size, GFP_KERNEL, ALLOCATE_RIGHT); > > + buf = test_alloc(test, 32, GFP_KERNEL, ALLOCATE_RIGHT); > > expect.addr = buf + size; > > READ_ONCE(*expect.addr); > > KUNIT_EXPECT_TRUE(test, report_matches(&expect)); > > @@ -519,11 +519,11 @@ static void test_free_bulk(struct kunit > > const size_t size = setup_test_cache(test, 8 + prandom_u32_max(300), 0, > > (iter & 1) ? ctor_set_x : NULL); > > void *objects[] = { > > - test_alloc(test, size, GFP_KERNEL, ALLOCATE_RIGHT), > > - test_alloc(test, size, GFP_KERNEL, ALLOCATE_NONE), > > - test_alloc(test, size, GFP_KERNEL, ALLOCATE_LEFT), > > - test_alloc(test, size, GFP_KERNEL, ALLOCATE_NONE), > > - test_alloc(test, size, GFP_KERNEL, ALLOCATE_NONE), > > + test_alloc(test, 32, GFP_KERNEL, ALLOCATE_RIGHT), > > + test_alloc(test, 32, GFP_KERNEL, ALLOCATE_NONE), > > + test_alloc(test, 32, GFP_KERNEL, ALLOCATE_LEFT), > > + test_alloc(test, 32, GFP_KERNEL, ALLOCATE_NONE), > > + test_alloc(test, 32, GFP_KERNEL, ALLOCATE_NONE), > > }; > > > > kmem_cache_free_bulk(test_cache, ARRAY_SIZE(objects), objects); > > > > > > Is gcc-10.3.0 simply confused? test_out_of_bounds_read() is clearly > > calling kmalloc_index(32) which is OK. > > > > Anyway, I'll drop this patch for now so I can compile a kernel! > > > > The error messages isn't so clear to me. > but one problem I can see is in kfence_test.c, there are many places that > are using size which is not constant. Ah, yes, of course, your patch changes kmalloc_index() to require that it always is called with a constant `size'. kfence_test doesn't do that. kfence is being a bit naughty here - the other kmalloc_index() callers only comple up the call after verifying that `size' is a compile-time constant. Would something like this work? include/linux/slab.h | 12 ++++++++---- mm/kfence/kfence_test.c | 4 ++-- 2 files changed, 10 insertions(+), 6 deletions(-) --- a/include/linux/slab.h~b +++ a/include/linux/slab.h @@ -374,7 +374,8 @@ static __always_inline enum kmalloc_cach * Note: there's no need to optimize kmalloc_index because it's evaluated * in compile-time. */ -static __always_inline unsigned int kmalloc_index(size_t size) +static __always_inline unsigned int kmalloc_index(size_t size, + bool size_is_constant) { if (!size) return 0; @@ -410,7 +411,10 @@ static __always_inline unsigned int kmal if (size <= 16 * 1024 * 1024) return 24; if (size <= 32 * 1024 * 1024) return 25; - BUILD_BUG_ON_MSG(1, "unexpected size in kmalloc_index()"); + if (size_is_constant) + BUILD_BUG_ON_MSG(1, "unexpected size in kmalloc_index()"); + else + BUG(); /* Will never be reached. Needed because the compiler may complain */ return -1; @@ -575,7 +579,7 @@ static __always_inline void *kmalloc(siz if (size > KMALLOC_MAX_CACHE_SIZE) return kmalloc_large(size, flags); #ifndef CONFIG_SLOB - index = kmalloc_index(size); + index = kmalloc_index(size, true); if (!index) return ZERO_SIZE_PTR; @@ -593,7 +597,7 @@ static __always_inline void *kmalloc_nod #ifndef CONFIG_SLOB if (__builtin_constant_p(size) && size <= KMALLOC_MAX_CACHE_SIZE) { - unsigned int i = kmalloc_index(size); + unsigned int i = kmalloc_index(size, true); if (!i) return ZERO_SIZE_PTR; --- a/mm/kfence/kfence_test.c~b +++ a/mm/kfence/kfence_test.c @@ -197,7 +197,7 @@ static void test_cache_destroy(void) static inline size_t kmalloc_cache_alignment(size_t size) { - return kmalloc_caches[kmalloc_type(GFP_KERNEL)][kmalloc_index(size)]->align; + return kmalloc_caches[kmalloc_type(GFP_KERNEL)][kmalloc_index(size, false)]->align; } /* Must always inline to match stack trace against caller. */ @@ -267,7 +267,7 @@ static void *test_alloc(struct kunit *te if (is_kfence_address(alloc)) { struct page *page = virt_to_head_page(alloc); - struct kmem_cache *s = test_cache ?: kmalloc_caches[kmalloc_type(GFP_KERNEL)][kmalloc_index(size)]; + struct kmem_cache *s = test_cache ?: kmalloc_caches[kmalloc_type(GFP_KERNEL)][kmalloc_index(size, false)]; /* * Verify that various helpers return the right values _