From patchwork Thu Apr 23 15:45:01 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Daniel Axtens X-Patchwork-Id: 11505961 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 27530159A for ; Thu, 23 Apr 2020 15:45:17 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id DE1EF20767 for ; Thu, 23 Apr 2020 15:45:16 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=axtens.net header.i=@axtens.net header.b="jPvZSIVR" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DE1EF20767 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=axtens.net Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 0BB958E0006; Thu, 23 Apr 2020 11:45:16 -0400 (EDT) Delivered-To: linux-mm-outgoing@kvack.org Received: by kanga.kvack.org (Postfix, from userid 40) id 06C578E0003; Thu, 23 Apr 2020 11:45:16 -0400 (EDT) X-Original-To: int-list-linux-mm@kvack.org X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EC4818E0006; Thu, 23 Apr 2020 11:45:15 -0400 (EDT) X-Original-To: linux-mm@kvack.org X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0183.hostedemail.com [216.40.44.183]) by kanga.kvack.org (Postfix) with ESMTP id D63438E0003 for ; Thu, 23 Apr 2020 11:45:15 -0400 (EDT) Received: from smtpin03.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 9BDBBA2BD for ; Thu, 23 Apr 2020 15:45:15 +0000 (UTC) X-FDA: 76739543790.03.wren72_680f88dbde153 X-Spam-Summary: 2,0,0,7d04e15ebc39d12d,d41d8cd98f00b204,dja@axtens.net,,RULES_HIT:2:41:69:152:355:379:541:800:960:966:973:988:989:1260:1277:1311:1313:1314:1345:1359:1431:1437:1515:1516:1518:1535:1593:1594:1605:1606:1730:1747:1777:1792:2196:2199:2380:2393:2559:2562:2693:3138:3139:3140:3141:3142:3865:3866:3867:3868:3870:3871:3874:4119:4250:4321:4385:4605:5007:6119:6261:6653:7514:7808:8603:9121:10004:11026:11233:11473:11658:11914:12043:12048:12291:12296:12297:12438:12517:12519:12555:12679:12683:12895:12986:13161:13229:13255:13894:14394:14659:21063:21080:21444:21451:21627:21796:21990:30012:30030:30034:30036:30054:30070,0,RBL:209.85.215.195:@axtens.net:.lbl8.mailshell.net-66.100.201.201 62.2.0.100,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:ft,MSBL:0,DNSBL:none,Custom_rules:0:0:0,LFtime:27,LUA_SUMMARY:none X-HE-Tag: wren72_680f88dbde153 X-Filterd-Recvd-Size: 8365 Received: from mail-pg1-f195.google.com (mail-pg1-f195.google.com [209.85.215.195]) by imf11.hostedemail.com (Postfix) with ESMTP for ; Thu, 23 Apr 2020 15:45:14 +0000 (UTC) Received: by mail-pg1-f195.google.com with SMTP id s18so407479pgl.12 for ; Thu, 23 Apr 2020 08:45:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=axtens.net; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=Rm9KJcgesHnxEUAmWee/mQjOLa+QTtuSP9X5sIq0tgA=; b=jPvZSIVRZtIuE/ljLJ3/1SMhxZNEYVUDCJn9nhJ1b8Rl/QIVp+M4bzvcP3LLZPvbj+ Ep1FWxzMDYCy8ID1pym0moYcnagwj44Z0fGnTNV/px2InnO20wPoAGY2Lehuaau4EKhV MgAEZsw3rHya1t16HQL+3GnyKd8WKAqsljxmA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=Rm9KJcgesHnxEUAmWee/mQjOLa+QTtuSP9X5sIq0tgA=; b=kNbNcxz7dB7wyfbBAHujS93iuqcdcWI7Glm/JrJ3sZRFEgBPWKF1Tf32pvRJDgQrHy Xg7i4vOmLfEtN3Ap0EbhNbgRc5V/+bdlbmME2dk5dtoY/eTeBbxb8PLAo7CGOjYQ8BFz j8pQolJO3FS6lY4iLYnoHiZ2nnyGX1ulqbbjZijeYDexRYDlhyj/Gd4mdGqSz5jdzrif ltEgYHRE2/j8ntpx8U9w1jdw2pHS+psb5w0WfAGIZ9pQTBKvjQj39wmU9cmqzzxFzN3n WVN/8ygdRTEaGmAbcFtP+qW0GUcu8qgwVlw75esKDxOb9IhkC9nkoUBMK2FkJCNyjqNk IbmA== X-Gm-Message-State: AGi0PuaHdYDlbb96hmDO89k9oewHvPMCBo5YCVdQZjI0eTvV63pGZVaX FxIxCwQKWfDh+BsRu/p8a8z18w== X-Google-Smtp-Source: APiQypJYVTiq1ojI2FxBtYaYdroS0cYv8IN+wrMRkcl61wXgxQMLYHKSmaDJF4pzqDaEJ9/oI6GZmg== X-Received: by 2002:a62:1d48:: with SMTP id d69mr4369984pfd.102.1587656714044; Thu, 23 Apr 2020 08:45:14 -0700 (PDT) Received: from localhost (2001-44b8-111e-5c00-7979-720a-9390-aec6.static.ipv6.internode.on.net. [2001:44b8:111e:5c00:7979:720a:9390:aec6]) by smtp.gmail.com with ESMTPSA id w125sm2435466pgw.22.2020.04.23.08.45.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 23 Apr 2020 08:45:13 -0700 (PDT) From: Daniel Axtens To: linux-kernel@vger.kernel.org, linux-mm@kvack.org, akpm@linux-foundation.org, kasan-dev@googlegroups.com Cc: dvyukov@google.com, christophe.leroy@c-s.fr, Daniel Axtens , Daniel Micay , Andrey Ryabinin , Alexander Potapenko Subject: [PATCH v3 1/3] kasan: stop tests being eliminated as dead code with FORTIFY_SOURCE Date: Fri, 24 Apr 2020 01:45:01 +1000 Message-Id: <20200423154503.5103-2-dja@axtens.net> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200423154503.5103-1-dja@axtens.net> References: <20200423154503.5103-1-dja@axtens.net> MIME-Version: 1.0 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: 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. Cc: Daniel Micay Cc: Andrey Ryabinin Cc: Alexander Potapenko Cc: Dmitry Vyukov Fixes: 0c96350a2d2f ("lib/test_kasan.c: add tests for several string/memory API functions") Reviewed-by: Dmitry Vyukov Signed-off-by: Daniel Axtens --- lib/test_kasan.c | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/lib/test_kasan.c b/lib/test_kasan.c index e3087d90e00d..939f395a5392 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); } @@ -637,8 +645,7 @@ static noinline void __init kasan_memcmp(void) if (!ptr) return; - memset(arr, 0, sizeof(arr)); - memcmp(ptr, arr, size+1); + kasan_int_result = memcmp(ptr, arr, size + 1); kfree(ptr); } @@ -661,22 +668,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 +750,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); }