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 3F8A8CF887B for ; Sat, 5 Oct 2024 10:37:27 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8F7736B0368; Sat, 5 Oct 2024 06:37:26 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8A7726B0373; Sat, 5 Oct 2024 06:37:26 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7478A6B0372; Sat, 5 Oct 2024 06:37:26 -0400 (EDT) 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 4F6136B035E for ; Sat, 5 Oct 2024 06:37:26 -0400 (EDT) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id D17E8ABD6C for ; Sat, 5 Oct 2024 10:37:25 +0000 (UTC) X-FDA: 82639196850.21.EE1EE53 Received: from mail-pj1-f49.google.com (mail-pj1-f49.google.com [209.85.216.49]) by imf17.hostedemail.com (Postfix) with ESMTP id 1B1834000A for ; Sat, 5 Oct 2024 10:37:23 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=uCVUzSre; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf17.hostedemail.com: domain of elver@google.com designates 209.85.216.49 as permitted sender) smtp.mailfrom=elver@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1728124620; a=rsa-sha256; cv=none; b=JgNE54x8doktaG3wND1bLtx9YkWergZVGURCB21+BawhyBHyBDoxRliTtqcrmwst3AGWm7 oBYAV7o2OwM7gaH/8mwbZu5NzuAss9gXrrz9p2+yMs5PS83gS7c6NTsX/ZeQNmDNhV04QM YvWl1Hv6ZLqilP3vdee2VUzt/GYA+80= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=uCVUzSre; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf17.hostedemail.com: domain of elver@google.com designates 209.85.216.49 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=1728124620; 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=B4ms11ZQkraDYH9Xo26ocEAmXpjpDosjseyHDy0gtFw=; b=IxsZopiBHxyQr++vSXfVaG2YtvACMHHVAVzYEiFjIv6qEcqQKP3sX9rVwsQT9tCzxfX4ts Vo1wbzh4lVvIjEGGPOyp9r2sve7HUyS2LI1zScaNgL1Mp0bXCbWsZ+6kzwxzqgImo5UTpP 8PvW+xpbG4sv7Dy7Ydd2PgiZEQYza/Y= Received: by mail-pj1-f49.google.com with SMTP id 98e67ed59e1d1-2e0af6e5da9so2241833a91.2 for ; Sat, 05 Oct 2024 03:37:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1728124643; x=1728729443; darn=kvack.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=B4ms11ZQkraDYH9Xo26ocEAmXpjpDosjseyHDy0gtFw=; b=uCVUzSreR01xJlJ1MqdheqLbpOP6osqMmcCudI9GkTQ60/DQYg/GlqBzIiZ40oSMi/ RE2O/Pm5wOVXopKvscjbNO/UvgGELdDv95BJXV2jv6LELKQQ0J7Di2YOnwgFCmpaNlQe 45EisKdxJv4Ocye2x+Yfub8l9m2ZOGariC5jy+wSu+PMgEJcDW0EEQVpZVHswAx5Egt3 hg5uAHucpJKwvkmOvRbdFxmEI63xUfh6R/DE6nEGASvjOLy2rLt73V8vEqXhqD3gSRtt sZcMELvTQos7s7KpvryQHlwRgwkUwaqxwz9Wy7oFKe3hp7wpp1w1LvsWwahSlZwSmoDB SS4w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1728124643; x=1728729443; 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=B4ms11ZQkraDYH9Xo26ocEAmXpjpDosjseyHDy0gtFw=; b=M1d1VXO8PL/IRoMI6I4EsSE7HLx7otSVe9OPxqF3pmLJ7GYCHqOtexdQhk+x3jlW3Y e+5UVmjBY+wYMRd66YjcmG0emGJGacBREEKWJ2YwNuhYvUkz18BttccIrvvWI/hoctzV +xT/zYFT6k+cZm0N9hzNDM0iNXBk79gaPIfCWUM4FdtY4MkwQKgMRNdnEQlBAXZqNNZD SmH+/rf/CRRsZKa/LooB6PhusCOw5fu0WGoQKoPbAndzv02IBnRCgIMQTsGTRZfYr3cV fYZJCFxllL/ar5UibJUx7s0EvJRKJnjkTtIhPXvvV9kDwTg/4w53r6rCqGeY2OSBJu28 pvjQ== X-Forwarded-Encrypted: i=1; AJvYcCXcdkgKx9yHULqE/ItyApqOxw8fSIY04qc66MjAMsoNRCuVLxk8Q21z8Zx3MelUNQK2kkktI/+ECQ==@kvack.org X-Gm-Message-State: AOJu0YwYCJkDHBZdgsGVSjcFrk3C8FNWWZKIfO7RIEVHBwx0LY5Sa73S vhOS+kvjaIYIOFe+fo6AojCuKk6gzWK7X19WlMEeScy1JsqBBPp9VuV1ttRdhqog6XFnsLwjt4X 4cIUQpKE9H+eIn37V/3niXYQCAfR5ycUEaeKr X-Google-Smtp-Source: AGHT+IFpelqnAaOBUIxS1I4oQr1q2aEbZ56jlxqBJluUueazlQUie6wZxLwd0KZecIyBO72JszXXXVvhtpu6+8xR5xo= X-Received: by 2002:a17:90b:494:b0:2c9:b72:7a1f with SMTP id 98e67ed59e1d1-2e1e632369fmr7417174a91.28.1728124642435; Sat, 05 Oct 2024 03:37:22 -0700 (PDT) MIME-Version: 1.0 References: <20241005092316.2471810-1-snovitoll@gmail.com> In-Reply-To: <20241005092316.2471810-1-snovitoll@gmail.com> From: Marco Elver Date: Sat, 5 Oct 2024 12:36:44 +0200 Message-ID: Subject: Re: [PATCH] mm, kmsan: instrument copy_from_kernel_nofault To: Sabyrzhan Tasbolatov Cc: ryabinin.a.a@gmail.com, glider@google.com, andreyknvl@gmail.com, dvyukov@google.com, akpm@linux-foundation.org, vincenzo.frascino@arm.com, kasan-dev@googlegroups.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org, bpf@vger.kernel.org, syzbot+61123a5daeb9f7454599@syzkaller.appspotmail.com Content-Type: text/plain; charset="UTF-8" X-Rspam-User: X-Stat-Signature: kdj7b5ah1at1ehur354r3rzt7rd1b38y X-Rspamd-Queue-Id: 1B1834000A X-Rspamd-Server: rspam02 X-HE-Tag: 1728124643-664925 X-HE-Meta: U2FsdGVkX198VXlj+rLiiDPbvzd8r2w3Mt+wU4vRdGsC66KySJWnuFdPOhyU4emLplzwcV8R1m1+LTn1VWm6FNHS0qp/Zv9dpANAtjd0YPoevCbNFLu3irXR7Ag558scwwwyI414OHjFAv0bBmyZTDI5Jvapc/ugfX21QbNmUWH180jpAOf74DHsr+9AifnMthNZuodCP/E4hS5pZdBbw1yU5effHmj4I1yohhOJDxwJbSUNpl5HolPhIXi3434DoSgQijCVkalj2O1tKclrovH0F9zpH4SpEO3Vf72dP8Px6Q/iWpedLu4VOlCUHkzmrcyX62O6rG4VfLMXnlUVqwdHtA9LKwYGGOYDEH0GtQ/SfSxF3s/JXHem6Y4hC9GHLxUwKxz96KaPDM7gk4PMSK86VB4pFQKcHUH8Wto6UMrmAGr85SARIMQ9wb4oBJoCIFG0CZxJ2Krbaq0FcoAY+V4lm/u109n1lVjpGYk4MhOjF3qcO+O84zw6AFwHiVDRLsb1SZ9GIFI7mbE+2rEtXHcySSodFdvUFSOLhk+ZNp4wB3qbk73LN+ooGew8sH2iZl3YPtkru1fwYufwagtChqMXBGXur1OuHyWVEs/wtzoXoAuQoaeJCII6LLGOkm4dAvl1yCWb1GHa0naYwdsQl9beLALZa3+kg45SX3EmqKKmerIxh8GzvxD4gZ2CI5IqMwIXpGQI6i5QFAsZiWLWQkkq+kyhr8gq13GsMQEbeJvULaTJHzskNa4Duml/TB0ekw7wR+B8rO8486n+2hU6p24fTQKjupIZ06b+PmQYHk5gTYfAZVfzGUk0W5Ew7qZu+JMoO5KEAicY0/xGQGQ5zju+q0gtw0U8yZRS7nLmr6L1gvqbd4vwlg0cIP0Ri1I1faiD1VuB9G7kKHsPfkupLMt/WHbg0Ynssna6mFz6Hg3+EUEK5KUU0fqkla3LYZ9xNJ7jeX7ClBer12+Q2o2 7LIUq1SL +Gfy12l8sOK4svj9uBX2UQggt/+7+tPynPCZiS+ZSXdzliwuIC5n5w+qIWYZp9gdHIGyVuYjl8IduGneCpA3TTbXgktIKCN7CXJdF98p6hT/nwiDoAa/MWFTT6dysn+957oAPAoNaAk7Q7vHZZVew+Npu/ptj6WPOAgASG0dpZt59HVYH+yE29HIUER+8T3/SrfsGJID3T6PVBbVmHRw/6E/S8o6VPhDc1NvIdDpVYGOwNQaVKSAxTCpAPurL/NaJOVWOKg2jDFr4DOI+Bp3kGCZwaLa5/POK6nmGBF41gjf7o8chPvTgVXVfenmC5Cz0XnXiDhamVD8+fpnUNe2oWEbBVo4gs6AM735a/+ygWDoOcibGRtjM0uvPmwiE+zPYGV6aUURG5RwQYc06rI1B6QllzSQZVacccBS66UTg51GWkoJCHc8aN8/wJ89m1DvNoeY5jq8qIlGzrc5OtghtvOzx5NW6iBGDe93u4/fFzxREsCtVvEN8JGS3SIYnEUb4mVzFWPFAsAm6FWReEs78QRWnxQ== 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, 5 Oct 2024 at 11:22, Sabyrzhan Tasbolatov wrote: > > syzbot reported that bpf_probe_read_kernel() kernel helper triggered > KASAN report via kasan_check_range() which is not the expected behaviour > as copy_from_kernel_nofault() is meant to be a non-faulting helper. > > Solution is, suggested by Marco Elver, to replace KASAN, KCSAN check in > copy_from_kernel_nofault() with KMSAN detection of copying uninitilaized > kernel memory. In copy_to_kernel_nofault() we can retain > instrument_write() for the memory corruption instrumentation but before > pagefault_disable(). > > Added KMSAN and modified KASAN kunit tests and tested on x86_64. > > This is the part of PATCH series attempting to properly address bugzilla > issue. > > Link: https://lore.kernel.org/linux-mm/CANpmjNMAVFzqnCZhEity9cjiqQ9CVN1X7qeeeAp_6yKjwKo8iw@mail.gmail.com/ > Suggested-by: Marco Elver > Reported-by: syzbot+61123a5daeb9f7454599@syzkaller.appspotmail.com > Closes: https://syzkaller.appspot.com/bug?extid=61123a5daeb9f7454599 > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=210505 > Signed-off-by: Sabyrzhan Tasbolatov I'm getting confused which parts are already picked up by Andrew into -mm, and which aren't. To clarify we have: 1. https://lore.kernel.org/mm-commits/20240927171751.D1BD9C4CEC4@smtp.kernel.org/ 2. https://lore.kernel.org/mm-commits/20240930162435.9B6CBC4CED0@smtp.kernel.org/ And this is the 3rd patch, which applies on top of the other 2. If my understanding is correct, rather than just adding fix on top of fix, in the interest of having one clean patch which can also be backported more easily, would it make sense to drop the first 2 patches from -mm, and you send out one clean patch series? Thanks, -- Marco > --- > mm/kasan/kasan_test_c.c | 8 ++------ > mm/kmsan/kmsan_test.c | 17 +++++++++++++++++ > mm/maccess.c | 5 +++-- > 3 files changed, 22 insertions(+), 8 deletions(-) > > diff --git a/mm/kasan/kasan_test_c.c b/mm/kasan/kasan_test_c.c > index 0a226ab032d..5cff90f831d 100644 > --- a/mm/kasan/kasan_test_c.c > +++ b/mm/kasan/kasan_test_c.c > @@ -1954,7 +1954,7 @@ static void rust_uaf(struct kunit *test) > KUNIT_EXPECT_KASAN_FAIL(test, kasan_test_rust_uaf()); > } > > -static void copy_from_to_kernel_nofault_oob(struct kunit *test) > +static void copy_to_kernel_nofault_oob(struct kunit *test) > { > char *ptr; > char buf[128]; > @@ -1973,10 +1973,6 @@ static void copy_from_to_kernel_nofault_oob(struct kunit *test) > KUNIT_EXPECT_LT(test, (u8)get_tag(ptr), (u8)KASAN_TAG_KERNEL); > } > > - KUNIT_EXPECT_KASAN_FAIL(test, > - copy_from_kernel_nofault(&buf[0], ptr, size)); > - KUNIT_EXPECT_KASAN_FAIL(test, > - copy_from_kernel_nofault(ptr, &buf[0], size)); > KUNIT_EXPECT_KASAN_FAIL(test, > copy_to_kernel_nofault(&buf[0], ptr, size)); > KUNIT_EXPECT_KASAN_FAIL(test, > @@ -2057,7 +2053,7 @@ static struct kunit_case kasan_kunit_test_cases[] = { > KUNIT_CASE(match_all_not_assigned), > KUNIT_CASE(match_all_ptr_tag), > KUNIT_CASE(match_all_mem_tag), > - KUNIT_CASE(copy_from_to_kernel_nofault_oob), > + KUNIT_CASE(copy_to_kernel_nofault_oob), > KUNIT_CASE(rust_uaf), > {} > }; > diff --git a/mm/kmsan/kmsan_test.c b/mm/kmsan/kmsan_test.c > index 13236d579eb..9733a22c46c 100644 > --- a/mm/kmsan/kmsan_test.c > +++ b/mm/kmsan/kmsan_test.c > @@ -640,6 +640,22 @@ static void test_unpoison_memory(struct kunit *test) > KUNIT_EXPECT_TRUE(test, report_matches(&expect)); > } > > +static void test_copy_from_kernel_nofault(struct kunit *test) > +{ > + long ret; > + char buf[4], src[4]; > + size_t size = sizeof(buf); > + > + EXPECTATION_UNINIT_VALUE_FN(expect, "copy_from_kernel_nofault"); > + kunit_info( > + test, > + "testing copy_from_kernel_nofault with uninitialized memory\n"); > + > + ret = copy_from_kernel_nofault((char *)&buf[0], (char *)&src[0], size); > + USE(ret); > + KUNIT_EXPECT_TRUE(test, report_matches(&expect)); > +} > + > static struct kunit_case kmsan_test_cases[] = { > KUNIT_CASE(test_uninit_kmalloc), > KUNIT_CASE(test_init_kmalloc), > @@ -664,6 +680,7 @@ static struct kunit_case kmsan_test_cases[] = { > KUNIT_CASE(test_long_origin_chain), > KUNIT_CASE(test_stackdepot_roundtrip), > KUNIT_CASE(test_unpoison_memory), > + KUNIT_CASE(test_copy_from_kernel_nofault), > {}, > }; > > diff --git a/mm/maccess.c b/mm/maccess.c > index f752f0c0fa3..a91a39a56cf 100644 > --- a/mm/maccess.c > +++ b/mm/maccess.c > @@ -31,8 +31,9 @@ long copy_from_kernel_nofault(void *dst, const void *src, size_t size) > if (!copy_from_kernel_nofault_allowed(src, size)) > return -ERANGE; > > + /* Make sure uninitialized kernel memory isn't copied. */ > + kmsan_check_memory(src, size); > pagefault_disable(); > - instrument_read(src, size); > if (!(align & 7)) > copy_from_kernel_nofault_loop(dst, src, size, u64, Efault); > if (!(align & 3)) > @@ -63,8 +64,8 @@ long copy_to_kernel_nofault(void *dst, const void *src, size_t size) > if (!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)) > align = (unsigned long)dst | (unsigned long)src; > > - pagefault_disable(); > instrument_write(dst, size); > + pagefault_disable(); > if (!(align & 7)) > copy_to_kernel_nofault_loop(dst, src, size, u64, Efault); > if (!(align & 3)) > -- > 2.34.1 >