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 X-Spam-Level: X-Spam-Status: No, score=-17.4 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BE0ABC433E7 for ; Fri, 16 Oct 2020 20:05:37 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id F257C20874 for ; Fri, 16 Oct 2020 20:05:36 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="t1Dv5NTl" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org F257C20874 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id F28FD6B005C; Fri, 16 Oct 2020 16:05:35 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id ED7ED6B005D; Fri, 16 Oct 2020 16:05:35 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DEF3E900002; Fri, 16 Oct 2020 16:05:35 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0141.hostedemail.com [216.40.44.141]) by kanga.kvack.org (Postfix) with ESMTP id 562176B005C for ; Fri, 16 Oct 2020 16:05:35 -0400 (EDT) Received: from smtpin29.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 573338249980 for ; Fri, 16 Oct 2020 20:05:34 +0000 (UTC) X-FDA: 77378868588.29.crook55_5f169402721f Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin29.hostedemail.com (Postfix) with ESMTP id 8A17B18086CC7 for ; Fri, 16 Oct 2020 20:05:33 +0000 (UTC) X-HE-Tag: crook55_5f169402721f X-Filterd-Recvd-Size: 13622 Received: from mail-pf1-f195.google.com (mail-pf1-f195.google.com [209.85.210.195]) by imf19.hostedemail.com (Postfix) with ESMTP for ; Fri, 16 Oct 2020 20:05:32 +0000 (UTC) Received: by mail-pf1-f195.google.com with SMTP id 144so2113660pfb.4 for ; Fri, 16 Oct 2020 13:05:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=JlmEeUtoxILUOsMKYCpMQdn4yPK9pTLKCLX4Si8MjAU=; b=t1Dv5NTlDwXbYXAQYRzsrIDZdbB/MF9AsR8YdHhz/45R310IsVYdrtBLl8T8cwKYBj 3xpXcEeHrSZGYKKYwpFec9j+2FWElN2nVfN3PDcOU2eXRYkgb8pWaAT/OoS/PIrmBz4h H1A8YLAK2rrOBydgtSbad0UHF3FNaPaKP9/oopZxRU8nr1FBEcPLUDANhbjKU5JBpRo6 WtP7MI73Je4UubwwpBP4i2yC1/Q0qaU9MW60IL+1SAHhXqnE3yDAvmJaAIz4fk7UmglV Qety6GIbPtUqu0oBJKYfRkw7XGUbILbAAoLFF3t5Ha2mj2lIa6mpBRsb2ihzULbWsfuf 3UMQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=JlmEeUtoxILUOsMKYCpMQdn4yPK9pTLKCLX4Si8MjAU=; b=WNL+vxQCw3FnISsQJZAalS+jS0kzRYyl6rNGmV9mR+4uGE923opQFtNNgKpY/xghbp OWgXmaYu6RBFAmUncuSanobAvm3yfXP2gzeqgtj+hUAs8WRpCawPzlAnS3pRCH3mhb5B u3/3/5hUjKc3GbDBJAwTdtqqFvgGnpVeYfTZYZx8JkPXD/T7Eb3bsDm8/weG3OktL58k CPNmZYvH4JMpCJIGo8THjuPGqwvzMsAihROXT0MCpSgaqzriBM6b2aiLxDtPtZhMR9n0 VDtWhowqa4Ag4EAcA8XL5c9/5+J/wTZX7bxFX4l7EyYCdep/UmMdb5qkQP1JUKno+YZC /DEQ== X-Gm-Message-State: AOAM5313t5OFaFTF12fWWeY+coOURAbDT/cGpzpRo3+ZW9KrjepmGuJf QT5IpeoJrTf3FIroKgkSmFHKNFEw6rWEOSrPVIGXlg== X-Google-Smtp-Source: ABdhPJyY0iL3mCeN7sysfzxRlTqNR6HarRP8wyQNgU5xI+aCw8nzRm4aJgTclvHo+lg1XAtCCJJ/3+T3KQbJJNKH+sw= X-Received: by 2002:a63:8c42:: with SMTP id q2mr4523721pgn.130.1602878731581; Fri, 16 Oct 2020 13:05:31 -0700 (PDT) MIME-Version: 1.0 References: <44861eaca17ffbb51726473bc8e86ad9e130c67e.1602876780.git.andreyknvl@google.com> In-Reply-To: <44861eaca17ffbb51726473bc8e86ad9e130c67e.1602876780.git.andreyknvl@google.com> From: Andrey Konovalov Date: Fri, 16 Oct 2020 22:05:20 +0200 Message-ID: Subject: Re: [PATCH] kasan: adopt KUNIT tests to SW_TAGS mode To: Andrew Morton Cc: Andrey Ryabinin , Alexander Potapenko , Dmitry Vyukov , Marco Elver , Vincenzo Frascino , David Gow , kasan-dev , Linux Memory Management List , LKML Content-Type: text/plain; charset="UTF-8" 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: On Fri, Oct 16, 2020 at 9:33 PM Andrey Konovalov wrote: > > Now that we have KASAN-KUNIT tests integration, it's easy to see that > some KASAN tests are not adopted to the SW_TAGS mode and are failing. > > Adjust the allocation size for kasan_memchr() and kasan_memcmp() by > roung it up to OOB_TAG_OFF so the bad access ends up in a separate > memory granule. > > Add new kmalloc_uaf_16() and kasan_bitops_uaf() tests that rely on UAFs, > as it's hard to adopt the existing kmalloc_oob_16() and kasan_bitops_oob() > (rename from kasan_bitops()) without losing the precision. > > Disable kasan_global_oob() and kasan_alloca_oob_left/right() as SW_TAGS > mode doesn't instrument globals nor dynamic allocas. > > Signed-off-by: Andrey Konovalov > --- > lib/test_kasan.c | 144 ++++++++++++++++++++++++++++++++--------------- > 1 file changed, 99 insertions(+), 45 deletions(-) > > diff --git a/lib/test_kasan.c b/lib/test_kasan.c > index 63c26171a791..3bff25a7fdcc 100644 > --- a/lib/test_kasan.c > +++ b/lib/test_kasan.c > @@ -216,6 +216,12 @@ static void kmalloc_oob_16(struct kunit *test) > u64 words[2]; > } *ptr1, *ptr2; > > + /* This test is specifically crafted for the generic mode. */ > + if (!IS_ENABLED(CONFIG_KASAN_GENERIC)) { > + kunit_info(test, "CONFIG_KASAN_GENERIC required\n"); > + return; > + } > + > ptr1 = kmalloc(sizeof(*ptr1) - 3, GFP_KERNEL); > KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr1); > > @@ -227,6 +233,23 @@ static void kmalloc_oob_16(struct kunit *test) > kfree(ptr2); > } > > +static void kmalloc_uaf_16(struct kunit *test) > +{ > + struct { > + u64 words[2]; > + } *ptr1, *ptr2; > + > + ptr1 = kmalloc(sizeof(*ptr1), GFP_KERNEL); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr1); > + > + ptr2 = kmalloc(sizeof(*ptr2), GFP_KERNEL); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr2); > + kfree(ptr2); > + > + KUNIT_EXPECT_KASAN_FAIL(test, *ptr1 = *ptr2); > + kfree(ptr1); > +} > + > static void kmalloc_oob_memset_2(struct kunit *test) > { > char *ptr; > @@ -429,6 +452,12 @@ static void kasan_global_oob(struct kunit *test) > volatile int i = 3; > char *p = &global_array[ARRAY_SIZE(global_array) + i]; > > + /* Only generic mode instruments globals. */ > + if (!IS_ENABLED(CONFIG_KASAN_GENERIC)) { > + kunit_info(test, "CONFIG_KASAN_GENERIC required"); > + return; > + } > + > KUNIT_EXPECT_KASAN_FAIL(test, *(volatile char *)p); > } > > @@ -467,6 +496,12 @@ static void kasan_alloca_oob_left(struct kunit *test) > char alloca_array[i]; > char *p = alloca_array - 1; > > + /* Only generic mode instruments dynamic allocas. */ > + if (!IS_ENABLED(CONFIG_KASAN_GENERIC)) { > + kunit_info(test, "CONFIG_KASAN_GENERIC required"); > + return; > + } > + > if (!IS_ENABLED(CONFIG_KASAN_STACK)) { > kunit_info(test, "CONFIG_KASAN_STACK is not enabled"); > return; > @@ -481,6 +516,12 @@ static void kasan_alloca_oob_right(struct kunit *test) > char alloca_array[i]; > char *p = alloca_array + i; > > + /* Only generic mode instruments dynamic allocas. */ > + if (!IS_ENABLED(CONFIG_KASAN_GENERIC)) { > + kunit_info(test, "CONFIG_KASAN_GENERIC required"); > + return; > + } > + > if (!IS_ENABLED(CONFIG_KASAN_STACK)) { > kunit_info(test, "CONFIG_KASAN_STACK is not enabled"); > return; > @@ -551,6 +592,9 @@ static void kasan_memchr(struct kunit *test) > return; > } > > + if (OOB_TAG_OFF) > + size = round_up(size, OOB_TAG_OFF); > + > ptr = kmalloc(size, GFP_KERNEL | __GFP_ZERO); > KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr); > > @@ -573,6 +617,9 @@ static void kasan_memcmp(struct kunit *test) > return; > } > > + if (OOB_TAG_OFF) > + size = round_up(size, OOB_TAG_OFF); > + > ptr = kmalloc(size, GFP_KERNEL | __GFP_ZERO); > KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr); > memset(arr, 0, sizeof(arr)); > @@ -619,13 +666,50 @@ static void kasan_strings(struct kunit *test) > KUNIT_EXPECT_KASAN_FAIL(test, kasan_int_result = strnlen(ptr, 1)); > } > > -static void kasan_bitops(struct kunit *test) > +static void kasan_bitops_modify(struct kunit *test, int nr, void *addr) > +{ > + KUNIT_EXPECT_KASAN_FAIL(test, set_bit(nr, addr)); > + KUNIT_EXPECT_KASAN_FAIL(test, __set_bit(nr, addr)); > + KUNIT_EXPECT_KASAN_FAIL(test, clear_bit(nr, addr)); > + KUNIT_EXPECT_KASAN_FAIL(test, __clear_bit(nr, addr)); > + KUNIT_EXPECT_KASAN_FAIL(test, clear_bit_unlock(nr, addr)); > + KUNIT_EXPECT_KASAN_FAIL(test, __clear_bit_unlock(nr, addr)); > + KUNIT_EXPECT_KASAN_FAIL(test, change_bit(nr, addr)); > + KUNIT_EXPECT_KASAN_FAIL(test, __change_bit(nr, addr)); > +} > + > +static void kasan_bitops_test_and_modify(struct kunit *test, int nr, void *addr) > { > + KUNIT_EXPECT_KASAN_FAIL(test, test_and_set_bit(nr, addr)); > + KUNIT_EXPECT_KASAN_FAIL(test, __test_and_set_bit(nr, addr)); > + KUNIT_EXPECT_KASAN_FAIL(test, test_and_set_bit_lock(nr, addr)); > + KUNIT_EXPECT_KASAN_FAIL(test, test_and_clear_bit(nr, addr)); > + KUNIT_EXPECT_KASAN_FAIL(test, __test_and_clear_bit(nr, addr)); > + KUNIT_EXPECT_KASAN_FAIL(test, test_and_change_bit(nr, addr)); > + KUNIT_EXPECT_KASAN_FAIL(test, __test_and_change_bit(nr, addr)); > + KUNIT_EXPECT_KASAN_FAIL(test, kasan_int_result = test_bit(nr, addr)); > + > +#if defined(clear_bit_unlock_is_negative_byte) > + KUNIT_EXPECT_KASAN_FAIL(test, kasan_int_result = > + clear_bit_unlock_is_negative_byte(nr, addr)); > +#endif > +} > + > +static void kasan_bitops_oob(struct kunit *test) > +{ > + long *bits; > + > + /* This test is specifically crafted for the generic mode. */ > + if (!IS_ENABLED(CONFIG_KASAN_GENERIC)) { > + kunit_info(test, "CONFIG_KASAN_GENERIC required\n"); > + return; > + } > + > /* > * Allocate 1 more byte, which causes kzalloc to round up to 16-bytes; > * this way we do not actually corrupt other memory. > */ > - long *bits = kzalloc(sizeof(*bits) + 1, GFP_KERNEL); > + bits = kzalloc(sizeof(*bits) + 1, GFP_KERNEL); > KUNIT_ASSERT_NOT_ERR_OR_NULL(test, bits); > > /* > @@ -633,56 +717,24 @@ static void kasan_bitops(struct kunit *test) > * below accesses are still out-of-bounds, since bitops are defined to > * operate on the whole long the bit is in. > */ > - KUNIT_EXPECT_KASAN_FAIL(test, set_bit(BITS_PER_LONG, bits)); > - > - KUNIT_EXPECT_KASAN_FAIL(test, __set_bit(BITS_PER_LONG, bits)); > - > - KUNIT_EXPECT_KASAN_FAIL(test, clear_bit(BITS_PER_LONG, bits)); > - > - KUNIT_EXPECT_KASAN_FAIL(test, __clear_bit(BITS_PER_LONG, bits)); > - > - KUNIT_EXPECT_KASAN_FAIL(test, clear_bit_unlock(BITS_PER_LONG, bits)); > - > - KUNIT_EXPECT_KASAN_FAIL(test, __clear_bit_unlock(BITS_PER_LONG, bits)); > - > - KUNIT_EXPECT_KASAN_FAIL(test, change_bit(BITS_PER_LONG, bits)); > - > - KUNIT_EXPECT_KASAN_FAIL(test, __change_bit(BITS_PER_LONG, bits)); > + kasan_bitops_modify(test, BITS_PER_LONG, bits); > > /* > * Below calls try to access bit beyond allocated memory. > */ > - KUNIT_EXPECT_KASAN_FAIL(test, > - test_and_set_bit(BITS_PER_LONG + BITS_PER_BYTE, bits)); > - > - KUNIT_EXPECT_KASAN_FAIL(test, > - __test_and_set_bit(BITS_PER_LONG + BITS_PER_BYTE, bits)); > + kasan_bitops_test_and_modify(test, BITS_PER_LONG + BITS_PER_BYTE, bits); > > - KUNIT_EXPECT_KASAN_FAIL(test, > - test_and_set_bit_lock(BITS_PER_LONG + BITS_PER_BYTE, bits)); > - > - KUNIT_EXPECT_KASAN_FAIL(test, > - test_and_clear_bit(BITS_PER_LONG + BITS_PER_BYTE, bits)); > - > - KUNIT_EXPECT_KASAN_FAIL(test, > - __test_and_clear_bit(BITS_PER_LONG + BITS_PER_BYTE, bits)); > - > - KUNIT_EXPECT_KASAN_FAIL(test, > - test_and_change_bit(BITS_PER_LONG + BITS_PER_BYTE, bits)); > - > - KUNIT_EXPECT_KASAN_FAIL(test, > - __test_and_change_bit(BITS_PER_LONG + BITS_PER_BYTE, bits)); > + kfree(bits); > +} > > - KUNIT_EXPECT_KASAN_FAIL(test, > - kasan_int_result = > - test_bit(BITS_PER_LONG + BITS_PER_BYTE, bits)); > +static void kasan_bitops_uaf(struct kunit *test) > +{ > + long *bits = kzalloc(sizeof(*bits), GFP_KERNEL); > > -#if defined(clear_bit_unlock_is_negative_byte) > - KUNIT_EXPECT_KASAN_FAIL(test, > - kasan_int_result = clear_bit_unlock_is_negative_byte( > - BITS_PER_LONG + BITS_PER_BYTE, bits)); > -#endif > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, bits); > kfree(bits); > + kasan_bitops_modify(test, BITS_PER_LONG, bits); > + kasan_bitops_test_and_modify(test, BITS_PER_LONG + BITS_PER_BYTE, bits); > } This actually ends up modifying the data in a freed object, which isn't a good idea. I'll change this to do an OOB too, but for the tag-based mode. > > static void kmalloc_double_kzfree(struct kunit *test) > @@ -728,6 +780,7 @@ static struct kunit_case kasan_kunit_test_cases[] = { > KUNIT_CASE(kmalloc_oob_krealloc_more), > KUNIT_CASE(kmalloc_oob_krealloc_less), > KUNIT_CASE(kmalloc_oob_16), > + KUNIT_CASE(kmalloc_uaf_16), > KUNIT_CASE(kmalloc_oob_in_memset), > KUNIT_CASE(kmalloc_oob_memset_2), > KUNIT_CASE(kmalloc_oob_memset_4), > @@ -751,7 +804,8 @@ static struct kunit_case kasan_kunit_test_cases[] = { > KUNIT_CASE(kasan_memchr), > KUNIT_CASE(kasan_memcmp), > KUNIT_CASE(kasan_strings), > - KUNIT_CASE(kasan_bitops), > + KUNIT_CASE(kasan_bitops_oob), > + KUNIT_CASE(kasan_bitops_uaf), > KUNIT_CASE(kmalloc_double_kzfree), > KUNIT_CASE(vmalloc_oob), > {} > -- > 2.29.0.rc1.297.gfa9743e501-goog >