From patchwork Fri Jun 19 14:32:52 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Greg Kroah-Hartman X-Patchwork-Id: 223995 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=-9.8 required=3.0 tests=DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH, MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_GIT 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 49BA2C433E0 for ; Fri, 19 Jun 2020 15:45:21 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 28F9020707 for ; Fri, 19 Jun 2020 15:45:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1592581521; bh=zS6gAEk0HKpJiDZ/kTr717G1d4Hq+xCpiqZybYupiOA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=iFGcFHOBvqpmX5SYpV2kroM+y5Bf4lax5p2i25S1+MNB2YTIGutqFpdWaAB8eT1yd TyfxFmDe89103L+I8n2sFeQORlUlGmqsnaH2osjSU28hY2huNUuKEvFV4B5N5E9TBk 0KBqHhRfaI1P3Uq7FCnijAFkkyb7DFsIyYN5md38= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2391223AbgFSPpJ (ORCPT ); Fri, 19 Jun 2020 11:45:09 -0400 Received: from mail.kernel.org ([198.145.29.99]:59250 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2391072AbgFSP1b (ORCPT ); Fri, 19 Jun 2020 11:27:31 -0400 Received: from localhost (83-86-89-107.cable.dynamic.v4.ziggo.nl [83.86.89.107]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 95B0421919; Fri, 19 Jun 2020 15:27:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1592580450; bh=zS6gAEk0HKpJiDZ/kTr717G1d4Hq+xCpiqZybYupiOA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ZHBUKdMzw8/NXXtnl028yYtoKcdN/tS/mCsol5V/Z2dRyCaCAzvsSBmD1BN1wRjl3 nBsUZkas7i2Jos02CzUdVF5TZgS8hiz0rpib5d2CnkjhfCsPTErtY8etpNbnIAZDXU Y8SLIkzA3sLNjQfG51hhoaQjb16aB6cwmQY3WfgQ= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Daniel Axtens , Andrew Morton , David Gow , Dmitry Vyukov , Daniel Micay , Andrey Ryabinin , Alexander Potapenko , Linus Torvalds , Sasha Levin Subject: [PATCH 5.7 254/376] kasan: stop tests being eliminated as dead code with FORTIFY_SOURCE Date: Fri, 19 Jun 2020 16:32:52 +0200 Message-Id: <20200619141722.348253156@linuxfoundation.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200619141710.350494719@linuxfoundation.org> References: <20200619141710.350494719@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Sender: stable-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org From: Daniel Axtens [ Upstream commit adb72ae1915db28f934e9e02c18bfcea2f3ed3b7 ] Patch series "Fix some incompatibilites between KASAN and FORTIFY_SOURCE", v4. 3 KASAN self-tests fail on a kernel with both KASAN and FORTIFY_SOURCE: memchr, memcmp and strlen. When FORTIFY_SOURCE is on, a number of functions are replaced with fortified versions, which attempt to check the sizes of the operands. However, these functions often directly invoke __builtin_foo() once they have performed the fortify check. The compiler can detect that the results of these functions are not used, and knows that they have no other side effects, and so can eliminate them as dead code. Why are only memchr, memcmp and strlen affected? ================================================ Of string and string-like functions, kasan_test tests: * strchr -> not affected, no fortified version * strrchr -> likewise * strcmp -> likewise * strncmp -> likewise * strnlen -> not affected, the fortify source implementation calls the underlying strnlen implementation which is instrumented, not a builtin * strlen -> affected, the fortify souce implementation calls a __builtin version which the compiler can determine is dead. * memchr -> likewise * memcmp -> likewise * memset -> not affected, the compiler knows that memset writes to its first argument and therefore is not dead. Why does this not affect the functions normally? ================================================ In string.h, these functions are not marked as __pure, so the compiler cannot know that they do not have side effects. If relevant functions are marked as __pure in string.h, we see the following warnings and the functions are elided: lib/test_kasan.c: In function `kasan_memchr': lib/test_kasan.c:606:2: warning: statement with no effect [-Wunused-value] memchr(ptr, '1', size + 1); ^~~~~~~~~~~~~~~~~~~~~~~~~~ lib/test_kasan.c: In function `kasan_memcmp': lib/test_kasan.c:622:2: warning: statement with no effect [-Wunused-value] memcmp(ptr, arr, size+1); ^~~~~~~~~~~~~~~~~~~~~~~~ lib/test_kasan.c: In function `kasan_strings': lib/test_kasan.c:645:2: warning: statement with no effect [-Wunused-value] strchr(ptr, '1'); ^~~~~~~~~~~~~~~~ ... This annotation would make sense to add and could be added at any point, so the behaviour of test_kasan.c should change. The fix ======= Make all the functions that are pure write their results to a global, which makes them live. The strlen and memchr tests now pass. The memcmp test still fails to trigger, which is addressed in the next patch. [dja@axtens.net: drop patch 3] Link: http://lkml.kernel.org/r/20200424145521.8203-2-dja@axtens.net Fixes: 0c96350a2d2f ("lib/test_kasan.c: add tests for several string/memory API functions") Signed-off-by: Daniel Axtens Signed-off-by: Andrew Morton Tested-by: David Gow Reviewed-by: Dmitry Vyukov Cc: Daniel Micay Cc: Andrey Ryabinin Cc: Alexander Potapenko Link: http://lkml.kernel.org/r/20200423154503.5103-1-dja@axtens.net Link: http://lkml.kernel.org/r/20200423154503.5103-2-dja@axtens.net Signed-off-by: Linus Torvalds Signed-off-by: Sasha Levin --- lib/test_kasan.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/lib/test_kasan.c b/lib/test_kasan.c index e3087d90e00d..dc2c6a51d11a 100644 --- a/lib/test_kasan.c +++ b/lib/test_kasan.c @@ -23,6 +23,14 @@ #include +/* + * We assign some test results to these globals to make sure the tests + * are not eliminated as dead code. + */ + +int kasan_int_result; +void *kasan_ptr_result; + /* * Note: test functions are marked noinline so that their names appear in * reports. @@ -622,7 +630,7 @@ static noinline void __init kasan_memchr(void) if (!ptr) return; - memchr(ptr, '1', size + 1); + kasan_ptr_result = memchr(ptr, '1', size + 1); kfree(ptr); } @@ -638,7 +646,7 @@ static noinline void __init kasan_memcmp(void) return; memset(arr, 0, sizeof(arr)); - memcmp(ptr, arr, size+1); + kasan_int_result = memcmp(ptr, arr, size + 1); kfree(ptr); } @@ -661,22 +669,22 @@ static noinline void __init kasan_strings(void) * will likely point to zeroed byte. */ ptr += 16; - strchr(ptr, '1'); + kasan_ptr_result = strchr(ptr, '1'); pr_info("use-after-free in strrchr\n"); - strrchr(ptr, '1'); + kasan_ptr_result = strrchr(ptr, '1'); pr_info("use-after-free in strcmp\n"); - strcmp(ptr, "2"); + kasan_int_result = strcmp(ptr, "2"); pr_info("use-after-free in strncmp\n"); - strncmp(ptr, "2", 1); + kasan_int_result = strncmp(ptr, "2", 1); pr_info("use-after-free in strlen\n"); - strlen(ptr); + kasan_int_result = strlen(ptr); pr_info("use-after-free in strnlen\n"); - strnlen(ptr, 1); + kasan_int_result = strnlen(ptr, 1); } static noinline void __init kasan_bitops(void) @@ -743,11 +751,12 @@ static noinline void __init kasan_bitops(void) __test_and_change_bit(BITS_PER_LONG + BITS_PER_BYTE, bits); pr_info("out-of-bounds in test_bit\n"); - (void)test_bit(BITS_PER_LONG + BITS_PER_BYTE, bits); + kasan_int_result = test_bit(BITS_PER_LONG + BITS_PER_BYTE, bits); #if defined(clear_bit_unlock_is_negative_byte) pr_info("out-of-bounds in clear_bit_unlock_is_negative_byte\n"); - clear_bit_unlock_is_negative_byte(BITS_PER_LONG + BITS_PER_BYTE, bits); + kasan_int_result = clear_bit_unlock_is_negative_byte(BITS_PER_LONG + + BITS_PER_BYTE, bits); #endif kfree(bits); }