[PATCH v3 0/3] kasan: add memory corruption identification support for hw tag-based kasan

[Kuan-Ying Lee]

Add memory corruption identification for hardware tag-based KASAN mode.

Changes since v3:
- Preserve Copyright from hw_tags.c/sw_tags.c and
report_sw_tags.c/report_hw_tags.c
- Make non-trivial change in kasan sw tag-based mode

Changes since v2:
- Thanks for Marco's Suggestion
- Rename the CONFIG_KASAN_SW_TAGS_IDENTIFY
- Integrate tag-based kasan common part
- Rebase to latest linux-next

Kuan-Ying Lee (3):
kasan: rename CONFIG_KASAN_SW_TAGS_IDENTIFY to
CONFIG_KASAN_TAGS_IDENTIFY
kasan: integrate the common part of two KASAN tag-based modes
kasan: add memory corruption identification support for hardware
tag-based mode

lib/Kconfig.kasan | 4 +--
mm/kasan/Makefile | 4 +--
mm/kasan/hw_tags.c | 22 ---------------
mm/kasan/kasan.h | 4 +--
mm/kasan/report_hw_tags.c | 6 +---
mm/kasan/report_sw_tags.c | 46 +-----------------------------
mm/kasan/report_tags.h | 55 ++++++++++++++++++++++++++++++++++++
mm/kasan/sw_tags.c | 41 ---------------------------
mm/kasan/tags.c | 59 +++++++++++++++++++++++++++++++++++++++
9 files changed, 122 insertions(+), 119 deletions(-)
create mode 100644 mm/kasan/report_tags.h
create mode 100644 mm/kasan/tags.c

--
2.18.0

[Kuan-Ying Lee]

Add memory corruption identification support for hardware tag-based

mode. We store one old free pointer tag and free backtrace.

Signed-off-by: Kuan-Ying Lee <Kuan-Y...@mediatek.com>
Suggested-by: Marco Elver <el...@google.com>
Cc: Andrey Ryabinin <ryabin...@gmail.com>
Cc: Alexander Potapenko <gli...@google.com>
Cc: Andrey Konovalov <andre...@gmail.com>
Cc: Dmitry Vyukov <dvy...@google.com>
Cc: Andrew Morton <ak...@linux-foundation.org>
---
lib/Kconfig.kasan | 2 +-
mm/kasan/kasan.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
index 6f5d48832139..2cc25792bc2f 100644
--- a/lib/Kconfig.kasan
+++ b/lib/Kconfig.kasan
@@ -157,7 +157,7 @@ config KASAN_STACK

config KASAN_TAGS_IDENTIFY
bool "Enable memory corruption identification"
- depends on KASAN_SW_TAGS
+ depends on KASAN_SW_TAGS || KASAN_HW_TAGS
help
This option enables best-effort identification of bug type
(use-after-free or out-of-bounds) at the cost of increased
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index b0fc9a1eb7e3..d6f982b8a84e 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -153,7 +153,7 @@ struct kasan_track {
depot_stack_handle_t stack;
};

-#ifdef CONFIG_KASAN_TAGS_IDENTIFY
+#if defined(CONFIG_KASAN_TAGS_IDENTIFY) && defined(CONFIG_KASAN_SW_TAGS)
#define KASAN_NR_FREE_STACKS 5
#else
#define KASAN_NR_FREE_STACKS 1
--
2.18.0

[Kuan-Ying Lee]

This patch renames CONFIG_KASAN_SW_TAGS_IDENTIFY to
CONFIG_KASAN_TAGS_IDENTIFY in order to be compatible
with hardware tag-based mode.

Signed-off-by: Kuan-Ying Lee <Kuan-Y...@mediatek.com>
Suggested-by: Marco Elver <el...@google.com>
Cc: Andrey Ryabinin <ryabin...@gmail.com>
Cc: Alexander Potapenko <gli...@google.com>
Cc: Andrey Konovalov <andre...@gmail.com>
Cc: Dmitry Vyukov <dvy...@google.com>
Cc: Andrew Morton <ak...@linux-foundation.org>
---
lib/Kconfig.kasan | 2 +-

