* [PATCH 01/18] mm/kasan: make some kasan functions global
2017-11-28 7:48 [PATCH 00/18] introduce a new tool, valid access checker js1304
@ 2017-11-28 7:48 ` js1304
2017-11-28 7:48 ` [PATCH 02/18] vchecker: introduce the valid access checker js1304
` (18 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: js1304 @ 2017-11-28 7:48 UTC (permalink / raw)
To: Andrew Morton
Cc: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel, Namhyung Kim, Wengang Wang, Joonsoo Kim
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
They will be used for the vchecker in the following patch.
Make it non-static and add declairation in header files.
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
include/linux/kasan.h | 1 +
mm/kasan/kasan.c | 2 +-
mm/kasan/kasan.h | 2 ++
mm/kasan/report.c | 2 +-
4 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index e3eb834..50f49fe 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -37,6 +37,7 @@ extern void kasan_enable_current(void);
/* Disable reporting bugs for current task */
extern void kasan_disable_current(void);
+void kasan_poison_shadow(const void *address, size_t size, u8 value);
void kasan_unpoison_shadow(const void *address, size_t size);
void kasan_unpoison_task_stack(struct task_struct *task);
diff --git a/mm/kasan/kasan.c b/mm/kasan/kasan.c
index 405bba4..2bcbdbd 100644
--- a/mm/kasan/kasan.c
+++ b/mm/kasan/kasan.c
@@ -54,7 +54,7 @@ void kasan_disable_current(void)
* Poisons the shadow memory for 'size' bytes starting from 'addr'.
* Memory addresses should be aligned to KASAN_SHADOW_SCALE_SIZE.
*/
-static void kasan_poison_shadow(const void *address, size_t size, u8 value)
+void kasan_poison_shadow(const void *address, size_t size, u8 value)
{
void *shadow_start, *shadow_end;
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index c70851a..b5d086d 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -99,6 +99,8 @@ static inline const void *kasan_shadow_to_mem(const void *shadow_addr)
void kasan_report(unsigned long addr, size_t size,
bool is_write, unsigned long ip);
+void describe_object(struct kmem_cache *cache, void *object,
+ const void *addr);
void kasan_report_double_free(struct kmem_cache *cache, void *object,
void *ip);
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 6bcfb01..b78735a 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -230,7 +230,7 @@ static void describe_object_addr(struct kmem_cache *cache, void *object,
(void *)(object_addr + cache->object_size));
}
-static void describe_object(struct kmem_cache *cache, void *object,
+void describe_object(struct kmem_cache *cache, void *object,
const void *addr)
{
struct kasan_alloc_meta *alloc_info = get_alloc_info(cache, object);
--
2.7.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 02/18] vchecker: introduce the valid access checker
2017-11-28 7:48 [PATCH 00/18] introduce a new tool, valid access checker js1304
2017-11-28 7:48 ` [PATCH 01/18] mm/kasan: make some kasan functions global js1304
@ 2017-11-28 7:48 ` js1304
2017-11-28 19:41 ` Andi Kleen
2017-12-01 5:08 ` kbuild test robot
2017-11-28 7:48 ` [PATCH 03/18] vchecker: mark/unmark the shadow of the allocated objects js1304
` (17 subsequent siblings)
19 siblings, 2 replies; 29+ messages in thread
From: js1304 @ 2017-11-28 7:48 UTC (permalink / raw)
To: Andrew Morton
Cc: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel, Namhyung Kim, Wengang Wang, Joonsoo Kim
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Vchecker is a dynamic memory error detector. It provides a new debug
feature that can find out an un-intended access to valid area. Valid
area here means the memory which is allocated and allowed to be
accessed by memory owner and un-intended access means the read/write
that is initiated by non-owner. Usual problem of this class is
memory overwritten.
Most of debug feature focused on finding out un-intended access to
in-valid area, for example, out-of-bound access and use-after-free, and,
there are many good tools for it. But, as far as I know, there is
no good tool to find out un-intended access to valid area. This kind
of problem is really hard to solve so this tool would be very useful.
Idea to implement this feature is so simple. Thanks to compile-time
instrumentation, we can audit all the accesses to memory. However,
since almost accesses to valid area are usually valid, we need a way
to distinguish an in-valid access. What this patch provides is
the interface to describe the information about the address that
user are interested on and the judgement criteria for in-valid access.
With this information, we can easily detect in-valid access to valid area.
For now, two kinds of criteria are supported. One is the value checker
that checks written value to the designated address. The other is the
callstack checker that checks in-valid callstack when read/write happens.
It is more than welcome if someone introduces more checkers.
Note that this patch itself just provides infrastructure of the vchecker
and sample checker. Proper checker implementation will be implemented
in the following patches.
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
include/linux/slab_def.h | 3 +
include/linux/slub_def.h | 3 +
lib/Kconfig.kasan | 12 +
mm/kasan/Makefile | 1 +
mm/kasan/kasan.c | 11 +-
mm/kasan/kasan.h | 1 +
mm/kasan/vchecker.c | 592 +++++++++++++++++++++++++++++++++++++++++++++++
mm/kasan/vchecker.h | 31 +++
mm/slab.h | 6 +
mm/slab_common.c | 19 ++
10 files changed, 677 insertions(+), 2 deletions(-)
create mode 100644 mm/kasan/vchecker.c
create mode 100644 mm/kasan/vchecker.h
diff --git a/include/linux/slab_def.h b/include/linux/slab_def.h
index 7385547..34e2ea7 100644
--- a/include/linux/slab_def.h
+++ b/include/linux/slab_def.h
@@ -80,6 +80,9 @@ struct kmem_cache {
#ifdef CONFIG_KASAN
struct kasan_cache kasan_info;
#endif
+#ifdef CONFIG_VCHECKER
+ struct vchecker_cache vchecker_cache;
+#endif
#ifdef CONFIG_SLAB_FREELIST_RANDOM
unsigned int *random_seq;
diff --git a/include/linux/slub_def.h b/include/linux/slub_def.h
index 8ad99c4..8a9deac 100644
--- a/include/linux/slub_def.h
+++ b/include/linux/slub_def.h
@@ -134,6 +134,9 @@ struct kmem_cache {
#ifdef CONFIG_KASAN
struct kasan_cache kasan_info;
#endif
+#ifdef CONFIG_VCHECKER
+ struct vchecker_cache vchecker_cache;
+#endif
size_t useroffset; /* Usercopy region offset */
size_t usersize; /* Usercopy region size */
diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
index bd38aab..51c0a05 100644
--- a/lib/Kconfig.kasan
+++ b/lib/Kconfig.kasan
@@ -53,4 +53,16 @@ config TEST_KASAN
out of bounds accesses, use after free. It is useful for testing
kernel debugging features like kernel address sanitizer.
+config VCHECKER
+ bool "Valid access checker"
+ help
+ Enables valid access checker - runtime memory debugger,
+ designed to find un-intended accesses to valid (allocated) area.
+ To debug something, you need to specify concrete area to be
+ debugged and register checker you'd like to check when access
+ happens at the area.
+
+ depends on KASAN && DEBUG_FS
+ select KASAN_OUTLINE
+
endif
diff --git a/mm/kasan/Makefile b/mm/kasan/Makefile
index 3289db3..f02ba99 100644
--- a/mm/kasan/Makefile
+++ b/mm/kasan/Makefile
@@ -9,3 +9,4 @@ CFLAGS_REMOVE_kasan.o = -pg
CFLAGS_kasan.o := $(call cc-option, -fno-conserve-stack -fno-stack-protector)
obj-y := kasan.o report.o kasan_init.o quarantine.o
+obj-$(CONFIG_VCHECKER) += vchecker.o
diff --git a/mm/kasan/kasan.c b/mm/kasan/kasan.c
index 2bcbdbd..8fc4ad8 100644
--- a/mm/kasan/kasan.c
+++ b/mm/kasan/kasan.c
@@ -39,6 +39,7 @@
#include "kasan.h"
#include "../slab.h"
+#include "vchecker.h"
void kasan_enable_current(void)
{
@@ -257,6 +258,9 @@ static __always_inline void check_memory_region_inline(unsigned long addr,
if (likely(!memory_is_poisoned(addr, size)))
return;
+ if (vchecker_check(addr, size, write, ret_ip))
+ return;
+
kasan_report(addr, size, write, ret_ip);
}
@@ -511,9 +515,11 @@ bool kasan_slab_free(struct kmem_cache *cache, void *object)
shadow_byte = READ_ONCE(*(s8 *)kasan_mem_to_shadow(object));
if (shadow_byte < 0 || shadow_byte >= KASAN_SHADOW_SCALE_SIZE) {
- kasan_report_double_free(cache, object,
+ if ((u8)shadow_byte != KASAN_VCHECKER_GRAYZONE) {
+ kasan_report_double_free(cache, object,
__builtin_return_address(1));
- return true;
+ return true;
+ }
}
kasan_poison_slab_free(cache, object);
@@ -546,6 +552,7 @@ void kasan_kmalloc(struct kmem_cache *cache, const void *object, size_t size,
kasan_unpoison_shadow(object, size);
kasan_poison_shadow((void *)redzone_start, redzone_end - redzone_start,
KASAN_KMALLOC_REDZONE);
+ vchecker_kmalloc(cache, object, size);
if (cache->flags & SLAB_KASAN)
set_track(&get_alloc_info(cache, object)->alloc_track, flags);
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index b5d086d..485a2c0 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -13,6 +13,7 @@
#define KASAN_KMALLOC_REDZONE 0xFC /* redzone inside slub object */
#define KASAN_KMALLOC_FREE 0xFB /* object was freed (kmem_cache_free/kfree) */
#define KASAN_GLOBAL_REDZONE 0xFA /* redzone for global variable */
+#define KASAN_VCHECKER_GRAYZONE 0xF0 /* area that should be checked when access */
/*
* Stack redzone shadow values
diff --git a/mm/kasan/vchecker.c b/mm/kasan/vchecker.c
new file mode 100644
index 0000000..0ac031c
--- /dev/null
+++ b/mm/kasan/vchecker.c
@@ -0,0 +1,592 @@
+/*
+ * Valid access checker
+ *
+ * Copyright (c) 2016-2017 Joonsoo Kim <iamjoonsoo.kim@lge.com>
+ */
+
+
+#include <linux/kernel.h>
+#include <linux/ctype.h>
+#include <linux/debugfs.h>
+#include <linux/list.h>
+#include <linux/rculist.h>
+#include <linux/mm.h>
+#include <linux/mm_types.h>
+#include <linux/slab.h>
+#include <linux/mutex.h>
+#include <linux/kasan.h>
+#include <linux/uaccess.h>
+
+#include "vchecker.h"
+#include "../slab.h"
+#include "kasan.h"
+
+struct vchecker {
+ bool enabled;
+ struct list_head cb_list;
+};
+
+enum vchecker_type_num {
+ VCHECKER_TYPE_VALUE = 0,
+ VCHECKER_TYPE_MAX,
+};
+
+struct vchecker_type {
+ char *name;
+ const struct file_operations *fops;
+ int (*init)(struct kmem_cache *s, struct vchecker_cb *cb,
+ char *buf, size_t cnt);
+ void (*fini)(struct vchecker_cb *cb);
+ void (*show)(struct kmem_cache *s, struct seq_file *f,
+ struct vchecker_cb *cb, void *object);
+ bool (*check)(struct kmem_cache *s, struct vchecker_cb *cb,
+ void *object, bool write,
+ unsigned long begin, unsigned long end);
+};
+
+struct vchecker_cb {
+ unsigned long begin;
+ unsigned long end;
+ void *arg;
+ struct vchecker_type *type;
+ struct list_head list;
+};
+
+struct vchecker_value_arg {
+ u64 mask;
+ u64 value;
+};
+
+static struct dentry *debugfs_root;
+static struct vchecker_type vchecker_types[VCHECKER_TYPE_MAX];
+static DEFINE_MUTEX(vchecker_meta);
+static DEFINE_SPINLOCK(report_lock);
+
+static bool need_check(struct vchecker_cb *cb,
+ unsigned long begin, unsigned long end)
+{
+ if (cb->end <= begin)
+ return false;
+
+ if (cb->begin >= end)
+ return false;
+
+ return true;
+}
+
+static void show_cb(struct kmem_cache *s, struct seq_file *f,
+ struct vchecker_cb *cb, void *object)
+{
+ if (f) {
+ seq_printf(f, "%s checker for offset %ld ~ %ld\n",
+ cb->type->name, cb->begin, cb->end);
+ } else {
+ pr_err("%s checker for offset %ld ~ %ld at %p\n",
+ cb->type->name, cb->begin, cb->end, object);
+ }
+
+ cb->type->show(s, f, cb, object);
+}
+
+static void add_cb(struct kmem_cache *s, struct vchecker_cb *cb)
+{
+ list_add_tail(&cb->list, &s->vchecker_cache.checker->cb_list);
+}
+
+static int remove_cbs(struct kmem_cache *s, struct vchecker_type *t)
+{
+ struct vchecker *checker = s->vchecker_cache.checker;
+ struct vchecker_cb *cb, *tmp;
+
+ list_for_each_entry_safe(cb, tmp, &checker->cb_list, list) {
+ if (cb->type == t) {
+ list_del(&cb->list);
+ t->fini(cb);
+ kfree(cb);
+ }
+ }
+
+ return 0;
+}
+
+void vchecker_kmalloc(struct kmem_cache *s, const void *object, size_t size)
+{
+ struct vchecker *checker;
+ struct vchecker_cb *cb;
+
+ rcu_read_lock();
+ checker = s->vchecker_cache.checker;
+ if (!checker || !checker->enabled) {
+ rcu_read_unlock();
+ return;
+ }
+
+ list_for_each_entry(cb, &checker->cb_list, list) {
+ kasan_poison_shadow(object + cb->begin,
+ round_up(cb->end - cb->begin,
+ KASAN_SHADOW_SCALE_SIZE),
+ KASAN_VCHECKER_GRAYZONE);
+ }
+ rcu_read_unlock();
+}
+
+static void vchecker_report(unsigned long addr, size_t size, bool write,
+ unsigned long ret_ip, struct kmem_cache *s,
+ struct vchecker_cb *cb, void *object)
+{
+ unsigned long flags;
+ const char *bug_type = "invalid access";
+
+ kasan_disable_current();
+ spin_lock_irqsave(&report_lock, flags);
+ pr_err("==================================================================\n");
+ pr_err("BUG: VCHECKER: %s in %pS at addr %p\n",
+ bug_type, (void *)ret_ip, (void *)addr);
+ pr_err("%s of size %zu by task %s/%d\n",
+ write ? "Write" : "Read", size,
+ current->comm, task_pid_nr(current));
+ show_cb(s, NULL, cb, object);
+
+ describe_object(s, object, (const void *)addr);
+ pr_err("==================================================================\n");
+ add_taint(TAINT_BAD_PAGE, LOCKDEP_NOW_UNRELIABLE);
+ spin_unlock_irqrestore(&report_lock, flags);
+ if (panic_on_warn)
+ panic("panic_on_warn set ...\n");
+ kasan_enable_current();
+}
+
+static bool vchecker_poisoned(void *addr, size_t size)
+{
+ s8 shadow_val;
+ s8 *shadow_addr = kasan_mem_to_shadow(addr);
+ size_t shadow_size = kasan_mem_to_shadow(addr + size - 1) -
+ (void *)shadow_addr + 1;
+
+ while (shadow_size) {
+ shadow_val = *shadow_addr;
+ shadow_size--;
+ shadow_addr++;
+
+ if (shadow_val == 0)
+ continue;
+
+ if (shadow_val == (s8)KASAN_VCHECKER_GRAYZONE)
+ continue;
+
+ if (shadow_val < 0)
+ return false;
+
+ if (shadow_size)
+ return false;
+
+ /* last byte check */
+ if ((((unsigned long)addr + size - 1) & KASAN_SHADOW_MASK) >=
+ shadow_val)
+ return false;
+ }
+
+ return true;
+}
+
+bool vchecker_check(unsigned long addr, size_t size,
+ bool write, unsigned long ret_ip)
+{
+ struct page *page;
+ struct kmem_cache *s;
+ void *object;
+ struct vchecker *checker;
+ struct vchecker_cb *cb;
+ unsigned long begin, end;
+ bool checked = false;
+
+ if (current->kasan_depth)
+ return false;
+
+ page = virt_to_head_page((void *)addr);
+ if (!PageSlab(page))
+ return false;
+
+ s = page->slab_cache;
+ object = nearest_obj(s, page, (void *)addr);
+ begin = addr - (unsigned long)object;
+ end = begin + size;
+
+ rcu_read_lock();
+ checker = s->vchecker_cache.checker;
+ if (!checker->enabled) {
+ rcu_read_unlock();
+ goto check_shadow;
+ }
+
+ list_for_each_entry(cb, &checker->cb_list, list) {
+ if (!need_check(cb, begin, end))
+ continue;
+
+ checked = true;
+ if (cb->type->check(s, cb, object, write, begin, end))
+ continue;
+
+ vchecker_report(addr, size, write, ret_ip, s, cb, object);
+ rcu_read_unlock();
+ return true;
+ }
+ rcu_read_unlock();
+
+ if (checked)
+ return true;
+
+check_shadow:
+ return vchecker_poisoned((void *)addr, size);
+}
+
+static ssize_t vchecker_type_write(struct file *filp, const char __user *ubuf,
+ size_t cnt, loff_t *ppos,
+ enum vchecker_type_num type)
+{
+ char *buf;
+ struct kmem_cache *s = file_inode(filp)->i_private;
+ struct vchecker_type *t = NULL;
+ struct vchecker_cb *cb = NULL;
+ bool remove = false;
+ int ret = -EINVAL;
+
+ if (cnt >= PAGE_SIZE)
+ return -EINVAL;
+
+ buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ if (copy_from_user(buf, ubuf, cnt)) {
+ kfree(buf);
+ return -EFAULT;
+ }
+
+ if (isspace(buf[0]))
+ remove = true;
+ buf[cnt - 1] = '\0';
+
+ mutex_lock(&vchecker_meta);
+ if (s->vchecker_cache.checker->enabled)
+ goto err;
+
+ t = &vchecker_types[type];
+
+ if (remove) {
+ remove_cbs(s, t);
+ goto out;
+ }
+
+ cb = kzalloc(sizeof(*cb), GFP_KERNEL);
+ if (!cb) {
+ ret = -ENOMEM;
+ goto err;
+ }
+
+ cb->type = t;
+ INIT_LIST_HEAD(&cb->list);
+
+ ret = t->init(s, cb, buf, cnt);
+ if (ret)
+ goto err;
+
+ add_cb(s, cb);
+
+out:
+ mutex_unlock(&vchecker_meta);
+ kfree(buf);
+
+ return cnt;
+
+err:
+ mutex_unlock(&vchecker_meta);
+ kfree(buf);
+ kfree(cb);
+
+ return ret;
+}
+
+static int vchecker_type_show(struct seq_file *f, enum vchecker_type_num type)
+{
+ struct kmem_cache *s = f->private;
+ struct vchecker *checker;
+ struct vchecker_cb *cb;
+
+ mutex_lock(&vchecker_meta);
+ checker = s->vchecker_cache.checker;
+ list_for_each_entry(cb, &checker->cb_list, list) {
+ if (cb->type != &vchecker_types[type])
+ continue;
+
+ show_cb(s, f, cb, NULL);
+ }
+ mutex_unlock(&vchecker_meta);
+
+ return 0;
+}
+
+static int enable_show(struct seq_file *f, void *v)
+{
+ struct kmem_cache *s = f->private;
+ struct vchecker *checker = s->vchecker_cache.checker;
+ struct vchecker_cb *cb;
+
+ mutex_lock(&vchecker_meta);
+
+ seq_printf(f, "%s\n", checker->enabled ? "1" : "0");
+ list_for_each_entry(cb, &checker->cb_list, list)
+ show_cb(s, f, cb, NULL);
+
+ mutex_unlock(&vchecker_meta);
+
+ return 0;
+}
+
+static int enable_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, enable_show, inode->i_private);
+}
+
+static ssize_t enable_write(struct file *filp, const char __user *ubuf,
+ size_t cnt, loff_t *ppos)
+{
+ char enable_char;
+ bool enable;
+ struct kmem_cache *s = file_inode(filp)->i_private;
+
+ if (cnt >= PAGE_SIZE || cnt == 0)
+ return -EINVAL;
+
+ if (copy_from_user(&enable_char, ubuf, 1))
+ return -EFAULT;
+
+ if (enable_char == '0')
+ enable = false;
+ else if (enable_char == '1')
+ enable = true;
+ else
+ return -EINVAL;
+
+ mutex_lock(&vchecker_meta);
+ if (enable && list_empty(&s->vchecker_cache.checker->cb_list)) {
+ mutex_unlock(&vchecker_meta);
+ return -EINVAL;
+ }
+ s->vchecker_cache.checker->enabled = enable;
+
+ /*
+ * After this operation, it is guaranteed that there is no user
+ * left that accesses checker's cb list if vchecker is disabled.
+ */
+ synchronize_sched();
+ mutex_unlock(&vchecker_meta);
+
+ return cnt;
+}
+
+static const struct file_operations enable_fops = {
+ .open = enable_open,
+ .write = enable_write,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
+static int init_value(struct kmem_cache *s, struct vchecker_cb *cb,
+ char *buf, size_t cnt)
+{
+ unsigned long begin;
+ u64 mask;
+ u64 value;
+ struct vchecker_value_arg *arg;
+ unsigned long max_size = round_up(s->object_size, sizeof(u64));
+
+ BUILD_BUG_ON(sizeof(u64) != KASAN_SHADOW_SCALE_SIZE);
+
+ if (sscanf(buf, "%lu %llx %llu", &begin, &mask, &value) != 3)
+ return -EINVAL;
+
+ if (!IS_ALIGNED(begin, KASAN_SHADOW_SCALE_SIZE))
+ return -EINVAL;
+
+ if (begin > max_size - sizeof(value))
+ return -EINVAL;
+
+ arg = kzalloc(sizeof(struct vchecker_value_arg), GFP_KERNEL);
+ if (!arg)
+ return -ENOMEM;
+
+ arg->mask = mask;
+ arg->value = value;
+
+ cb->begin = begin;
+ cb->end = begin + sizeof(value);
+ cb->arg = arg;
+
+ return 0;
+}
+
+static void fini_value(struct vchecker_cb *cb)
+{
+ kfree(cb->arg);
+}
+
+static void show_value(struct kmem_cache *s, struct seq_file *f,
+ struct vchecker_cb *cb, void *object)
+{
+ struct vchecker_value_arg *arg = cb->arg;
+
+ if (f)
+ seq_printf(f, "(mask 0x%llx value %llu) invalid value %llu\n\n",
+ arg->mask, arg->value, arg->value & arg->mask);
+ else
+ pr_err("(mask 0x%llx value %llu) invalid value %llu\n\n",
+ arg->mask, arg->value, arg->value & arg->mask);
+}
+
+static bool check_value(struct kmem_cache *s, struct vchecker_cb *cb,
+ void *object, bool write,
+ unsigned long begin, unsigned long end)
+{
+ struct vchecker_value_arg *arg;
+ u64 value;
+
+ arg = cb->arg;
+ value = *(u64 *)(object + begin);
+ if ((value & arg->mask) != (arg->value & arg->mask))
+ return true;
+
+ return false;
+}
+
+static int value_show(struct seq_file *f, void *v)
+{
+ return vchecker_type_show(f, VCHECKER_TYPE_VALUE);
+}
+
+static int value_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, value_show, inode->i_private);
+}
+
+static ssize_t value_write(struct file *filp, const char __user *ubuf,
+ size_t cnt, loff_t *ppos)
+{
+ return vchecker_type_write(filp, ubuf, cnt, ppos,
+ VCHECKER_TYPE_VALUE);
+}
+
+static const struct file_operations fops_value = {
+ .open = value_open,
+ .write = value_write,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
+static struct vchecker_type vchecker_types[VCHECKER_TYPE_MAX] = {
+ { "value", &fops_value, init_value, fini_value,
+ show_value, check_value },
+};
+
+static void free_vchecker(struct kmem_cache *s)
+{
+ int i;
+
+ if (!s->vchecker_cache.checker)
+ return;
+
+ for (i = 0; i < ARRAY_SIZE(vchecker_types); i++)
+ remove_cbs(s, &vchecker_types[i]);
+ kfree(s->vchecker_cache.checker);
+}
+
+static void __fini_vchecker(struct kmem_cache *s)
+{
+ debugfs_remove_recursive(s->vchecker_cache.dir);
+ free_vchecker(s);
+}
+
+void fini_vchecker(struct kmem_cache *s)
+{
+ mutex_lock(&vchecker_meta);
+ __fini_vchecker(s);
+ mutex_unlock(&vchecker_meta);
+}
+
+static int alloc_vchecker(struct kmem_cache *s)
+{
+ struct vchecker *checker;
+
+ if (s->vchecker_cache.checker)
+ return 0;
+
+ checker = kzalloc(sizeof(*checker), GFP_KERNEL);
+ if (!checker)
+ return -ENOMEM;
+
+ INIT_LIST_HEAD(&checker->cb_list);
+ s->vchecker_cache.checker = checker;
+
+ return 0;
+}
+
+static int register_debugfs(struct kmem_cache *s)
+{
+ int i;
+ struct dentry *dir;
+ struct vchecker_type *t;
+
+ if (s->vchecker_cache.dir)
+ return 0;
+
+ dir = debugfs_create_dir(s->name, debugfs_root);
+ if (!dir)
+ return -ENOMEM;
+
+ s->vchecker_cache.dir = dir;
+ if (!debugfs_create_file("enable", 0600, dir, s, &enable_fops))
+ return -ENOMEM;
+
+ for (i = 0; i < ARRAY_SIZE(vchecker_types); i++) {
+ t = &vchecker_types[i];
+ if (!debugfs_create_file(t->name, 0600, dir, s, t->fops))
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
+int init_vchecker(struct kmem_cache *s)
+{
+ if (!debugfs_root || !s->name)
+ return 0;
+
+ mutex_lock(&vchecker_meta);
+ if (alloc_vchecker(s)) {
+ mutex_unlock(&vchecker_meta);
+ return -ENOMEM;
+ }
+
+ if (register_debugfs(s)) {
+ __fini_vchecker(s);
+ mutex_unlock(&vchecker_meta);
+ return -ENOMEM;
+ }
+ mutex_unlock(&vchecker_meta);
+
+ return 0;
+}
+
+static int __init vchecker_debugfs_init(void)
+{
+ debugfs_root = debugfs_create_dir("vchecker", NULL);
+ if (!debugfs_root)
+ return -ENOMEM;
+
+ init_vcheckers();
+
+ return 0;
+}
+core_initcall(vchecker_debugfs_init);
diff --git a/mm/kasan/vchecker.h b/mm/kasan/vchecker.h
new file mode 100644
index 0000000..77ba07d
--- /dev/null
+++ b/mm/kasan/vchecker.h
@@ -0,0 +1,31 @@
+#ifndef __MM_KASAN_VCHECKER_H
+#define __MM_KASAN_VCHECKER_H
+
+struct vchecker;
+struct vchecker_cb;
+
+struct vchecker_cache {
+ struct vchecker *checker;
+ struct dentry *dir;
+};
+
+
+#ifdef CONFIG_VCHECKER
+void vchecker_kmalloc(struct kmem_cache *s, const void *object, size_t size);
+bool vchecker_check(unsigned long addr, size_t size,
+ bool write, unsigned long ret_ip);
+int init_vchecker(struct kmem_cache *s);
+void fini_vchecker(struct kmem_cache *s);
+
+#else
+static inline void vchecker_kmalloc(struct kmem_cache *s,
+ const void *object, size_t size) { }
+static inline bool vchecker_check(unsigned long addr, size_t size,
+ bool write, unsigned long ret_ip) { return false; }
+static inline int init_vchecker(struct kmem_cache *s) { return 0; }
+static inline void fini_vchecker(struct kmem_cache *s) { }
+
+#endif
+
+
+#endif
diff --git a/mm/slab.h b/mm/slab.h
index 1f013f7..d054da8 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -32,6 +32,8 @@ struct kmem_cache {
#endif /* CONFIG_SLOB */
+#include "kasan/vchecker.h"
+
#ifdef CONFIG_SLAB
#include <linux/slab_def.h>
#endif
@@ -530,4 +532,8 @@ static inline int cache_random_seq_create(struct kmem_cache *cachep,
static inline void cache_random_seq_destroy(struct kmem_cache *cachep) { }
#endif /* CONFIG_SLAB_FREELIST_RANDOM */
+#ifdef CONFIG_VCHECKER
+void init_vcheckers(void);
+#endif
+
#endif /* MM_SLAB_H */
diff --git a/mm/slab_common.c b/mm/slab_common.c
index e96fb23d..6f700f3 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -25,6 +25,7 @@
#include <trace/events/kmem.h>
#include "slab.h"
+#include "kasan/vchecker.h"
enum slab_state slab_state;
LIST_HEAD(slab_caches);
@@ -396,6 +397,10 @@ static struct kmem_cache *create_cache(const char *name,
if (err)
goto out_free_cache;
+ err = init_vchecker(s);
+ if (err)
+ goto out_free_cache;
+
err = __kmem_cache_create(s, flags);
if (err)
goto out_free_cache;
@@ -409,6 +414,7 @@ static struct kmem_cache *create_cache(const char *name,
return s;
out_free_cache:
+ fini_vchecker(s);
destroy_memcg_params(s);
kmem_cache_free(kmem_cache, s);
goto out;
@@ -841,6 +847,7 @@ void slab_kmem_cache_release(struct kmem_cache *s)
{
__kmem_cache_release(s);
destroy_memcg_params(s);
+ fini_vchecker(s);
kfree_const(s->name);
kmem_cache_free(kmem_cache, s);
}
@@ -1216,6 +1223,18 @@ void cache_random_seq_destroy(struct kmem_cache *cachep)
}
#endif /* CONFIG_SLAB_FREELIST_RANDOM */
+#ifdef CONFIG_VCHECKER
+void __init init_vcheckers(void)
+{
+ struct kmem_cache *s;
+
+ mutex_lock(&slab_mutex);
+ list_for_each_entry(s, &slab_caches, list)
+ init_vchecker(s);
+ mutex_unlock(&slab_mutex);
+}
+#endif
+
#if defined(CONFIG_SLAB) || defined(CONFIG_SLUB_DEBUG)
#ifdef CONFIG_SLAB
#define SLABINFO_RIGHTS (S_IWUSR | S_IRUSR)
--
2.7.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 02/18] vchecker: introduce the valid access checker
2017-11-28 7:48 ` [PATCH 02/18] vchecker: introduce the valid access checker js1304
@ 2017-11-28 19:41 ` Andi Kleen
2017-11-29 5:36 ` Joonsoo Kim
2017-12-01 5:08 ` kbuild test robot
1 sibling, 1 reply; 29+ messages in thread
From: Andi Kleen @ 2017-11-28 19:41 UTC (permalink / raw)
To: js1304
Cc: Andrew Morton, Andrey Ryabinin, Alexander Potapenko,
Dmitry Vyukov, kasan-dev, linux-mm, linux-kernel, Namhyung Kim,
Wengang Wang, Joonsoo Kim
js1304@gmail.com writes:
> From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Looks useful. Essentially unlimited hardware break points, combined
with slab.
Didn't do a full review, but noticed some things below.
> +
> + buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
> +
> + if (copy_from_user(buf, ubuf, cnt)) {
> + kfree(buf);
> + return -EFAULT;
> + }
> +
> + if (isspace(buf[0]))
> + remove = true;
and that may be uninitialized.
and the space changes the operation? That's a strange syntax.
> + buf[cnt - 1] = '\0';
That's an underflow of one byte if cnt is 0.
-Andi
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 02/18] vchecker: introduce the valid access checker
2017-11-28 19:41 ` Andi Kleen
@ 2017-11-29 5:36 ` Joonsoo Kim
0 siblings, 0 replies; 29+ messages in thread
From: Joonsoo Kim @ 2017-11-29 5:36 UTC (permalink / raw)
To: Andi Kleen
Cc: Andrew Morton, Andrey Ryabinin, Alexander Potapenko,
Dmitry Vyukov, kasan-dev, linux-mm, linux-kernel, Namhyung Kim,
Wengang Wang
On Tue, Nov 28, 2017 at 11:41:08AM -0800, Andi Kleen wrote:
> js1304@gmail.com writes:
>
> > From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>
> Looks useful. Essentially unlimited hardware break points, combined
> with slab.
Thanks!!!
>
> Didn't do a full review, but noticed some things below.
> > +
> > + buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
> > + if (!buf)
> > + return -ENOMEM;
> > +
> > + if (copy_from_user(buf, ubuf, cnt)) {
> > + kfree(buf);
> > + return -EFAULT;
> > + }
> > +
> > + if (isspace(buf[0]))
> > + remove = true;
>
> and that may be uninitialized.
I will add 'cnt == 0' check above.
> and the space changes the operation? That's a strange syntax.
Intention is to clear the all the previous configuration when user
input is '\n'. Will fix it by checking '\n' directly.
>
> > + buf[cnt - 1] = '\0';
>
> That's an underflow of one byte if cnt is 0.
Will add 'cnt == 0' check above.
String parsing part in this patchset will not work properly when the
last input character is not '\n'. I will fix it on the next spin.
Thanks.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 02/18] vchecker: introduce the valid access checker
2017-11-28 7:48 ` [PATCH 02/18] vchecker: introduce the valid access checker js1304
2017-11-28 19:41 ` Andi Kleen
@ 2017-12-01 5:08 ` kbuild test robot
2017-12-01 8:01 ` Joonsoo Kim
1 sibling, 1 reply; 29+ messages in thread
From: kbuild test robot @ 2017-12-01 5:08 UTC (permalink / raw)
To: js1304
Cc: kbuild-all, Andrew Morton, Andrey Ryabinin, Alexander Potapenko,
Dmitry Vyukov, kasan-dev, linux-mm, linux-kernel, Namhyung Kim,
Wengang Wang, Joonsoo Kim
[-- Attachment #1: Type: text/plain, Size: 1374 bytes --]
Hi Joonsoo,
I love your patch! Yet something to improve:
[auto build test ERROR on mmotm/master]
[also build test ERROR on v4.15-rc1 next-20171130]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/js1304-gmail-com/introduce-a-new-tool-valid-access-checker/20171201-090908
base: git://git.cmpxchg.org/linux-mmotm.git master
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
In file included from drivers//block/skd_main.c:35:0:
>> include/linux/slab_def.h:84:24: error: field 'vchecker_cache' has incomplete type
struct vchecker_cache vchecker_cache;
^~~~~~~~~~~~~~
vim +/vchecker_cache +84 include/linux/slab_def.h
76
77 #ifdef CONFIG_MEMCG
78 struct memcg_cache_params memcg_params;
79 #endif
80 #ifdef CONFIG_KASAN
81 struct kasan_cache kasan_info;
82 #endif
83 #ifdef CONFIG_VCHECKER
> 84 struct vchecker_cache vchecker_cache;
85 #endif
86
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 62460 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 02/18] vchecker: introduce the valid access checker
2017-12-01 5:08 ` kbuild test robot
@ 2017-12-01 8:01 ` Joonsoo Kim
0 siblings, 0 replies; 29+ messages in thread
From: Joonsoo Kim @ 2017-12-01 8:01 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, Andrew Morton, Andrey Ryabinin, Alexander Potapenko,
Dmitry Vyukov, kasan-dev, linux-mm, linux-kernel, Namhyung Kim,
Wengang Wang
On Fri, Dec 01, 2017 at 01:08:13PM +0800, kbuild test robot wrote:
> Hi Joonsoo,
>
> I love your patch! Yet something to improve:
Thanks! I will fix all the error from kbuild bot on next spin.
Thanks.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 03/18] vchecker: mark/unmark the shadow of the allocated objects
2017-11-28 7:48 [PATCH 00/18] introduce a new tool, valid access checker js1304
2017-11-28 7:48 ` [PATCH 01/18] mm/kasan: make some kasan functions global js1304
2017-11-28 7:48 ` [PATCH 02/18] vchecker: introduce the valid access checker js1304
@ 2017-11-28 7:48 ` js1304
2017-11-28 7:48 ` [PATCH 04/18] vchecker: prepare per object memory for vchecker js1304
` (16 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: js1304 @ 2017-11-28 7:48 UTC (permalink / raw)
To: Andrew Morton
Cc: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel, Namhyung Kim, Wengang Wang, Joonsoo Kim
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Mark/unmark the shadow of the objects that is allocated before the
vchecker is enabled/disabled. It is necessary to fully debug the system.
Since there is no synchronization way to prevent slab object free,
we cannot synchronously mark/unmark the shadow of the allocated object.
Therefore, with this patch, it would be possible to overwrite
KASAN_KMALLOC_FREE shadow value to KASAN_VCHECKER_GRAYZONE/0 and
UAF check in KASAN would be missed. However, it is okay since
it happens rarely and we would decide to use this feature
as a last resort.
We can solve this race problem if another shadow memory is introduced.
It will be considered after the usefulness of the feature is justified.
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
include/linux/slab.h | 6 ++++++
mm/kasan/vchecker.c | 27 +++++++++++++++++++++++++++
mm/kasan/vchecker.h | 7 +++++++
mm/slab.c | 31 +++++++++++++++++++++++++++++++
mm/slab.h | 4 ++--
mm/slub.c | 36 ++++++++++++++++++++++++++++++++++--
6 files changed, 107 insertions(+), 4 deletions(-)
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 47e70e6..f6efbbe 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -108,6 +108,12 @@
#define SLAB_KASAN 0
#endif
+#ifdef CONFIG_VCHECKER
+#define SLAB_VCHECKER 0x10000000UL
+#else
+#define SLAB_VCHECKER 0x00000000UL
+#endif
+
/* The following flags affect the page allocator grouping pages by mobility */
/* Objects are reclaimable */
#define SLAB_RECLAIM_ACCOUNT ((slab_flags_t __force)0x00020000U)
diff --git a/mm/kasan/vchecker.c b/mm/kasan/vchecker.c
index 0ac031c..0b8a1e7 100644
--- a/mm/kasan/vchecker.c
+++ b/mm/kasan/vchecker.c
@@ -109,6 +109,12 @@ static int remove_cbs(struct kmem_cache *s, struct vchecker_type *t)
return 0;
}
+void vchecker_cache_create(struct kmem_cache *s,
+ size_t *size, slab_flags_t *flags)
+{
+ *flags |= SLAB_VCHECKER;
+}
+
void vchecker_kmalloc(struct kmem_cache *s, const void *object, size_t size)
{
struct vchecker *checker;
@@ -130,6 +136,26 @@ void vchecker_kmalloc(struct kmem_cache *s, const void *object, size_t size)
rcu_read_unlock();
}
+void vchecker_enable_obj(struct kmem_cache *s, const void *object,
+ size_t size, bool enable)
+{
+ struct vchecker *checker;
+ struct vchecker_cb *cb;
+ s8 shadow_val = READ_ONCE(*(s8 *)kasan_mem_to_shadow(object));
+ s8 mark = enable ? KASAN_VCHECKER_GRAYZONE : 0;
+
+ /* It would be freed object. We don't need to mark it */
+ if (shadow_val < 0 && (u8)shadow_val != KASAN_VCHECKER_GRAYZONE)
+ return;
+
+ checker = s->vchecker_cache.checker;
+ list_for_each_entry(cb, &checker->cb_list, list) {
+ kasan_poison_shadow(object + cb->begin,
+ round_up(cb->end - cb->begin,
+ KASAN_SHADOW_SCALE_SIZE), mark);
+ }
+}
+
static void vchecker_report(unsigned long addr, size_t size, bool write,
unsigned long ret_ip, struct kmem_cache *s,
struct vchecker_cb *cb, void *object)
@@ -380,6 +406,7 @@ static ssize_t enable_write(struct file *filp, const char __user *ubuf,
* left that accesses checker's cb list if vchecker is disabled.
*/
synchronize_sched();
+ vchecker_enable_cache(s, enable);
mutex_unlock(&vchecker_meta);
return cnt;
diff --git a/mm/kasan/vchecker.h b/mm/kasan/vchecker.h
index 77ba07d..aa22e8d 100644
--- a/mm/kasan/vchecker.h
+++ b/mm/kasan/vchecker.h
@@ -16,6 +16,11 @@ bool vchecker_check(unsigned long addr, size_t size,
bool write, unsigned long ret_ip);
int init_vchecker(struct kmem_cache *s);
void fini_vchecker(struct kmem_cache *s);
+void vchecker_cache_create(struct kmem_cache *s, size_t *size,
+ slab_flags_t *flags);
+void vchecker_enable_cache(struct kmem_cache *s, bool enable);
+void vchecker_enable_obj(struct kmem_cache *s, const void *object,
+ size_t size, bool enable);
#else
static inline void vchecker_kmalloc(struct kmem_cache *s,
@@ -24,6 +29,8 @@ static inline bool vchecker_check(unsigned long addr, size_t size,
bool write, unsigned long ret_ip) { return false; }
static inline int init_vchecker(struct kmem_cache *s) { return 0; }
static inline void fini_vchecker(struct kmem_cache *s) { }
+static inline void vchecker_cache_create(struct kmem_cache *s,
+ size_t *size, slab_flags_t *flags) {}
#endif
diff --git a/mm/slab.c b/mm/slab.c
index 78ea436..ba45c15 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -2551,6 +2551,37 @@ static inline bool shuffle_freelist(struct kmem_cache *cachep,
}
#endif /* CONFIG_SLAB_FREELIST_RANDOM */
+#ifdef CONFIG_VCHECKER
+static void __vchecker_enable_cache(struct kmem_cache *s,
+ struct page *page, bool enable)
+{
+ int i;
+ void *p;
+
+ for (i = 0; i < s->num; i++) {
+ p = index_to_obj(s, page, i);
+ vchecker_enable_obj(s, p, s->object_size, enable);
+ }
+}
+
+void vchecker_enable_cache(struct kmem_cache *s, bool enable)
+{
+ int node;
+ struct kmem_cache_node *n;
+ struct page *page;
+ unsigned long flags;
+
+ for_each_kmem_cache_node(s, node, n) {
+ spin_lock_irqsave(&n->list_lock, flags);
+ list_for_each_entry(page, &n->slabs_partial, lru)
+ __vchecker_enable_cache(s, page, enable);
+ list_for_each_entry(page, &n->slabs_full, lru)
+ __vchecker_enable_cache(s, page, enable);
+ spin_unlock_irqrestore(&n->list_lock, flags);
+ }
+}
+#endif
+
static void cache_init_objs(struct kmem_cache *cachep,
struct page *page)
{
diff --git a/mm/slab.h b/mm/slab.h
index d054da8..c1cf486 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -136,10 +136,10 @@ static inline slab_flags_t kmem_cache_flags(unsigned long object_size,
SLAB_TYPESAFE_BY_RCU | SLAB_DEBUG_OBJECTS )
#if defined(CONFIG_DEBUG_SLAB)
-#define SLAB_DEBUG_FLAGS (SLAB_RED_ZONE | SLAB_POISON | SLAB_STORE_USER)
+#define SLAB_DEBUG_FLAGS (SLAB_RED_ZONE | SLAB_POISON | SLAB_STORE_USER | SLAB_VCHECKER)
#elif defined(CONFIG_SLUB_DEBUG)
#define SLAB_DEBUG_FLAGS (SLAB_RED_ZONE | SLAB_POISON | SLAB_STORE_USER | \
- SLAB_TRACE | SLAB_CONSISTENCY_CHECKS)
+ SLAB_TRACE | SLAB_CONSISTENCY_CHECKS | SLAB_VCHECKER)
#else
#define SLAB_DEBUG_FLAGS (0)
#endif
diff --git a/mm/slub.c b/mm/slub.c
index bb8c949..67364cb 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1026,7 +1026,7 @@ static void trace(struct kmem_cache *s, struct page *page, void *object,
static void add_full(struct kmem_cache *s,
struct kmem_cache_node *n, struct page *page)
{
- if (!(s->flags & SLAB_STORE_USER))
+ if (!(s->flags & (SLAB_STORE_USER | SLAB_VCHECKER)))
return;
lockdep_assert_held(&n->list_lock);
@@ -1035,7 +1035,7 @@ static void add_full(struct kmem_cache *s,
static void remove_full(struct kmem_cache *s, struct kmem_cache_node *n, struct page *page)
{
- if (!(s->flags & SLAB_STORE_USER))
+ if (!(s->flags & (SLAB_STORE_USER | SLAB_VCHECKER)))
return;
lockdep_assert_held(&n->list_lock);
@@ -1555,6 +1555,38 @@ static inline bool shuffle_freelist(struct kmem_cache *s, struct page *page)
}
#endif /* CONFIG_SLAB_FREELIST_RANDOM */
+#ifdef CONFIG_VCHECKER
+static void __vchecker_enable_cache(struct kmem_cache *s,
+ struct page *page, bool enable)
+{
+ void *p;
+ void *addr = page_address(page);
+
+ if (!page->inuse)
+ return;
+
+ for_each_object(p, s, addr, page->objects)
+ vchecker_enable_obj(s, p, s->object_size, enable);
+}
+
+void vchecker_enable_cache(struct kmem_cache *s, bool enable)
+{
+ int node;
+ struct kmem_cache_node *n;
+ struct page *page;
+ unsigned long flags;
+
+ for_each_kmem_cache_node(s, node, n) {
+ spin_lock_irqsave(&n->list_lock, flags);
+ list_for_each_entry(page, &n->partial, lru)
+ __vchecker_enable_cache(s, page, enable);
+ list_for_each_entry(page, &n->full, lru)
+ __vchecker_enable_cache(s, page, enable);
+ spin_unlock_irqrestore(&n->list_lock, flags);
+ }
+}
+#endif
+
static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node)
{
struct page *page;
--
2.7.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 04/18] vchecker: prepare per object memory for vchecker
2017-11-28 7:48 [PATCH 00/18] introduce a new tool, valid access checker js1304
` (2 preceding siblings ...)
2017-11-28 7:48 ` [PATCH 03/18] vchecker: mark/unmark the shadow of the allocated objects js1304
@ 2017-11-28 7:48 ` js1304
2017-11-28 7:48 ` [PATCH 05/18] vchecker: store/report callstack of value writer js1304
` (15 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: js1304 @ 2017-11-28 7:48 UTC (permalink / raw)
To: Andrew Morton
Cc: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel, Namhyung Kim, Wengang Wang, Joonsoo Kim
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
To prepare per object memory for vchecker, we need to change the layout
of the object when kmem_cache initialization. Add such code on
vchecker_cache_create() which is called when kmem_cache initialization.
And, this memory should be initialized when object is populated. Do it
with another hook.
This memory will be used in the following patch.
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
mm/kasan/vchecker.c | 15 +++++++++++++++
mm/kasan/vchecker.h | 4 ++++
mm/slab.c | 2 ++
mm/slub.c | 2 ++
4 files changed, 23 insertions(+)
diff --git a/mm/kasan/vchecker.c b/mm/kasan/vchecker.c
index 0b8a1e7..be0f0cd 100644
--- a/mm/kasan/vchecker.c
+++ b/mm/kasan/vchecker.c
@@ -31,6 +31,10 @@ enum vchecker_type_num {
VCHECKER_TYPE_MAX,
};
+struct vchecker_data {
+ void *dummy;
+};
+
struct vchecker_type {
char *name;
const struct file_operations *fops;
@@ -109,10 +113,21 @@ static int remove_cbs(struct kmem_cache *s, struct vchecker_type *t)
return 0;
}
+void vchecker_init_slab_obj(struct kmem_cache *s, const void *object)
+{
+ struct vchecker_data *data;
+
+ data = (void *)object + s->vchecker_cache.data_offset;
+ __memset(data, 0, sizeof(*data));
+}
+
void vchecker_cache_create(struct kmem_cache *s,
size_t *size, slab_flags_t *flags)
{
*flags |= SLAB_VCHECKER;
+
+ s->vchecker_cache.data_offset = *size;
+ *size += sizeof(struct vchecker_data);
}
void vchecker_kmalloc(struct kmem_cache *s, const void *object, size_t size)
diff --git a/mm/kasan/vchecker.h b/mm/kasan/vchecker.h
index aa22e8d..efebc63 100644
--- a/mm/kasan/vchecker.h
+++ b/mm/kasan/vchecker.h
@@ -7,6 +7,7 @@ struct vchecker_cb;
struct vchecker_cache {
struct vchecker *checker;
struct dentry *dir;
+ int data_offset;
};
@@ -18,6 +19,7 @@ int init_vchecker(struct kmem_cache *s);
void fini_vchecker(struct kmem_cache *s);
void vchecker_cache_create(struct kmem_cache *s, size_t *size,
slab_flags_t *flags);
+void vchecker_init_slab_obj(struct kmem_cache *s, const void *object);
void vchecker_enable_cache(struct kmem_cache *s, bool enable);
void vchecker_enable_obj(struct kmem_cache *s, const void *object,
size_t size, bool enable);
@@ -31,6 +33,8 @@ static inline int init_vchecker(struct kmem_cache *s) { return 0; }
static inline void fini_vchecker(struct kmem_cache *s) { }
static inline void vchecker_cache_create(struct kmem_cache *s,
size_t *size, slab_flags_t *flags) {}
+static inline void vchecker_init_slab_obj(struct kmem_cache *s,
+ const void *object) {}
#endif
diff --git a/mm/slab.c b/mm/slab.c
index ba45c15..64d768b 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -2076,6 +2076,7 @@ int __kmem_cache_create(struct kmem_cache *cachep, slab_flags_t flags)
}
#endif
+ vchecker_cache_create(cachep, &size, &flags);
kasan_cache_create(cachep, &size, &flags);
size = ALIGN(size, cachep->align);
@@ -2601,6 +2602,7 @@ static void cache_init_objs(struct kmem_cache *cachep,
for (i = 0; i < cachep->num; i++) {
objp = index_to_obj(cachep, page, i);
+ vchecker_init_slab_obj(cachep, objp);
kasan_init_slab_obj(cachep, objp);
/* constructor could break poison info */
diff --git a/mm/slub.c b/mm/slub.c
index 67364cb..c099b33 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1418,6 +1418,7 @@ static void setup_object(struct kmem_cache *s, struct page *page,
void *object)
{
setup_object_debug(s, page, object);
+ vchecker_init_slab_obj(s, object);
kasan_init_slab_obj(s, object);
if (unlikely(s->ctor)) {
kasan_unpoison_object_data(s, object);
@@ -3550,6 +3551,7 @@ static int calculate_sizes(struct kmem_cache *s, int forced_order)
size += 2 * sizeof(struct track);
#endif
+ vchecker_cache_create(s, &size, &s->flags);
kasan_cache_create(s, &size, &s->flags);
#ifdef CONFIG_SLUB_DEBUG
if (flags & SLAB_RED_ZONE) {
--
2.7.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 05/18] vchecker: store/report callstack of value writer
2017-11-28 7:48 [PATCH 00/18] introduce a new tool, valid access checker js1304
` (3 preceding siblings ...)
2017-11-28 7:48 ` [PATCH 04/18] vchecker: prepare per object memory for vchecker js1304
@ 2017-11-28 7:48 ` js1304
2017-11-28 7:48 ` [PATCH 06/18] lib/stackdepot: Add is_new arg to depot_save_stack js1304
` (14 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: js1304 @ 2017-11-28 7:48 UTC (permalink / raw)
To: Andrew Morton
Cc: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel, Namhyung Kim, Wengang Wang, Joonsoo Kim
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
The purpose of the value checker is finding invalid user writing
invalid value at the moment that the value is written. However, there is
not enough infrastructure so that we cannot easily detect this case
in time.
However, by following way, we can emulate similar effect.
1. Store callstack when memory is written.
2. If check is failed when next access happen due to invalid written
value from previous write, report previous write-access callstack
It will caught offending user properly.
Following output "Invalid writer:" part is the result of this patch.
We find the invalid value writer at workfn_old_obj+0x14/0x50.
[ 49.400673] ==================================================================
[ 49.402297] BUG: VCHECKER: invalid access in workfn_old_obj+0x14/0x50 [vchecker_test] at addr ffff88002e9dc000
[ 49.403899] Write of size 8 by task kworker/0:2/465
[ 49.404538] value checker for offset 0 ~ 8 at ffff88002e9dc000
[ 49.405374] (mask 0xffff value 7) invalid value 7
[ 49.406016] Invalid writer:
[ 49.406302] workfn_old_obj+0x14/0x50 [vchecker_test]
[ 49.406973] process_one_work+0x3b5/0x9f0
[ 49.407463] worker_thread+0x87/0x750
[ 49.407895] kthread+0x1b2/0x200
[ 49.408252] ret_from_fork+0x24/0x30
[ 49.408723] Allocated by task 1326:
[ 49.409126] kasan_kmalloc+0xb9/0xe0
[ 49.409571] kmem_cache_alloc+0xd1/0x250
[ 49.410046] 0xffffffffa00c8157
[ 49.410389] do_one_initcall+0x82/0x1cf
[ 49.410851] do_init_module+0xe7/0x333
[ 49.411296] load_module+0x406b/0x4b40
[ 49.411745] SYSC_finit_module+0x14d/0x180
[ 49.412247] do_syscall_64+0xf0/0x340
[ 49.412674] return_from_SYSCALL_64+0x0/0x75
[ 49.413276] Freed by task 0:
[ 49.413566] (stack is not available)
[ 49.414034] The buggy address belongs to the object at ffff88002e9dc000
which belongs to the cache vchecker_test of size 8
[ 49.415708] The buggy address is located 0 bytes inside of
8-byte region [ffff88002e9dc000, ffff88002e9dc008)
[ 49.417148] ==================================================================
Correct implementation needs more modifications to various layers
so it is postponed until feasibility is proved.
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
mm/kasan/vchecker.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 59 insertions(+), 5 deletions(-)
diff --git a/mm/kasan/vchecker.c b/mm/kasan/vchecker.c
index be0f0cd..2e9f461 100644
--- a/mm/kasan/vchecker.c
+++ b/mm/kasan/vchecker.c
@@ -16,11 +16,14 @@
#include <linux/mutex.h>
#include <linux/kasan.h>
#include <linux/uaccess.h>
+#include <linux/stackdepot.h>
#include "vchecker.h"
#include "../slab.h"
#include "kasan.h"
+#define VCHECKER_STACK_DEPTH (16)
+
struct vchecker {
bool enabled;
struct list_head cb_list;
@@ -32,7 +35,7 @@ enum vchecker_type_num {
};
struct vchecker_data {
- void *dummy;
+ depot_stack_handle_t write_handle;
};
struct vchecker_type {
@@ -281,6 +284,24 @@ bool vchecker_check(unsigned long addr, size_t size,
return vchecker_poisoned((void *)addr, size);
}
+static noinline depot_stack_handle_t save_stack(void)
+{
+ unsigned long entries[VCHECKER_STACK_DEPTH];
+ struct stack_trace trace = {
+ .nr_entries = 0,
+ .entries = entries,
+ .max_entries = VCHECKER_STACK_DEPTH,
+ .skip = 0
+ };
+
+ save_stack_trace(&trace);
+ if (trace.nr_entries != 0 &&
+ trace.entries[trace.nr_entries-1] == ULONG_MAX)
+ trace.nr_entries--;
+
+ return depot_save_stack(&trace, GFP_NOWAIT);
+}
+
static ssize_t vchecker_type_write(struct file *filp, const char __user *ubuf,
size_t cnt, loff_t *ppos,
enum vchecker_type_num type)
@@ -474,17 +495,35 @@ static void fini_value(struct vchecker_cb *cb)
kfree(cb->arg);
}
+static void show_value_stack(struct vchecker_data *data)
+{
+ struct stack_trace trace;
+
+ if (!data->write_handle)
+ return;
+
+ pr_err("Invalid writer:\n");
+ depot_fetch_stack(data->write_handle, &trace);
+ print_stack_trace(&trace, 0);
+ pr_err("\n");
+}
+
static void show_value(struct kmem_cache *s, struct seq_file *f,
struct vchecker_cb *cb, void *object)
{
struct vchecker_value_arg *arg = cb->arg;
+ struct vchecker_data *data;
if (f)
seq_printf(f, "(mask 0x%llx value %llu) invalid value %llu\n\n",
arg->mask, arg->value, arg->value & arg->mask);
- else
+ else {
+ data = (void *)object + s->vchecker_cache.data_offset;
+
pr_err("(mask 0x%llx value %llu) invalid value %llu\n\n",
arg->mask, arg->value, arg->value & arg->mask);
+ show_value_stack(data);
+ }
}
static bool check_value(struct kmem_cache *s, struct vchecker_cb *cb,
@@ -492,14 +531,29 @@ static bool check_value(struct kmem_cache *s, struct vchecker_cb *cb,
unsigned long begin, unsigned long end)
{
struct vchecker_value_arg *arg;
+ struct vchecker_data *data;
u64 value;
+ depot_stack_handle_t handle;
+
+ if (!write)
+ goto check;
+
+ handle = save_stack();
+ if (!handle) {
+ pr_err("VCHECKER: %s: fail at addr %p\n", __func__, object);
+ dump_stack();
+ }
+ data = (void *)object + s->vchecker_cache.data_offset;
+ data->write_handle = handle;
+
+check:
arg = cb->arg;
value = *(u64 *)(object + begin);
- if ((value & arg->mask) != (arg->value & arg->mask))
- return true;
+ if ((value & arg->mask) == (arg->value & arg->mask))
+ return false;
- return false;
+ return true;
}
static int value_show(struct seq_file *f, void *v)
--
2.7.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 06/18] lib/stackdepot: Add is_new arg to depot_save_stack
2017-11-28 7:48 [PATCH 00/18] introduce a new tool, valid access checker js1304
` (4 preceding siblings ...)
2017-11-28 7:48 ` [PATCH 05/18] vchecker: store/report callstack of value writer js1304
@ 2017-11-28 7:48 ` js1304
2017-11-28 7:48 ` [PATCH 07/18] lib/stackdepot: extend stackdepot API to support per-user stackdepot js1304
` (13 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: js1304 @ 2017-11-28 7:48 UTC (permalink / raw)
To: Andrew Morton
Cc: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel, Namhyung Kim, Wengang Wang, Joonsoo Kim
From: Namhyung Kim <namhyung@kernel.org>
The is_new argument is to check whether the given stack trace was
already in the stack depot or newly added. It'll be used by vchecker
callstack in the next patch.
Also add WARN_ONCE if stack depot failed to allocate stack slab for some
reason. This is unusual as it allocates the stack slab before its use
but sometimes users might want to know its failure. Passing
__GFP_NOWARN in the alloc_flags will bypass it though.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
include/linux/stackdepot.h | 3 ++-
lib/stackdepot.c | 15 +++++++++++++--
mm/kasan/kasan.c | 2 +-
mm/kasan/vchecker.c | 2 +-
mm/page_owner.c | 4 ++--
5 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/include/linux/stackdepot.h b/include/linux/stackdepot.h
index 7978b3e..93363f2 100644
--- a/include/linux/stackdepot.h
+++ b/include/linux/stackdepot.h
@@ -25,7 +25,8 @@ typedef u32 depot_stack_handle_t;
struct stack_trace;
-depot_stack_handle_t depot_save_stack(struct stack_trace *trace, gfp_t flags);
+depot_stack_handle_t depot_save_stack(struct stack_trace *trace, gfp_t flags,
+ bool *is_new);
void depot_fetch_stack(depot_stack_handle_t handle, struct stack_trace *trace);
diff --git a/lib/stackdepot.c b/lib/stackdepot.c
index f87d138..e40ccb6 100644
--- a/lib/stackdepot.c
+++ b/lib/stackdepot.c
@@ -130,8 +130,11 @@ static struct stack_record *depot_alloc_stack(unsigned long *entries, int size,
smp_store_release(&next_slab_inited, 0);
}
init_stack_slab(prealloc);
- if (stack_slabs[depot_index] == NULL)
+ if (stack_slabs[depot_index] == NULL) {
+ if (!(alloc_flags & __GFP_NOWARN))
+ WARN_ONCE(1, "Stack depot failed to allocate stack_slabs");
return NULL;
+ }
stack = stack_slabs[depot_index] + depot_offset;
@@ -198,11 +201,12 @@ EXPORT_SYMBOL_GPL(depot_fetch_stack);
* depot_save_stack - save stack in a stack depot.
* @trace - the stacktrace to save.
* @alloc_flags - flags for allocating additional memory if required.
+ * @is_new - set #true when @trace was not in the stack depot.
*
* Returns the handle of the stack struct stored in depot.
*/
depot_stack_handle_t depot_save_stack(struct stack_trace *trace,
- gfp_t alloc_flags)
+ gfp_t alloc_flags, bool *is_new)
{
u32 hash;
depot_stack_handle_t retval = 0;
@@ -236,6 +240,8 @@ depot_stack_handle_t depot_save_stack(struct stack_trace *trace,
* |next_slab_inited| in depot_alloc_stack() and init_stack_slab().
*/
if (unlikely(!smp_load_acquire(&next_slab_inited))) {
+ gfp_t orig_flags = alloc_flags;
+
/*
* Zero out zone modifiers, as we don't have specific zone
* requirements. Keep the flags related to allocation in atomic
@@ -247,6 +253,9 @@ depot_stack_handle_t depot_save_stack(struct stack_trace *trace,
page = alloc_pages(alloc_flags, STACK_ALLOC_ORDER);
if (page)
prealloc = page_address(page);
+
+ /* restore flags to report failure in depot_alloc_stack() */
+ alloc_flags = orig_flags;
}
spin_lock_irqsave(&depot_lock, flags);
@@ -264,6 +273,8 @@ depot_stack_handle_t depot_save_stack(struct stack_trace *trace,
*/
smp_store_release(bucket, new);
found = new;
+ if (is_new)
+ *is_new = true;
}
} else if (prealloc) {
/*
diff --git a/mm/kasan/kasan.c b/mm/kasan/kasan.c
index 8fc4ad8..1b37e12 100644
--- a/mm/kasan/kasan.c
+++ b/mm/kasan/kasan.c
@@ -454,7 +454,7 @@ static inline depot_stack_handle_t save_stack(gfp_t flags)
trace.entries[trace.nr_entries-1] == ULONG_MAX)
trace.nr_entries--;
- return depot_save_stack(&trace, flags);
+ return depot_save_stack(&trace, flags, NULL);
}
static inline void set_track(struct kasan_track *track, gfp_t flags)
diff --git a/mm/kasan/vchecker.c b/mm/kasan/vchecker.c
index 2e9f461..82d4f1d 100644
--- a/mm/kasan/vchecker.c
+++ b/mm/kasan/vchecker.c
@@ -299,7 +299,7 @@ static noinline depot_stack_handle_t save_stack(void)
trace.entries[trace.nr_entries-1] == ULONG_MAX)
trace.nr_entries--;
- return depot_save_stack(&trace, GFP_NOWAIT);
+ return depot_save_stack(&trace, GFP_NOWAIT, NULL);
}
static ssize_t vchecker_type_write(struct file *filp, const char __user *ubuf,
diff --git a/mm/page_owner.c b/mm/page_owner.c
index f948acc..0e22eee 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -66,7 +66,7 @@ static __always_inline depot_stack_handle_t create_dummy_stack(void)
dummy.skip = 0;
save_stack_trace(&dummy);
- return depot_save_stack(&dummy, GFP_KERNEL);
+ return depot_save_stack(&dummy, GFP_KERNEL, NULL);
}
static noinline void register_dummy_stack(void)
@@ -162,7 +162,7 @@ static noinline depot_stack_handle_t save_stack(gfp_t flags)
if (check_recursive_alloc(&trace, _RET_IP_))
return dummy_handle;
- handle = depot_save_stack(&trace, flags);
+ handle = depot_save_stack(&trace, flags, NULL);
if (!handle)
handle = failure_handle;
--
2.7.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 07/18] lib/stackdepot: extend stackdepot API to support per-user stackdepot
2017-11-28 7:48 [PATCH 00/18] introduce a new tool, valid access checker js1304
` (5 preceding siblings ...)
2017-11-28 7:48 ` [PATCH 06/18] lib/stackdepot: Add is_new arg to depot_save_stack js1304
@ 2017-11-28 7:48 ` js1304
2017-11-28 7:48 ` [PATCH 08/18] vchecker: Add 'callstack' checker js1304
` (12 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: js1304 @ 2017-11-28 7:48 UTC (permalink / raw)
To: Andrew Morton
Cc: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel, Namhyung Kim, Wengang Wang, Joonsoo Kim
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
There is a usecase that check if stack trace is new or not during specific
period. Since stackdepot library doesn't support removal of stack trace,
it's impossible to know above thing. Since removal of stack trace is not
easy in the design of stackdepot library, we need another way to support
it. Therefore, this patch introduces per-user stackdepot. Although it
still cannot support removal of individual stack trace, it can be
destroyed totally. With it, we can implement correct is_new check
by using per-user stackdepot for specific period.
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
drivers/gpu/drm/drm_mm.c | 4 +-
include/linux/stackdepot.h | 11 +++--
lib/stackdepot.c | 115 ++++++++++++++++++++++++++++-----------------
mm/kasan/kasan.c | 2 +-
mm/kasan/report.c | 2 +-
mm/kasan/vchecker.c | 4 +-
mm/page_owner.c | 8 ++--
7 files changed, 90 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index eb86bc3..95b8291 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -118,7 +118,7 @@ static noinline void save_stack(struct drm_mm_node *node)
trace.nr_entries--;
/* May be called under spinlock, so avoid sleeping */
- node->stack = depot_save_stack(&trace, GFP_NOWAIT);
+ node->stack = depot_save_stack(NULL, &trace, GFP_NOWAIT, NULL);
}
static void show_leaks(struct drm_mm *mm)
@@ -143,7 +143,7 @@ static void show_leaks(struct drm_mm *mm)
continue;
}
- depot_fetch_stack(node->stack, &trace);
+ depot_fetch_stack(NULL, node->stack, &trace);
snprint_stack_trace(buf, BUFSZ, &trace, 0);
DRM_ERROR("node [%08llx + %08llx]: inserted at\n%s",
node->start, node->size, buf);
diff --git a/include/linux/stackdepot.h b/include/linux/stackdepot.h
index 93363f2..abcfe1b 100644
--- a/include/linux/stackdepot.h
+++ b/include/linux/stackdepot.h
@@ -24,10 +24,15 @@
typedef u32 depot_stack_handle_t;
struct stack_trace;
+struct stackdepot;
-depot_stack_handle_t depot_save_stack(struct stack_trace *trace, gfp_t flags,
- bool *is_new);
+depot_stack_handle_t depot_save_stack(struct stackdepot *s,
+ struct stack_trace *trace, gfp_t flags, bool *is_new);
-void depot_fetch_stack(depot_stack_handle_t handle, struct stack_trace *trace);
+void depot_fetch_stack(struct stackdepot *s,
+ depot_stack_handle_t handle, struct stack_trace *trace);
+
+struct stackdepot *create_stackdepot(void);
+void destroy_stackdepot(struct stackdepot *s);
#endif
diff --git a/lib/stackdepot.c b/lib/stackdepot.c
index e40ccb6..0a4fcb5 100644
--- a/lib/stackdepot.c
+++ b/lib/stackdepot.c
@@ -39,6 +39,7 @@
#include <linux/stackdepot.h>
#include <linux/string.h>
#include <linux/types.h>
+#include <linux/vmalloc.h>
#define DEPOT_STACK_BITS (sizeof(depot_stack_handle_t) * 8)
@@ -55,6 +56,11 @@
(((1LL << (STACK_ALLOC_INDEX_BITS)) < STACK_ALLOC_SLABS_CAP) ? \
(1LL << (STACK_ALLOC_INDEX_BITS)) : STACK_ALLOC_SLABS_CAP)
+#define STACK_HASH_ORDER 20
+#define STACK_HASH_SIZE (1L << STACK_HASH_ORDER)
+#define STACK_HASH_MASK (STACK_HASH_SIZE - 1)
+#define STACK_HASH_SEED 0x9747b28c
+
/* The compact structure to store the reference to stacks. */
union handle_parts {
depot_stack_handle_t handle;
@@ -73,14 +79,21 @@ struct stack_record {
unsigned long entries[1]; /* Variable-sized array of entries. */
};
-static void *stack_slabs[STACK_ALLOC_MAX_SLABS];
+struct stackdepot {
+ spinlock_t lock;
+ int depot_index;
+ int next_slab_inited;
+ size_t depot_offset;
-static int depot_index;
-static int next_slab_inited;
-static size_t depot_offset;
-static DEFINE_SPINLOCK(depot_lock);
+ void *stack_slabs[STACK_ALLOC_MAX_SLABS];
+ struct stack_record *stack_table[STACK_HASH_SIZE];
+};
-static bool init_stack_slab(void **prealloc)
+static struct stackdepot global_stackdepot = {
+ .lock = __SPIN_LOCK_UNLOCKED(global_stackdepot.lock),
+};
+
+static bool init_stack_slab(struct stackdepot *s, void **prealloc)
{
if (!*prealloc)
return false;
@@ -88,24 +101,25 @@ static bool init_stack_slab(void **prealloc)
* This smp_load_acquire() pairs with smp_store_release() to
* |next_slab_inited| below and in depot_alloc_stack().
*/
- if (smp_load_acquire(&next_slab_inited))
+ if (smp_load_acquire(&s->next_slab_inited))
return true;
- if (stack_slabs[depot_index] == NULL) {
- stack_slabs[depot_index] = *prealloc;
+ if (s->stack_slabs[s->depot_index] == NULL) {
+ s->stack_slabs[s->depot_index] = *prealloc;
} else {
- stack_slabs[depot_index + 1] = *prealloc;
+ s->stack_slabs[s->depot_index + 1] = *prealloc;
/*
* This smp_store_release pairs with smp_load_acquire() from
* |next_slab_inited| above and in depot_save_stack().
*/
- smp_store_release(&next_slab_inited, 1);
+ smp_store_release(&s->next_slab_inited, 1);
}
*prealloc = NULL;
return true;
}
/* Allocation of a new stack in raw storage */
-static struct stack_record *depot_alloc_stack(unsigned long *entries, int size,
+static struct stack_record *depot_alloc_stack(struct stackdepot *s,
+ unsigned long *entries, int size,
u32 hash, void **prealloc, gfp_t alloc_flags)
{
int required_size = offsetof(struct stack_record, entries) +
@@ -114,50 +128,41 @@ static struct stack_record *depot_alloc_stack(unsigned long *entries, int size,
required_size = ALIGN(required_size, 1 << STACK_ALLOC_ALIGN);
- if (unlikely(depot_offset + required_size > STACK_ALLOC_SIZE)) {
- if (unlikely(depot_index + 1 >= STACK_ALLOC_MAX_SLABS)) {
+ if (unlikely(s->depot_offset + required_size > STACK_ALLOC_SIZE)) {
+ if (unlikely(s->depot_index + 1 >= STACK_ALLOC_MAX_SLABS)) {
WARN_ONCE(1, "Stack depot reached limit capacity");
return NULL;
}
- depot_index++;
- depot_offset = 0;
+ s->depot_index++;
+ s->depot_offset = 0;
/*
* smp_store_release() here pairs with smp_load_acquire() from
* |next_slab_inited| in depot_save_stack() and
* init_stack_slab().
*/
- if (depot_index + 1 < STACK_ALLOC_MAX_SLABS)
- smp_store_release(&next_slab_inited, 0);
+ if (s->depot_index + 1 < STACK_ALLOC_MAX_SLABS)
+ smp_store_release(&s->next_slab_inited, 0);
}
- init_stack_slab(prealloc);
- if (stack_slabs[depot_index] == NULL) {
+ init_stack_slab(s, prealloc);
+ if (s->stack_slabs[s->depot_index] == NULL) {
if (!(alloc_flags & __GFP_NOWARN))
WARN_ONCE(1, "Stack depot failed to allocate stack_slabs");
return NULL;
}
- stack = stack_slabs[depot_index] + depot_offset;
+ stack = s->stack_slabs[s->depot_index] + s->depot_offset;
stack->hash = hash;
stack->size = size;
- stack->handle.slabindex = depot_index;
- stack->handle.offset = depot_offset >> STACK_ALLOC_ALIGN;
+ stack->handle.slabindex = s->depot_index;
+ stack->handle.offset = s->depot_offset >> STACK_ALLOC_ALIGN;
stack->handle.valid = 1;
memcpy(stack->entries, entries, size * sizeof(unsigned long));
- depot_offset += required_size;
+ s->depot_offset += required_size;
return stack;
}
-#define STACK_HASH_ORDER 20
-#define STACK_HASH_SIZE (1L << STACK_HASH_ORDER)
-#define STACK_HASH_MASK (STACK_HASH_SIZE - 1)
-#define STACK_HASH_SEED 0x9747b28c
-
-static struct stack_record *stack_table[STACK_HASH_SIZE] = {
- [0 ... STACK_HASH_SIZE - 1] = NULL
-};
-
/* Calculate hash for a stack */
static inline u32 hash_stack(unsigned long *entries, unsigned int size)
{
@@ -184,10 +189,12 @@ static inline struct stack_record *find_stack(struct stack_record *bucket,
return NULL;
}
-void depot_fetch_stack(depot_stack_handle_t handle, struct stack_trace *trace)
+void depot_fetch_stack(struct stackdepot *s,
+ depot_stack_handle_t handle, struct stack_trace *trace)
{
union handle_parts parts = { .handle = handle };
- void *slab = stack_slabs[parts.slabindex];
+ struct stackdepot *s2 = s ? : &global_stackdepot;
+ void *slab = s2->stack_slabs[parts.slabindex];
size_t offset = parts.offset << STACK_ALLOC_ALIGN;
struct stack_record *stack = slab + offset;
@@ -205,8 +212,8 @@ EXPORT_SYMBOL_GPL(depot_fetch_stack);
*
* Returns the handle of the stack struct stored in depot.
*/
-depot_stack_handle_t depot_save_stack(struct stack_trace *trace,
- gfp_t alloc_flags, bool *is_new)
+depot_stack_handle_t depot_save_stack(struct stackdepot *s,
+ struct stack_trace *trace, gfp_t alloc_flags, bool *is_new)
{
u32 hash;
depot_stack_handle_t retval = 0;
@@ -218,8 +225,11 @@ depot_stack_handle_t depot_save_stack(struct stack_trace *trace,
if (unlikely(trace->nr_entries == 0))
goto fast_exit;
+ if (!s)
+ s = &global_stackdepot;
+
hash = hash_stack(trace->entries, trace->nr_entries);
- bucket = &stack_table[hash & STACK_HASH_MASK];
+ bucket = &s->stack_table[hash & STACK_HASH_MASK];
/*
* Fast path: look the stack trace up without locking.
@@ -239,7 +249,7 @@ depot_stack_handle_t depot_save_stack(struct stack_trace *trace,
* The smp_load_acquire() here pairs with smp_store_release() to
* |next_slab_inited| in depot_alloc_stack() and init_stack_slab().
*/
- if (unlikely(!smp_load_acquire(&next_slab_inited))) {
+ if (unlikely(!smp_load_acquire(&s->next_slab_inited))) {
gfp_t orig_flags = alloc_flags;
/*
@@ -258,12 +268,12 @@ depot_stack_handle_t depot_save_stack(struct stack_trace *trace,
alloc_flags = orig_flags;
}
- spin_lock_irqsave(&depot_lock, flags);
+ spin_lock_irqsave(&s->lock, flags);
found = find_stack(*bucket, trace->entries, trace->nr_entries, hash);
if (!found) {
struct stack_record *new =
- depot_alloc_stack(trace->entries, trace->nr_entries,
+ depot_alloc_stack(s, trace->entries, trace->nr_entries,
hash, &prealloc, alloc_flags);
if (new) {
new->next = *bucket;
@@ -281,10 +291,10 @@ depot_stack_handle_t depot_save_stack(struct stack_trace *trace,
* We didn't need to store this stack trace, but let's keep
* the preallocated memory for the future.
*/
- WARN_ON(!init_stack_slab(&prealloc));
+ WARN_ON(!init_stack_slab(s, &prealloc));
}
- spin_unlock_irqrestore(&depot_lock, flags);
+ spin_unlock_irqrestore(&s->lock, flags);
exit:
if (prealloc) {
/* Nobody used this memory, ok to free it. */
@@ -296,3 +306,22 @@ depot_stack_handle_t depot_save_stack(struct stack_trace *trace,
return retval;
}
EXPORT_SYMBOL_GPL(depot_save_stack);
+
+struct stackdepot *create_stackdepot(void)
+{
+ struct stackdepot *s;
+
+ s = vzalloc(sizeof(*s));
+ if (!s)
+ return NULL;
+
+ spin_lock_init(&s->lock);
+ return s;
+}
+EXPORT_SYMBOL_GPL(create_stackdepot);
+
+void destroy_stackdepot(struct stackdepot *s)
+{
+ vfree(s);
+}
+EXPORT_SYMBOL_GPL(destroy_stackdepot);
diff --git a/mm/kasan/kasan.c b/mm/kasan/kasan.c
index 1b37e12..984e423 100644
--- a/mm/kasan/kasan.c
+++ b/mm/kasan/kasan.c
@@ -454,7 +454,7 @@ static inline depot_stack_handle_t save_stack(gfp_t flags)
trace.entries[trace.nr_entries-1] == ULONG_MAX)
trace.nr_entries--;
- return depot_save_stack(&trace, flags, NULL);
+ return depot_save_stack(NULL, &trace, flags, NULL);
}
static inline void set_track(struct kasan_track *track, gfp_t flags)
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index b78735a..6c83631 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -183,7 +183,7 @@ static void print_track(struct kasan_track *track, const char *prefix)
if (track->stack) {
struct stack_trace trace;
- depot_fetch_stack(track->stack, &trace);
+ depot_fetch_stack(NULL, track->stack, &trace);
print_stack_trace(&trace, 0);
} else {
pr_err("(stack is not available)\n");
diff --git a/mm/kasan/vchecker.c b/mm/kasan/vchecker.c
index 82d4f1d..15a1b18 100644
--- a/mm/kasan/vchecker.c
+++ b/mm/kasan/vchecker.c
@@ -299,7 +299,7 @@ static noinline depot_stack_handle_t save_stack(void)
trace.entries[trace.nr_entries-1] == ULONG_MAX)
trace.nr_entries--;
- return depot_save_stack(&trace, GFP_NOWAIT, NULL);
+ return depot_save_stack(NULL, &trace, GFP_NOWAIT, NULL);
}
static ssize_t vchecker_type_write(struct file *filp, const char __user *ubuf,
@@ -503,7 +503,7 @@ static void show_value_stack(struct vchecker_data *data)
return;
pr_err("Invalid writer:\n");
- depot_fetch_stack(data->write_handle, &trace);
+ depot_fetch_stack(NULL, data->write_handle, &trace);
print_stack_trace(&trace, 0);
pr_err("\n");
}
diff --git a/mm/page_owner.c b/mm/page_owner.c
index 0e22eee..627a955 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -66,7 +66,7 @@ static __always_inline depot_stack_handle_t create_dummy_stack(void)
dummy.skip = 0;
save_stack_trace(&dummy);
- return depot_save_stack(&dummy, GFP_KERNEL, NULL);
+ return depot_save_stack(NULL, &dummy, GFP_KERNEL, NULL);
}
static noinline void register_dummy_stack(void)
@@ -162,7 +162,7 @@ static noinline depot_stack_handle_t save_stack(gfp_t flags)
if (check_recursive_alloc(&trace, _RET_IP_))
return dummy_handle;
- handle = depot_save_stack(&trace, flags, NULL);
+ handle = depot_save_stack(NULL, &trace, flags, NULL);
if (!handle)
handle = failure_handle;
@@ -377,7 +377,7 @@ print_page_owner(char __user *buf, size_t count, unsigned long pfn,
if (ret >= count)
goto err;
- depot_fetch_stack(handle, &trace);
+ depot_fetch_stack(NULL, handle, &trace);
ret += snprint_stack_trace(kbuf + ret, count - ret, &trace, 0);
if (ret >= count)
goto err;
@@ -440,7 +440,7 @@ void __dump_page_owner(struct page *page)
return;
}
- depot_fetch_stack(handle, &trace);
+ depot_fetch_stack(NULL, handle, &trace);
pr_alert("page allocated via order %u, migratetype %s, gfp_mask %#x(%pGg)\n",
page_owner->order, migratetype_names[mt], gfp_mask, &gfp_mask);
print_stack_trace(&trace, 0);
--
2.7.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 08/18] vchecker: Add 'callstack' checker
2017-11-28 7:48 [PATCH 00/18] introduce a new tool, valid access checker js1304
` (6 preceding siblings ...)
2017-11-28 7:48 ` [PATCH 07/18] lib/stackdepot: extend stackdepot API to support per-user stackdepot js1304
@ 2017-11-28 7:48 ` js1304
2017-11-28 7:48 ` [PATCH 09/18] vchecker: Support toggle on/off of callstack check js1304
` (11 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: js1304 @ 2017-11-28 7:48 UTC (permalink / raw)
To: Andrew Morton
Cc: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel, Namhyung Kim, Wengang Wang, Joonsoo Kim
From: Namhyung Kim <namhyung@kernel.org>
The callstack checker is to find invalid code paths accessing to a
certain field in an object. Currently it only saves all stack traces at
the given offset. Reporting will be added in the next patch.
The below example checks callstack of anon_vma:
# cd /sys/kernel/debug/vchecker
# echo 0 8 > anon_vma/callstack # offset 0, size 8
# echo 1 > anon_vma/enable
# cat anon_vma/callstack # show saved callstacks
0x0 0x8 callstack
total: 42
callstack #0
anon_vma_fork+0x101/0x280
copy_process.part.10+0x15ff/0x2a40
_do_fork+0x155/0x7d0
SyS_clone+0x19/0x20
do_syscall_64+0xdf/0x460
return_from_SYSCALL_64+0x0/0x7a
...
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
mm/kasan/vchecker.c | 172 ++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 161 insertions(+), 11 deletions(-)
diff --git a/mm/kasan/vchecker.c b/mm/kasan/vchecker.c
index 15a1b18..0c9a4fc 100644
--- a/mm/kasan/vchecker.c
+++ b/mm/kasan/vchecker.c
@@ -31,6 +31,7 @@ struct vchecker {
enum vchecker_type_num {
VCHECKER_TYPE_VALUE = 0,
+ VCHECKER_TYPE_CALLSTACK,
VCHECKER_TYPE_MAX,
};
@@ -45,7 +46,7 @@ struct vchecker_type {
char *buf, size_t cnt);
void (*fini)(struct vchecker_cb *cb);
void (*show)(struct kmem_cache *s, struct seq_file *f,
- struct vchecker_cb *cb, void *object);
+ struct vchecker_cb *cb, void *object, bool verbose);
bool (*check)(struct kmem_cache *s, struct vchecker_cb *cb,
void *object, bool write,
unsigned long begin, unsigned long end);
@@ -64,6 +65,12 @@ struct vchecker_value_arg {
u64 value;
};
+#define CALLSTACK_MAX_HANDLE (PAGE_SIZE / sizeof(depot_stack_handle_t))
+struct vchecker_callstack_arg {
+ depot_stack_handle_t *handles;
+ atomic_t count;
+};
+
static struct dentry *debugfs_root;
static struct vchecker_type vchecker_types[VCHECKER_TYPE_MAX];
static DEFINE_MUTEX(vchecker_meta);
@@ -82,7 +89,7 @@ static bool need_check(struct vchecker_cb *cb,
}
static void show_cb(struct kmem_cache *s, struct seq_file *f,
- struct vchecker_cb *cb, void *object)
+ struct vchecker_cb *cb, void *object, bool verbose)
{
if (f) {
seq_printf(f, "%s checker for offset %ld ~ %ld\n",
@@ -92,7 +99,7 @@ static void show_cb(struct kmem_cache *s, struct seq_file *f,
cb->type->name, cb->begin, cb->end, object);
}
- cb->type->show(s, f, cb, object);
+ cb->type->show(s, f, cb, object, verbose);
}
static void add_cb(struct kmem_cache *s, struct vchecker_cb *cb)
@@ -189,7 +196,7 @@ static void vchecker_report(unsigned long addr, size_t size, bool write,
pr_err("%s of size %zu by task %s/%d\n",
write ? "Write" : "Read", size,
current->comm, task_pid_nr(current));
- show_cb(s, NULL, cb, object);
+ show_cb(s, NULL, cb, object, true);
describe_object(s, object, (const void *)addr);
pr_err("==================================================================\n");
@@ -284,14 +291,14 @@ bool vchecker_check(unsigned long addr, size_t size,
return vchecker_poisoned((void *)addr, size);
}
-static noinline depot_stack_handle_t save_stack(void)
+static noinline depot_stack_handle_t save_stack(int skip, bool *is_new)
{
unsigned long entries[VCHECKER_STACK_DEPTH];
struct stack_trace trace = {
.nr_entries = 0,
.entries = entries,
.max_entries = VCHECKER_STACK_DEPTH,
- .skip = 0
+ .skip = skip,
};
save_stack_trace(&trace);
@@ -299,7 +306,7 @@ static noinline depot_stack_handle_t save_stack(void)
trace.entries[trace.nr_entries-1] == ULONG_MAX)
trace.nr_entries--;
- return depot_save_stack(NULL, &trace, GFP_NOWAIT, NULL);
+ return depot_save_stack(NULL, &trace, GFP_NOWAIT, is_new);
}
static ssize_t vchecker_type_write(struct file *filp, const char __user *ubuf,
@@ -381,7 +388,7 @@ static int vchecker_type_show(struct seq_file *f, enum vchecker_type_num type)
if (cb->type != &vchecker_types[type])
continue;
- show_cb(s, f, cb, NULL);
+ show_cb(s, f, cb, NULL, true);
}
mutex_unlock(&vchecker_meta);
@@ -398,7 +405,7 @@ static int enable_show(struct seq_file *f, void *v)
seq_printf(f, "%s\n", checker->enabled ? "1" : "0");
list_for_each_entry(cb, &checker->cb_list, list)
- show_cb(s, f, cb, NULL);
+ show_cb(s, f, cb, NULL, false);
mutex_unlock(&vchecker_meta);
@@ -509,7 +516,7 @@ static void show_value_stack(struct vchecker_data *data)
}
static void show_value(struct kmem_cache *s, struct seq_file *f,
- struct vchecker_cb *cb, void *object)
+ struct vchecker_cb *cb, void *object, bool verbose)
{
struct vchecker_value_arg *arg = cb->arg;
struct vchecker_data *data;
@@ -538,7 +545,7 @@ static bool check_value(struct kmem_cache *s, struct vchecker_cb *cb,
if (!write)
goto check;
- handle = save_stack();
+ handle = save_stack(0, NULL);
if (!handle) {
pr_err("VCHECKER: %s: fail at addr %p\n", __func__, object);
dump_stack();
@@ -581,9 +588,152 @@ static const struct file_operations fops_value = {
.release = single_release,
};
+static int init_callstack(struct kmem_cache *s, struct vchecker_cb *cb,
+ char *buf, size_t cnt)
+{
+ unsigned long begin, len;
+ struct vchecker_callstack_arg *arg;
+ unsigned long max_size = round_up(s->object_size, sizeof(u64));
+
+ BUILD_BUG_ON(sizeof(u64) != KASAN_SHADOW_SCALE_SIZE);
+
+ if (sscanf(buf, "%lu %lu", &begin, &len) != 2)
+ return -EINVAL;
+
+ if (len > max_size || begin > max_size - len)
+ return -EINVAL;
+
+ arg = kzalloc(sizeof(struct vchecker_callstack_arg), GFP_KERNEL);
+ if (!arg)
+ return -ENOMEM;
+
+ arg->handles = (void *)get_zeroed_page(GFP_KERNEL);
+ if (!arg->handles) {
+ kfree(arg);
+ return -ENOMEM;
+ }
+ atomic_set(&arg->count, 0);
+
+ cb->begin = begin;
+ cb->end = begin + len;
+ cb->arg = arg;
+
+ return 0;
+}
+
+static void fini_callstack(struct vchecker_cb *cb)
+{
+ struct vchecker_callstack_arg *arg = cb->arg;
+
+ free_page((unsigned long)arg->handles);
+ kfree(arg);
+}
+
+static void show_callstack_handle(struct seq_file *f, int idx,
+ struct vchecker_callstack_arg *arg)
+{
+ struct stack_trace trace;
+ unsigned int i;
+
+ seq_printf(f, "callstack #%d\n", idx);
+
+ depot_fetch_stack(NULL, arg->handles[idx], &trace);
+
+ for (i = 0; i < trace.nr_entries; i++)
+ seq_printf(f, " %pS\n", (void *)trace.entries[i]);
+ seq_putc(f, '\n');
+}
+
+static void show_callstack(struct kmem_cache *s, struct seq_file *f,
+ struct vchecker_cb *cb, void *object, bool verbose)
+{
+ struct vchecker_callstack_arg *arg = cb->arg;
+ int count = atomic_read(&arg->count);
+ int i;
+
+ if (f) {
+ seq_printf(f, "total: %d\n", count);
+
+ if (!verbose)
+ return;
+
+ if (count > CALLSTACK_MAX_HANDLE) {
+ seq_printf(f, "callstack is overflowed: (%d / %ld)\n",
+ count, CALLSTACK_MAX_HANDLE);
+ count = CALLSTACK_MAX_HANDLE;
+ }
+
+ for (i = 0; i < count; i++)
+ show_callstack_handle(f, i, arg);
+ } else {
+ pr_err("invalid callstack found #%d\n", count - 1);
+ /* current stack trace will be shown by kasan_object_err() */
+ }
+}
+
+/*
+ * number of stacks to skip (at least).
+ *
+ * __asan_loadX -> vchecker_check -> cb->check() -> save_stack
+ * -> save_stack_trace
+ */
+#define STACK_SKIP 5
+
+static bool check_callstack(struct kmem_cache *s, struct vchecker_cb *cb,
+ void *object, bool write,
+ unsigned long begin, unsigned long end)
+{
+ u32 handle;
+ bool is_new = false;
+ struct vchecker_callstack_arg *arg = cb->arg;
+ int idx;
+
+ handle = save_stack(STACK_SKIP, &is_new);
+ if (!is_new)
+ return true;
+
+ idx = atomic_fetch_inc(&arg->count);
+
+ /* TODO: support handle table in multiple pages */
+ if (idx < CALLSTACK_MAX_HANDLE)
+ arg->handles[idx] = handle;
+
+ /* TODO: support reporting new callstack */
+ return true;
+}
+
+static int callstack_show(struct seq_file *f, void *v)
+{
+ return vchecker_type_show(f, VCHECKER_TYPE_CALLSTACK);
+}
+
+static int callstack_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, callstack_show, inode->i_private);
+}
+
+static ssize_t callstack_write(struct file *filp, const char __user *ubuf,
+ size_t cnt, loff_t *ppos)
+{
+ /* add a new (disabled) callstack checker at the given offset */
+ return vchecker_type_write(filp, ubuf, cnt, ppos,
+ VCHECKER_TYPE_CALLSTACK);
+}
+
+static const struct file_operations fops_callstack = {
+ .open = callstack_open,
+ .write = callstack_write,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
+/* also need to update enum VCHECKER_TYPE_XXX */
static struct vchecker_type vchecker_types[VCHECKER_TYPE_MAX] = {
{ "value", &fops_value, init_value, fini_value,
show_value, check_value },
+ { "callstack", &fops_callstack, init_callstack, fini_callstack,
+ show_callstack, check_callstack },
};
static void free_vchecker(struct kmem_cache *s)
--
2.7.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 09/18] vchecker: Support toggle on/off of callstack check
2017-11-28 7:48 [PATCH 00/18] introduce a new tool, valid access checker js1304
` (7 preceding siblings ...)
2017-11-28 7:48 ` [PATCH 08/18] vchecker: Add 'callstack' checker js1304
@ 2017-11-28 7:48 ` js1304
2017-11-28 7:48 ` [PATCH 10/18] vchecker: Use __GFP_ATOMIC to save stacktrace js1304
` (10 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: js1304 @ 2017-11-28 7:48 UTC (permalink / raw)
To: Andrew Morton
Cc: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel, Namhyung Kim, Wengang Wang, Joonsoo Kim
From: Namhyung Kim <namhyung@kernel.org>
By default, callstack checker only collects callchains. When a user
writes 'on' to the callstack file in debugfs, it checks and reports new
callstacks. Writing 'off' to disable it again.
# cd /sys/kernel/debug/vchecker
# echo 0 8 > anon_vma/callstack
# echo 1 > anon_vma/enable
... (do some work to collect enough callstacks) ...
# echo on > anon_vma/callstack
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
mm/kasan/vchecker.c | 31 +++++++++++++++++++++++++++++--
1 file changed, 29 insertions(+), 2 deletions(-)
diff --git a/mm/kasan/vchecker.c b/mm/kasan/vchecker.c
index 0c9a4fc..6b3824f 100644
--- a/mm/kasan/vchecker.c
+++ b/mm/kasan/vchecker.c
@@ -69,6 +69,7 @@ struct vchecker_value_arg {
struct vchecker_callstack_arg {
depot_stack_handle_t *handles;
atomic_t count;
+ bool enabled;
};
static struct dentry *debugfs_root;
@@ -698,8 +699,7 @@ static bool check_callstack(struct kmem_cache *s, struct vchecker_cb *cb,
if (idx < CALLSTACK_MAX_HANDLE)
arg->handles[idx] = handle;
- /* TODO: support reporting new callstack */
- return true;
+ return !arg->enabled;
}
static int callstack_show(struct seq_file *f, void *v)
@@ -712,9 +712,36 @@ static int callstack_open(struct inode *inode, struct file *file)
return single_open(file, callstack_show, inode->i_private);
}
+static void callstack_onoff(struct file *filp, bool enable)
+{
+ struct kmem_cache *s = file_inode(filp)->i_private;
+ struct vchecker_cb *cb;
+
+ mutex_lock(&vchecker_meta);
+ list_for_each_entry(cb, &s->vchecker_cache.checker->cb_list, list) {
+ if (cb->type == &vchecker_types[VCHECKER_TYPE_CALLSTACK]) {
+ struct vchecker_callstack_arg *arg = cb->arg;
+
+ arg->enabled = enable;
+ }
+ }
+ mutex_unlock(&vchecker_meta);
+}
+
static ssize_t callstack_write(struct file *filp, const char __user *ubuf,
size_t cnt, loff_t *ppos)
{
+ char buf[4];
+
+ if (copy_from_user(buf, ubuf, 4))
+ return -EFAULT;
+
+ /* turn on/off existing callstack checkers */
+ if (!strncmp(buf, "on", 2) || !strncmp(buf, "off", 3)) {
+ callstack_onoff(filp, buf[1] == 'n');
+ return cnt;
+ }
+
/* add a new (disabled) callstack checker at the given offset */
return vchecker_type_write(filp, ubuf, cnt, ppos,
VCHECKER_TYPE_CALLSTACK);
--
2.7.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 10/18] vchecker: Use __GFP_ATOMIC to save stacktrace
2017-11-28 7:48 [PATCH 00/18] introduce a new tool, valid access checker js1304
` (8 preceding siblings ...)
2017-11-28 7:48 ` [PATCH 09/18] vchecker: Support toggle on/off of callstack check js1304
@ 2017-11-28 7:48 ` js1304
2017-11-28 7:48 ` [PATCH 11/18] vchecker: consistently exclude vchecker's stacktrace js1304
` (9 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: js1304 @ 2017-11-28 7:48 UTC (permalink / raw)
To: Andrew Morton
Cc: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel, Namhyung Kim, Wengang Wang, Joonsoo Kim
From: Namhyung Kim <namhyung@kernel.org>
Since we're finding a cause of broken data, it'd be desired not to miss
any suspects. It doesn't use GFP_ATOMIC since it includes __GFP_HIGH
which is for system making forward progress.
It also adds a WARN_ON whenever it fails to allocate pages even with
__GFP_ATOMIC.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
mm/kasan/vchecker.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/mm/kasan/vchecker.c b/mm/kasan/vchecker.c
index 6b3824f..df480d5 100644
--- a/mm/kasan/vchecker.c
+++ b/mm/kasan/vchecker.c
@@ -301,13 +301,20 @@ static noinline depot_stack_handle_t save_stack(int skip, bool *is_new)
.max_entries = VCHECKER_STACK_DEPTH,
.skip = skip,
};
+ depot_stack_handle_t handle;
save_stack_trace(&trace);
if (trace.nr_entries != 0 &&
trace.entries[trace.nr_entries-1] == ULONG_MAX)
trace.nr_entries--;
- return depot_save_stack(NULL, &trace, GFP_NOWAIT, is_new);
+ if (trace.nr_entries == 0)
+ return 0;
+
+ handle = depot_save_stack(NULL, &trace, __GFP_ATOMIC, is_new);
+ WARN_ON(!handle);
+
+ return handle;
}
static ssize_t vchecker_type_write(struct file *filp, const char __user *ubuf,
--
2.7.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 11/18] vchecker: consistently exclude vchecker's stacktrace
2017-11-28 7:48 [PATCH 00/18] introduce a new tool, valid access checker js1304
` (9 preceding siblings ...)
2017-11-28 7:48 ` [PATCH 10/18] vchecker: Use __GFP_ATOMIC to save stacktrace js1304
@ 2017-11-28 7:48 ` js1304
2017-11-28 7:48 ` [PATCH 12/18] vchecker: fix 'remove' handling on callstack checker js1304
` (8 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: js1304 @ 2017-11-28 7:48 UTC (permalink / raw)
To: Andrew Morton
Cc: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel, Namhyung Kim, Wengang Wang, Joonsoo Kim
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Since there is a different callpath even in the vchecker, static skip
value doesn't always exclude vchecker's stacktrace. Fix it through
checking stacktrace dynamically.
v2: skip two depth of stack at default, it's safe!
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
mm/kasan/vchecker.c | 41 +++++++++++++++++++++++++----------------
1 file changed, 25 insertions(+), 16 deletions(-)
diff --git a/mm/kasan/vchecker.c b/mm/kasan/vchecker.c
index df480d5..dc3a9a7 100644
--- a/mm/kasan/vchecker.c
+++ b/mm/kasan/vchecker.c
@@ -23,6 +23,7 @@
#include "kasan.h"
#define VCHECKER_STACK_DEPTH (16)
+#define VCHECKER_SKIP_DEPTH (2)
struct vchecker {
bool enabled;
@@ -48,7 +49,7 @@ struct vchecker_type {
void (*show)(struct kmem_cache *s, struct seq_file *f,
struct vchecker_cb *cb, void *object, bool verbose);
bool (*check)(struct kmem_cache *s, struct vchecker_cb *cb,
- void *object, bool write,
+ void *object, bool write, unsigned long ret_ip,
unsigned long begin, unsigned long end);
};
@@ -276,7 +277,7 @@ bool vchecker_check(unsigned long addr, size_t size,
continue;
checked = true;
- if (cb->type->check(s, cb, object, write, begin, end))
+ if (cb->type->check(s, cb, object, write, ret_ip, begin, end))
continue;
vchecker_report(addr, size, write, ret_ip, s, cb, object);
@@ -292,14 +293,29 @@ bool vchecker_check(unsigned long addr, size_t size,
return vchecker_poisoned((void *)addr, size);
}
-static noinline depot_stack_handle_t save_stack(int skip, bool *is_new)
+static void filter_vchecker_stacks(struct stack_trace *trace,
+ unsigned long ret_ip)
+{
+ int i;
+
+ for (i = 0; i < trace->nr_entries; i++) {
+ if (trace->entries[i] == ret_ip) {
+ trace->entries = &trace->entries[i];
+ trace->nr_entries -= i;
+ break;
+ }
+ }
+}
+
+static noinline depot_stack_handle_t save_stack(unsigned long ret_ip,
+ bool *is_new)
{
unsigned long entries[VCHECKER_STACK_DEPTH];
struct stack_trace trace = {
.nr_entries = 0,
.entries = entries,
.max_entries = VCHECKER_STACK_DEPTH,
- .skip = skip,
+ .skip = VCHECKER_SKIP_DEPTH,
};
depot_stack_handle_t handle;
@@ -311,6 +327,7 @@ static noinline depot_stack_handle_t save_stack(int skip, bool *is_new)
if (trace.nr_entries == 0)
return 0;
+ filter_vchecker_stacks(&trace, ret_ip);
handle = depot_save_stack(NULL, &trace, __GFP_ATOMIC, is_new);
WARN_ON(!handle);
@@ -542,7 +559,7 @@ static void show_value(struct kmem_cache *s, struct seq_file *f,
}
static bool check_value(struct kmem_cache *s, struct vchecker_cb *cb,
- void *object, bool write,
+ void *object, bool write, unsigned long ret_ip,
unsigned long begin, unsigned long end)
{
struct vchecker_value_arg *arg;
@@ -553,7 +570,7 @@ static bool check_value(struct kmem_cache *s, struct vchecker_cb *cb,
if (!write)
goto check;
- handle = save_stack(0, NULL);
+ handle = save_stack(ret_ip, NULL);
if (!handle) {
pr_err("VCHECKER: %s: fail at addr %p\n", __func__, object);
dump_stack();
@@ -679,16 +696,8 @@ static void show_callstack(struct kmem_cache *s, struct seq_file *f,
}
}
-/*
- * number of stacks to skip (at least).
- *
- * __asan_loadX -> vchecker_check -> cb->check() -> save_stack
- * -> save_stack_trace
- */
-#define STACK_SKIP 5
-
static bool check_callstack(struct kmem_cache *s, struct vchecker_cb *cb,
- void *object, bool write,
+ void *object, bool write, unsigned long ret_ip,
unsigned long begin, unsigned long end)
{
u32 handle;
@@ -696,7 +705,7 @@ static bool check_callstack(struct kmem_cache *s, struct vchecker_cb *cb,
struct vchecker_callstack_arg *arg = cb->arg;
int idx;
- handle = save_stack(STACK_SKIP, &is_new);
+ handle = save_stack(ret_ip, &is_new);
if (!is_new)
return true;
--
2.7.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 12/18] vchecker: fix 'remove' handling on callstack checker
2017-11-28 7:48 [PATCH 00/18] introduce a new tool, valid access checker js1304
` (10 preceding siblings ...)
2017-11-28 7:48 ` [PATCH 11/18] vchecker: consistently exclude vchecker's stacktrace js1304
@ 2017-11-28 7:48 ` js1304
2017-11-28 7:48 ` [PATCH 13/18] mm/vchecker: support inline KASAN build js1304
` (7 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: js1304 @ 2017-11-28 7:48 UTC (permalink / raw)
To: Andrew Morton
Cc: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel, Namhyung Kim, Wengang Wang, Joonsoo Kim
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Since stack depot library doesn't support removal operation,
after removing and adding again callstack cb, callstack checker cannot
correctly judge whether this callstack is new or not for current cb
if the same callstack happens for previous cb.
This problem can be fixed by per-user stack depot since
we can create/destroy it when callstack cb is created/destroyed.
With that, is_new always shows correct result for current cb.
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
mm/kasan/vchecker.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/mm/kasan/vchecker.c b/mm/kasan/vchecker.c
index dc3a9a7..acead62 100644
--- a/mm/kasan/vchecker.c
+++ b/mm/kasan/vchecker.c
@@ -68,6 +68,7 @@ struct vchecker_value_arg {
#define CALLSTACK_MAX_HANDLE (PAGE_SIZE / sizeof(depot_stack_handle_t))
struct vchecker_callstack_arg {
+ struct stackdepot *s;
depot_stack_handle_t *handles;
atomic_t count;
bool enabled;
@@ -307,8 +308,8 @@ static void filter_vchecker_stacks(struct stack_trace *trace,
}
}
-static noinline depot_stack_handle_t save_stack(unsigned long ret_ip,
- bool *is_new)
+static noinline depot_stack_handle_t save_stack(struct stackdepot *s,
+ unsigned long ret_ip, bool *is_new)
{
unsigned long entries[VCHECKER_STACK_DEPTH];
struct stack_trace trace = {
@@ -328,7 +329,7 @@ static noinline depot_stack_handle_t save_stack(unsigned long ret_ip,
return 0;
filter_vchecker_stacks(&trace, ret_ip);
- handle = depot_save_stack(NULL, &trace, __GFP_ATOMIC, is_new);
+ handle = depot_save_stack(s, &trace, __GFP_ATOMIC, is_new);
WARN_ON(!handle);
return handle;
@@ -570,7 +571,7 @@ static bool check_value(struct kmem_cache *s, struct vchecker_cb *cb,
if (!write)
goto check;
- handle = save_stack(ret_ip, NULL);
+ handle = save_stack(NULL, ret_ip, NULL);
if (!handle) {
pr_err("VCHECKER: %s: fail at addr %p\n", __func__, object);
dump_stack();
@@ -637,6 +638,14 @@ static int init_callstack(struct kmem_cache *s, struct vchecker_cb *cb,
kfree(arg);
return -ENOMEM;
}
+
+ arg->s = create_stackdepot();
+ if (!arg->s) {
+ free_page((unsigned long)arg->handles);
+ kfree(arg);
+ return -ENOMEM;
+ }
+
atomic_set(&arg->count, 0);
cb->begin = begin;
@@ -650,6 +659,7 @@ static void fini_callstack(struct vchecker_cb *cb)
{
struct vchecker_callstack_arg *arg = cb->arg;
+ destroy_stackdepot(arg->s);
free_page((unsigned long)arg->handles);
kfree(arg);
}
@@ -662,7 +672,7 @@ static void show_callstack_handle(struct seq_file *f, int idx,
seq_printf(f, "callstack #%d\n", idx);
- depot_fetch_stack(NULL, arg->handles[idx], &trace);
+ depot_fetch_stack(arg->s, arg->handles[idx], &trace);
for (i = 0; i < trace.nr_entries; i++)
seq_printf(f, " %pS\n", (void *)trace.entries[i]);
@@ -705,7 +715,7 @@ static bool check_callstack(struct kmem_cache *s, struct vchecker_cb *cb,
struct vchecker_callstack_arg *arg = cb->arg;
int idx;
- handle = save_stack(ret_ip, &is_new);
+ handle = save_stack(arg->s, ret_ip, &is_new);
if (!is_new)
return true;
--
2.7.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 13/18] mm/vchecker: support inline KASAN build
2017-11-28 7:48 [PATCH 00/18] introduce a new tool, valid access checker js1304
` (11 preceding siblings ...)
2017-11-28 7:48 ` [PATCH 12/18] vchecker: fix 'remove' handling on callstack checker js1304
@ 2017-11-28 7:48 ` js1304
2017-11-28 7:48 ` [PATCH 14/18] mm/vchecker: make callstack depth configurable js1304
` (6 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: js1304 @ 2017-11-28 7:48 UTC (permalink / raw)
To: Andrew Morton
Cc: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel, Namhyung Kim, Wengang Wang, Joonsoo Kim
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
There is no reason not to support inline KASAN build. Support it.
Note that vchecker_check() function is now placed on kasan report function
to support inline build because gcc generates the inline check code and
then directly jump to kasan report function when poisoned value is found.
Name is somewhat misleading but there is no problem in the view of
implementation.
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
lib/Kconfig.kasan | 1 -
mm/kasan/report.c | 8 ++++++++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
index 51c0a05..d3552f3 100644
--- a/lib/Kconfig.kasan
+++ b/lib/Kconfig.kasan
@@ -63,6 +63,5 @@ config VCHECKER
happens at the area.
depends on KASAN && DEBUG_FS
- select KASAN_OUTLINE
endif
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 6c83631..3d002aa 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -413,6 +413,8 @@ void kasan_report(unsigned long addr, size_t size,
#define DEFINE_ASAN_REPORT_LOAD(size) \
void __asan_report_load##size##_noabort(unsigned long addr) \
{ \
+ if (vchecker_check(addr, size, false, _RET_IP_)) \
+ return; \
kasan_report(addr, size, false, _RET_IP_); \
} \
EXPORT_SYMBOL(__asan_report_load##size##_noabort)
@@ -420,6 +422,8 @@ EXPORT_SYMBOL(__asan_report_load##size##_noabort)
#define DEFINE_ASAN_REPORT_STORE(size) \
void __asan_report_store##size##_noabort(unsigned long addr) \
{ \
+ if (vchecker_check(addr, size, true, _RET_IP_)) \
+ return; \
kasan_report(addr, size, true, _RET_IP_); \
} \
EXPORT_SYMBOL(__asan_report_store##size##_noabort)
@@ -437,12 +441,16 @@ DEFINE_ASAN_REPORT_STORE(16);
void __asan_report_load_n_noabort(unsigned long addr, size_t size)
{
+ if (vchecker_check(addr, size, false, _RET_IP_))
+ return;
kasan_report(addr, size, false, _RET_IP_);
}
EXPORT_SYMBOL(__asan_report_load_n_noabort);
void __asan_report_store_n_noabort(unsigned long addr, size_t size)
{
+ if (vchecker_check(addr, size, true, _RET_IP_))
+ return;
kasan_report(addr, size, true, _RET_IP_);
}
EXPORT_SYMBOL(__asan_report_store_n_noabort);
--
2.7.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 14/18] mm/vchecker: make callstack depth configurable
2017-11-28 7:48 [PATCH 00/18] introduce a new tool, valid access checker js1304
` (12 preceding siblings ...)
2017-11-28 7:48 ` [PATCH 13/18] mm/vchecker: support inline KASAN build js1304
@ 2017-11-28 7:48 ` js1304
2017-11-28 7:48 ` [PATCH 15/18] mm/vchecker: pass allocation caller address to vchecker hook js1304
` (5 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: js1304 @ 2017-11-28 7:48 UTC (permalink / raw)
To: Andrew Morton
Cc: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel, Namhyung Kim, Wengang Wang, Joonsoo Kim
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Getting full callstack is heavy job so it's sometimes better to
reduce this overhead by limiting callstack depth. So, this patch
makes the callstack depth configurable by using debugfs interface.
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
mm/kasan/vchecker.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 77 insertions(+), 4 deletions(-)
diff --git a/mm/kasan/vchecker.c b/mm/kasan/vchecker.c
index acead62..4d140e7 100644
--- a/mm/kasan/vchecker.c
+++ b/mm/kasan/vchecker.c
@@ -27,6 +27,7 @@
struct vchecker {
bool enabled;
+ unsigned int callstack_depth;
struct list_head cb_list;
};
@@ -309,13 +310,14 @@ static void filter_vchecker_stacks(struct stack_trace *trace,
}
static noinline depot_stack_handle_t save_stack(struct stackdepot *s,
- unsigned long ret_ip, bool *is_new)
+ unsigned long ret_ip, unsigned int max_entries,
+ bool *is_new)
{
unsigned long entries[VCHECKER_STACK_DEPTH];
struct stack_trace trace = {
.nr_entries = 0,
.entries = entries,
- .max_entries = VCHECKER_STACK_DEPTH,
+ .max_entries = max_entries,
.skip = VCHECKER_SKIP_DEPTH,
};
depot_stack_handle_t handle;
@@ -489,6 +491,70 @@ static const struct file_operations enable_fops = {
.release = single_release,
};
+static int callstack_depth_show(struct seq_file *f, void *v)
+{
+ struct kmem_cache *s = f->private;
+ struct vchecker *checker = s->vchecker_cache.checker;
+
+ mutex_lock(&vchecker_meta);
+ seq_printf(f, "%u\n", checker->callstack_depth);
+ mutex_unlock(&vchecker_meta);
+
+ return 0;
+}
+
+static int callstack_depth_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, callstack_depth_show, inode->i_private);
+}
+
+static ssize_t callstack_depth_write(struct file *filp, const char __user *ubuf,
+ size_t cnt, loff_t *ppos)
+{
+ char callstack_depth_chars[32];
+ unsigned int callstack_depth;
+ struct kmem_cache *s = file_inode(filp)->i_private;
+
+ if (cnt >= 32 || cnt == 0)
+ return -EINVAL;
+
+ if (copy_from_user(&callstack_depth_chars, ubuf, cnt))
+ return -EFAULT;
+
+ if (isspace(callstack_depth_chars[0])) {
+ callstack_depth = VCHECKER_STACK_DEPTH;
+ goto setup;
+ }
+
+ callstack_depth_chars[cnt - 1] = '\0';
+ if (kstrtouint(callstack_depth_chars, 10, &callstack_depth))
+ return -EINVAL;
+
+ if (callstack_depth > VCHECKER_STACK_DEPTH)
+ callstack_depth = VCHECKER_STACK_DEPTH;
+
+setup:
+ mutex_lock(&vchecker_meta);
+ if (s->vchecker_cache.checker->enabled ||
+ !list_empty(&s->vchecker_cache.checker->cb_list)) {
+ mutex_unlock(&vchecker_meta);
+ return -EINVAL;
+ }
+
+ s->vchecker_cache.checker->callstack_depth = callstack_depth;
+ mutex_unlock(&vchecker_meta);
+
+ return cnt;
+}
+
+static const struct file_operations callstack_depth_fops = {
+ .open = callstack_depth_open,
+ .write = callstack_depth_write,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
static int init_value(struct kmem_cache *s, struct vchecker_cb *cb,
char *buf, size_t cnt)
{
@@ -571,7 +637,8 @@ static bool check_value(struct kmem_cache *s, struct vchecker_cb *cb,
if (!write)
goto check;
- handle = save_stack(NULL, ret_ip, NULL);
+ handle = save_stack(NULL, ret_ip,
+ s->vchecker_cache.checker->callstack_depth, NULL);
if (!handle) {
pr_err("VCHECKER: %s: fail at addr %p\n", __func__, object);
dump_stack();
@@ -715,7 +782,8 @@ static bool check_callstack(struct kmem_cache *s, struct vchecker_cb *cb,
struct vchecker_callstack_arg *arg = cb->arg;
int idx;
- handle = save_stack(arg->s, ret_ip, &is_new);
+ handle = save_stack(arg->s, ret_ip,
+ s->vchecker_cache.checker->callstack_depth, &is_new);
if (!is_new)
return true;
@@ -825,6 +893,7 @@ static int alloc_vchecker(struct kmem_cache *s)
if (!checker)
return -ENOMEM;
+ checker->callstack_depth = VCHECKER_STACK_DEPTH;
INIT_LIST_HEAD(&checker->cb_list);
s->vchecker_cache.checker = checker;
@@ -848,6 +917,10 @@ static int register_debugfs(struct kmem_cache *s)
if (!debugfs_create_file("enable", 0600, dir, s, &enable_fops))
return -ENOMEM;
+ if (!debugfs_create_file("callstack_depth", 0600, dir, s,
+ &callstack_depth_fops))
+ return -ENOMEM;
+
for (i = 0; i < ARRAY_SIZE(vchecker_types); i++) {
t = &vchecker_types[i];
if (!debugfs_create_file(t->name, 0600, dir, s, t->fops))
--
2.7.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 15/18] mm/vchecker: pass allocation caller address to vchecker hook
2017-11-28 7:48 [PATCH 00/18] introduce a new tool, valid access checker js1304
` (13 preceding siblings ...)
2017-11-28 7:48 ` [PATCH 14/18] mm/vchecker: make callstack depth configurable js1304
@ 2017-11-28 7:48 ` js1304
2017-12-01 2:39 ` kbuild test robot
2017-12-01 3:01 ` kbuild test robot
2017-11-28 7:48 ` [PATCH 16/18] mm/vchecker: support allocation caller filter js1304
` (4 subsequent siblings)
19 siblings, 2 replies; 29+ messages in thread
From: js1304 @ 2017-11-28 7:48 UTC (permalink / raw)
To: Andrew Morton
Cc: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel, Namhyung Kim, Wengang Wang, Joonsoo Kim
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Vchecker requires kmalloc caller address to support validation on
*specific* kmalloc user. Therefore, this patch passes slab allocation
caller address to vchecker hook. This caller address will be used in the
following patch.
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
include/linux/slab.h | 2 ++
mm/kasan/kasan.c | 1 -
mm/kasan/vchecker.c | 3 ++-
mm/kasan/vchecker.h | 5 +++--
mm/slab.c | 14 ++++++++++----
mm/slab.h | 4 +++-
mm/slab_common.c | 6 ++++++
mm/slub.c | 11 ++++++++---
8 files changed, 34 insertions(+), 12 deletions(-)
diff --git a/include/linux/slab.h b/include/linux/slab.h
index f6efbbe..25a74f3c 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -421,6 +421,7 @@ static __always_inline void *kmem_cache_alloc_trace(struct kmem_cache *s,
void *ret = kmem_cache_alloc(s, flags);
kasan_kmalloc(s, ret, size, flags);
+ vchecker_kmalloc(s, ret, size, _THIS_IP_);
return ret;
}
@@ -432,6 +433,7 @@ kmem_cache_alloc_node_trace(struct kmem_cache *s,
void *ret = kmem_cache_alloc_node(s, gfpflags, node);
kasan_kmalloc(s, ret, size, gfpflags);
+ vchecker_kmalloc(s, ret, size, _THIS_IP_);
return ret;
}
#endif /* CONFIG_TRACING */
diff --git a/mm/kasan/kasan.c b/mm/kasan/kasan.c
index 984e423..8634e43 100644
--- a/mm/kasan/kasan.c
+++ b/mm/kasan/kasan.c
@@ -552,7 +552,6 @@ void kasan_kmalloc(struct kmem_cache *cache, const void *object, size_t size,
kasan_unpoison_shadow(object, size);
kasan_poison_shadow((void *)redzone_start, redzone_end - redzone_start,
KASAN_KMALLOC_REDZONE);
- vchecker_kmalloc(cache, object, size);
if (cache->flags & SLAB_KASAN)
set_track(&get_alloc_info(cache, object)->alloc_track, flags);
diff --git a/mm/kasan/vchecker.c b/mm/kasan/vchecker.c
index 4d140e7..918f05a 100644
--- a/mm/kasan/vchecker.c
+++ b/mm/kasan/vchecker.c
@@ -144,7 +144,8 @@ void vchecker_cache_create(struct kmem_cache *s,
*size += sizeof(struct vchecker_data);
}
-void vchecker_kmalloc(struct kmem_cache *s, const void *object, size_t size)
+void vchecker_kmalloc(struct kmem_cache *s, const void *object, size_t size,
+ unsigned long ret_ip)
{
struct vchecker *checker;
struct vchecker_cb *cb;
diff --git a/mm/kasan/vchecker.h b/mm/kasan/vchecker.h
index efebc63..ab5a6f6 100644
--- a/mm/kasan/vchecker.h
+++ b/mm/kasan/vchecker.h
@@ -12,7 +12,8 @@ struct vchecker_cache {
#ifdef CONFIG_VCHECKER
-void vchecker_kmalloc(struct kmem_cache *s, const void *object, size_t size);
+void vchecker_kmalloc(struct kmem_cache *s, const void *object, size_t size,
+ unsigned long ret_ip);
bool vchecker_check(unsigned long addr, size_t size,
bool write, unsigned long ret_ip);
int init_vchecker(struct kmem_cache *s);
@@ -26,7 +27,7 @@ void vchecker_enable_obj(struct kmem_cache *s, const void *object,
#else
static inline void vchecker_kmalloc(struct kmem_cache *s,
- const void *object, size_t size) { }
+ const void *object, size_t size, unsigned long ret_ip) { }
static inline bool vchecker_check(unsigned long addr, size_t size,
bool write, unsigned long ret_ip) { return false; }
static inline int init_vchecker(struct kmem_cache *s) { return 0; }
diff --git a/mm/slab.c b/mm/slab.c
index 64d768b..f6b1adf 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -3359,7 +3359,7 @@ slab_alloc_node(struct kmem_cache *cachep, gfp_t flags, int nodeid,
if (unlikely(flags & __GFP_ZERO) && ptr)
memset(ptr, 0, cachep->object_size);
- slab_post_alloc_hook(cachep, flags, 1, &ptr);
+ slab_post_alloc_hook(cachep, flags, 1, &ptr, caller);
return ptr;
}
@@ -3416,7 +3416,7 @@ slab_alloc(struct kmem_cache *cachep, gfp_t flags, unsigned long caller)
if (unlikely(flags & __GFP_ZERO) && objp)
memset(objp, 0, cachep->object_size);
- slab_post_alloc_hook(cachep, flags, 1, &objp);
+ slab_post_alloc_hook(cachep, flags, 1, &objp, caller);
return objp;
}
@@ -3579,6 +3579,7 @@ void *kmem_cache_alloc(struct kmem_cache *cachep, gfp_t flags)
void *ret = slab_alloc(cachep, flags, _RET_IP_);
kasan_slab_alloc(cachep, ret, flags);
+ vchecker_kmalloc(cachep, ret, cachep->object_size, _RET_IP_);
trace_kmem_cache_alloc(_RET_IP_, ret,
cachep->object_size, cachep->size, flags);
@@ -3624,13 +3625,13 @@ int kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t size,
for (i = 0; i < size; i++)
memset(p[i], 0, s->object_size);
- slab_post_alloc_hook(s, flags, size, p);
+ slab_post_alloc_hook(s, flags, size, p, _RET_IP_);
/* FIXME: Trace call missing. Christoph would like a bulk variant */
return size;
error:
local_irq_enable();
cache_alloc_debugcheck_after_bulk(s, flags, i, p, _RET_IP_);
- slab_post_alloc_hook(s, flags, i, p);
+ slab_post_alloc_hook(s, flags, i, p, _RET_IP_);
__kmem_cache_free_bulk(s, i, p);
return 0;
}
@@ -3645,6 +3646,7 @@ kmem_cache_alloc_trace(struct kmem_cache *cachep, gfp_t flags, size_t size)
ret = slab_alloc(cachep, flags, _RET_IP_);
kasan_kmalloc(cachep, ret, size, flags);
+ vchecker_kmalloc(cachep, ret, size, _RET_IP_);
trace_kmalloc(_RET_IP_, ret,
size, cachep->size, flags);
return ret;
@@ -3669,6 +3671,7 @@ void *kmem_cache_alloc_node(struct kmem_cache *cachep, gfp_t flags, int nodeid)
void *ret = slab_alloc_node(cachep, flags, nodeid, _RET_IP_);
kasan_slab_alloc(cachep, ret, flags);
+ vchecker_kmalloc(cachep, ret, cachep->object_size, _RET_IP_);
trace_kmem_cache_alloc_node(_RET_IP_, ret,
cachep->object_size, cachep->size,
flags, nodeid);
@@ -3688,6 +3691,7 @@ void *kmem_cache_alloc_node_trace(struct kmem_cache *cachep,
ret = slab_alloc_node(cachep, flags, nodeid, _RET_IP_);
kasan_kmalloc(cachep, ret, size, flags);
+ vchecker_kmalloc(cachep, ret, size, _RET_IP_);
trace_kmalloc_node(_RET_IP_, ret,
size, cachep->size,
flags, nodeid);
@@ -3707,6 +3711,7 @@ __do_kmalloc_node(size_t size, gfp_t flags, int node, unsigned long caller)
return cachep;
ret = kmem_cache_alloc_node_trace(cachep, flags, node, size);
kasan_kmalloc(cachep, ret, size, flags);
+ vchecker_kmalloc(cachep, ret, size, _RET_IP_);
return ret;
}
@@ -3743,6 +3748,7 @@ static __always_inline void *__do_kmalloc(size_t size, gfp_t flags,
ret = slab_alloc(cachep, flags, caller);
kasan_kmalloc(cachep, ret, size, flags);
+ vchecker_kmalloc(cachep, ret, size, _RET_IP_);
trace_kmalloc(caller, ret,
size, cachep->size, flags);
diff --git a/mm/slab.h b/mm/slab.h
index c1cf486..9fce8ab 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -435,7 +435,8 @@ static inline struct kmem_cache *slab_pre_alloc_hook(struct kmem_cache *s,
}
static inline void slab_post_alloc_hook(struct kmem_cache *s, gfp_t flags,
- size_t size, void **p)
+ size_t size, void **p,
+ unsigned long caller)
{
size_t i;
@@ -446,6 +447,7 @@ static inline void slab_post_alloc_hook(struct kmem_cache *s, gfp_t flags,
kmemleak_alloc_recursive(object, s->object_size, 1,
s->flags, flags);
kasan_slab_alloc(s, object, flags);
+ vchecker_kmalloc(s, object, s->object_size, caller);
}
if (memcg_kmem_enabled())
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 6f700f3..ffc7515 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -1446,12 +1446,18 @@ static __always_inline void *__do_krealloc(const void *p, size_t new_size,
{
void *ret;
size_t ks = 0;
+ struct page *page;
if (p)
ks = ksize(p);
if (ks >= new_size) {
kasan_krealloc((void *)p, new_size, flags);
+ page = virt_to_head_page(p);
+ if (PageSlab(page)) {
+ vchecker_kmalloc(page->slab_cache, p,
+ new_size, _RET_IP_);
+ }
return (void *)p;
}
diff --git a/mm/slub.c b/mm/slub.c
index c099b33..d37f023 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2755,7 +2755,7 @@ static __always_inline void *slab_alloc_node(struct kmem_cache *s,
if (unlikely(gfpflags & __GFP_ZERO) && object)
memset(object, 0, s->object_size);
- slab_post_alloc_hook(s, gfpflags, 1, &object);
+ slab_post_alloc_hook(s, gfpflags, 1, &object, addr);
return object;
}
@@ -2783,6 +2783,7 @@ void *kmem_cache_alloc_trace(struct kmem_cache *s, gfp_t gfpflags, size_t size)
void *ret = slab_alloc(s, gfpflags, _RET_IP_);
trace_kmalloc(_RET_IP_, ret, size, s->size, gfpflags);
kasan_kmalloc(s, ret, size, gfpflags);
+ vchecker_kmalloc(s, ret, size, _RET_IP_);
return ret;
}
EXPORT_SYMBOL(kmem_cache_alloc_trace);
@@ -2811,6 +2812,7 @@ void *kmem_cache_alloc_node_trace(struct kmem_cache *s,
size, s->size, gfpflags, node);
kasan_kmalloc(s, ret, size, gfpflags);
+ vchecker_kmalloc(s, ret, size, _RET_IP_);
return ret;
}
EXPORT_SYMBOL(kmem_cache_alloc_node_trace);
@@ -3184,11 +3186,11 @@ int kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t size,
}
/* memcg and kmem_cache debug support */
- slab_post_alloc_hook(s, flags, size, p);
+ slab_post_alloc_hook(s, flags, size, p, _RET_IP_);
return i;
error:
local_irq_enable();
- slab_post_alloc_hook(s, flags, i, p);
+ slab_post_alloc_hook(s, flags, i, p, _RET_IP_);
__kmem_cache_free_bulk(s, i, p);
return 0;
}
@@ -3388,6 +3390,7 @@ static void early_kmem_cache_node_alloc(int node)
#endif
kasan_kmalloc(kmem_cache_node, n, sizeof(struct kmem_cache_node),
GFP_KERNEL);
+ vchecker_kmalloc(kmem_cache_node, n, sizeof(*n), _RET_IP_);
init_kmem_cache_node(n);
inc_slabs_node(kmem_cache_node, node, page->objects);
@@ -3794,6 +3797,7 @@ void *__kmalloc(size_t size, gfp_t flags)
trace_kmalloc(_RET_IP_, ret, size, s->size, flags);
kasan_kmalloc(s, ret, size, flags);
+ vchecker_kmalloc(s, ret, size, _RET_IP_);
return ret;
}
@@ -3839,6 +3843,7 @@ void *__kmalloc_node(size_t size, gfp_t flags, int node)
trace_kmalloc_node(_RET_IP_, ret, size, s->size, flags, node);
kasan_kmalloc(s, ret, size, flags);
+ vchecker_kmalloc(s, ret, size, _RET_IP_);
return ret;
}
--
2.7.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 15/18] mm/vchecker: pass allocation caller address to vchecker hook
2017-11-28 7:48 ` [PATCH 15/18] mm/vchecker: pass allocation caller address to vchecker hook js1304
@ 2017-12-01 2:39 ` kbuild test robot
2017-12-01 3:01 ` kbuild test robot
1 sibling, 0 replies; 29+ messages in thread
From: kbuild test robot @ 2017-12-01 2:39 UTC (permalink / raw)
To: js1304
Cc: kbuild-all, Andrew Morton, Andrey Ryabinin, Alexander Potapenko,
Dmitry Vyukov, kasan-dev, linux-mm, linux-kernel, Namhyung Kim,
Wengang Wang, Joonsoo Kim
[-- Attachment #1: Type: text/plain, Size: 1921 bytes --]
Hi Joonsoo,
I love your patch! Yet something to improve:
[auto build test ERROR on mmotm/master]
[also build test ERROR on v4.15-rc1 next-20171130]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/js1304-gmail-com/introduce-a-new-tool-valid-access-checker/20171201-090908
base: git://git.cmpxchg.org/linux-mmotm.git master
config: x86_64-randconfig-x011-201748 (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
In file included from include/linux/crypto.h:24:0,
from arch/x86/kernel/asm-offsets.c:9:
include/linux/slab.h: In function 'kmem_cache_alloc_trace':
>> include/linux/slab.h:424:2: error: implicit declaration of function 'vchecker_kmalloc'; did you mean '__kmalloc'? [-Werror=implicit-function-declaration]
vchecker_kmalloc(s, ret, size, _THIS_IP_);
^~~~~~~~~~~~~~~~
__kmalloc
cc1: some warnings being treated as errors
make[2]: *** [arch/x86/kernel/asm-offsets.s] Error 1
make[2]: Target '__build' not remade because of errors.
make[1]: *** [prepare0] Error 2
make[1]: Target 'prepare' not remade because of errors.
make: *** [sub-make] Error 2
vim +424 include/linux/slab.h
416
417 #else /* CONFIG_TRACING */
418 static __always_inline void *kmem_cache_alloc_trace(struct kmem_cache *s,
419 gfp_t flags, size_t size)
420 {
421 void *ret = kmem_cache_alloc(s, flags);
422
423 kasan_kmalloc(s, ret, size, flags);
> 424 vchecker_kmalloc(s, ret, size, _THIS_IP_);
425 return ret;
426 }
427
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33760 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 15/18] mm/vchecker: pass allocation caller address to vchecker hook
2017-11-28 7:48 ` [PATCH 15/18] mm/vchecker: pass allocation caller address to vchecker hook js1304
2017-12-01 2:39 ` kbuild test robot
@ 2017-12-01 3:01 ` kbuild test robot
1 sibling, 0 replies; 29+ messages in thread
From: kbuild test robot @ 2017-12-01 3:01 UTC (permalink / raw)
To: js1304
Cc: kbuild-all, Andrew Morton, Andrey Ryabinin, Alexander Potapenko,
Dmitry Vyukov, kasan-dev, linux-mm, linux-kernel, Namhyung Kim,
Wengang Wang, Joonsoo Kim
[-- Attachment #1: Type: text/plain, Size: 1891 bytes --]
Hi Joonsoo,
I love your patch! Yet something to improve:
[auto build test ERROR on mmotm/master]
[also build test ERROR on v4.15-rc1 next-20171130]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/js1304-gmail-com/introduce-a-new-tool-valid-access-checker/20171201-090908
base: git://git.cmpxchg.org/linux-mmotm.git master
config: i386-randconfig-s0-201748 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
In file included from include/linux/crypto.h:24:0,
from arch/x86/kernel/asm-offsets.c:9:
include/linux/slab.h: In function 'kmem_cache_alloc_trace':
>> include/linux/slab.h:424:2: error: implicit declaration of function 'vchecker_kmalloc' [-Werror=implicit-function-declaration]
vchecker_kmalloc(s, ret, size, _THIS_IP_);
^~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
make[2]: *** [arch/x86/kernel/asm-offsets.s] Error 1
make[2]: Target '__build' not remade because of errors.
make[1]: *** [prepare0] Error 2
make[1]: Target 'prepare' not remade because of errors.
make: *** [sub-make] Error 2
vim +/vchecker_kmalloc +424 include/linux/slab.h
416
417 #else /* CONFIG_TRACING */
418 static __always_inline void *kmem_cache_alloc_trace(struct kmem_cache *s,
419 gfp_t flags, size_t size)
420 {
421 void *ret = kmem_cache_alloc(s, flags);
422
423 kasan_kmalloc(s, ret, size, flags);
> 424 vchecker_kmalloc(s, ret, size, _THIS_IP_);
425 return ret;
426 }
427
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28245 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 16/18] mm/vchecker: support allocation caller filter
2017-11-28 7:48 [PATCH 00/18] introduce a new tool, valid access checker js1304
` (14 preceding siblings ...)
2017-11-28 7:48 ` [PATCH 15/18] mm/vchecker: pass allocation caller address to vchecker hook js1304
@ 2017-11-28 7:48 ` js1304
2017-11-28 7:48 ` [PATCH 17/18] lib/vchecker_test: introduce a sample for vchecker test js1304
` (3 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: js1304 @ 2017-11-28 7:48 UTC (permalink / raw)
To: Andrew Morton
Cc: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel, Namhyung Kim, Wengang Wang, Joonsoo Kim
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
kmalloc() is used everywhere in the kernel and it doesn't distiniguish
the callers since it doesn't much help to efficiently manage the memory.
However, there is a difference in the view of the debugging. A bug usually
happens on the objects allocated by specific allocation caller. So,
it is useful to distiniguish them. Let's call it as a same class object.
This patch implements an allocation caller filter to distiniguish
the class. With it, vchecker can be applied only to a specific class
and debugging it could be possible.
Note that it's not easy to distiniguish allocation caller of existing
allocated memory. Therefore, existing allocated memory will not be
included to debugging target if allocation caller filter is enabled.
If it is really required, feel free to ask me. I have a rough idea
to implement it.
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
lib/Kconfig.kasan | 1 +
mm/kasan/vchecker.c | 126 +++++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 126 insertions(+), 1 deletion(-)
diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
index d3552f3..4b8e748 100644
--- a/lib/Kconfig.kasan
+++ b/lib/Kconfig.kasan
@@ -63,5 +63,6 @@ config VCHECKER
happens at the area.
depends on KASAN && DEBUG_FS
+ select KALLSYMS
endif
diff --git a/mm/kasan/vchecker.c b/mm/kasan/vchecker.c
index 918f05a..9f2b164 100644
--- a/mm/kasan/vchecker.c
+++ b/mm/kasan/vchecker.c
@@ -17,6 +17,7 @@
#include <linux/kasan.h>
#include <linux/uaccess.h>
#include <linux/stackdepot.h>
+#include <linux/kallsyms.h>
#include "vchecker.h"
#include "../slab.h"
@@ -28,6 +29,7 @@
struct vchecker {
bool enabled;
unsigned int callstack_depth;
+ struct list_head alloc_filter_list;
struct list_head cb_list;
};
@@ -75,6 +77,12 @@ struct vchecker_callstack_arg {
bool enabled;
};
+struct vchecker_alloc_filter {
+ unsigned long begin;
+ unsigned long end;
+ struct list_head list;
+};
+
static struct dentry *debugfs_root;
static struct vchecker_type vchecker_types[VCHECKER_TYPE_MAX];
static DEFINE_MUTEX(vchecker_meta);
@@ -149,6 +157,7 @@ void vchecker_kmalloc(struct kmem_cache *s, const void *object, size_t size,
{
struct vchecker *checker;
struct vchecker_cb *cb;
+ struct vchecker_alloc_filter *af;
rcu_read_lock();
checker = s->vchecker_cache.checker;
@@ -157,6 +166,18 @@ void vchecker_kmalloc(struct kmem_cache *s, const void *object, size_t size,
return;
}
+ if (list_empty(&checker->alloc_filter_list))
+ goto mark;
+
+ list_for_each_entry(af, &checker->alloc_filter_list, list) {
+ if (af->begin <= ret_ip && ret_ip < af->end)
+ goto mark;
+ }
+
+ rcu_read_unlock();
+ return;
+
+mark:
list_for_each_entry(cb, &checker->cb_list, list) {
kasan_poison_shadow(object + cb->begin,
round_up(cb->end - cb->begin,
@@ -476,9 +497,13 @@ static ssize_t enable_write(struct file *filp, const char __user *ubuf,
/*
* After this operation, it is guaranteed that there is no user
* left that accesses checker's cb list if vchecker is disabled.
+ * Don't mark the object if alloc_filter is enabled. We cannot
+ * know the allocation caller at this moment.
*/
synchronize_sched();
- vchecker_enable_cache(s, enable);
+ if (!enable ||
+ list_empty(&s->vchecker_cache.checker->alloc_filter_list))
+ vchecker_enable_cache(s, enable);
mutex_unlock(&vchecker_meta);
return cnt;
@@ -556,6 +581,99 @@ static const struct file_operations callstack_depth_fops = {
.release = single_release,
};
+static int alloc_filter_show(struct seq_file *f, void *v)
+{
+ char name[KSYM_NAME_LEN];
+ struct kmem_cache *s = f->private;
+ struct vchecker *checker = s->vchecker_cache.checker;
+ struct vchecker_alloc_filter *af;
+
+ mutex_lock(&vchecker_meta);
+ list_for_each_entry(af, &checker->alloc_filter_list, list) {
+ if (!lookup_symbol_name(af->begin, name))
+ seq_printf(f, "%s: ", name);
+ seq_printf(f, "0x%lx - 0x%lx\n", af->begin, af->end);
+ }
+ mutex_unlock(&vchecker_meta);
+
+ return 0;
+}
+
+static int alloc_filter_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, alloc_filter_show, inode->i_private);
+}
+
+static void remove_alloc_filters(struct vchecker *checker)
+{
+ struct vchecker_alloc_filter *af, *tmp;
+
+ list_for_each_entry_safe(af, tmp, &checker->alloc_filter_list, list) {
+ list_del(&af->list);
+ kfree(af);
+ }
+}
+
+static ssize_t alloc_filter_write(struct file *filp, const char __user *ubuf,
+ size_t cnt, loff_t *ppos)
+{
+ char filter_chars[KSYM_NAME_LEN];
+ struct kmem_cache *s = file_inode(filp)->i_private;
+ struct vchecker *checker = s->vchecker_cache.checker;
+ unsigned long begin;
+ unsigned long size;
+ struct vchecker_alloc_filter *af = NULL;
+
+ if (cnt >= KSYM_NAME_LEN || cnt == 0)
+ return -EINVAL;
+
+ if (copy_from_user(&filter_chars, ubuf, cnt))
+ return -EFAULT;
+
+ if (isspace(filter_chars[0]))
+ goto change;
+
+ filter_chars[cnt - 1] = '\0';
+ begin = kallsyms_lookup_name(filter_chars);
+ if (!begin)
+ return -EINVAL;
+
+ kallsyms_lookup_size_offset(begin, &size, NULL);
+
+ af = kzalloc(sizeof(*af), GFP_KERNEL);
+ if (!af)
+ return -ENOMEM;
+
+ af->begin = begin;
+ af->end = begin + size;
+
+change:
+ mutex_lock(&vchecker_meta);
+ if (checker->enabled || !list_empty(&checker->cb_list)) {
+ mutex_unlock(&vchecker_meta);
+ kfree(af);
+
+ return -EINVAL;
+ }
+
+ if (af)
+ list_add_tail(&af->list, &checker->alloc_filter_list);
+ else
+ remove_alloc_filters(checker);
+
+ mutex_unlock(&vchecker_meta);
+
+ return cnt;
+}
+
+static const struct file_operations alloc_filter_fops = {
+ .open = alloc_filter_open,
+ .write = alloc_filter_write,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
static int init_value(struct kmem_cache *s, struct vchecker_cb *cb,
char *buf, size_t cnt)
{
@@ -865,6 +983,7 @@ static void free_vchecker(struct kmem_cache *s)
if (!s->vchecker_cache.checker)
return;
+ remove_alloc_filters(s->vchecker_cache.checker);
for (i = 0; i < ARRAY_SIZE(vchecker_types); i++)
remove_cbs(s, &vchecker_types[i]);
kfree(s->vchecker_cache.checker);
@@ -895,6 +1014,7 @@ static int alloc_vchecker(struct kmem_cache *s)
return -ENOMEM;
checker->callstack_depth = VCHECKER_STACK_DEPTH;
+ INIT_LIST_HEAD(&checker->alloc_filter_list);
INIT_LIST_HEAD(&checker->cb_list);
s->vchecker_cache.checker = checker;
@@ -922,6 +1042,10 @@ static int register_debugfs(struct kmem_cache *s)
&callstack_depth_fops))
return -ENOMEM;
+ if (!debugfs_create_file("alloc_filter", 0600, dir, s,
+ &alloc_filter_fops))
+ return -ENOMEM;
+
for (i = 0; i < ARRAY_SIZE(vchecker_types); i++) {
t = &vchecker_types[i];
if (!debugfs_create_file(t->name, 0600, dir, s, t->fops))
--
2.7.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 17/18] lib/vchecker_test: introduce a sample for vchecker test
2017-11-28 7:48 [PATCH 00/18] introduce a new tool, valid access checker js1304
` (15 preceding siblings ...)
2017-11-28 7:48 ` [PATCH 16/18] mm/vchecker: support allocation caller filter js1304
@ 2017-11-28 7:48 ` js1304
2017-11-28 7:48 ` [PATCH 18/18] doc: add vchecker document js1304
` (2 subsequent siblings)
19 siblings, 0 replies; 29+ messages in thread
From: js1304 @ 2017-11-28 7:48 UTC (permalink / raw)
To: Andrew Morton
Cc: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel, Namhyung Kim, Wengang Wang, Joonsoo Kim
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
It's not easy to understand what can be done by the vchecker.
This sample could explain it and help to understand the vchecker.
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
lib/Kconfig.kasan | 9 ++++
lib/Makefile | 1 +
lib/vchecker_test.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 127 insertions(+)
create mode 100644 lib/vchecker_test.c
diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
index 4b8e748..9983ec8 100644
--- a/lib/Kconfig.kasan
+++ b/lib/Kconfig.kasan
@@ -65,4 +65,13 @@ config VCHECKER
depends on KASAN && DEBUG_FS
select KALLSYMS
+config TEST_VCHECKER
+ tristate "Module for testing vchecker"
+ depends on m && KASAN
+ help
+ This is a test module doing memory over-write. If vchecker is
+ properly set up to check that over-write, memory over-written
+ problem would be detected. See the help text in the
+ lib/vchecker_test.c for vchecker sample run.
+
endif
diff --git a/lib/Makefile b/lib/Makefile
index d11c48e..cc1f5ec 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -244,6 +244,7 @@ clean-files += oid_registry_data.c
obj-$(CONFIG_UCS2_STRING) += ucs2_string.o
obj-$(CONFIG_UBSAN) += ubsan.o
+obj-$(CONFIG_TEST_VCHECKER) += vchecker_test.o
UBSAN_SANITIZE_ubsan.o := n
diff --git a/lib/vchecker_test.c b/lib/vchecker_test.c
new file mode 100644
index 0000000..fcb4b7f
--- /dev/null
+++ b/lib/vchecker_test.c
@@ -0,0 +1,117 @@
+#include <linux/kernel.h>
+#include <linux/printk.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/workqueue.h>
+
+/*
+ * How to use this sample for vchecker sample-run
+ *
+ * 1. Insert this module
+ * 2. Do following command on debugfs directory
+ * # cd /sys/kernel/debug/vchecker
+ * # echo 0 0xffff 7 > vchecker_test/value # offset 0, mask 0xffff, value 7
+ * # echo 1 > vchecker_test/enable
+ * # echo workfn_kmalloc_obj > kmalloc-8/alloc_filter
+ * # echo "0 8" > kmalloc-8/callstack
+ * # echo on > kmalloc-8/callstack
+ * # echo 1 > kmalloc-8/enable
+ * 3. Check the error report due to invalid written value
+ */
+
+struct object {
+ volatile unsigned long v[1];
+};
+
+static struct kmem_cache *s;
+static void *old_obj;
+static struct delayed_work dwork_old_obj;
+static struct delayed_work dwork_new_obj;
+static struct delayed_work dwork_kmalloc_obj;
+
+static void workfn_old_obj(struct work_struct *work)
+{
+ struct object *obj = old_obj;
+ struct delayed_work *dwork = (struct delayed_work *)work;
+
+ obj->v[0] = 7;
+
+ mod_delayed_work(system_wq, dwork, HZ * 5);
+}
+
+static void workfn_new_obj(struct work_struct *work)
+{
+ struct object *obj;
+ struct delayed_work *dwork = (struct delayed_work *)work;
+
+ obj = kmem_cache_alloc(s, GFP_KERNEL);
+
+ obj->v[0] = 7;
+ /*
+ * Need one more access to detect wrong value since there is
+ * no proper infrastructure yet and the feature is just emulated.
+ */
+ obj->v[0] = 0;
+
+ kmem_cache_free(s, obj);
+ mod_delayed_work(system_wq, dwork, HZ * 5);
+}
+
+static void workfn_kmalloc_obj(struct work_struct *work)
+{
+ struct object *obj;
+ struct delayed_work *dwork = (struct delayed_work *)work;
+
+ obj = kmalloc(sizeof(*obj), GFP_KERNEL);
+
+ obj->v[0] = 7;
+ /*
+ * Need one more access to detect wrong value since there is
+ * no proper infrastructure yet and the feature is just emulated.
+ */
+ obj->v[0] = 0;
+
+ kfree(obj);
+ mod_delayed_work(system_wq, dwork, HZ * 5);
+}
+
+static int __init vchecker_test_init(void)
+{
+ s = kmem_cache_create("vchecker_test",
+ sizeof(struct object), 0, SLAB_NOLEAKTRACE, NULL);
+ if (!s)
+ return 1;
+
+ old_obj = kmem_cache_alloc(s, GFP_KERNEL);
+ if (!old_obj) {
+ kmem_cache_destroy(s);
+ return 1;
+ }
+
+ INIT_DELAYED_WORK(&dwork_old_obj, workfn_old_obj);
+ INIT_DELAYED_WORK(&dwork_new_obj, workfn_new_obj);
+ INIT_DELAYED_WORK(&dwork_kmalloc_obj, workfn_kmalloc_obj);
+
+ mod_delayed_work(system_wq, &dwork_old_obj, HZ * 5);
+ mod_delayed_work(system_wq, &dwork_new_obj, HZ * 5);
+ mod_delayed_work(system_wq, &dwork_kmalloc_obj, HZ * 5);
+
+ return 0;
+}
+
+static void __exit vchecker_test_fini(void)
+{
+ cancel_delayed_work_sync(&dwork_old_obj);
+ cancel_delayed_work_sync(&dwork_new_obj);
+ cancel_delayed_work_sync(&dwork_kmalloc_obj);
+
+ kmem_cache_free(s, old_obj);
+ kmem_cache_destroy(s);
+}
+
+
+module_init(vchecker_test_init);
+module_exit(vchecker_test_fini)
+
+MODULE_LICENSE("GPL");
+
--
2.7.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 18/18] doc: add vchecker document
2017-11-28 7:48 [PATCH 00/18] introduce a new tool, valid access checker js1304
` (16 preceding siblings ...)
2017-11-28 7:48 ` [PATCH 17/18] lib/vchecker_test: introduce a sample for vchecker test js1304
@ 2017-11-28 7:48 ` js1304
2017-11-29 9:27 ` [PATCH 00/18] introduce a new tool, valid access checker Dmitry Vyukov
2017-12-22 1:51 ` Joonsoo Kim
19 siblings, 0 replies; 29+ messages in thread
From: js1304 @ 2017-11-28 7:48 UTC (permalink / raw)
To: Andrew Morton
Cc: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel, Namhyung Kim, Wengang Wang, Joonsoo Kim
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
This is a main document for vchecker user.
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
Documentation/dev-tools/vchecker.rst | 200 +++++++++++++++++++++++++++++++++++
1 file changed, 200 insertions(+)
create mode 100644 Documentation/dev-tools/vchecker.rst
diff --git a/Documentation/dev-tools/vchecker.rst b/Documentation/dev-tools/vchecker.rst
new file mode 100644
index 0000000..136e5d9
--- /dev/null
+++ b/Documentation/dev-tools/vchecker.rst
@@ -0,0 +1,200 @@
+The Valid Access Checker (VCHECKER)
+===================================
+
+Overview
+--------
+Vchecker is a dynamic memory error detector. It provides a new debug feature
+that can find out an un-intended access to valid area. Valid area here means
+the memory which is allocated and allowed to be accessed by memory owner and
+un-intended access means the read/write that is initiated by non-owner.
+Usual problem of this class is memory overwritten.
+
+Most of debug feature focused on finding out un-intended access to
+in-valid area, for example, out-of-bound access and use-after-free, and,
+there are many good tools for it. But, as far as I know, there is no good tool
+to find out un-intended access to valid area. This kind of problem is really
+hard to solve so this tool would be very useful.
+
+This tool doesn't automatically catch a problem. Manual configuration to
+specify the target object is required. See the usage part on the below.
+
+Usage
+-----
+
+To enable vchecker configure kernel with
+
+::
+
+ CONFIG_VCHECKER = y
+
+and choose the target slab object type and the type of the checkers by debugfs
+interface at the runtime. Following is the hierarchy of the interface.
+
+::
+
+ - debugfs root for vchecker
+ - directory per slab cache
+ - alloc_filter
+ - callstack_depth
+ - enable
+ - value
+ - callstack
+
+
+alloc_filter can be used to apply the checker to specific allocation caller
+for this slab cache. For example, there are multiple users for kmalloc-N
+slab cache and it's better to filter out un-related allocation caller
+when debugging. Note that, if alloc_filter is specified, vchecker doesn't
+regard existing allocated objects as debugging target since it's not easy
+to know the allocation caller for existing allocated objects.
+
+callstack_depth can be used to limit the depth of callstack. It would be
+helpful to reduce overhead.
+
+enable can be used to begin/end the checker for this slab cache.
+
+There are two checkers now, value checker and callstack checker.
+
+Value checker checks the value stored in the target object.
+See following example.
+
+::
+
+ static void workfn(struct work_struct *work)
+ {
+ struct object *obj;
+ struct delayed_work *dwork = (struct delayed_work *)work;
+
+ obj = kmem_cache_alloc(s, GFP_KERNEL);
+
+ obj->v[0] = 7;
+ obj->v[0] = 0;
+
+ kmem_cache_free(s, obj);
+ mod_delayed_work(system_wq, dwork, HZ);
+ }
+
+Assume that v[0] should not be the value, 7, however, there is a code
+to set v[0] to the value, 7. To detect this error, register the value checker
+for this object and specify that invalid value is '7'. After registration,
+if someone stores '7' to this object, the error will be reported. Registration
+can be done by three parameter as following.
+
+::
+
+ # cd /sys/kernel/debug/vchecker
+ # echo 0 0xffff 7 > [slab cache]/value
+ // offset 0 (dec)
+ // mask 0xffff (hex)
+ // value 7 (dec)
+ # echo 1 > [slab cache]/enable
+
+Before describing the each parameters, one thing should be noted. One value
+checker works for 8 bytes at maximum. If more bytes should be checked,
+please register multiple value checkers.
+
+First parameter is a target offset from the object base. It should be aligned
+by 8 bytes due to implementation constraint. Second parameter is a mask that
+is used to specify the range in the specified 8 bytes. Occasionally, we want to
+check just 1 byte or 1 bit and this mask makes it possible to check such
+a small range. Third parameter is the value that is assumed as invalid.
+
+Second checker is the callstack checker. It checks the read/write callstack
+of the target object. Overwritten problem usually happens by non-owner and
+it would have odd callstack. By checking the oddity of the callstack, vchecker
+can report the possible error candidate. Currently, the oddity of the callstack
+is naively determined by checking whether it is a new callstack or not. It can
+be extended to use whitelist but not yet implemented.
+
+::
+
+ # echo 0 8 > [slab cache]/callstack
+ // offset 0 (dec)
+ // size 8 (dec)
+ # echo 1 > [slab cache]/enable
+
+First parameter is a target offset as usual and the second one is the size to
+determine the range. Unlike the value checker, callstack checker can check
+more than 8 bytes by just one checker.
+
+::
+
+ # echo off > [slab cache]/callstack
+ // ... (do some work to collect enough valid callstacks) ...
+ # echo on > [slab cache]/callstack
+
+You can collect potential valid callstack during 'off state'. If you think
+that enough callstacks are collected, turn on the checker. It will report
+a new callstack and it may be a bug candidate.
+
+Error reports
+-------------
+
+Report format looks very similar with the report of KASAN
+
+::
+
+ [ 49.400673] ==================================================================
+ [ 49.402297] BUG: VCHECKER: invalid access in workfn_old_obj+0x14/0x50 [vchecker_test] at addr ffff88002e9dc000
+ [ 49.403899] Write of size 8 by task kworker/0:2/465
+ [ 49.404538] value checker for offset 0 ~ 8 at ffff88002e9dc000
+ [ 49.405374] (mask 0xffff value 7) invalid value 7
+
+ [ 49.406016] Invalid writer:
+ [ 49.406302] workfn_old_obj+0x14/0x50 [vchecker_test]
+ [ 49.406973] process_one_work+0x3b5/0x9f0
+ [ 49.407463] worker_thread+0x87/0x750
+ [ 49.407895] kthread+0x1b2/0x200
+ [ 49.408252] ret_from_fork+0x24/0x30
+
+ [ 49.408723] Allocated by task 1326:
+ [ 49.409126] kasan_kmalloc+0xb9/0xe0
+ [ 49.409571] kmem_cache_alloc+0xd1/0x250
+ [ 49.410046] 0xffffffffa00c8157
+ [ 49.410389] do_one_initcall+0x82/0x1cf
+ [ 49.410851] do_init_module+0xe7/0x333
+ [ 49.411296] load_module+0x406b/0x4b40
+ [ 49.411745] SYSC_finit_module+0x14d/0x180
+ [ 49.412247] do_syscall_64+0xf0/0x340
+ [ 49.412674] return_from_SYSCALL_64+0x0/0x75
+
+ [ 49.413276] Freed by task 0:
+ [ 49.413566] (stack is not available)
+
+ [ 49.414034] The buggy address belongs to the object at ffff88002e9dc000
+ which belongs to the cache vchecker_test of size 8
+ [ 49.415708] The buggy address is located 0 bytes inside of
+ 8-byte region [ffff88002e9dc000, ffff88002e9dc008)
+ [ 49.417148] ==================================================================
+
+It shows that vchecker find the invalid value writing
+at workfn_old_obj+0x14/0x50. Object information is also reported.
+
+Implementation details
+----------------------
+This part requires some understanding of how KASAN works since vchecker is
+highly depends on shadow memory of KASAN. Vchecker uses the shadow to
+distinguish interesting memory address for validation. If it finds the special
+value on the shadow of the accessing address, it means that this address is
+the target for validation check. Then, it tries to do additional checks to this
+address. With this way, vchecker can filter out un-interesting memory access
+very efficiently.
+
+A new type of checks can be added by implementing following callback structure.
+check() callback is the main function that checks whether the access is valid
+or not. Please reference existing checkers for more information.
+
+::
+
+ struct vchecker_type {
+ char *name;
+ const struct file_operations *fops;
+ int (*init)(struct kmem_cache *s, struct vchecker_cb *cb,
+ char *buf, size_t cnt);
+ void (*fini)(struct vchecker_cb *cb);
+ void (*show)(struct kmem_cache *s, struct seq_file *f,
+ struct vchecker_cb *cb, void *object, bool verbose);
+ bool (*check)(struct kmem_cache *s, struct vchecker_cb *cb,
+ void *object, bool write, unsigned long ret_ip,
+ unsigned long begin, unsigned long end);
+ };
--
2.7.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 00/18] introduce a new tool, valid access checker
2017-11-28 7:48 [PATCH 00/18] introduce a new tool, valid access checker js1304
` (17 preceding siblings ...)
2017-11-28 7:48 ` [PATCH 18/18] doc: add vchecker document js1304
@ 2017-11-29 9:27 ` Dmitry Vyukov
2017-12-01 7:46 ` Joonsoo Kim
2017-12-22 1:51 ` Joonsoo Kim
19 siblings, 1 reply; 29+ messages in thread
From: Dmitry Vyukov @ 2017-11-29 9:27 UTC (permalink / raw)
To: Joonsoo Kim
Cc: Andrew Morton, Andrey Ryabinin, Alexander Potapenko, kasan-dev,
Linux-MM, LKML, Namhyung Kim, Wengang Wang, Joonsoo Kim,
Andi Kleen
On Tue, Nov 28, 2017 at 8:48 AM, <js1304@gmail.com> wrote:
> From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>
> Hello,
>
> This patchset introduces a new tool, valid access checker.
>
> Vchecker is a dynamic memory error detector. It provides a new debug feature
> that can find out an un-intended access to valid area. Valid area here means
> the memory which is allocated and allowed to be accessed by memory owner and
> un-intended access means the read/write that is initiated by non-owner.
> Usual problem of this class is memory overwritten.
>
> Most of debug feature focused on finding out un-intended access to
> in-valid area, for example, out-of-bound access and use-after-free, and,
> there are many good tools for it. But, as far as I know, there is no good tool
> to find out un-intended access to valid area. This kind of problem is really
> hard to solve so this tool would be very useful.
>
> This tool doesn't automatically catch a problem. Manual runtime configuration
> to specify the target object is required.
>
> Note that there was a similar attempt for the debugging overwritten problem
> however it requires manual code modifying and recompile.
>
> http://lkml.kernel.org/r/<20171117223043.7277-1-wen.gang.wang@oracle.com>
>
> To get more information about vchecker, please see a documention at
> the last patch.
>
> Patchset can also be available at
>
> https://github.com/JoonsooKim/linux/tree/vchecker-master-v1.0-next-20171122
>
> Enjoy it.
Hi Joonsoo,
I skimmed through the code and this looks fine from KASAN point of
view (minimal code changes and no perf impact).
I don't feel like I can judge if this should go in or not. I will not
use this, we use KASAN for large-scale testing, but vchecker is in a
different bucket, it is meant for developers debugging hard bugs.
Wengang come up with a very similar change, and Andi said that this
looks useful.
If the decision is that this goes in, please let me take a closer look
before this is merged.
Thanks
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 00/18] introduce a new tool, valid access checker
2017-11-29 9:27 ` [PATCH 00/18] introduce a new tool, valid access checker Dmitry Vyukov
@ 2017-12-01 7:46 ` Joonsoo Kim
0 siblings, 0 replies; 29+ messages in thread
From: Joonsoo Kim @ 2017-12-01 7:46 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: Andrew Morton, Andrey Ryabinin, Alexander Potapenko, kasan-dev,
Linux-MM, LKML, Namhyung Kim, Wengang Wang, Andi Kleen
On Wed, Nov 29, 2017 at 10:27:00AM +0100, Dmitry Vyukov wrote:
> On Tue, Nov 28, 2017 at 8:48 AM, <js1304@gmail.com> wrote:
> > From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> >
> > Hello,
> >
> > This patchset introduces a new tool, valid access checker.
> >
> > Vchecker is a dynamic memory error detector. It provides a new debug feature
> > that can find out an un-intended access to valid area. Valid area here means
> > the memory which is allocated and allowed to be accessed by memory owner and
> > un-intended access means the read/write that is initiated by non-owner.
> > Usual problem of this class is memory overwritten.
> >
> > Most of debug feature focused on finding out un-intended access to
> > in-valid area, for example, out-of-bound access and use-after-free, and,
> > there are many good tools for it. But, as far as I know, there is no good tool
> > to find out un-intended access to valid area. This kind of problem is really
> > hard to solve so this tool would be very useful.
> >
> > This tool doesn't automatically catch a problem. Manual runtime configuration
> > to specify the target object is required.
> >
> > Note that there was a similar attempt for the debugging overwritten problem
> > however it requires manual code modifying and recompile.
> >
> > http://lkml.kernel.org/r/<20171117223043.7277-1-wen.gang.wang@oracle.com>
> >
> > To get more information about vchecker, please see a documention at
> > the last patch.
> >
> > Patchset can also be available at
> >
> > https://github.com/JoonsooKim/linux/tree/vchecker-master-v1.0-next-20171122
> >
> > Enjoy it.
>
>
> Hi Joonsoo,
>
> I skimmed through the code and this looks fine from KASAN point of
> view (minimal code changes and no perf impact).
> I don't feel like I can judge if this should go in or not. I will not
> use this, we use KASAN for large-scale testing, but vchecker is in a
> different bucket, it is meant for developers debugging hard bugs.
> Wengang come up with a very similar change, and Andi said that this
> looks useful.
Thanks for comment.
Hello, other reviewers!
Please let me know more opinions about this patchset.
>
> If the decision is that this goes in, please let me take a closer look
> before this is merged.
I will let you know when the decision is made.
Thanks.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 00/18] introduce a new tool, valid access checker
2017-11-28 7:48 [PATCH 00/18] introduce a new tool, valid access checker js1304
` (18 preceding siblings ...)
2017-11-29 9:27 ` [PATCH 00/18] introduce a new tool, valid access checker Dmitry Vyukov
@ 2017-12-22 1:51 ` Joonsoo Kim
2018-01-18 22:39 ` Andrew Morton
19 siblings, 1 reply; 29+ messages in thread
From: Joonsoo Kim @ 2017-12-22 1:51 UTC (permalink / raw)
To: Andrew Morton
Cc: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel, Namhyung Kim, Wengang Wang
On Tue, Nov 28, 2017 at 04:48:35PM +0900, js1304@gmail.com wrote:
> From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>
> Hello,
>
> This patchset introduces a new tool, valid access checker.
>
> Vchecker is a dynamic memory error detector. It provides a new debug feature
> that can find out an un-intended access to valid area. Valid area here means
> the memory which is allocated and allowed to be accessed by memory owner and
> un-intended access means the read/write that is initiated by non-owner.
> Usual problem of this class is memory overwritten.
>
> Most of debug feature focused on finding out un-intended access to
> in-valid area, for example, out-of-bound access and use-after-free, and,
> there are many good tools for it. But, as far as I know, there is no good tool
> to find out un-intended access to valid area. This kind of problem is really
> hard to solve so this tool would be very useful.
>
> This tool doesn't automatically catch a problem. Manual runtime configuration
> to specify the target object is required.
>
> Note that there was a similar attempt for the debugging overwritten problem
> however it requires manual code modifying and recompile.
>
> http://lkml.kernel.org/r/<20171117223043.7277-1-wen.gang.wang@oracle.com>
>
> To get more information about vchecker, please see a documention at
> the last patch.
>
> Patchset can also be available at
>
> https://github.com/JoonsooKim/linux/tree/vchecker-master-v1.0-next-20171122
>
> Enjoy it.
>
> Thanks.
Hello, Andrew.
Before the fixing some build failure on this patchset, I'd like to know
other reviewer's opinion on this patchset, especially, yours. :)
There are some interests on this patchset from some developers. Wengang
come up with a very similar change and Andi said that this looks useful.
Do you think that this tool is useful and can be merged?
Thanks.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 00/18] introduce a new tool, valid access checker
2017-12-22 1:51 ` Joonsoo Kim
@ 2018-01-18 22:39 ` Andrew Morton
0 siblings, 0 replies; 29+ messages in thread
From: Andrew Morton @ 2018-01-18 22:39 UTC (permalink / raw)
To: Joonsoo Kim
Cc: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel, Namhyung Kim, Wengang Wang
On Fri, 22 Dec 2017 10:51:15 +0900 Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
> On Tue, Nov 28, 2017 at 04:48:35PM +0900, js1304@gmail.com wrote:
> > From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> >
> > Hello,
> >
> > This patchset introduces a new tool, valid access checker.
> >
> > Vchecker is a dynamic memory error detector. It provides a new debug feature
> > that can find out an un-intended access to valid area. Valid area here means
> > the memory which is allocated and allowed to be accessed by memory owner and
> > un-intended access means the read/write that is initiated by non-owner.
> > Usual problem of this class is memory overwritten.
> >
> > Most of debug feature focused on finding out un-intended access to
> > in-valid area, for example, out-of-bound access and use-after-free, and,
> > there are many good tools for it. But, as far as I know, there is no good tool
> > to find out un-intended access to valid area. This kind of problem is really
> > hard to solve so this tool would be very useful.
> >
> > This tool doesn't automatically catch a problem. Manual runtime configuration
> > to specify the target object is required.
> >
> > Note that there was a similar attempt for the debugging overwritten problem
> > however it requires manual code modifying and recompile.
> >
> > http://lkml.kernel.org/r/<20171117223043.7277-1-wen.gang.wang@oracle.com>
> >
> > To get more information about vchecker, please see a documention at
> > the last patch.
> >
> > Patchset can also be available at
> >
> > https://github.com/JoonsooKim/linux/tree/vchecker-master-v1.0-next-20171122
> >
> > Enjoy it.
> >
> > Thanks.
>
> Hello, Andrew.
>
> Before the fixing some build failure on this patchset, I'd like to know
> other reviewer's opinion on this patchset, especially, yours. :)
>
> There are some interests on this patchset from some developers. Wengang
> come up with a very similar change and Andi said that this looks useful.
> Do you think that this tool is useful and can be merged?
>
My main fear is that the feature will sit there and nobody will use it.
Are there ways around that? For example, can we arrange with the test
robot(s) to get vchecker operating on their setups in some automatable
fashion and have them checking for bugs?
Any other suggestions as to how we could get this feature to be used by
others and producing useful results?
And has vchecker actually found any real bugs in existing code? If so,
a description of that would be illuminating.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 29+ messages in thread