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 ABC2AC48291 for ; Fri, 2 Feb 2024 10:03:34 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 24ECB6B007B; Fri, 2 Feb 2024 05:03:34 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 1FECF6B007D; Fri, 2 Feb 2024 05:03:34 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0C69C6B007E; Fri, 2 Feb 2024 05:03:34 -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 ED4CF6B007B for ; Fri, 2 Feb 2024 05:03:33 -0500 (EST) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 4EAFF140F13 for ; Fri, 2 Feb 2024 10:03:33 +0000 (UTC) X-FDA: 81746426706.18.27FD0C1 Received: from postout2.mail.lrz.de (postout2.mail.lrz.de [129.187.255.138]) by imf22.hostedemail.com (Postfix) with ESMTP id A6046C0020 for ; Fri, 2 Feb 2024 10:03:30 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=tum.de header.s=tu-postout21 header.b=RPVzcIDU; dmarc=pass (policy=none) header.from=tum.de; spf=pass (imf22.hostedemail.com: domain of paul.heidekrueger@tum.de designates 129.187.255.138 as permitted sender) smtp.mailfrom=paul.heidekrueger@tum.de ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1706868211; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=IRwZ7u3dYjdCFrPcqazENsa1lO8v+rGpqJG9/l92qYo=; b=jKXC7hlGsbkBJidgfVFp+8laey+V4KGmv83UePEx7gp+TJrgJp2gXazhD+/bwxuLKZ0/ed MhJPufJPfq7P3skhxN/OUcPC14ircVdAHVNwVjrQ7Ys91Pf5+dpx7KM9H4c0CFf/Iv2m2/ hdT/3foIU2fH8eA6Oti20ZbDo85azaI= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=pass header.d=tum.de header.s=tu-postout21 header.b=RPVzcIDU; dmarc=pass (policy=none) header.from=tum.de; spf=pass (imf22.hostedemail.com: domain of paul.heidekrueger@tum.de designates 129.187.255.138 as permitted sender) smtp.mailfrom=paul.heidekrueger@tum.de ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1706868211; a=rsa-sha256; cv=none; b=PKScbp1pixX7UBano7VS42YQVuWvdPe/JW8lA1+hXqg0MIC6e/Djj0IyHdrOMcdZrErCD1 SbcSdCqNpb3SQ5snr6e9Yk19OwG/bp1lnKBok3KfGLTuDCGlRCb7ewWIT5vyxOHZ4mhoEB /xjwaCD0QgkSXCdXTKOVX7oR58dMJXI= Received: from lxmhs52.srv.lrz.de (localhost [127.0.0.1]) by postout2.mail.lrz.de (Postfix) with ESMTP id 4TRBDc2LD0zyVJ; Fri, 2 Feb 2024 11:03:28 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tum.de; h= in-reply-to:content-transfer-encoding:content-disposition :content-type:content-type:mime-version:references:message-id :subject:subject:from:from:date:date:received:received; s= tu-postout21; t=1706868205; bh=mZPhnsfQIs9PURvzCAM3bG3CdSibjO9GC 8sexiOx9X8=; b=RPVzcIDUiPEmO9JesrdxFvrjHa8vxKD/wLe0hWLVqjD/z1s+h fcyDXwLG7Zx/t4p2XorZpdpPb7lreB2Cd+PYYHUbRsJf7LWGzTVyN+ql2UMMBlSD +Tp/DpNKxG4lvpn8diBfeJvPanX2nC88QhaFU9GWzDHiEhqPBgmDcOuBgBzXk/yl pKubtpeb5X/w1/z7kwZ9YA05sglLkBUh4jVtMtGJ1XLKJJYSBO00aU5Jgm5MRjUs xns0TZrb2+ONtNHb50higuGnMUTLX7/FLOHEuNkCcbBSjGCv8VS560eQ2CIasNEN IyYUq9Ti4Ga6mKl/40TuuEJzxxyFUKsnevlmQ== X-Virus-Scanned: by amavisd-new at lrz.de in lxmhs52.srv.lrz.de Received: from postout2.mail.lrz.de ([127.0.0.1]) by lxmhs52.srv.lrz.de (lxmhs52.srv.lrz.de [127.0.0.1]) (amavisd-new, port 20024) with LMTP id 8CO4Xq_tL921; Fri, 2 Feb 2024 11:03:25 +0100 (CET) Received: from pine.fritz.box (unknown [IPv6:2001:a61:2531:301:2520:4fa:71b2:b582]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by postout2.mail.lrz.de (Postfix) with ESMTPSA id 4TRBDX3p7czyV1; Fri, 2 Feb 2024 11:03:24 +0100 (CET) Date: Fri, 2 Feb 2024 11:03:18 +0100 From: Paul =?utf-8?Q?Heidekr=C3=BCger?= To: Marco Elver Cc: Andrey Ryabinin , Alexander Potapenko , Andrey Konovalov , Dmitry Vyukov , Vincenzo Frascino , Andrew Morton , kasan-dev@googlegroups.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: Re: [PATCH RFC v2] kasan: add atomic tests Message-ID: References: <20240131210041.686657-1-paul.heidekrueger@tum.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspamd-Queue-Id: A6046C0020 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: kxgyki5drp7gkjc4se41mwdd5yazbe3k X-HE-Tag: 1706868210-482080 X-HE-Meta: U2FsdGVkX188M+3THH6IQN0auTp2qeaQqYV8yGp3+Fv5N9gPXILm+ov9atVYa13DcJrzmYdOQpY3cLYH1qKBycGCmQeZ8ZWzCXcpoEQTzu82Cle6jBmJPU3Ggj2IYtyvJzGG2X1tuifB13rpRE3QPapfaxi/yYh5BTJkkobzIst4id6I6yNm6ZdjEWtWiQUjX79B9Ocy3VqE9oC02oH28d4IHO4gBYE+9qjtI7+b6e+hPZ5AxvkBuE9lcNy0/M//3dCJJfXubbehqVD0Plf9RL5ecz4uITBwbVvA2i6OHgJdz+Cy6KC71tNgMJaTjL84zEeULqYnshTkLez2wc1w5z4iv/EMURgn/9GOH2ojeIPu5/JgtyR43BXOTvFSxVDkdyr4JM1GW945X6aYkqHaM7KH+k9QTpjmx8zktfJXFjlHBPqB4qTR2RoV8L9PWEKibSvgwociOgiahlvA//tod5zAJUANE9wiCg9OLt9gN5085sgXS04V/OxPGpUkblohWYuREE3njcxWvSoaRhopMinifYnAcx2xncHUIqCleuBgTTlnro6Mh2T4PDd5jYz9wiJs1FXnz8YdCHwOS5CQ2dvi94mbCapk311FFZphWKhPqVx/wvZuxALsdihPUjPsN7RdLmpCjsGBH1PAVlTKHSMf2K+Kb9zwVRg3Vo2ZXYfn1WtOr+CQ1SCLIiU4xSBifHT7GEHGX6wUQPvqJAOk2Rs901A8oZgQJKx6tB9xLJDybA3YyYjbyx+5hcZZ2Em2gyidn4DUZEolTDVgwDRccn2GunRFnbs4AzmM/LQIq9/S01zv7leH8iMRpFjqDth7Szcm53eOnpggYVn1HgcKR11bcEnxOY81bPpT9/e5lwe+jo389EIdhYE5fM/ikCjYB76EZEsXY+L+59v8xOYqHDBkSGRMX5c8kWoGmdhqok+GR/oSMEMOo1CvSyYQAHeN5j8kkJ3HE0tQH8hzBc4 c0QlrRy+ JvVF1L3VPKPHfxKI/37asPrPxpgEcBxbC1XGPJWbiNGJyVreJp0hn6a0aXwp/n88bLmJyO9DKo1dp5RQDZ60eSgEDbQ7alT5FYOD0ei8q0cgMQ0ssqzL9RDLJSHPiV6t7L93s5gYFW0iajWwaM0WEKFldpUSB43AeAfPA9ceLRu4dIfai5YcfbrVbdPKTijNIy/Ye8mZ2w9/PeYKI2FvO1F6mVQe148R2Q15D2lB3eYVs1A+6dE+VLCswZBUEtpDHA+lMi0g8ai9FXUvtgpl4JQuneQ1QnREbSn0C1a0mDtL6gM0kb+cX8Ube9A+SC65FdiujgvcMvY9rw5u7g9QlCypNqAwmn2T+py+ochcfIWJ4eWWVg9dAX38l/Z0tl7sgxnEf0vRYVJRBuvi2j5EEBFdV2cCyzuqRNO2u4T7GrjKX3NY= 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: List-Subscribe: List-Unsubscribe: On 01.02.2024 10:38, Marco Elver wrote: > On Wed, 31 Jan 2024 at 22:01, Paul Heidekrüger wrote: > > > > Hi! > > > > This RFC patch adds tests that detect whether KASan is able to catch > > unsafe atomic accesses. > > > > Since v1, which can be found on Bugzilla (see "Closes:" tag), I've made > > the following suggested changes: > > > > * Adjust size of allocations to make kasan_atomics() work with all KASan modes > > * Remove comments and move tests closer to the bitops tests > > * For functions taking two addresses as an input, test each address in a separate function call. > > * Rename variables for clarity > > * Add tests for READ_ONCE(), WRITE_ONCE(), smp_load_acquire() and smp_store_release() > > > > I'm still uncelar on which kinds of atomic accesses we should be testing > > though. The patch below only covers a subset, and I don't know if it > > would be feasible to just manually add all atomics of interest. Which > > ones would those be exactly? > > The atomics wrappers are generated by a script. An exhaustive test > case would, if generated by hand, be difficult to keep in sync if some > variants are removed or renamed (although that's probably a relatively > rare occurrence). > > I would probably just cover some of the most common ones that all > architectures (that support KASAN) provide. I think you are already > covering some of the most important ones, and I'd just say it's good > enough for the test. > > > As Andrey pointed out on Bugzilla, if we > > were to include all of the atomic64_* ones, that would make a lot of > > function calls. > > Just include a few atomic64_ cases, similar to the ones you already > include for atomic_. Although beware that the atomic64_t helpers are > likely not available on 32-bit architectures, so you need an #ifdef > CONFIG_64BIT. > > Alternatively, there is also atomic_long_t, which (on 64-bit > architectures) just wraps atomic64_t helpers, and on 32-bit the > atomic_t ones. I'd probably opt for the atomic_long_t variants, just > to keep it simpler and get some additional coverage on 32-bit > architectures. If I were to add some atomic_long_* cases, e.g. atomic_long_read() or atomic_long_write(), in addition to the test cases I already have, wouldn't that mean that on 32-bit architectures we would have the same test case twice because atomic_read() and long_atomic_read() both boil down to raw_atomic_read() and raw_atomic_write() respectively? Or did I misunderstand and I should only be covering long_atomic_* functions whose atomic_* counterpart doesn't exist in the test cases already? > > Also, the availability of atomics varies between architectures; I did my > > testing on arm64. Is something like gen-atomic-instrumented.sh required? > > I would not touch gen-atomic-instrumented.sh for the test. > > > Many thanks, > > Paul > > > > CC: Marco Elver > > CC: Andrey Konovalov > > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=214055 > > Signed-off-by: Paul Heidekrüger > > --- > > mm/kasan/kasan_test.c | 50 +++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 50 insertions(+) > > > > diff --git a/mm/kasan/kasan_test.c b/mm/kasan/kasan_test.c > > index 8281eb42464b..1ab4444fe4a0 100644 > > --- a/mm/kasan/kasan_test.c > > +++ b/mm/kasan/kasan_test.c > > @@ -1150,6 +1150,55 @@ static void kasan_bitops_tags(struct kunit *test) > > kfree(bits); > > } > > > > +static void kasan_atomics_helper(struct kunit *test, void *unsafe, void *safe) > > +{ > > + int *i_safe = (int *)safe; > > + int *i_unsafe = (int *)unsafe; > > + > > + KUNIT_EXPECT_KASAN_FAIL(test, READ_ONCE(*i_unsafe)); > > + KUNIT_EXPECT_KASAN_FAIL(test, WRITE_ONCE(*i_unsafe, 42)); > > + KUNIT_EXPECT_KASAN_FAIL(test, smp_load_acquire(i_unsafe)); > > + KUNIT_EXPECT_KASAN_FAIL(test, smp_store_release(i_unsafe, 42)); > > + KUNIT_EXPECT_KASAN_FAIL(test, atomic_read(unsafe)); > > + KUNIT_EXPECT_KASAN_FAIL(test, atomic_set(unsafe, 42)); > > + KUNIT_EXPECT_KASAN_FAIL(test, atomic_add(42, unsafe)); > > + KUNIT_EXPECT_KASAN_FAIL(test, atomic_sub(42, unsafe)); > > + KUNIT_EXPECT_KASAN_FAIL(test, atomic_inc(unsafe)); > > + KUNIT_EXPECT_KASAN_FAIL(test, atomic_dec(unsafe)); > > + KUNIT_EXPECT_KASAN_FAIL(test, atomic_and(42, unsafe)); > > + KUNIT_EXPECT_KASAN_FAIL(test, atomic_andnot(42, unsafe)); > > + KUNIT_EXPECT_KASAN_FAIL(test, atomic_or(42, unsafe)); > > + KUNIT_EXPECT_KASAN_FAIL(test, atomic_xor(42, unsafe)); > > + KUNIT_EXPECT_KASAN_FAIL(test, atomic_xchg(unsafe, 42)); > > + KUNIT_EXPECT_KASAN_FAIL(test, atomic_cmpxchg(unsafe, 21, 42)); > > + KUNIT_EXPECT_KASAN_FAIL(test, atomic_try_cmpxchg(unsafe, safe, 42)); > > + KUNIT_EXPECT_KASAN_FAIL(test, atomic_try_cmpxchg(safe, unsafe, 42)); > > + KUNIT_EXPECT_KASAN_FAIL(test, atomic_sub_and_test(42, unsafe)); > > + KUNIT_EXPECT_KASAN_FAIL(test, atomic_dec_and_test(unsafe)); > > + KUNIT_EXPECT_KASAN_FAIL(test, atomic_inc_and_test(unsafe)); > > + KUNIT_EXPECT_KASAN_FAIL(test, atomic_add_negative(42, unsafe)); > > + KUNIT_EXPECT_KASAN_FAIL(test, atomic_add_unless(unsafe, 21, 42)); > > + KUNIT_EXPECT_KASAN_FAIL(test, atomic_inc_not_zero(unsafe)); > > + KUNIT_EXPECT_KASAN_FAIL(test, atomic_inc_unless_negative(unsafe)); > > + KUNIT_EXPECT_KASAN_FAIL(test, atomic_dec_unless_positive(unsafe)); > > + KUNIT_EXPECT_KASAN_FAIL(test, atomic_dec_if_positive(unsafe)); > > +} > > + > > +static void kasan_atomics(struct kunit *test) > > +{ > > + int *a1, *a2; > > If you're casting it to void* below and never using as an int* in this > function, just make these void* (the sizeof can just be sizeof(int)). > > > + a1 = kzalloc(48, GFP_KERNEL); > > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, a1); > > + a2 = kzalloc(sizeof(*a1), GFP_KERNEL); > > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, a1); > > + > > + kasan_atomics_helper(test, (void *)a1 + 48, (void *)a2); > > We try to ensure (where possible) that the KASAN tests are not > destructive to the rest of the kernel. I think the size of "48" was > chosen to fall into the 64-byte size class, similar to the bitops. I > would just copy that comment, so nobody attempts to change it in > future. :-) And yes to all the rest - thanks for the feedback! > > + kfree(a1); > > + kfree(a2); > > Thanks, > -- Marco Many thanks, Paul