mm/kasan/kasan.h | 4 ++--
mm/kasan/report_sw_tags.c | 2 +-
mm/kasan/sw_tags.c | 4 ++--
4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
index cffc2ebbf185..6f5d48832139 100644
--- a/lib/Kconfig.kasan
+++ b/lib/Kconfig.kasan
@@ -155,7 +155,7 @@ config KASAN_STACK
CONFIG_COMPILE_TEST. On gcc it is assumed to always be safe
to use and enabled by default.

-config KASAN_SW_TAGS_IDENTIFY
+config KASAN_TAGS_IDENTIFY

bool "Enable memory corruption identification"

depends on KASAN_SW_TAGS
help
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 8f450bc28045..b0fc9a1eb7e3 100644

--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -153,7 +153,7 @@ struct kasan_track {
depot_stack_handle_t stack;
};

-#ifdef CONFIG_KASAN_SW_TAGS_IDENTIFY
+#ifdef CONFIG_KASAN_TAGS_IDENTIFY

#define KASAN_NR_FREE_STACKS 5
#else
#define KASAN_NR_FREE_STACKS 1

@@ -170,7 +170,7 @@ struct kasan_alloc_meta {
#else
struct kasan_track free_track[KASAN_NR_FREE_STACKS];
#endif
-#ifdef CONFIG_KASAN_SW_TAGS_IDENTIFY
+#ifdef CONFIG_KASAN_TAGS_IDENTIFY
u8 free_pointer_tag[KASAN_NR_FREE_STACKS];
u8 free_track_idx;
#endif
diff --git a/mm/kasan/report_sw_tags.c b/mm/kasan/report_sw_tags.c
index 3d20d3451d9e..821a14a19a92 100644
--- a/mm/kasan/report_sw_tags.c
+++ b/mm/kasan/report_sw_tags.c
@@ -31,7 +31,7 @@

const char *kasan_get_bug_type(struct kasan_access_info *info)
{
-#ifdef CONFIG_KASAN_SW_TAGS_IDENTIFY
+#ifdef CONFIG_KASAN_TAGS_IDENTIFY
struct kasan_alloc_meta *alloc_meta;
struct kmem_cache *cache;
struct page *page;
diff --git a/mm/kasan/sw_tags.c b/mm/kasan/sw_tags.c
index 9362938abbfa..dd05e6c801fa 100644
--- a/mm/kasan/sw_tags.c
+++ b/mm/kasan/sw_tags.c
@@ -177,7 +177,7 @@ void kasan_set_free_info(struct kmem_cache *cache,
if (!alloc_meta)
return;

-#ifdef CONFIG_KASAN_SW_TAGS_IDENTIFY
+#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;
@@ -196,7 +196,7 @@ struct kasan_track *kasan_get_free_track(struct kmem_cache *cache,
if (!alloc_meta)
return NULL;

-#ifdef CONFIG_KASAN_SW_TAGS_IDENTIFY
+#ifdef CONFIG_KASAN_TAGS_IDENTIFY
for (i = 0; i < KASAN_NR_FREE_STACKS; i++) {
if (alloc_meta->free_pointer_tag[i] == tag)
break;
--
2.18.0

[Kuan-Ying Lee]

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 <Kuan-Y...@mediatek.com>
Suggested-by: Marco Elver <el...@google.com>

Suggested-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>

Cc: Andrey Ryabinin <ryabin...@gmail.com>
Cc: Alexander Potapenko <gli...@google.com>
Cc: Andrey Konovalov <andre...@gmail.com>
Cc: Dmitry Vyukov <dvy...@google.com>
Cc: Andrew Morton <ak...@linux-foundation.org>
---

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 | 55 ++++++++++++++++++++++++++++++++++++
mm/kasan/sw_tags.c | 41 ---------------------------
mm/kasan/tags.c | 59 +++++++++++++++++++++++++++++++++++++++

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/Makefile b/mm/kasan/Makefile
index 9fe39a66388a..634de6c1da9b 100644
--- a/mm/kasan/Makefile
+++ b/mm/kasan/Makefile
@@ -37,5 +37,5 @@ CFLAGS_sw_tags.o := $(CC_FLAGS_KASAN_RUNTIME)

obj-$(CONFIG_KASAN) := common.o report.o
obj-$(CONFIG_KASAN_GENERIC) += init.o generic.o report_generic.o shadow.o quarantine.o
-obj-$(CONFIG_KASAN_HW_TAGS) += hw_tags.o report_hw_tags.o
-obj-$(CONFIG_KASAN_SW_TAGS) += init.o report_sw_tags.o shadow.o sw_tags.o
+obj-$(CONFIG_KASAN_HW_TAGS) += hw_tags.o report_hw_tags.o tags.o
+obj-$(CONFIG_KASAN_SW_TAGS) += init.o report_sw_tags.o shadow.o sw_tags.o tags.o
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 <linux/types.h>

#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 <asm/sections.h>

-#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..1cb872177904
--- /dev/null
+++ b/mm/kasan/report_tags.h
@@ -0,0 +1,55 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2020 Google, Inc.
+ */
+#ifndef __MM_KASAN_REPORT_TAGS_H
+#define __MM_KASAN_REPORT_TAGS_H
+
+#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";
+}
+
+#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..8f48b9502a17
--- /dev/null
+++ b/mm/kasan/tags.c
@@ -0,0 +1,59 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * This file contains common tag-based KASAN code.
+ *
+ * Copyright (c) 2018 Google, Inc.
+ * Copyright (c) 2020 Google, Inc.
+ */
+
+#include <linux/init.h>
+#include <linux/kasan.h>
+#include <linux/kernel.h>
+#include <linux/memory.h>
+#include <linux/mm.h>
+#include <linux/static_key.h>
+#include <linux/string.h>
+#include <linux/types.h>
+
+#include "kasan.h"
+
+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];
+}
--
2.18.0

[Marco Elver]

On Sun, 20 Jun 2021 at 13:48, Kuan-Ying Lee <Kuan-Y...@mediatek.com> wrote:
>
> Add memory corruption identification for hardware tag-based KASAN mode.
>
> Changes since v3:
> - Preserve Copyright from hw_tags.c/sw_tags.c and
> report_sw_tags.c/report_hw_tags.c
> - Make non-trivial change in kasan sw tag-based mode
>
> Changes since v2:
> - Thanks for Marco's Suggestion
> - Rename the CONFIG_KASAN_SW_TAGS_IDENTIFY
> - Integrate tag-based kasan common part
> - Rebase to latest linux-next
>
> Kuan-Ying Lee (3):
> kasan: rename CONFIG_KASAN_SW_TAGS_IDENTIFY to
> CONFIG_KASAN_TAGS_IDENTIFY
> kasan: integrate the common part of two KASAN tag-based modes
> kasan: add memory corruption identification support for hardware
> tag-based mode

I think this looks fine, thank you for your efforts. How did you test
this? Did you run the lib/test_kasan module with both SW_TAGS and
HW_TAGS mode? I was about to run that before adding my Reviewed-by.

Andrey, Alex, if you have time, please have a quick look at the series.

Thanks,
-- Marco

[Alexander Potapenko]

> diff --git a/mm/kasan/report_tags.h b/mm/kasan/report_tags.h
> new file mode 100644
> index 000000000000..1cb872177904
> --- /dev/null
> +++ b/mm/kasan/report_tags.h

Why don't you make it a C file instead?

> +const char *kasan_get_bug_type(struct kasan_access_info *info)

If this function has to be in the header, it should be declared as
static inline.
But I don't think it has to be there in the first place.

[Alexander Potapenko]

On Sun, Jun 20, 2021 at 1:48 PM Kuan-Ying Lee
<Kuan-Y...@mediatek.com> wrote:
>
> This patch renames CONFIG_KASAN_SW_TAGS_IDENTIFY to
> CONFIG_KASAN_TAGS_IDENTIFY in order to be compatible
> with hardware tag-based mode.
>
> Signed-off-by: Kuan-Ying Lee <Kuan-Y...@mediatek.com>
> Suggested-by: Marco Elver <el...@google.com>
> Cc: Andrey Ryabinin <ryabin...@gmail.com>
> Cc: Alexander Potapenko <gli...@google.com>
> Cc: Andrey Konovalov <andre...@gmail.com>
> Cc: Dmitry Vyukov <dvy...@google.com>
> Cc: Andrew Morton <ak...@linux-foundation.org>

Reviewed-by: Alexander Potapenko <gli...@google.com>

[Alexander Potapenko]

On Sun, Jun 20, 2021 at 1:48 PM Kuan-Ying Lee
<Kuan-Y...@mediatek.com> wrote:
>

> Add memory corruption identification support for hardware tag-based
> mode. We store one old free pointer tag and free backtrace.
>
> Signed-off-by: Kuan-Ying Lee <Kuan-Y...@mediatek.com>
> Suggested-by: Marco Elver <el...@google.com>
> Cc: Andrey Ryabinin <ryabin...@gmail.com>
> Cc: Alexander Potapenko <gli...@google.com>
> Cc: Andrey Konovalov <andre...@gmail.com>
> Cc: Dmitry Vyukov <dvy...@google.com>
> Cc: Andrew Morton <ak...@linux-foundation.org>

Reviewed-by: Alexander Potapenko <gli...@google.com>

> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20210620114756.31304-4-Kuan-Ying.Lee%40mediatek.com.



--
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

[Marco Elver]

On Tue, 22 Jun 2021 at 11:28, Alexander Potapenko <gli...@google.com> wrote:
>
> > diff --git a/mm/kasan/report_tags.h b/mm/kasan/report_tags.h
> > new file mode 100644
> > index 000000000000..1cb872177904
> > --- /dev/null
> > +++ b/mm/kasan/report_tags.h
> Why don't you make it a C file instead?

Yes, good point. report_{hw,sw}_tags.c essentially define it, but it's
called by report.c code.

I think I suggested to make it a header first because there were still
parts that were report_{hw,sw}_tags.c specific, and the helper
function would be used by those 2 to build their version of
kasan_get_bug_type(), but that doesn't seem to be the case anymore.

[Andrey Konovalov]

Reviewed-by: Andrey Konovalov <andre...@gmail.com>

[Andrey Konovalov]

On Sun, Jun 20, 2021 at 2:48 PM Kuan-Ying Lee
<Kuan-Y...@mediatek.com> wrote:
>

> 1. Move kasan_get_free_track() and kasan_set_free_info()
> into tags.c

Please mention that the patch doesn't only move but also combines
these functions for SW_TAGS and HW_TAGS modes.

> --- /dev/null
> +++ b/mm/kasan/report_tags.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
> + * Copyright (c) 2020 Google, Inc.
> + */
> +#ifndef __MM_KASAN_REPORT_TAGS_H
> +#define __MM_KASAN_REPORT_TAGS_H
> +
> +#include "kasan.h"
> +#include "../slab.h"
> +
> +const char *kasan_get_bug_type(struct kasan_access_info *info)

As mentioned by Alex, don't put this implementation into a header. Put
it into report_tags.c. The declaration is already in kasan.h.

[Andrey Konovalov]

On Sun, Jun 20, 2021 at 2:48 PM Kuan-Ying Lee
<Kuan-Y...@mediatek.com> wrote:
>

> Add memory corruption identification support for hardware tag-based
> mode. We store one old free pointer tag and free backtrace.

Please explain why only one.

Other than that:

Reviewed-by: Andrey Konovalov <andre...@gmail.com>

[Kuan-Ying Lee]

On Tue, 2021-06-22 at 16:54 +0300, Andrey Konovalov wrote:
> On Sun, Jun 20, 2021 at 2:48 PM Kuan-Ying Lee
> <Kuan-Y...@mediatek.com> wrote:
> >
> > 1. Move kasan_get_free_track() and kasan_set_free_info()
> > into tags.c
>
> Please mention that the patch doesn't only move but also combines
> these functions for SW_TAGS and HW_TAGS modes.
>

Got it.

> > --- /dev/null
> > +++ b/mm/kasan/report_tags.h
> > @@ -0,0 +1,55 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
> > + * Copyright (c) 2020 Google, Inc.
> > + */
> > +#ifndef __MM_KASAN_REPORT_TAGS_H
> > +#define __MM_KASAN_REPORT_TAGS_H
> > +
> > +#include "kasan.h"
> > +#include "../slab.h"
> > +
> > +const char *kasan_get_bug_type(struct kasan_access_info *info)
>
> As mentioned by Alex, don't put this implementation into a header.
> Put
> it into report_tags.c. The declaration is already in kasan.h.
>

Ok. I will refactor in v4.
Thanks for suggestions.

[Kuan-Ying Lee]

On Tue, 2021-06-22 at 16:54 +0300, Andrey Konovalov wrote:

> On Sun, Jun 20, 2021 at 2:48 PM Kuan-Ying Lee
> <Kuan-Y...@mediatek.com> wrote:
> >
> > Add memory corruption identification support for hardware tag-based
> > mode. We store one old free pointer tag and free backtrace.
>
> Please explain why only one.
>

Got it. I will rewrite this commit message in v4.

[Kuan-Ying Lee]

This patch renames CONFIG_KASAN_SW_TAGS_IDENTIFY to
CONFIG_KASAN_TAGS_IDENTIFY in order to be compatible
with hardware tag-based mode.

Signed-off-by: Kuan-Ying Lee <Kuan-Y...@mediatek.com>
Suggested-by: Marco Elver <el...@google.com>

Reviewed-by: Alexander Potapenko <gli...@google.com>
Reviewed-by: Andrey Konovalov <andre...@gmail.com>
Cc: Andrey Ryabinin <ryabin...@gmail.com>

Cc: Dmitry Vyukov <dvy...@google.com>
Cc: Andrew Morton <ak...@linux-foundation.org>
---
lib/Kconfig.kasan | 2 +-

mm/kasan/kasan.h | 4 ++--
mm/kasan/report_sw_tags.c | 2 +-
mm/kasan/sw_tags.c | 4 ++--

4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
index c3b228828a80..fdb4a08dba83 100644
--- a/lib/Kconfig.kasan
+++ b/lib/Kconfig.kasan
@@ -167,7 +167,7 @@ config KASAN_STACK
instrumentation is also disabled as it adds inline-style
instrumentation that is run unconditionally.

-config KASAN_SW_TAGS_IDENTIFY
+config KASAN_TAGS_IDENTIFY

bool "Enable memory corruption identification"

depends on KASAN_SW_TAGS
help
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 7b45b17a8106..952df2db7fdd 100644

--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -153,7 +153,7 @@ struct kasan_track {
depot_stack_handle_t stack;
};

-#ifdef CONFIG_KASAN_SW_TAGS_IDENTIFY
+#ifdef CONFIG_KASAN_TAGS_IDENTIFY

#define KASAN_NR_FREE_STACKS 5
#else
#define KASAN_NR_FREE_STACKS 1

for (i = 0; i < KASAN_NR_FREE_STACKS; i++) {

if (alloc_meta->free_pointer_tag[i] == tag)

break;
--
2.18.0

[Kuan-Ying Lee]

Add memory corruption identification for hardware tag-based KASAN mode.

Changes since v4:
- Change report_tags.h to report_tags.c
- Refine the commit message
- Test lib/test_kasan module with SW_TAGS mode
- Test lib/test_kasan module with HW_TAGS mode

- Rebase to latest linux-next

Changes since v3:
- Preserve Copyright from hw_tags.c/sw_tags.c and
report_sw_tags.c/report_hw_tags.c
- Make non-trivial change in kasan sw tag-based mode

Changes since v2:
- Thanks for Marco's Suggestion
- Rename the CONFIG_KASAN_SW_TAGS_IDENTIFY
- Integrate tag-based kasan common part
- Rebase to latest linux-next

Kuan-Ying Lee (3):
kasan: rename CONFIG_KASAN_SW_TAGS_IDENTIFY to
CONFIG_KASAN_TAGS_IDENTIFY
kasan: integrate the common part of two KASAN tag-based modes

kasan: add memory corruption identification support for hardware
tag-based mode

lib/Kconfig.kasan | 4 +--

mm/kasan/Makefile | 4 +--
mm/kasan/hw_tags.c | 22 ---------------

mm/kasan/kasan.h | 4 +--
mm/kasan/report_hw_tags.c | 5 ----
mm/kasan/report_sw_tags.c | 43 ----------------------------
mm/kasan/report_tags.c | 51 +++++++++++++++++++++++++++++++++

mm/kasan/sw_tags.c | 41 ---------------------------
mm/kasan/tags.c | 59 +++++++++++++++++++++++++++++++++++++++

9 files changed, 116 insertions(+), 117 deletions(-)
create mode 100644 mm/kasan/report_tags.c

[Kuan-Ying Lee]

Add memory corruption identification support for hardware tag-based

mode. We store one old free pointer tag and free backtrace instead
of five because hardware tag-based kasan only has 16 different tags.

If we store as many stacks as SW tag-based kasan does(5 stacks),
there is high probability to find the same tag in the stacks when
out-of-bound issues happened and we will mistake out-of-bound
issue for use-after-free.

Signed-off-by: Kuan-Ying Lee <Kuan-Y...@mediatek.com>
Suggested-by: Marco Elver <el...@google.com>
Reviewed-by: Alexander Potapenko <gli...@google.com>
Reviewed-by: Andrey Konovalov <andre...@gmail.com>
Cc: Andrey Ryabinin <ryabin...@gmail.com>
Cc: Dmitry Vyukov <dvy...@google.com>
Cc: Andrew Morton <ak...@linux-foundation.org>
---
lib/Kconfig.kasan | 2 +-

mm/kasan/kasan.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
index fdb4a08dba83..1e2d10f86011 100644
--- a/lib/Kconfig.kasan
+++ b/lib/Kconfig.kasan
@@ -169,7 +169,7 @@ config KASAN_STACK

config KASAN_TAGS_IDENTIFY
bool "Enable memory corruption identification"

- depends on KASAN_SW_TAGS
+ depends on KASAN_SW_TAGS || KASAN_HW_TAGS
help
This option enables best-effort identification of bug type
(use-after-free or out-of-bounds) at the cost of increased

diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 952df2db7fdd..f58672f6029a 100644

--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -153,7 +153,7 @@ struct kasan_track {
depot_stack_handle_t stack;
};

-#ifdef CONFIG_KASAN_TAGS_IDENTIFY
+#if defined(CONFIG_KASAN_TAGS_IDENTIFY) && defined(CONFIG_KASAN_SW_TAGS)

#define KASAN_NR_FREE_STACKS 5
#else
#define KASAN_NR_FREE_STACKS 1

--
2.18.0

[Kuan-Ying Lee]

1. Move kasan_get_free_track() and kasan_set_free_info()

into tags.c and combine these two functions for
SW_TAGS and HW_TAGS kasan mode.
2. Move kasan_get_bug_type() to report_tags.c and
make this function compatible for SW_TAGS and
HW_TAGS kasan mode.

Signed-off-by: Kuan-Ying Lee <Kuan-Y...@mediatek.com>
Suggested-by: Marco Elver <el...@google.com>

Suggested-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: Andrey Ryabinin <ryabin...@gmail.com>

Cc: Alexander Potapenko <gli...@google.com>
Cc: Andrey Konovalov <andre...@gmail.com>

Cc: Dmitry Vyukov <dvy...@google.com>
Cc: Andrew Morton <ak...@linux-foundation.org>
---

mm/kasan/Makefile | 4 +--
mm/kasan/hw_tags.c | 22 ---------------

mm/kasan/report_hw_tags.c | 5 ----
mm/kasan/report_sw_tags.c | 43 ----------------------------
mm/kasan/report_tags.c | 51 +++++++++++++++++++++++++++++++++
mm/kasan/sw_tags.c | 41 ---------------------------
mm/kasan/tags.c | 59 +++++++++++++++++++++++++++++++++++++++

7 files changed, 112 insertions(+), 113 deletions(-)

create mode 100644 mm/kasan/report_tags.c
create mode 100644 mm/kasan/tags.c

diff --git a/mm/kasan/Makefile b/mm/kasan/Makefile
index 9fe39a66388a..adcd9acaef61 100644

--- a/mm/kasan/Makefile
+++ b/mm/kasan/Makefile
@@ -37,5 +37,5 @@ CFLAGS_sw_tags.o := $(CC_FLAGS_KASAN_RUNTIME)

obj-$(CONFIG_KASAN) := common.o report.o
obj-$(CONFIG_KASAN_GENERIC) += init.o generic.o report_generic.o shadow.o quarantine.o
-obj-$(CONFIG_KASAN_HW_TAGS) += hw_tags.o report_hw_tags.o
-obj-$(CONFIG_KASAN_SW_TAGS) += init.o report_sw_tags.o shadow.o sw_tags.o

+obj-$(CONFIG_KASAN_HW_TAGS) += hw_tags.o report_hw_tags.o tags.o report_tags.o
+obj-$(CONFIG_KASAN_SW_TAGS) += init.o report_sw_tags.o shadow.o sw_tags.o tags.o report_tags.o

index 42b2168755d6..5dbbbb930e7a 100644
--- a/mm/kasan/report_hw_tags.c
+++ b/mm/kasan/report_hw_tags.c
@@ -15,11 +15,6 @@

#include "kasan.h"

-const char *kasan_get_bug_type(struct kasan_access_info *info)
-{
- return "invalid-access";
-}

-

void *kasan_find_first_bad_addr(void *addr, size_t size)
{

return kasan_reset_tag(addr);
diff --git a/mm/kasan/report_sw_tags.c b/mm/kasan/report_sw_tags.c
index 821a14a19a92..d2298c357834 100644
--- a/mm/kasan/report_sw_tags.c
+++ b/mm/kasan/report_sw_tags.c
@@ -29,49 +29,6 @@
#include "kasan.h"
#include "../slab.h"

-

void *kasan_find_first_bad_addr(void *addr, size_t size)
{

u8 tag = get_tag(addr);
diff --git a/mm/kasan/report_tags.c b/mm/kasan/report_tags.c
new file mode 100644
index 000000000000..8a319fc16dab
--- /dev/null
+++ b/mm/kasan/report_tags.c
@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: GPL-2.0

+/*
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2020 Google, Inc.
+ */

+
+#include "kasan.h"
+#include "../slab.h"
+
+const char *kasan_get_bug_type(struct kasan_access_info *info)

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

new file mode 100644

index 000000000000..8f48b9502a17
--- /dev/null
+++ b/mm/kasan/tags.c
@@ -0,0 +1,59 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * This file contains common tag-based KASAN code.
+ *
+ * Copyright (c) 2018 Google, Inc.

+ * Copyright (c) 2020 Google, Inc.
+ */

+
+#include <linux/init.h>
+#include <linux/kasan.h>
+#include <linux/kernel.h>
+#include <linux/memory.h>
+#include <linux/mm.h>
+#include <linux/static_key.h>
+#include <linux/string.h>
+#include <linux/types.h>
+
+#include "kasan.h"
+
+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);

[Kuan-Ying Lee]

Thanks for the reminder.
Yes, I run the lib/test_kasan module with SW_TAGS and HW_TAGS mode. :)

[Andrey Konovalov]

Reviewed-by: Andrey Konovalov <andre...@gmail.com>

[Marco Elver]

On Sat, 26 Jun 2021 at 12:09, Kuan-Ying Lee <Kuan-Y...@mediatek.com> wrote:
>
> This patch renames CONFIG_KASAN_SW_TAGS_IDENTIFY to
> CONFIG_KASAN_TAGS_IDENTIFY in order to be compatible
> with hardware tag-based mode.
>
> Signed-off-by: Kuan-Ying Lee <Kuan-Y...@mediatek.com>
> Suggested-by: Marco Elver <el...@google.com>
> Reviewed-by: Alexander Potapenko <gli...@google.com>
> Reviewed-by: Andrey Konovalov <andre...@gmail.com>
> Cc: Andrey Ryabinin <ryabin...@gmail.com>
> Cc: Dmitry Vyukov <dvy...@google.com>
> Cc: Andrew Morton <ak...@linux-foundation.org>

Reviewed-by: Marco Elver <el...@google.com>

> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20210626100931.22794-2-Kuan-Ying.Lee%40mediatek.com.

[Marco Elver]

On Sat, 26 Jun 2021 at 12:09, Kuan-Ying Lee <Kuan-Y...@mediatek.com> wrote:
>

> 1. Move kasan_get_free_track() and kasan_set_free_info()
> into tags.c and combine these two functions for
> SW_TAGS and HW_TAGS kasan mode.
> 2. Move kasan_get_bug_type() to report_tags.c and
> make this function compatible for SW_TAGS and
> HW_TAGS kasan mode.
>
> Signed-off-by: Kuan-Ying Lee <Kuan-Y...@mediatek.com>
> Suggested-by: Marco Elver <el...@google.com>
> Suggested-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>
> Cc: Andrey Ryabinin <ryabin...@gmail.com>
> Cc: Alexander Potapenko <gli...@google.com>
> Cc: Andrey Konovalov <andre...@gmail.com>
> Cc: Dmitry Vyukov <dvy...@google.com>
> Cc: Andrew Morton <ak...@linux-foundation.org>

Reviewed-by: Marco Elver <el...@google.com>

> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20210626100931.22794-3-Kuan-Ying.Lee%40mediatek.com.

[Marco Elver]

On Sat, 26 Jun 2021 at 12:09, Kuan-Ying Lee <Kuan-Y...@mediatek.com> wrote:
>

> Add memory corruption identification support for hardware tag-based
> mode. We store one old free pointer tag and free backtrace instead
> of five because hardware tag-based kasan only has 16 different tags.
>
> If we store as many stacks as SW tag-based kasan does(5 stacks),
> there is high probability to find the same tag in the stacks when
> out-of-bound issues happened and we will mistake out-of-bound
> issue for use-after-free.
>
> Signed-off-by: Kuan-Ying Lee <Kuan-Y...@mediatek.com>
> Suggested-by: Marco Elver <el...@google.com>
> Reviewed-by: Alexander Potapenko <gli...@google.com>
> Reviewed-by: Andrey Konovalov <andre...@gmail.com>
> Cc: Andrey Ryabinin <ryabin...@gmail.com>
> Cc: Dmitry Vyukov <dvy...@google.com>
> Cc: Andrew Morton <ak...@linux-foundation.org>

Reviewed-by: Marco Elver <el...@google.com>