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=-23.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_IN_DEF_DKIM_WL autolearn=ham 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 2CE3DC48BE5 for ; Sat, 12 Jun 2021 14:43:00 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 80AEB6138C for ; Sat, 12 Jun 2021 14:42:59 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 80AEB6138C 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 B79DF6B006C; Sat, 12 Jun 2021 10:42:58 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B2A016B006E; Sat, 12 Jun 2021 10:42:58 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 97C386B0070; Sat, 12 Jun 2021 10:42:58 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0056.hostedemail.com [216.40.44.56]) by kanga.kvack.org (Postfix) with ESMTP id 664A46B006C for ; Sat, 12 Jun 2021 10:42:58 -0400 (EDT) Received: from smtpin12.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id E73035DEA for ; Sat, 12 Jun 2021 14:42:57 +0000 (UTC) X-FDA: 78245338794.12.8DD7B1F Received: from mail-ot1-f44.google.com (mail-ot1-f44.google.com [209.85.210.44]) by imf30.hostedemail.com (Postfix) with ESMTP id 5733AE000240 for ; Sat, 12 Jun 2021 14:42:53 +0000 (UTC) Received: by mail-ot1-f44.google.com with SMTP id 36-20020a9d0ba70000b02902e0a0a8fe36so6194874oth.8 for ; Sat, 12 Jun 2021 07:42:57 -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=iH7EgL9X46ZYFZJtSIwwFSnS3uKaFXLbIujUQXunbi0=; b=wNxXyE/0okBDTG9XmRTHjerp356Pfi5go1rJh6p6vqJuOZDqvO/1DMRy1LLkCezXFr TBbJQo08srak+Nx2xyguxnoqNOiMM/xDH/v9RqAcITpn/VUsEJC2S4bmfS3lo4rn+up8 gh0GCZxE6FN8z5zsmkhEJIAzMnQlGeQDYULtdyskGHIAzkVAG6Xa9koNb5uzhsj6UN37 psWiIwk8Ve3qnNfd3xzf3oyXy1IUaxZDCfybFJArdYSixI5CRF7NLfc8utPz3YhMvJ0m XzN9p/3OSeBSNFhyPfp00qFyS5Lmb8YD6IUXFGjNNFVZiQsBmEiGl1r7iwYRNskSI07K DBUw== 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=iH7EgL9X46ZYFZJtSIwwFSnS3uKaFXLbIujUQXunbi0=; b=I4mBbBppzeA67o4jJLR4u62VPL2WlgFXf9URw+uS0oO/XPxG/P5gDJ0DNQClK4q3zB rCmX8vSNi8VFl+foPofN/R0y4MNfX56Y5fvX+uht9ACakS5oENNlXmQ+Y6/cb3mwn6bT 91M1BzrNstE7Uc0+eTpIp3ybZKY5FDAPQ2h5SvCBPZ7eTgsEJugs1OeOvjLr3AVSe7T6 LoIu3Gtp8v+64RCvgpWBM31pVIu2IRl/r/8jfCT0kDrRRZCkuSXiHYp3rTTl11ezV7Mg jiAfbPWIRnj6uWe/WTQlpZAFcYUkQLBrG6YLIsKcqUINt7c7Rm+cm3QZob8BEvG9LcOp lg2A== X-Gm-Message-State: AOAM532J/EC2eQ+mo6EKKwu7456GGJB7V9cac1IXNSLeG7gKZM/RMzev 4GAikYA2r3ybZpQRSNU7kPjTMXcA9n8ePsrPj5X/Pg== X-Google-Smtp-Source: ABdhPJyBUGhk0Z9OC45WMmiRoeWwNVm++xa8VAU766aiv7xkUKDVGHjax+HBCNIyA2St+uM3/cLLEb5DZlgRxuPmJp8= X-Received: by 2002:a05:6830:1c7b:: with SMTP id s27mr7290621otg.233.1623508976605; Sat, 12 Jun 2021 07:42:56 -0700 (PDT) MIME-Version: 1.0 References: <20210612045156.44763-1-kylee0686026@gmail.com> <20210612045156.44763-3-kylee0686026@gmail.com> In-Reply-To: <20210612045156.44763-3-kylee0686026@gmail.com> From: Marco Elver Date: Sat, 12 Jun 2021 16:42:44 +0200 Message-ID: Subject: Re: [PATCH v2 2/3] kasan: integrate the common part of two KASAN tag-based modes To: Kuan-Ying Lee , Greg Kroah-Hartman Cc: Andrey Ryabinin , Alexander Potapenko , Andrey Konovalov , Dmitry Vyukov , Andrew Morton , kasan-dev , LKML , Linux Memory Management List Content-Type: text/plain; charset="UTF-8" Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=google.com header.s=20161025 header.b="wNxXyE/0"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf30.hostedemail.com: domain of elver@google.com designates 209.85.210.44 as permitted sender) smtp.mailfrom=elver@google.com X-Stat-Signature: 6ec9p1j3ufbwkybsq1nqs1udxxjyggaf X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 5733AE000240 X-HE-Tag: 1623508973-148828 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 Sat, 12 Jun 2021 at 06:52, Kuan-Ying Lee wrote: > 1. Move kasan_get_free_track() and kasan_set_free_info() > into tags.c > 2. Move kasan_get_bug_type() to header file > > Signed-off-by: Kuan-Ying Lee > Suggested-by: Marco Elver > Cc: Andrey Ryabinin > Cc: Alexander Potapenko > Cc: Andrey Konovalov > Cc: Dmitry Vyukov > Cc: Andrew Morton > --- > mm/kasan/Makefile | 4 +-- > mm/kasan/hw_tags.c | 22 --------------- > mm/kasan/report_hw_tags.c | 6 +--- > mm/kasan/report_sw_tags.c | 46 +------------------------------ > mm/kasan/report_tags.h | 56 +++++++++++++++++++++++++++++++++++++ > mm/kasan/sw_tags.c | 41 --------------------------- > mm/kasan/tags.c | 58 +++++++++++++++++++++++++++++++++++++++ > 7 files changed, 118 insertions(+), 115 deletions(-) > create mode 100644 mm/kasan/report_tags.h > create mode 100644 mm/kasan/tags.c [...] > diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c > index ed5e5b833d61..4ea8c368b5b8 100644 > --- a/mm/kasan/hw_tags.c > +++ b/mm/kasan/hw_tags.c > @@ -216,28 +216,6 @@ void __init kasan_init_hw_tags(void) > pr_info("KernelAddressSanitizer initialized\n"); > } > > -void kasan_set_free_info(struct kmem_cache *cache, > - void *object, u8 tag) > -{ > - struct kasan_alloc_meta *alloc_meta; > - > - alloc_meta = kasan_get_alloc_meta(cache, object); > - if (alloc_meta) > - kasan_set_track(&alloc_meta->free_track[0], GFP_NOWAIT); > -} > - > -struct kasan_track *kasan_get_free_track(struct kmem_cache *cache, > - void *object, u8 tag) > -{ > - struct kasan_alloc_meta *alloc_meta; > - > - alloc_meta = kasan_get_alloc_meta(cache, object); > - if (!alloc_meta) > - return NULL; > - > - return &alloc_meta->free_track[0]; > -} > - > void kasan_alloc_pages(struct page *page, unsigned int order, gfp_t flags) > { > /* > diff --git a/mm/kasan/report_hw_tags.c b/mm/kasan/report_hw_tags.c > index 42b2168755d6..ef5e7378f3aa 100644 > --- a/mm/kasan/report_hw_tags.c > +++ b/mm/kasan/report_hw_tags.c > @@ -14,11 +14,7 @@ > #include > > #include "kasan.h" > - > -const char *kasan_get_bug_type(struct kasan_access_info *info) > -{ > - return "invalid-access"; > -} > +#include "report_tags.h" > > void *kasan_find_first_bad_addr(void *addr, size_t size) > { > diff --git a/mm/kasan/report_sw_tags.c b/mm/kasan/report_sw_tags.c > index 821a14a19a92..d965a170083e 100644 > --- a/mm/kasan/report_sw_tags.c > +++ b/mm/kasan/report_sw_tags.c > @@ -26,51 +26,7 @@ > > #include > > -#include "kasan.h" > -#include "../slab.h" > - > -const char *kasan_get_bug_type(struct kasan_access_info *info) > -{ > -#ifdef CONFIG_KASAN_TAGS_IDENTIFY > - struct kasan_alloc_meta *alloc_meta; > - struct kmem_cache *cache; > - struct page *page; > - const void *addr; > - void *object; > - u8 tag; > - int i; > - > - tag = get_tag(info->access_addr); > - addr = kasan_reset_tag(info->access_addr); > - page = kasan_addr_to_page(addr); > - if (page && PageSlab(page)) { > - cache = page->slab_cache; > - object = nearest_obj(cache, page, (void *)addr); > - alloc_meta = kasan_get_alloc_meta(cache, object); > - > - if (alloc_meta) { > - for (i = 0; i < KASAN_NR_FREE_STACKS; i++) { > - if (alloc_meta->free_pointer_tag[i] == tag) > - return "use-after-free"; > - } > - } > - return "out-of-bounds"; > - } > - > -#endif > - /* > - * If access_size is a negative number, then it has reason to be > - * defined as out-of-bounds bug type. > - * > - * 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. > - */ > - if (info->access_addr + info->access_size < info->access_addr) > - return "out-of-bounds"; > - > - return "invalid-access"; > -} > +#include "report_tags.h" > > void *kasan_find_first_bad_addr(void *addr, size_t size) > { > diff --git a/mm/kasan/report_tags.h b/mm/kasan/report_tags.h > new file mode 100644 > index 000000000000..4f740d4d99ee > --- /dev/null > +++ b/mm/kasan/report_tags.h > @@ -0,0 +1,56 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef __MM_KASAN_REPORT_TAGS_H > +#define __MM_KASAN_REPORT_TAGS_H > + > +#include "kasan.h" > +#include "../slab.h" > + > +#ifdef CONFIG_KASAN_TAGS_IDENTIFY > +const char *kasan_get_bug_type(struct kasan_access_info *info) > +{ [...] > + /* > + * If access_size is a negative number, then it has reason to be > + * defined as out-of-bounds bug type. > + * > + * 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. > + */ > + if (info->access_addr + info->access_size < info->access_addr) > + return "out-of-bounds"; This seems to change behaviour for SW_TAGS because it was there even if !CONFIG_KASAN_TAGS_IDENTIFY. Does it still work as before? > + > + return "invalid-access"; > +} > +#else > +const char *kasan_get_bug_type(struct kasan_access_info *info) > +{ > + return "invalid-access"; > +} > +#endif > + > +#endif > diff --git a/mm/kasan/sw_tags.c b/mm/kasan/sw_tags.c > index dd05e6c801fa..bd3f540feb47 100644 > --- a/mm/kasan/sw_tags.c > +++ b/mm/kasan/sw_tags.c > @@ -167,47 +167,6 @@ void __hwasan_tag_memory(unsigned long addr, u8 tag, unsigned long size) > } > EXPORT_SYMBOL(__hwasan_tag_memory); > > -void kasan_set_free_info(struct kmem_cache *cache, > - void *object, u8 tag) > -{ > - struct kasan_alloc_meta *alloc_meta; > - u8 idx = 0; > - > - alloc_meta = kasan_get_alloc_meta(cache, object); > - if (!alloc_meta) > - return; > - > -#ifdef CONFIG_KASAN_TAGS_IDENTIFY > - idx = alloc_meta->free_track_idx; > - alloc_meta->free_pointer_tag[idx] = tag; > - alloc_meta->free_track_idx = (idx + 1) % KASAN_NR_FREE_STACKS; > -#endif > - > - kasan_set_track(&alloc_meta->free_track[idx], GFP_NOWAIT); > -} > - > -struct kasan_track *kasan_get_free_track(struct kmem_cache *cache, > - void *object, u8 tag) > -{ > - struct kasan_alloc_meta *alloc_meta; > - int i = 0; > - > - alloc_meta = kasan_get_alloc_meta(cache, object); > - if (!alloc_meta) > - return NULL; > - > -#ifdef CONFIG_KASAN_TAGS_IDENTIFY > - for (i = 0; i < KASAN_NR_FREE_STACKS; i++) { > - if (alloc_meta->free_pointer_tag[i] == tag) > - break; > - } > - if (i == KASAN_NR_FREE_STACKS) > - i = alloc_meta->free_track_idx; > -#endif > - > - return &alloc_meta->free_track[i]; > -} > - > void kasan_tag_mismatch(unsigned long addr, unsigned long access_info, > unsigned long ret_ip) > { > diff --git a/mm/kasan/tags.c b/mm/kasan/tags.c > new file mode 100644 > index 000000000000..9c33c0ebe1d1 > --- /dev/null > +++ b/mm/kasan/tags.c > @@ -0,0 +1,58 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * This file contains common tag-based KASAN code. > + * > + * Author: Kuan-Ying Lee We appreciate your work on this, but this is misleading. Because you merely copied/moved the code, have a look what sw_tags.c says -- that should either be preserved, or we add nothing here. I prefer to add nothing or the bare minimum (e.g. if the company requires a Copyright line) for non-substantial additions because this stuff becomes out-of-date fast and just isn't useful at all. 'git log' is the source of truth. Cc'ing Greg for process advice. For moved code, does it have to preserve the original Copyright line if there was one? Thanks, -- Marco