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 E46C6C41513 for ; Tue, 28 May 2024 10:21:01 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7F6F46B0092; Tue, 28 May 2024 06:21:01 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7A82F6B0093; Tue, 28 May 2024 06:21:01 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 66EF66B0095; Tue, 28 May 2024 06:21:01 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 48F276B0092 for ; Tue, 28 May 2024 06:21:01 -0400 (EDT) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id E141C1403A0 for ; Tue, 28 May 2024 10:20:59 +0000 (UTC) X-FDA: 82167411438.16.BFFDDC6 Received: from mail-yb1-f172.google.com (mail-yb1-f172.google.com [209.85.219.172]) by imf03.hostedemail.com (Postfix) with ESMTP id 1ECCF20005 for ; Tue, 28 May 2024 10:20:57 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=2z4BWGo5; spf=pass (imf03.hostedemail.com: domain of glider@google.com designates 209.85.219.172 as permitted sender) smtp.mailfrom=glider@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1716891658; a=rsa-sha256; cv=none; b=b5lpA9+jOMTVvdhlgp2VJsayJfmXNihtW209WdzrnS+Dp23y0SipwE7HQK3WT+ZoqR5j6T l8jfk5k+AdeUSZLO+2YPtbB7120tpMY0VFy94RGl/5Dz78ApcaiotVUQ5+g/kImP/JRaAL OONHnv8AapiRDQSEUG8QMVFBdEJFmMI= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=2z4BWGo5; spf=pass (imf03.hostedemail.com: domain of glider@google.com designates 209.85.219.172 as permitted sender) smtp.mailfrom=glider@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1716891658; 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=gL61El2MUvMXM1QO7xxPZA2/oELsPgEiA0VxGFI8MCw=; b=DsqlIarJoOyDVZBMhyNAq5tCzI+BGz3acgh3i39CsxmlkdBrnRdpwsOHy+EsovzYOE1eF6 j79Fw9aXJXxFeJqikQLVp4XKDWSgV8xG7nIb/5PNAQ4FejRe/HarI0RPEeHToNI1Z/Rp7h 4aMAXJPd/CtZhamqdvumC5oftWcKBK0= Received: by mail-yb1-f172.google.com with SMTP id 3f1490d57ef6-df771db8b24so604447276.3 for ; Tue, 28 May 2024 03:20:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1716891657; x=1717496457; 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=gL61El2MUvMXM1QO7xxPZA2/oELsPgEiA0VxGFI8MCw=; b=2z4BWGo5uob/Pr7h0lYkXTRmci2b6jo8jptepXKOeGaonyT6ycW4tNqqu2vFnBkakz bDG1HFb3rgrW3Grw69oLQxdHAAZF/xlh3ct9rCE47dT2PQh1DyWCI+SJPerch7+iySgu mg37zTw422vw3zrklR8njknlQjUAexAYBHFN17LHvYR8eSURCHkc6+q1El3fw1x91bB8 aroDUXcJ/5WvsNY5wrCqOyn7FUCgSOkbHpVkmYhjI16u9KMm1uaC3Dxy3OIyTCQrh8RI Ezi34bcdRkFgzBBXfvcXbg58ABJzUCWqQn3OiqvnY92zec92pY+c/9oGVW+81DbXLnpg ZSkw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1716891657; x=1717496457; 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=gL61El2MUvMXM1QO7xxPZA2/oELsPgEiA0VxGFI8MCw=; b=Q7Q1E93DxVAhhWLryLm6C3MtT2ydugHzEsTdH2MKhJJ52OKHuZa6iq6M1AAA6G3eEG ewniITJm5E4RldWNbEJrqv6O19uEVgpgMlamMkP0bskweKxr7RiuekRWDadpaXwHurDd R20kwdqy0+YYPzlNfJG7EPFVPY+M5vhv/vdxHq2Vtf6+V4/59bBw83LcRJTV+j5fYg2H QeTwrFwjhq9EjC5MC2P05ij6wMZ1HPAEsqnlRH7ki9mnEiLmF3cKzKzu0IpZ7DWKp5ka X2V+p+a08yU3jfCv5gXeVVUYS4+2EY7hpdPPXc0cGCyxOXuTQO3viUQxJPvvuDy4SUba BLJg== X-Forwarded-Encrypted: i=1; AJvYcCUzNdjun/wYr2m6Vrp6WjwBw+CmxloTYAodve2wTtyHstSfCoKnpQRKNu1LcPRhP6gcu9ujqYtewFMpYvBKe4R8UPM= X-Gm-Message-State: AOJu0Yy3Toknknvso81sC8IqsrFllKf5aJDM3A+8/M9I8EEfG1/MZbbE 3IjXbk3rgvx84Xc9OTOeBQp8Yr9nsxG39o+9so0gLQhwpwuWa8iG6zapUaQcdIiLdf3ctVaXIHd Hp0BO/cFf31SegmvPGYjb6zy1PYAch6Q7zgsj X-Google-Smtp-Source: AGHT+IGVefpydQUP287F0eKALWCJSpkCj+j+25zdNrdIpDbUXLUcnBDJt6zW3qBe07KpY0NpzvdOvUjXQ8IToL/ELxo= X-Received: by 2002:a25:ad0b:0:b0:df4:db5c:99f4 with SMTP id 3f1490d57ef6-df77224071fmr10979349276.53.1716891656941; Tue, 28 May 2024 03:20:56 -0700 (PDT) MIME-Version: 1.0 References: <20240524232804.1984355-1-bjohannesmeyer@gmail.com> In-Reply-To: <20240524232804.1984355-1-bjohannesmeyer@gmail.com> From: Alexander Potapenko Date: Tue, 28 May 2024 12:20:15 +0200 Message-ID: Subject: Re: [PATCH] kmsan: introduce test_unpoison_memory() To: Brian Johannesmeyer Cc: Marco Elver , Dmitry Vyukov , 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: 1ECCF20005 X-Rspam-User: X-Rspamd-Server: rspam12 X-Stat-Signature: ns6ssnscd5yz547ce19p5d54a5qt471c X-HE-Tag: 1716891657-578986 X-HE-Meta: U2FsdGVkX1/WPmq8f9ESMpoqMw5Zyrip4fiIbiRiBtVMROroKUhbkOxXu6O9pHGfgVkEyhyNisUklDlpb8TgW1WL6nMKokn3gcvBg+ncUuWgmzTTOoLWPzqfYNRUmJQ/2JdTSGpSxmdcwoobmbmy7e2ilW199mu7YMOGAitZH/bFZepGJcpR4qxisDG8oBcggi25gLcc8QSzFnRXoDwwYFOgN8POCk9mm4WhHs4Wuu2WF2os774vIUh4BrTsVr1VFDhUDkPLYRECsosESIXtjHHgD/WFErCVDcCwUFyEcTfc64SA5EIAkxsad1sjMs4DugCAtnRgIAhiJQyxezTB7hEtLkIg+v7gP9gLFNmP6ilq3Ynqw6LqqhWOQUUUKlmN7gy65wQPU11Gy3enJrTtNuHo80RGeGH8oXgXCMIOxdYnDMsTGvrt8kwJg/gQ+MFcMs63lbEe+4Mp7e+7EGPnjBKEcgvl0pJL3E0LQnVVEpkj9qD5p1j8XtYz5HzTas/Xbx2ptsnLibRFAKxkGrgwmGcmdb9TTz6nxHkmdVUD07Iw5VDtD9FNgaTH5K5LmFUqJ1fAygzGaKOxLZe6YopzjdXek8hJPQa5RNj9xcyNkQOJ9OGDdE3dkv8tIqC63asz4qB3dW5MCS6JqRcHfI60at5alMbBuAnE0cPCRcvqhd4BY48DVMlYmxCHSNJROxSgUdxc3gs2s6YK6RUgbcYatR052AgXxBaeI/9ouWIOuFcaAc6cbA715LYxfx4fkPEHt67y9Vn4I/ANx7NGnc3Bq+t+E9RKoa77b3nVPksevfhs5crA5KLevzhnPtm7+pxTnCDtSLVdBlA8zaQtolbU+S08XSyc8hk+w/4EV1T6QnHHOs6fwmDI8IvN7DGYoPnb023BRUyr3H09niACwdW7Ua9mt7Ys4n8zmhi6dzRa4YgIZapIRo0b7oPBKz2xLRt4Ryr+W2R2+DghTvRvtCn PD6WaxV/ jCWR4fhr4gbO+2cGm6T6Um0nYu3k5PKQVDr+ZGgYk42LgXQx1KxKo8cdY4BTmbKukX8rRGucipc7IG3ofQN3WyzwweWBaa+3q9dMT4tmmKvLYzNKWYr/wcVhGga69AG04+QtdC6g2twUggzpA8mtQ9KepZ78IHVwrfoBckYkXbM0RceT0sFsg29I/fOmi4VQvXf4mXN9UhuA6L0HtKH70jl+ODOZSLmrH9kvcbEbGnUDm2LKsjz/djvoakQ34EdD2Y0Fxbc0ZHIO7xCLMxij9FJhx5YHT5SgF30rW56ODBGPOGwlXH/c1PQ6HWL2Af9wXekRr6iKecP+/CChbhTGedmKU/KR9obZcfU2KH1/YMhnPl1/mq9GtP3JEdDjhRBFdUFTNt7lWtvABoRX/2bIBJPnVW+pWmtwq8p3LCUFxc29R8n4JeS8PUGizVeEJ/TzylCd2 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 Sat, May 25, 2024 at 1:28=E2=80=AFAM Brian Johannesmeyer wrote: > > Add a regression test to ensure that kmsan_unpoison_memory() works the sa= me > as an unpoisoning operation added by the instrumentation. (Of course, > please correct me if I'm misunderstanding how these should work). > > The test has two subtests: one that checks the instrumentation, and one > that checks kmsan_unpoison_memory(). Each subtest initializes the first > byte of a 4-byte buffer, then checks that the other 3 bytes are > uninitialized. Unfortunately, the test for kmsan_unpoison_memory() fails = to > identify the 3 bytes as uninitialized (i.e., the line with the comment > "Fail: No UMR report"). > > As to my guess why this is happening: From kmsan_unpoison_memory(), the > backing shadow is indeed correctly overwritten in > kmsan_internal_set_shadow_origin() via `__memset(shadow_start, b, size);`= . > Instead, the issue seems to stem from overwriting the backing origin, in > the following `origin_start[i] =3D origin;` loop; if we return before tha= t > loop on this specific call to kmsan_unpoison_memory(), then the test > passes. Hi Brian, You are right with your analysis. KMSAN stores a single origin for every aligned four-byte granule of memory, so we lose some information when more than one uninitialized value is combined in that granule. When writing an uninitialized value to memory, a viable strategy is to always update the origin. But if we partially initialize the granule with a store, it is better to preserve that granule's origin to prevent false negatives, so we need to check the resulting shadow slot before updating the origin. This is what the compiler instrumentation does, so kmsan_internal_set_shadow_origin() should behave in the same way. I found a similar bug in kmsan_internal_memmove_metadata() last year, but missed this one. I am going to send a patch fixing this along with your test (with an updated description), if you don't object. > Signed-off-by: Brian Johannesmeyer > --- > mm/kmsan/kmsan_test.c | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/mm/kmsan/kmsan_test.c b/mm/kmsan/kmsan_test.c > index 07d3a3a5a9c5..c3ab90df0abf 100644 > --- a/mm/kmsan/kmsan_test.c > +++ b/mm/kmsan/kmsan_test.c > @@ -614,6 +614,30 @@ static void test_stackdepot_roundtrip(struct kunit *= test) > KUNIT_EXPECT_TRUE(test, report_matches(&expect)); > } > > +/* > + * Test case: ensure that kmsan_unpoison_memory() and the instrumentatio= n work > + * the same > + */ > +static void test_unpoison_memory(struct kunit *test) > +{ > + EXPECTATION_UNINIT_VALUE_FN(expect, "test_unpoison_memory"); > + volatile char a[4], b[4]; > + > + kunit_info( > + test, > + "unpoisoning via the instrumentation vs. kmsan_unpoison_m= emory() (2 UMR reports)\n"); > + > + a[0] =3D 0; // Initialize a[0= ] > + kmsan_check_memory((char *)&a[1], 3); // Check a[1]--a[3] > + KUNIT_EXPECT_TRUE(test, report_matches(&expect)); // Pass: UMR re= port > + > + report_reset(); > + > + kmsan_unpoison_memory((char *)&b[0], 1); // Initialize b[0] > + kmsan_check_memory((char *)&b[1], 3); // Check b[1]--b[3] > + KUNIT_EXPECT_TRUE(test, report_matches(&expect)); // Fail: No UMR= report > +} > + > static struct kunit_case kmsan_test_cases[] =3D { > KUNIT_CASE(test_uninit_kmalloc), > KUNIT_CASE(test_init_kmalloc), > @@ -637,6 +661,7 @@ static struct kunit_case kmsan_test_cases[] =3D { > KUNIT_CASE(test_memset64), > KUNIT_CASE(test_long_origin_chain), > KUNIT_CASE(test_stackdepot_roundtrip), > + KUNIT_CASE(test_unpoison_memory), > {}, > }; > > -- > 2.34.1 > > -- > You received this message because you are subscribed to the Google Groups= "kasan-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an= email to kasan-dev+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgi= d/kasan-dev/20240524232804.1984355-1-bjohannesmeyer%40gmail.com. --=20 Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Stra=C3=9Fe, 33 80636 M=C3=BCnchen Gesch=C3=A4ftsf=C3=BChrer: Paul Manicle, Liana Sebastian Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg