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 37CE2C47DDF for ; Thu, 1 Feb 2024 09:38:50 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BE14D6B0087; Thu, 1 Feb 2024 04:38:49 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B91526B0088; Thu, 1 Feb 2024 04:38:49 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A59356B0089; Thu, 1 Feb 2024 04:38:49 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 956946B0087 for ; Thu, 1 Feb 2024 04:38:49 -0500 (EST) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 6466540682 for ; Thu, 1 Feb 2024 09:38:49 +0000 (UTC) X-FDA: 81742735578.10.93D2A4D Received: from mail-vk1-f174.google.com (mail-vk1-f174.google.com [209.85.221.174]) by imf29.hostedemail.com (Postfix) with ESMTP id 89D56120031 for ; Thu, 1 Feb 2024 09:38:47 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=siLPQUhG; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf29.hostedemail.com: domain of elver@google.com designates 209.85.221.174 as permitted sender) smtp.mailfrom=elver@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1706780327; 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=XD0yQ/tI+tg2dm8uEuvvq9XaoXhsRoncokKKzr2jeMM=; b=S1h3WBQzXPqyNEgQWH6yRe0IK8oYfeK3QtxETj9gU1UyJNKyKnnpUyDS4FpVdnMoadCZE9 U2ux0xPBfuxddsPRWpoU4lT1MLxyUEvZUgQFYfX3y7bdwnkXeYAjOWR55AKU1dTIwHOYKz RhwbqsgU7H7WE6BInOW6SfmN1tRIw/A= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=siLPQUhG; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf29.hostedemail.com: domain of elver@google.com designates 209.85.221.174 as permitted sender) smtp.mailfrom=elver@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1706780327; a=rsa-sha256; cv=none; b=K+txgohZk8GCkr2jOX+VRmfayNjXIDPieItlkVaFSHjvVhCUrIi+PR+oNahYMDvN7vMdYH L8V9n9aI4+TC6hNITko15Mm4ohbeiKvG4QFA+BK2nKz34wgw28G6W9lF+k8HsEPWPYXdAR 91hYQNkeUD7A+1rl2QX/5BJy0SB4vSQ= Received: by mail-vk1-f174.google.com with SMTP id 71dfb90a1353d-4bda5740d2dso299247e0c.1 for ; Thu, 01 Feb 2024 01:38:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1706780326; x=1707385126; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=XD0yQ/tI+tg2dm8uEuvvq9XaoXhsRoncokKKzr2jeMM=; b=siLPQUhGhXUm6pl1zO0U2NLBSHIzSSkJ854DjNwGuynGm07BsBLMjfqBChmbSWLFVC h4l2vktzBCPUDWx2T5X20MfLhuQjPS8usiT9tE/O8YfLSeUCQFC0CY/Rg1NX7iA3WiPd Xh7Etg+EQTKeYCHW/kEuAM0la9aY5Ij1/9YzOvjXxLsWsN04UTV90VD5vEJZiWpruCjl 8obXR1v1NXwjNVSFcxju4lr3Tq8HN6VrLfiOYmUqHEuToxk4cJ4vRcYwUdN+Qk4tfRIw bmoLg4MJearaBVOejn66rqX6WfNV7klzZM+Ilj66FZSy6m12A7u+jwA4Ydsv4TMw/j1D Epdw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706780326; x=1707385126; h=content-transfer-encoding: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=XD0yQ/tI+tg2dm8uEuvvq9XaoXhsRoncokKKzr2jeMM=; b=GlzxOj/STDq1ji+qDJHFAfNssgdIVgf3TBpvtn1ClV8ZfwpG6Gkt1583a2QBCvS1NG /fIO31UjQK5VQNlbFsG0M6VoVy+fERxdq61LfjLId2AEOZOgd7TFsWkQX9oP4e13JW4z thWpWQHF5X2zec5kOU+0m8/yWPsfxJrSdQTW9vA7/AiACIEekGSv04hPmvUhTT+5TOZY fdj2anI7Wz2YfU9PK31ILZ9dBFsPhmEy5qSkyhTFks+sDT1CaktBAp+MsT4Ub27mhS9M BY3jlCV0oWE676rI9YyM+I3qSm5E2IsYSC+h7a9iBwInO7jzcn/wF7ZOT4y14PEYVnIQ MWHQ== X-Gm-Message-State: AOJu0YwpTMX058NiZV4YU8fyrdcL/XZvCLaTnVAfQPPhviOOgeH61Qm/ Md/YAPM6lfEKywTLutr9nJssb/HMYRK8Bg5y3FhXQJ4KO70hBaYpgCw76HKrxgIfxUt0cKb2RHE i4uolAatWoPIS/sDuodJBwmctRnpm7jdVfbQ5 X-Google-Smtp-Source: AGHT+IH+sgJyGjTNJ8nrAZACpyhjF1aGCbvp4J/0sKBmH4acCQHArxhx13e7Uh8RuNCOYXyQzWjA12EqMJKHy6Nh8Ic= X-Received: by 2002:a05:6122:1d87:b0:4bd:7da1:a2ea with SMTP id gg7-20020a0561221d8700b004bd7da1a2eamr1519212vkb.14.1706780326396; Thu, 01 Feb 2024 01:38:46 -0800 (PST) MIME-Version: 1.0 References: <20240131210041.686657-1-paul.heidekrueger@tum.de> In-Reply-To: <20240131210041.686657-1-paul.heidekrueger@tum.de> From: Marco Elver Date: Thu, 1 Feb 2024 10:38:07 +0100 Message-ID: Subject: Re: [PATCH RFC v2] kasan: add atomic tests To: =?UTF-8?Q?Paul_Heidekr=C3=BCger?= 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 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 89D56120031 X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: ds67sykwi67e3ado8xu5unj6w6gri4bu X-HE-Tag: 1706780327-829253 X-HE-Meta: U2FsdGVkX1/bhm43NJ8QiRAfRH/dEHaMV/J+rFa7pK62Sv+joO0iyQwuGlJe3IDf3ZN8n5Nd/+ylrNlzzQJk/NlGBcQp7JaBQSIdPr+W1SLGYMAkBOqRYgjlhjNWCyDxrB/SHkqhgsTH8S8UGOZk8yM+h9JFo4uecR9UAzDZcnPqXcqSxlRVTlN5ZgTw21gihTCH6gWmztOmBgOWVKlawXkyByMJPkTq5EgPTzOLFSl2HNI/1UvlCLkGlIFufM1qtHzqrDCYjnGKS0ocgVzAvgbWKZVo5rRmA5ZbB0L/yxa54KwIYZ2RbjYolOlnA1uREu+EsfZvKD3r5wegsRZDfDMy7e7l+16RMVyB1HoKPsz4uGnQ1zGu3e9OPVoxLHQ2d7PQVeuZo66+X4wEXSBtPB9U5nYljZmhPkKNy+3/UsU1/ISr84ZpPlTa3UWOlLD5z5H0kgLooIR8HiIV6r7Y29sve6swpdD7co7tKgcPlXd/6kMmvHVqT9uQgUkImCdkOFE3MgpScv013Rgss6tZwhiKhbJJ51Gk0sNxo3qwHmraoiKMFVmYRjLRE5gWIU+fDq7pwwMX3FM9GA4bSiU9XzZkwjiM8Nnkn8GTmhRmffqTv9jW0Lt1MqF00yu24Gd9h9hjvmSDnxEKnwCRs7/nsJ1+TU+Ycw2LmIDLjPK0VYzWa98vSL2dwx/ZNHi0Ju0YtrhpuG6bfD7SDbDKCYdoRjt7D9s3ybHMT6tcgJAXcsx1pGtLsFqUQOFmcTeKKvKtKFpkpt884zET1UC074jybtt3yZ4Ubq639SZ2gSC8Zt4oDbJG2tqMGwqQoURzR6M12U0Rqk9NhD7iEP3rMwi8HsSjh7sDvTOPfA1LWA/F/0QjEbI38D3uVuXzfz70s5GA2l9nDPuolTlBekl2m9EfXN6NJu/B2S80YjLDSTelIVSxusPW37LVPCh50m9fc137XEA9qyB371I1g4rrj1n 4ziPoffx lSeK6SeDlNsdStwS7gvxraK4XlFzGc4EfHDrvpylet9cP5+q9NclQQfxJRN2E5DSqf+eIxQeVNzKfZzzueZUhygHEd6iSvU7bqwFGZPyy8NC/ALtUAENoDPCm8FbmZybjjBG/L6GDI5QdzRW0n6akspPXFeEMEsZCgJCdLz4KRkcpI7p+x57BCkypdrYWeMCzEJGdWTqndIVvZ9kUUrIdONQTOMSpJ0YBRCRx5nv0VN/NQApUnXpYc8L2at89SKoPFOPKt+zrDQeBqNHx0PD4CMt7Z894BoUHhCerRcXN/7XNmr3R2+EkcLzN1ed0tDseyrxWyXY/yLw6mFgiHrV+lSL3lJW10Hbep9DsvdcZcvQO+jiehluZXTR42dKCHhice5e5c4NNqSWc7lKexp/Yuqaegw== 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 Wed, 31 Jan 2024 at 22:01, Paul Heidekr=C3=BCger 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_sto= re_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. > 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=3D214055 > Signed-off-by: Paul Heidekr=C3=BCger > --- > 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 =3D (int *)safe; > + int *i_unsafe =3D (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 =3D kzalloc(48, GFP_KERNEL); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, a1); > + a2 =3D 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. :-) > + kfree(a1); > + kfree(a2); Thanks, -- Marco