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 D068BCEE345 for ; Wed, 9 Oct 2024 20:19:01 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5531A6B00A9; Wed, 9 Oct 2024 16:19:01 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 503466B00AA; Wed, 9 Oct 2024 16:19:01 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3A3816B00AB; Wed, 9 Oct 2024 16:19:01 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 1BB6D6B00A9 for ; Wed, 9 Oct 2024 16:19:01 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id B57F7419FC for ; Wed, 9 Oct 2024 20:18:58 +0000 (UTC) X-FDA: 82655177640.28.4779220 Received: from mail-wm1-f42.google.com (mail-wm1-f42.google.com [209.85.128.42]) by imf28.hostedemail.com (Postfix) with ESMTP id E54ECC001A for ; Wed, 9 Oct 2024 20:18:58 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=L+DUHW23; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf28.hostedemail.com: domain of andreyknvl@gmail.com designates 209.85.128.42 as permitted sender) smtp.mailfrom=andreyknvl@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1728505095; a=rsa-sha256; cv=none; b=ewxT1Q3TBuyzs42r5rnxuVdqnsnqKrURHUTiGaf/PtbY27IwyFKV0EI1Oxj6+Bn7CeZsn0 uZdl0oORB/5PSEqE9V5tqXBJvZJCTsjBx+2qp95RjLpn67iYvj5WmROq3BzCsDu1l0fipJ lxDZraOeo6o3sYBIk/TK/BhvPIsJCrQ= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=L+DUHW23; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf28.hostedemail.com: domain of andreyknvl@gmail.com designates 209.85.128.42 as permitted sender) smtp.mailfrom=andreyknvl@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1728505095; 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=7c35+JpWEpsPpCJZNGS7NrTRM+BKUrPMr11LkkZQ0go=; b=7EG0REM54WgX7z0YjN95hAX/XnDst24OWg5ZvmoxZvDk+CuilQM9HEpd4lcmJJbyKokUW5 +AzbY7uQFhOIQRbOuWNVbx6UPcJT14AChZQjUpcfZE8g8IFKgqvxS1bIp1Eanc+1e7D7n6 IvioTLonTITn8WUr+up1EdSb96beS3k= Received: by mail-wm1-f42.google.com with SMTP id 5b1f17b1804b1-42cc43454d5so1095215e9.3 for ; Wed, 09 Oct 2024 13:18:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1728505137; x=1729109937; 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=7c35+JpWEpsPpCJZNGS7NrTRM+BKUrPMr11LkkZQ0go=; b=L+DUHW23KXGfiisQ4kIxzBcdy+rOQZVJWx+G57paYqyBCtGdOHrTLlXe8tjSk32b4y SkLnVpivotuwXcQ9Ne5yn7hNZHm4u+GY52x4xI1yottwVRyS2jfCLNKjngIdcCibokGd Kty4q7YugxOGtP1f46u9YXRaeuO9Oe7GtQ/6MW7ybGP/pJ/xKxnyO5xpq+7dxcUiRLeI Ysm3TPMiQmil4msDMwcWN5yYzAOEDUMZ0XwNY8a32c6y3TEDeWnysmqYivw3Lo8Iv7DK pP3wcYB2Sd50ucmc5e1UxG2XAGBE2QXPx1dsFPvKFDrwHydeUK6NbeS8zF44yc6/rp1M k5fg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1728505137; x=1729109937; 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=7c35+JpWEpsPpCJZNGS7NrTRM+BKUrPMr11LkkZQ0go=; b=RJ2rHaMqcFPAB9wytsr2XBVmH1iNywdww0aSnh6tSS5A9u/QlILBRpTN/221wQjq7i JqPYn+7/7gFRmIb5Kin29e5/H8ncO1oIKUoiIHgiuPlpWFaDjmpyNaARlkLzDAjmRCEq lgFTYiUjOdP6/fr9sc5LPFRRQdbqeDCerHl8C3cRup7KOcVa34XQ5TRc/Jtd9dd4Cx0o 0frLdy0B9foPjUl6gjvqsjy919XKXfmhSo8qzQ2s6sQBtOBm/Vl8b0MWEDFoNh4XZ8++ Tz106SIlyT++QivQbNoYxEYqb1Y3FkLCpX+aQth/Fl41mHoxhE8k52yXTyoCTsQoHePx w+wA== X-Forwarded-Encrypted: i=1; AJvYcCUPQsszEJpPL0/h+kf9qRbIuRABXekKbNIbXkbEkOqFEUmehfNEjVfsUDrhdlP1Z7bhIJZu0rh/Ig==@kvack.org X-Gm-Message-State: AOJu0YwF/mNcBpvxY5ViGsubOYVYxbe4K1vh8eB5rEs3nGqhefscOINH bo2Ag4UQ6OqVtmmJsLBxB7GGT5kwy2dKX0lim0emrGqTFUA417xeUIqBFKuwWpDrINdv8XyUaPg N7arw71XggOOM+hB7CmhO9uvBTkg= X-Google-Smtp-Source: AGHT+IEQx0AGyWtXNRqY8P/dVmpR3p8HBSZumK19Zat2IZqc0Ucy0FVIpxOiDWV8yz0RdA+qpWPoNIIl9Gw1vdotbQk= X-Received: by 2002:adf:dd8d:0:b0:37d:31a7:2814 with SMTP id ffacd0b85a97d-37d3aa579famr2243483f8f.29.1728505137061; Wed, 09 Oct 2024 13:18:57 -0700 (PDT) MIME-Version: 1.0 References: <20241008192910.2823726-1-snovitoll@gmail.com> In-Reply-To: <20241008192910.2823726-1-snovitoll@gmail.com> From: Andrey Konovalov Date: Wed, 9 Oct 2024 22:18:46 +0200 Message-ID: Subject: Re: [PATCH v4] mm, kasan, kmsan: copy_from/to_kernel_nofault To: Sabyrzhan Tasbolatov Cc: elver@google.com, akpm@linux-foundation.org, bpf@vger.kernel.org, dvyukov@google.com, glider@google.com, kasan-dev@googlegroups.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, ryabinin.a.a@gmail.com, syzbot+61123a5daeb9f7454599@syzkaller.appspotmail.com, vincenzo.frascino@arm.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Queue-Id: E54ECC001A X-Rspamd-Server: rspam01 X-Stat-Signature: 31dzrp85jjphnh69dqjyb49r9gf7tr5h X-HE-Tag: 1728505138-493222 X-HE-Meta: U2FsdGVkX1+85KyttOcHvC2UKrM7N5slI1EZS/l+6YgdJ5V0azMj50i9U17rJHxlN7uw0uVsY1hfJIB2FPFWmifnDUMjNnAUwNP6qI2BccU962OiONgboMme0FytQh7DYX/I6RK3sArONAoE74AnMiQtBjFCLeI8BQKyg+7clfcX6NyvqwrHh7Dc2uQ5FsWWZBxF8m7Dhgmlq0mYpVw6cbI2Wt/GtMlQIFKufY5M7QfKa9XshTmO6cKrc/t5HPq4D0RuqZoFbXxR/sIoCbVXTGq2B5w+wxKDaoxcd9yn+gBv9XVOhGNdUFErGrXsqtvP83gxzhr/+w9MkilVzVnwWhs/RIsUsSWKtY7U0x1+MAGc09nLtbTms++AeQ1zZl0uNmBJ964p8JeNTSdTuim4ThvAHZv2Qh/F5dJf+nFAlCBETTsiqUxj2xDE1KnMHjYlZBVjoKJBdbyHxPUXs3dUDPrJl9DHS0OVMDG9KwDUCheHO4EEKHqvNqq9Td6BG9Je3t8QbyAksFACH7KG23a/4PyG/Sq7Ph/xTTbwy65T6xZlNC3rug5s1OBb1kgLIFNr30OoAT2R078MdrFia5eYny+8roQfwTkaIsUQF3AsCIdbxYucE/QjYvuS/aWHk29wAe0hSBAivTChLl9Te0mmfyqHXTcmv/vCgvo2/qerVtQs9Va2CJuk+NhA848KaBmICl+T6+2pYsK6EfYrGLWiNTx2CyN/yRbClBw+VUlPpsxUnGyOu3bU3UkCAR90NP915VRpfCdMG4f18tsK/p6bYi1BrKmCs2y+N2T5E3CvLU/20JByqARlavsKY/HBr1bhDKLPOvn0ClQCS3z0vqQtyM3SWJzKdpUMvzP0FInLv2J59HJU5AfxgBl3F4LIt+Uym4ssYCI39LxGgYjqNdSfPGwK0ZEhtnQ4fUVvqgOxQQCETBFH7wtx78WcOg1p8v2oM40ogAJx0yR4RUVAaAk rvdebyPJ hVWpCWuEiuA7bUylmozua5hnd28jvkNCnqla/1OCi4Y2zQ2lxU09MNtYcUbejYSOtXEWYsfqBwwzQC5o7s3e+h1aBqO9r963XAPiu+Rm6XeLQ4tgJ6nb2t71QFQAJ83uEEi6/ElQUUBN+YJEtERZM1g0AoLix0WzmsdmfE5b5DK5SVuSp/2hQygNwVNIZtATK0xCGV+H1XhJ9kgy6wD+euorHTwctjXR+yfUpFOUQ3az8qMiEeDlNMf3noHW9I881Zryx64UuI5lfWo9HK38zVwWwr6CE+xEdDuPaAawKqZEMxPAEeY1M7fE19OEUxsUmKVWaf1dgOXr/GW84xPQOyVZm7UdN0a5lSaDnYywAMzP9u8w24NtmRxbLvpedwVCf4hCdVCTKjubqnGNp5VCeqa7BGy8I9QCK+SgCjKzaviVdNAQ6TKaj5aIGrEfWT5FwSCPakHm7PYqZ8cMUHHOJb0tRcW2cbLictK4E3RUIRL0CJBeGCOwLgLhjAWYn8xiLNafYWfMwBjuNBppx7CSaahUICsylVgiyWCJzdSijacFkXTG/dSL0tUhcyOSuWmkVLZrv40afvv/590BqEo7rpG4r/JH2ZXZ0/PkMnAvEKRDzB2XnDcyZknT6+A== 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 Tue, Oct 8, 2024 at 9:28=E2=80=AFPM Sabyrzhan Tasbolatov wrote: > > Instrument copy_from_kernel_nofault() with KMSAN for uninitialized kernel > memory check and copy_to_kernel_nofault() with KASAN, KCSAN to detect > the memory corruption. > > 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() explicitly for the memory corruption instrumentation. > > copy_to_kernel_nofault() is tested on x86_64 and arm64 with > CONFIG_KASAN_SW_TAGS. On arm64 with CONFIG_KASAN_HW_TAGS, > kunit test currently fails. Need more clarification on it > - currently, disabled in kunit test. > > Link: https://lore.kernel.org/linux-mm/CANpmjNMAVFzqnCZhEity9cjiqQ9CVN1X7= qeeeAp_6yKjwKo8iw@mail.gmail.com/ > Reviewed-by: Marco Elver > Reported-by: syzbot+61123a5daeb9f7454599@syzkaller.appspotmail.com > Closes: https://syzkaller.appspot.com/bug?extid=3D61123a5daeb9f7454599 > Reported-by: Andrey Konovalov > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=3D210505 > Signed-off-by: Sabyrzhan Tasbolatov (Back from travels, looking at the patches again.) > --- > v2: > - squashed previous submitted in -mm tree 2 patches based on Linus tree > v3: > - moved checks to *_nofault_loop macros per Marco's comments > - edited the commit message > v4: > - replaced Suggested-By with Reviewed-By: Marco Elver > --- > mm/kasan/kasan_test_c.c | 27 +++++++++++++++++++++++++++ > mm/kmsan/kmsan_test.c | 17 +++++++++++++++++ > mm/maccess.c | 10 ++++++++-- > 3 files changed, 52 insertions(+), 2 deletions(-) > > diff --git a/mm/kasan/kasan_test_c.c b/mm/kasan/kasan_test_c.c > index a181e4780d9d..5cff90f831db 100644 > --- a/mm/kasan/kasan_test_c.c > +++ b/mm/kasan/kasan_test_c.c > @@ -1954,6 +1954,32 @@ static void rust_uaf(struct kunit *test) > KUNIT_EXPECT_KASAN_FAIL(test, kasan_test_rust_uaf()); > } > > +static void copy_to_kernel_nofault_oob(struct kunit *test) > +{ > + char *ptr; > + char buf[128]; > + size_t size =3D sizeof(buf); > + > + /* Not detecting fails currently with HW_TAGS */ Let's reword this to: This test currently fails with the HW_TAGS mode. The reason is unknown and needs to be investigated. > + KASAN_TEST_NEEDS_CONFIG_OFF(test, CONFIG_KASAN_HW_TAGS); > + > + ptr =3D kmalloc(size - KASAN_GRANULE_SIZE, GFP_KERNEL); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr); > + OPTIMIZER_HIDE_VAR(ptr); > + > + if (IS_ENABLED(CONFIG_KASAN_SW_TAGS)) { > + /* Check that the returned pointer is tagged. */ > + KUNIT_EXPECT_GE(test, (u8)get_tag(ptr), (u8)KASAN_TAG_MIN= ); > + KUNIT_EXPECT_LT(test, (u8)get_tag(ptr), (u8)KASAN_TAG_KER= NEL); > + } Let's drop the checks above: if pointers returned by kmalloc are not tagged, the checks below (and many other tests) will fail. > + Please add a comment here explaining why we only check copy_to_kernel_nofault and not copy_from_kernel_nofault (is this because we cannot add KASAN instrumentation to copy_from_kernel_nofault?). > + KUNIT_EXPECT_KASAN_FAIL(test, > + copy_to_kernel_nofault(&buf[0], ptr, size)); > + KUNIT_EXPECT_KASAN_FAIL(test, > + copy_to_kernel_nofault(ptr, &buf[0], size)); > + kfree(ptr); > +} > + > static struct kunit_case kasan_kunit_test_cases[] =3D { > KUNIT_CASE(kmalloc_oob_right), > KUNIT_CASE(kmalloc_oob_left), > @@ -2027,6 +2053,7 @@ static struct kunit_case kasan_kunit_test_cases[] = =3D { > KUNIT_CASE(match_all_not_assigned), > KUNIT_CASE(match_all_ptr_tag), > KUNIT_CASE(match_all_mem_tag), > + 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 13236d579eba..9733a22c46c1 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 =3D sizeof(buf); > + > + EXPECTATION_UNINIT_VALUE_FN(expect, "copy_from_kernel_nofault"); > + kunit_info( > + test, > + "testing copy_from_kernel_nofault with uninitialized memo= ry\n"); > + > + ret =3D 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[] =3D { > KUNIT_CASE(test_uninit_kmalloc), > KUNIT_CASE(test_init_kmalloc), > @@ -664,6 +680,7 @@ static struct kunit_case kmsan_test_cases[] =3D { > 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 518a25667323..3ca55ec63a6a 100644 > --- a/mm/maccess.c > +++ b/mm/maccess.c > @@ -13,9 +13,14 @@ bool __weak copy_from_kernel_nofault_allowed(const voi= d *unsafe_src, > return true; > } > > +/* > + * The below only uses kmsan_check_memory() to ensure uninitialized kern= el > + * memory isn't leaked. > + */ > #define copy_from_kernel_nofault_loop(dst, src, len, type, err_label) \ > while (len >=3D sizeof(type)) { = \ > - __get_kernel_nofault(dst, src, type, err_label); = \ > + __get_kernel_nofault(dst, src, type, err_label); \ > + kmsan_check_memory(src, sizeof(type)); \ > dst +=3D sizeof(type); = \ > src +=3D sizeof(type); = \ > len -=3D sizeof(type); = \ > @@ -49,7 +54,8 @@ EXPORT_SYMBOL_GPL(copy_from_kernel_nofault); > > #define copy_to_kernel_nofault_loop(dst, src, len, type, err_label) \ > while (len >=3D sizeof(type)) { = \ > - __put_kernel_nofault(dst, src, type, err_label); = \ > + __put_kernel_nofault(dst, src, type, err_label); \ > + instrument_write(dst, sizeof(type)); \ > dst +=3D sizeof(type); = \ > src +=3D sizeof(type); = \ > len -=3D sizeof(type); = \ > -- > 2.34.1 >