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=-9.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, UNPARSEABLE_RELAY,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 3E686C4CECE for ; Mon, 14 Oct 2019 16:23:27 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 19BA3217D9 for ; Mon, 14 Oct 2019 16:23:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2387666AbfJNQX0 (ORCPT ); Mon, 14 Oct 2019 12:23:26 -0400 Received: from mailgw02.mediatek.com ([210.61.82.184]:34785 "EHLO mailgw02.mediatek.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1729271AbfJNQXY (ORCPT ); Mon, 14 Oct 2019 12:23:24 -0400 X-UUID: 00e5f5011c1b421da9e2c7e6370c8c49-20191015 X-UUID: 00e5f5011c1b421da9e2c7e6370c8c49-20191015 Received: from mtkcas06.mediatek.inc [(172.21.101.30)] by mailgw02.mediatek.com (envelope-from ) (Cellopoint E-mail Firewall v4.1.10 Build 0809 with TLS) with ESMTP id 1840082822; Tue, 15 Oct 2019 00:23:19 +0800 Received: from MTKCAS06.mediatek.inc (172.21.101.30) by mtkmbs07n1.mediatek.inc (172.21.101.16) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Tue, 15 Oct 2019 00:23:17 +0800 Received: from mtksdccf07.mediatek.inc (172.21.84.99) by MTKCAS06.mediatek.inc (172.21.101.73) with Microsoft SMTP Server id 15.0.1395.4 via Frontend Transport; Tue, 15 Oct 2019 00:23:15 +0800 From: Walter Wu To: Andrey Ryabinin , Alexander Potapenko , Dmitry Vyukov , Matthias Brugger CC: , , , , , , Walter Wu Subject: [PATCH v2 1/2] kasan: detect negative size in memory operation function Date: Tue, 15 Oct 2019 00:23:16 +0800 Message-ID: <20191014162316.28314-1-walter-zh.wu@mediatek.com> X-Mailer: git-send-email 2.18.0 MIME-Version: 1.0 Content-Type: text/plain X-MTK: N Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org KASAN missed detecting size is negative numbers in memset(), memcpy(), and memmove(), it will cause out-of-bounds bug, so needs to be detected by KASAN. If size is negative numbers, then it has three reasons to be defined as heap-out-of-bounds bug type. 1) Casting negative numbers to size_t would indeed turn up as a large size_t and its value will be larger than ULONG_MAX/2, so that this can qualify as out-of-bounds. 2) If KASAN has new bug type and user-space passes negative size, then there are duplicate reports. So don't produce new bug type in order to prevent duplicate reports by some systems (e.g. syzbot) to report the same bug twice. 3) When size is negative numbers, it may be passed from user-space. So we always print heap-out-of-bounds in order to prevent that kernel-space and user-space have the same bug but have duplicate reports. KASAN report: BUG: KASAN: heap-out-of-bounds in kmalloc_memmove_invalid_size+0x70/0xa0 Read of size 18446744073709551608 at addr ffffff8069660904 by task cat/72 CPU: 2 PID: 72 Comm: cat Not tainted 5.4.0-rc1-next-20191004ajb-00001-gdb8af2f372b2-dirty #1 Hardware name: linux,dummy-virt (DT) Call trace: dump_backtrace+0x0/0x288 show_stack+0x14/0x20 dump_stack+0x10c/0x164 print_address_description.isra.9+0x68/0x378 __kasan_report+0x164/0x1a0 kasan_report+0xc/0x18 check_memory_region+0x174/0x1d0 memmove+0x34/0x88 kmalloc_memmove_invalid_size+0x70/0xa0 [1] https://bugzilla.kernel.org/show_bug.cgi?id=199341 Changes in v2: Fix the indentation bug, thanks for the reminder Matthew. Signed-off-by: Walter Wu Reported -by: Dmitry Vyukov Suggested-by: Dmitry Vyukov Reviewed-by: Dmitry Vyukov --- mm/kasan/common.c | 13 ++++++++----- mm/kasan/generic.c | 5 +++++ mm/kasan/generic_report.c | 18 ++++++++++++++++++ mm/kasan/tags.c | 5 +++++ mm/kasan/tags_report.c | 18 ++++++++++++++++++ 5 files changed, 54 insertions(+), 5 deletions(-) diff --git a/mm/kasan/common.c b/mm/kasan/common.c index 6814d6d6a023..16a370023425 100644 --- a/mm/kasan/common.c +++ b/mm/kasan/common.c @@ -102,7 +102,8 @@ EXPORT_SYMBOL(__kasan_check_write); #undef memset void *memset(void *addr, int c, size_t len) { - check_memory_region((unsigned long)addr, len, true, _RET_IP_); + if (!check_memory_region((unsigned long)addr, len, true, _RET_IP_)) + return NULL; return __memset(addr, c, len); } @@ -110,8 +111,9 @@ void *memset(void *addr, int c, size_t len) #undef memmove void *memmove(void *dest, const void *src, size_t len) { - check_memory_region((unsigned long)src, len, false, _RET_IP_); - check_memory_region((unsigned long)dest, len, true, _RET_IP_); + if (!check_memory_region((unsigned long)src, len, false, _RET_IP_) || + !check_memory_region((unsigned long)dest, len, true, _RET_IP_)) + return NULL; return __memmove(dest, src, len); } @@ -119,8 +121,9 @@ void *memmove(void *dest, const void *src, size_t len) #undef memcpy void *memcpy(void *dest, const void *src, size_t len) { - check_memory_region((unsigned long)src, len, false, _RET_IP_); - check_memory_region((unsigned long)dest, len, true, _RET_IP_); + if (!check_memory_region((unsigned long)src, len, false, _RET_IP_) || + !check_memory_region((unsigned long)dest, len, true, _RET_IP_)) + return NULL; return __memcpy(dest, src, len); } diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c index 616f9dd82d12..02148a317d27 100644 --- a/mm/kasan/generic.c +++ b/mm/kasan/generic.c @@ -173,6 +173,11 @@ static __always_inline bool check_memory_region_inline(unsigned long addr, if (unlikely(size == 0)) return true; + if (unlikely((long)size < 0)) { + kasan_report(addr, size, write, ret_ip); + return false; + } + if (unlikely((void *)addr < kasan_shadow_to_mem((void *)KASAN_SHADOW_START))) { kasan_report(addr, size, write, ret_ip); diff --git a/mm/kasan/generic_report.c b/mm/kasan/generic_report.c index 36c645939bc9..52a92c7db697 100644 --- a/mm/kasan/generic_report.c +++ b/mm/kasan/generic_report.c @@ -107,6 +107,24 @@ static const char *get_wild_bug_type(struct kasan_access_info *info) const char *get_bug_type(struct kasan_access_info *info) { + /* + * If access_size is negative numbers, then it has three reasons + * to be defined as heap-out-of-bounds bug type. + * 1) Casting negative numbers to size_t would indeed turn up as + * a large size_t and its value will be larger than ULONG_MAX/2, + * so that this can qualify as out-of-bounds. + * 2) If KASAN has new bug type and user-space passes negative size, + * then there are duplicate reports. So don't produce new bug type + * in order to prevent duplicate reports by some systems + * (e.g. syzbot) to report the same bug twice. + * 3) When size is negative numbers, it may be passed from user-space. + * So we always print heap-out-of-bounds in order to prevent that + * kernel-space and user-space have the same bug but have duplicate + * reports. + */ + if ((long)info->access_size < 0) + return "heap-out-of-bounds"; + if (addr_has_shadow(info->access_addr)) return get_shadow_bug_type(info); return get_wild_bug_type(info); diff --git a/mm/kasan/tags.c b/mm/kasan/tags.c index 0e987c9ca052..b829535a3ad7 100644 --- a/mm/kasan/tags.c +++ b/mm/kasan/tags.c @@ -86,6 +86,11 @@ bool check_memory_region(unsigned long addr, size_t size, bool write, if (unlikely(size == 0)) return true; + if (unlikely((long)size < 0)) { + kasan_report(addr, size, write, ret_ip); + return false; + } + tag = get_tag((const void *)addr); /* diff --git a/mm/kasan/tags_report.c b/mm/kasan/tags_report.c index 969ae08f59d7..f7ae474aef3a 100644 --- a/mm/kasan/tags_report.c +++ b/mm/kasan/tags_report.c @@ -36,6 +36,24 @@ const char *get_bug_type(struct kasan_access_info *info) { + /* + * If access_size is negative numbers, then it has three reasons + * to be defined as heap-out-of-bounds bug type. + * 1) Casting negative numbers to size_t would indeed turn up as + * a large size_t and its value will be larger than ULONG_MAX/2, + * so that this can qualify as out-of-bounds. + * 2) If KASAN has new bug type and user-space passes negative size, + * then there are duplicate reports. So don't produce new bug type + * in order to prevent duplicate reports by some systems + * (e.g. syzbot) to report the same bug twice. + * 3) When size is negative numbers, it may be passed from user-space. + * So we always print heap-out-of-bounds in order to prevent that + * kernel-space and user-space have the same bug but have duplicate + * reports. + */ + if ((long)info->access_size < 0) + return "heap-out-of-bounds"; + #ifdef CONFIG_KASAN_SW_TAGS_IDENTIFY struct kasan_alloc_meta *alloc_meta; struct kmem_cache *cache; -- 2.18.0 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Walter Wu Subject: [PATCH v2 1/2] kasan: detect negative size in memory operation function Date: Tue, 15 Oct 2019 00:23:16 +0800 Message-ID: <20191014162316.28314-1-walter-zh.wu@mediatek.com> Mime-Version: 1.0 Content-Type: text/plain Return-path: Sender: linux-kernel-owner@vger.kernel.org To: Andrey Ryabinin , Alexander Potapenko , Dmitry Vyukov , Matthias Brugger Cc: kasan-dev@googlegroups.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, wsd_upstream@mediatek.com, Walter Wu List-Id: linux-mediatek@lists.infradead.org KASAN missed detecting size is negative numbers in memset(), memcpy(), and memmove(), it will cause out-of-bounds bug, so needs to be detected by KASAN. If size is negative numbers, then it has three reasons to be defined as heap-out-of-bounds bug type. 1) Casting negative numbers to size_t would indeed turn up as a large size_t and its value will be larger than ULONG_MAX/2, so that this can qualify as out-of-bounds. 2) If KASAN has new bug type and user-space passes negative size, then there are duplicate reports. So don't produce new bug type in order to prevent duplicate reports by some systems (e.g. syzbot) to report the same bug twice. 3) When size is negative numbers, it may be passed from user-space. So we always print heap-out-of-bounds in order to prevent that kernel-space and user-space have the same bug but have duplicate reports. KASAN report: BUG: KASAN: heap-out-of-bounds in kmalloc_memmove_invalid_size+0x70/0xa0 Read of size 18446744073709551608 at addr ffffff8069660904 by task cat/72 CPU: 2 PID: 72 Comm: cat Not tainted 5.4.0-rc1-next-20191004ajb-00001-gdb8af2f372b2-dirty #1 Hardware name: linux,dummy-virt (DT) Call trace: dump_backtrace+0x0/0x288 show_stack+0x14/0x20 dump_stack+0x10c/0x164 print_address_description.isra.9+0x68/0x378 __kasan_report+0x164/0x1a0 kasan_report+0xc/0x18 check_memory_region+0x174/0x1d0 memmove+0x34/0x88 kmalloc_memmove_invalid_size+0x70/0xa0 [1] https://bugzilla.kernel.org/show_bug.cgi?id=199341 Changes in v2: Fix the indentation bug, thanks for the reminder Matthew. Signed-off-by: Walter Wu Reported -by: Dmitry Vyukov Suggested-by: Dmitry Vyukov Reviewed-by: Dmitry Vyukov --- mm/kasan/common.c | 13 ++++++++----- mm/kasan/generic.c | 5 +++++ mm/kasan/generic_report.c | 18 ++++++++++++++++++ mm/kasan/tags.c | 5 +++++ mm/kasan/tags_report.c | 18 ++++++++++++++++++ 5 files changed, 54 insertions(+), 5 deletions(-) diff --git a/mm/kasan/common.c b/mm/kasan/common.c index 6814d6d6a023..16a370023425 100644 --- a/mm/kasan/common.c +++ b/mm/kasan/common.c @@ -102,7 +102,8 @@ EXPORT_SYMBOL(__kasan_check_write); #undef memset void *memset(void *addr, int c, size_t len) { - check_memory_region((unsigned long)addr, len, true, _RET_IP_); + if (!check_memory_region((unsigned long)addr, len, true, _RET_IP_)) + return NULL; return __memset(addr, c, len); } @@ -110,8 +111,9 @@ void *memset(void *addr, int c, size_t len) #undef memmove void *memmove(void *dest, const void *src, size_t len) { - check_memory_region((unsigned long)src, len, false, _RET_IP_); - check_memory_region((unsigned long)dest, len, true, _RET_IP_); + if (!check_memory_region((unsigned long)src, len, false, _RET_IP_) || + !check_memory_region((unsigned long)dest, len, true, _RET_IP_)) + return NULL; return __memmove(dest, src, len); } @@ -119,8 +121,9 @@ void *memmove(void *dest, const void *src, size_t len) #undef memcpy void *memcpy(void *dest, const void *src, size_t len) { - check_memory_region((unsigned long)src, len, false, _RET_IP_); - check_memory_region((unsigned long)dest, len, true, _RET_IP_); + if (!check_memory_region((unsigned long)src, len, false, _RET_IP_) || + !check_memory_region((unsigned long)dest, len, true, _RET_IP_)) + return NULL; return __memcpy(dest, src, len); } diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c index 616f9dd82d12..02148a317d27 100644 --- a/mm/kasan/generic.c +++ b/mm/kasan/generic.c @@ -173,6 +173,11 @@ static __always_inline bool check_memory_region_inline(unsigned long addr, if (unlikely(size == 0)) return true; + if (unlikely((long)size < 0)) { + kasan_report(addr, size, write, ret_ip); + return false; + } + if (unlikely((void *)addr < kasan_shadow_to_mem((void *)KASAN_SHADOW_START))) { kasan_report(addr, size, write, ret_ip); diff --git a/mm/kasan/generic_report.c b/mm/kasan/generic_report.c index 36c645939bc9..52a92c7db697 100644 --- a/mm/kasan/generic_report.c +++ b/mm/kasan/generic_report.c @@ -107,6 +107,24 @@ static const char *get_wild_bug_type(struct kasan_access_info *info) const char *get_bug_type(struct kasan_access_info *info) { + /* + * If access_size is negative numbers, then it has three reasons + * to be defined as heap-out-of-bounds bug type. + * 1) Casting negative numbers to size_t would indeed turn up as + * a large size_t and its value will be larger than ULONG_MAX/2, + * so that this can qualify as out-of-bounds. + * 2) If KASAN has new bug type and user-space passes negative size, + * then there are duplicate reports. So don't produce new bug type + * in order to prevent duplicate reports by some systems + * (e.g. syzbot) to report the same bug twice. + * 3) When size is negative numbers, it may be passed from user-space. + * So we always print heap-out-of-bounds in order to prevent that + * kernel-space and user-space have the same bug but have duplicate + * reports. + */ + if ((long)info->access_size < 0) + return "heap-out-of-bounds"; + if (addr_has_shadow(info->access_addr)) return get_shadow_bug_type(info); return get_wild_bug_type(info); diff --git a/mm/kasan/tags.c b/mm/kasan/tags.c index 0e987c9ca052..b829535a3ad7 100644 --- a/mm/kasan/tags.c +++ b/mm/kasan/tags.c @@ -86,6 +86,11 @@ bool check_memory_region(unsigned long addr, size_t size, bool write, if (unlikely(size == 0)) return true; + if (unlikely((long)size < 0)) { + kasan_report(addr, size, write, ret_ip); + return false; + } + tag = get_tag((const void *)addr); /* diff --git a/mm/kasan/tags_report.c b/mm/kasan/tags_report.c index 969ae08f59d7..f7ae474aef3a 100644 --- a/mm/kasan/tags_report.c +++ b/mm/kasan/tags_report.c @@ -36,6 +36,24 @@ const char *get_bug_type(struct kasan_access_info *info) { + /* + * If access_size is negative numbers, then it has three reasons + * to be defined as heap-out-of-bounds bug type. + * 1) Casting negative numbers to size_t would indeed turn up as + * a large size_t and its value will be larger than ULONG_MAX/2, + * so that this can qualify as out-of-bounds. + * 2) If KASAN has new bug type and user-space passes negative size, + * then there are duplicate reports. So don't produce new bug type + * in order to prevent duplicate reports by some systems + * (e.g. syzbot) to report the same bug twice. + * 3) When size is negative numbers, it may be passed from user-space. + * So we always print heap-out-of-bounds in order to prevent that + * kernel-space and user-space have the same bug but have duplicate + * reports. + */ + if ((long)info->access_size < 0) + return "heap-out-of-bounds"; + #ifdef CONFIG_KASAN_SW_TAGS_IDENTIFY struct kasan_alloc_meta *alloc_meta; struct kmem_cache *cache; -- 2.18.0 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=-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,UNPARSEABLE_RELAY,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 740BDECE587 for ; Mon, 14 Oct 2019 16:23:41 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 44C912133F for ; Mon, 14 Oct 2019 16:23:41 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="aRMSSh9c" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 44C912133F Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=mediatek.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-ID:Date:Subject:To :From:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References: List-Owner; bh=q23df0oSfIWVe7U0CMPI/L5rnl4+oeOPZpzOIYKxoZc=; b=aRMSSh9c8aUilG UkJ4jZctYez0p0ey3x84II5URpUgIGR2oSSO9K14jOGERA71/7VCohqz9hixiU/sNV0JWFE2bCPAA tpiudj/EPTSYxDdsVVIRh/tNH4SurDrw3OtUZxGqC+gXeBzfgsDO6vQ31Wlqw/eZdA7cc77+zzVvS VvMrvlWVaOosu6aJL6RaPdBSDheJ9OCfF1KndopMTg5blxY2lFCrHBxibCl9UPfuf43ogUGBkBpZm yBN+z14eCkwTAqR/k3rb29J89wKrOL0h3OdNhxWMe90NfMnmL+rvhsZM5rJt64INL0EczaQ8cZHz/ wWaLuRZ4zcpEj6XAJ0Ww==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1iK38B-0005IP-BF; Mon, 14 Oct 2019 16:23:35 +0000 Received: from mailgw02.mediatek.com ([216.200.240.185]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1iK382-0005Gw-3J; Mon, 14 Oct 2019 16:23:30 +0000 X-UUID: bcc38493f70642568ff5ae4ef0bbe282-20191014 X-UUID: bcc38493f70642568ff5ae4ef0bbe282-20191014 Received: from mtkcas66.mediatek.inc [(172.29.193.44)] by mailgw02.mediatek.com (envelope-from ) (musrelay.mediatek.com ESMTP with TLS) with ESMTP id 126172491; Mon, 14 Oct 2019 08:23:19 -0800 Received: from mtkmbs07n1.mediatek.inc (172.21.101.16) by MTKMBS62N2.mediatek.inc (172.29.193.42) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Mon, 14 Oct 2019 09:23:18 -0700 Received: from MTKCAS06.mediatek.inc (172.21.101.30) by mtkmbs07n1.mediatek.inc (172.21.101.16) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Tue, 15 Oct 2019 00:23:17 +0800 Received: from mtksdccf07.mediatek.inc (172.21.84.99) by MTKCAS06.mediatek.inc (172.21.101.73) with Microsoft SMTP Server id 15.0.1395.4 via Frontend Transport; Tue, 15 Oct 2019 00:23:15 +0800 From: Walter Wu To: Andrey Ryabinin , Alexander Potapenko , Dmitry Vyukov , Matthias Brugger Subject: [PATCH v2 1/2] kasan: detect negative size in memory operation function Date: Tue, 15 Oct 2019 00:23:16 +0800 Message-ID: <20191014162316.28314-1-walter-zh.wu@mediatek.com> X-Mailer: git-send-email 2.18.0 MIME-Version: 1.0 X-MTK: N X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20191014_092326_144084_CEA802AE X-CRM114-Status: GOOD ( 13.38 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Walter Wu , wsd_upstream@mediatek.com, linux-kernel@vger.kernel.org, kasan-dev@googlegroups.com, linux-mm@kvack.org, linux-mediatek@lists.infradead.org, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org KASAN missed detecting size is negative numbers in memset(), memcpy(), and memmove(), it will cause out-of-bounds bug, so needs to be detected by KASAN. If size is negative numbers, then it has three reasons to be defined as heap-out-of-bounds bug type. 1) Casting negative numbers to size_t would indeed turn up as a large size_t and its value will be larger than ULONG_MAX/2, so that this can qualify as out-of-bounds. 2) If KASAN has new bug type and user-space passes negative size, then there are duplicate reports. So don't produce new bug type in order to prevent duplicate reports by some systems (e.g. syzbot) to report the same bug twice. 3) When size is negative numbers, it may be passed from user-space. So we always print heap-out-of-bounds in order to prevent that kernel-space and user-space have the same bug but have duplicate reports. KASAN report: BUG: KASAN: heap-out-of-bounds in kmalloc_memmove_invalid_size+0x70/0xa0 Read of size 18446744073709551608 at addr ffffff8069660904 by task cat/72 CPU: 2 PID: 72 Comm: cat Not tainted 5.4.0-rc1-next-20191004ajb-00001-gdb8af2f372b2-dirty #1 Hardware name: linux,dummy-virt (DT) Call trace: dump_backtrace+0x0/0x288 show_stack+0x14/0x20 dump_stack+0x10c/0x164 print_address_description.isra.9+0x68/0x378 __kasan_report+0x164/0x1a0 kasan_report+0xc/0x18 check_memory_region+0x174/0x1d0 memmove+0x34/0x88 kmalloc_memmove_invalid_size+0x70/0xa0 [1] https://bugzilla.kernel.org/show_bug.cgi?id=199341 Changes in v2: Fix the indentation bug, thanks for the reminder Matthew. Signed-off-by: Walter Wu Reported -by: Dmitry Vyukov Suggested-by: Dmitry Vyukov Reviewed-by: Dmitry Vyukov --- mm/kasan/common.c | 13 ++++++++----- mm/kasan/generic.c | 5 +++++ mm/kasan/generic_report.c | 18 ++++++++++++++++++ mm/kasan/tags.c | 5 +++++ mm/kasan/tags_report.c | 18 ++++++++++++++++++ 5 files changed, 54 insertions(+), 5 deletions(-) diff --git a/mm/kasan/common.c b/mm/kasan/common.c index 6814d6d6a023..16a370023425 100644 --- a/mm/kasan/common.c +++ b/mm/kasan/common.c @@ -102,7 +102,8 @@ EXPORT_SYMBOL(__kasan_check_write); #undef memset void *memset(void *addr, int c, size_t len) { - check_memory_region((unsigned long)addr, len, true, _RET_IP_); + if (!check_memory_region((unsigned long)addr, len, true, _RET_IP_)) + return NULL; return __memset(addr, c, len); } @@ -110,8 +111,9 @@ void *memset(void *addr, int c, size_t len) #undef memmove void *memmove(void *dest, const void *src, size_t len) { - check_memory_region((unsigned long)src, len, false, _RET_IP_); - check_memory_region((unsigned long)dest, len, true, _RET_IP_); + if (!check_memory_region((unsigned long)src, len, false, _RET_IP_) || + !check_memory_region((unsigned long)dest, len, true, _RET_IP_)) + return NULL; return __memmove(dest, src, len); } @@ -119,8 +121,9 @@ void *memmove(void *dest, const void *src, size_t len) #undef memcpy void *memcpy(void *dest, const void *src, size_t len) { - check_memory_region((unsigned long)src, len, false, _RET_IP_); - check_memory_region((unsigned long)dest, len, true, _RET_IP_); + if (!check_memory_region((unsigned long)src, len, false, _RET_IP_) || + !check_memory_region((unsigned long)dest, len, true, _RET_IP_)) + return NULL; return __memcpy(dest, src, len); } diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c index 616f9dd82d12..02148a317d27 100644 --- a/mm/kasan/generic.c +++ b/mm/kasan/generic.c @@ -173,6 +173,11 @@ static __always_inline bool check_memory_region_inline(unsigned long addr, if (unlikely(size == 0)) return true; + if (unlikely((long)size < 0)) { + kasan_report(addr, size, write, ret_ip); + return false; + } + if (unlikely((void *)addr < kasan_shadow_to_mem((void *)KASAN_SHADOW_START))) { kasan_report(addr, size, write, ret_ip); diff --git a/mm/kasan/generic_report.c b/mm/kasan/generic_report.c index 36c645939bc9..52a92c7db697 100644 --- a/mm/kasan/generic_report.c +++ b/mm/kasan/generic_report.c @@ -107,6 +107,24 @@ static const char *get_wild_bug_type(struct kasan_access_info *info) const char *get_bug_type(struct kasan_access_info *info) { + /* + * If access_size is negative numbers, then it has three reasons + * to be defined as heap-out-of-bounds bug type. + * 1) Casting negative numbers to size_t would indeed turn up as + * a large size_t and its value will be larger than ULONG_MAX/2, + * so that this can qualify as out-of-bounds. + * 2) If KASAN has new bug type and user-space passes negative size, + * then there are duplicate reports. So don't produce new bug type + * in order to prevent duplicate reports by some systems + * (e.g. syzbot) to report the same bug twice. + * 3) When size is negative numbers, it may be passed from user-space. + * So we always print heap-out-of-bounds in order to prevent that + * kernel-space and user-space have the same bug but have duplicate + * reports. + */ + if ((long)info->access_size < 0) + return "heap-out-of-bounds"; + if (addr_has_shadow(info->access_addr)) return get_shadow_bug_type(info); return get_wild_bug_type(info); diff --git a/mm/kasan/tags.c b/mm/kasan/tags.c index 0e987c9ca052..b829535a3ad7 100644 --- a/mm/kasan/tags.c +++ b/mm/kasan/tags.c @@ -86,6 +86,11 @@ bool check_memory_region(unsigned long addr, size_t size, bool write, if (unlikely(size == 0)) return true; + if (unlikely((long)size < 0)) { + kasan_report(addr, size, write, ret_ip); + return false; + } + tag = get_tag((const void *)addr); /* diff --git a/mm/kasan/tags_report.c b/mm/kasan/tags_report.c index 969ae08f59d7..f7ae474aef3a 100644 --- a/mm/kasan/tags_report.c +++ b/mm/kasan/tags_report.c @@ -36,6 +36,24 @@ const char *get_bug_type(struct kasan_access_info *info) { + /* + * If access_size is negative numbers, then it has three reasons + * to be defined as heap-out-of-bounds bug type. + * 1) Casting negative numbers to size_t would indeed turn up as + * a large size_t and its value will be larger than ULONG_MAX/2, + * so that this can qualify as out-of-bounds. + * 2) If KASAN has new bug type and user-space passes negative size, + * then there are duplicate reports. So don't produce new bug type + * in order to prevent duplicate reports by some systems + * (e.g. syzbot) to report the same bug twice. + * 3) When size is negative numbers, it may be passed from user-space. + * So we always print heap-out-of-bounds in order to prevent that + * kernel-space and user-space have the same bug but have duplicate + * reports. + */ + if ((long)info->access_size < 0) + return "heap-out-of-bounds"; + #ifdef CONFIG_KASAN_SW_TAGS_IDENTIFY struct kasan_alloc_meta *alloc_meta; struct kmem_cache *cache; -- 2.18.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel