[PATCH] mm-kasan-initial-memory-quarantine-implementation-v8-fix

From: Andrey Ryabinin
Date: Tue May 10 2016 - 09:38:34 EST

* Fix comment styles,
* Get rid of some ifdefs
* Revert needless functions renames in quarantine patch
* Remove needless local_irq_save()/restore() in per_cpu_remove_cache()
* Add new 'struct qlist_node' instead of 'void **' types. This makes
code a bit more redable.

Signed-off-by: Andrey Ryabinin <aryabinin@xxxxxxxxxxxxx>
---
include/linux/kasan.h | 17 +++-----
mm/kasan/Makefile | 5 +--
mm/kasan/kasan.c | 14 ++-----
mm/kasan/kasan.h | 12 +++++-
mm/kasan/quarantine.c | 110 +++++++++++++++++++++++++-------------------------
mm/mempool.c | 5 +--
mm/page_alloc.c | 2 +-
mm/slab.c | 7 +---
mm/slub.c | 4 +-
9 files changed, 84 insertions(+), 92 deletions(-)

diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index 645c280..611927f 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -46,7 +46,7 @@ void kasan_unpoison_shadow(const void *address, size_t size);
void kasan_unpoison_task_stack(struct task_struct *task);

void kasan_alloc_pages(struct page *page, unsigned int order);
-void kasan_poison_free_pages(struct page *page, unsigned int order);
+void kasan_free_pages(struct page *page, unsigned int order);

void kasan_cache_create(struct kmem_cache *cache, size_t *size,
unsigned long *flags);
@@ -58,15 +58,13 @@ void kasan_unpoison_object_data(struct kmem_cache *cache, void *object);
void kasan_poison_object_data(struct kmem_cache *cache, void *object);

void kasan_kmalloc_large(const void *ptr, size_t size, gfp_t flags);
-void kasan_poison_kfree_large(const void *ptr);
-void kasan_poison_kfree(void *ptr);
+void kasan_kfree_large(const void *ptr);
+void kasan_kfree(void *ptr);
void kasan_kmalloc(struct kmem_cache *s, const void *object, size_t size,
gfp_t flags);
void kasan_krealloc(const void *object, size_t new_size, gfp_t flags);

void kasan_slab_alloc(struct kmem_cache *s, void *object, gfp_t flags);
-/* kasan_slab_free() returns true if the object has been put into quarantine.
- */
bool kasan_slab_free(struct kmem_cache *s, void *object);
void kasan_poison_slab_free(struct kmem_cache *s, void *object);

@@ -88,8 +86,7 @@ static inline void kasan_enable_current(void) {}
static inline void kasan_disable_current(void) {}

static inline void kasan_alloc_pages(struct page *page, unsigned int order) {}
-static inline void kasan_poison_free_pages(struct page *page,
- unsigned int order) {}
+static inline void kasan_free_pages(struct page *page, unsigned int order) {}

static inline void kasan_cache_create(struct kmem_cache *cache,
size_t *size,
@@ -104,8 +101,8 @@ static inline void kasan_poison_object_data(struct kmem_cache *cache,
void *object) {}

static inline void kasan_kmalloc_large(void *ptr, size_t size, gfp_t flags) {}
-static inline void kasan_poison_kfree_large(const void *ptr) {}
-static inline void kasan_poison_kfree(void *ptr) {}
+static inline void kasan_kfree_large(const void *ptr) {}
+static inline void kasan_kfree(void *ptr) {}
static inline void kasan_kmalloc(struct kmem_cache *s, const void *object,
size_t size, gfp_t flags) {}
static inline void kasan_krealloc(const void *object, size_t new_size,
@@ -113,8 +110,6 @@ static inline void kasan_krealloc(const void *object, size_t new_size,

static inline void kasan_slab_alloc(struct kmem_cache *s, void *object,
gfp_t flags) {}
-/* kasan_slab_free() returns true if the object has been put into quarantine.
- */
static inline bool kasan_slab_free(struct kmem_cache *s, void *object)
{
return false;
diff --git a/mm/kasan/Makefile b/mm/kasan/Makefile
index 63b54aa..1548749 100644
--- a/mm/kasan/Makefile
+++ b/mm/kasan/Makefile
@@ -8,7 +8,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
-
-ifdef CONFIG_SLAB
- obj-y += quarantine.o
-endif
+obj-$(CONFIG_SLAB) += quarantine.o
diff --git a/mm/kasan/kasan.c b/mm/kasan/kasan.c
index ef2e87b..8df666b 100644
--- a/mm/kasan/kasan.c
+++ b/mm/kasan/kasan.c
@@ -327,7 +327,7 @@ void kasan_alloc_pages(struct page *page, unsigned int order)
kasan_unpoison_shadow(page_address(page), PAGE_SIZE << order);
}

-void kasan_poison_free_pages(struct page *page, unsigned int order)
+void kasan_free_pages(struct page *page, unsigned int order)
{
if (likely(!PageHighMem(page)))
kasan_poison_shadow(page_address(page),
@@ -390,16 +390,12 @@ void kasan_cache_create(struct kmem_cache *cache, size_t *size,

void kasan_cache_shrink(struct kmem_cache *cache)
{
-#ifdef CONFIG_SLAB
quarantine_remove_cache(cache);
-#endif
}

void kasan_cache_destroy(struct kmem_cache *cache)
{
-#ifdef CONFIG_SLAB
quarantine_remove_cache(cache);
-#endif
}

void kasan_poison_slab(struct page *page)
@@ -550,10 +546,8 @@ void kasan_kmalloc(struct kmem_cache *cache, const void *object, size_t size,
unsigned long redzone_start;
unsigned long redzone_end;

-#ifdef CONFIG_SLAB
if (flags & __GFP_RECLAIM)
quarantine_reduce();
-#endif

if (unlikely(object == NULL))
return;
@@ -585,10 +579,8 @@ void kasan_kmalloc_large(const void *ptr, size_t size, gfp_t flags)
unsigned long redzone_start;
unsigned long redzone_end;

-#ifdef CONFIG_SLAB
if (flags & __GFP_RECLAIM)
quarantine_reduce();
-#endif

if (unlikely(ptr == NULL))
return;
@@ -618,7 +610,7 @@ void kasan_krealloc(const void *object, size_t size, gfp_t flags)
kasan_kmalloc(page->slab_cache, object, size, flags);
}

-void kasan_poison_kfree(void *ptr)
+void kasan_kfree(void *ptr)
{
struct page *page;

@@ -631,7 +623,7 @@ void kasan_poison_kfree(void *ptr)
kasan_slab_free(page->slab_cache, ptr);
}

-void kasan_poison_kfree_large(const void *ptr)
+void kasan_kfree_large(const void *ptr)
{
struct page *page = virt_to_page(ptr);

diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 7da78a6..7f7ac51 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -80,11 +80,14 @@ struct kasan_alloc_meta {
u32 reserved;
};

+struct qlist_node {
+ struct qlist_node *next;
+};
struct kasan_free_meta {
/* This field is used while the object is in the quarantine.
* Otherwise it might be used for the allocator freelist.
*/
- void **quarantine_link;
+ struct qlist_node quarantine_link;
struct kasan_track track;
};

@@ -108,8 +111,15 @@ static inline bool kasan_report_enabled(void)
void kasan_report(unsigned long addr, size_t size,
bool is_write, unsigned long ip);

+#ifdef CONFIG_SLAB
void quarantine_put(struct kasan_free_meta *info, struct kmem_cache *cache);
void quarantine_reduce(void);
void quarantine_remove_cache(struct kmem_cache *cache);
+#else
+static inline void quarantine_put(struct kasan_free_meta *info,
+ struct kmem_cache *cache) { }
+static inline void quarantine_reduce(void) { }
+static inline void quarantine_remove_cache(struct kmem_cache *cache) { }
+#endif

#endif
diff --git a/mm/kasan/quarantine.c b/mm/kasan/quarantine.c
index 40159a6..1e687d7 100644
--- a/mm/kasan/quarantine.c
+++ b/mm/kasan/quarantine.c
@@ -33,40 +33,42 @@

/* Data structure and operations for quarantine queues. */

-/* Each queue is a signle-linked list, which also stores the total size of
+/*
+ * Each queue is a signle-linked list, which also stores the total size of
* objects inside of it.
*/
-struct qlist {
- void **head;
- void **tail;
+struct qlist_head {
+ struct qlist_node *head;
+ struct qlist_node *tail;
size_t bytes;
};

#define QLIST_INIT { NULL, NULL, 0 }

-static bool qlist_empty(struct qlist *q)
+static bool qlist_empty(struct qlist_head *q)
{
return !q->head;
}

-static void qlist_init(struct qlist *q)
+static void qlist_init(struct qlist_head *q)
{
q->head = q->tail = NULL;
q->bytes = 0;
}

-static void qlist_put(struct qlist *q, void **qlink, size_t size)
+static void qlist_put(struct qlist_head *q, struct qlist_node *qlink,
+ size_t size)
{
if (unlikely(qlist_empty(q)))
q->head = qlink;
else
- *q->tail = qlink;
+ q->tail->next = qlink;
q->tail = qlink;
- *qlink = NULL;
+ qlink->next = NULL;
q->bytes += size;
}

-static void qlist_move_all(struct qlist *from, struct qlist *to)
+static void qlist_move_all(struct qlist_head *from, struct qlist_head *to)
{
if (unlikely(qlist_empty(from)))
return;
@@ -77,15 +79,15 @@ static void qlist_move_all(struct qlist *from, struct qlist *to)
return;
}

- *to->tail = from->head;
+ to->tail->next = from->head;
to->tail = from->tail;
to->bytes += from->bytes;

qlist_init(from);
}

-static void qlist_move(struct qlist *from, void **last, struct qlist *to,
- size_t size)
+static void qlist_move(struct qlist_head *from, struct qlist_node *last,
+ struct qlist_head *to, size_t size)
{
if (unlikely(last == from->tail)) {
qlist_move_all(from, to);
@@ -94,53 +96,56 @@ static void qlist_move(struct qlist *from, void **last, struct qlist *to,
if (qlist_empty(to))
to->head = from->head;
else
- *to->tail = from->head;
+ to->tail->next = from->head;
to->tail = last;
- from->head = *last;
- *last = NULL;
+ from->head = last->next;
+ last->next = NULL;
from->bytes -= size;
to->bytes += size;
}


-/* The object quarantine consists of per-cpu queues and a global queue,
+/*
+ * The object quarantine consists of per-cpu queues and a global queue,
* guarded by quarantine_lock.
*/
-static DEFINE_PER_CPU(struct qlist, cpu_quarantine);
+static DEFINE_PER_CPU(struct qlist_head, cpu_quarantine);

-static struct qlist global_quarantine;
+static struct qlist_head global_quarantine;
static DEFINE_SPINLOCK(quarantine_lock);

/* Maximum size of the global queue. */
static unsigned long quarantine_size;

-/* The fraction of physical memory the quarantine is allowed to occupy.
+/*
+ * The fraction of physical memory the quarantine is allowed to occupy.
* Quarantine doesn't support memory shrinker with SLAB allocator, so we keep
* the ratio low to avoid OOM.
*/
#define QUARANTINE_FRACTION 32

-/* smp_load_acquire() here pairs with smp_store_release() in
+/*
+ * smp_load_acquire() here pairs with smp_store_release() in
* quarantine_reduce().
*/
#define QUARANTINE_LOW_SIZE (smp_load_acquire(&quarantine_size) * 3 / 4)
#define QUARANTINE_PERCPU_SIZE (1 << 20)

-static struct kmem_cache *qlink_to_cache(void **qlink)
+static struct kmem_cache *qlink_to_cache(struct qlist_node *qlink)
{
return virt_to_head_page(qlink)->slab_cache;
}

-static void *qlink_to_object(void **qlink, struct kmem_cache *cache)
+static void *qlink_to_object(struct qlist_node *qlink, struct kmem_cache *cache)
{
struct kasan_free_meta *free_info =
- container_of((void ***)qlink, struct kasan_free_meta,
+ container_of(qlink, struct kasan_free_meta,
quarantine_link);

return ((void *)free_info) - cache->kasan_info.free_meta_offset;
}

-static void qlink_free(void **qlink, struct kmem_cache *cache)
+static void qlink_free(struct qlist_node *qlink, struct kmem_cache *cache)
{
void *object = qlink_to_object(qlink, cache);
struct kasan_alloc_meta *alloc_info = get_alloc_info(cache, object);
@@ -152,9 +157,9 @@ static void qlink_free(void **qlink, struct kmem_cache *cache)
local_irq_restore(flags);
}

-static void qlist_free_all(struct qlist *q, struct kmem_cache *cache)
+static void qlist_free_all(struct qlist_head *q, struct kmem_cache *cache)
{
- void **qlink;
+ struct qlist_node *qlink;

if (unlikely(qlist_empty(q)))
return;
@@ -163,7 +168,7 @@ static void qlist_free_all(struct qlist *q, struct kmem_cache *cache)
while (qlink) {
struct kmem_cache *obj_cache =
cache ? cache : qlink_to_cache(qlink);
- void **next = *qlink;
+ struct qlist_node *next = qlink->next;

qlink_free(qlink, obj_cache);
qlink = next;
@@ -174,13 +179,13 @@ static void qlist_free_all(struct qlist *q, struct kmem_cache *cache)
void quarantine_put(struct kasan_free_meta *info, struct kmem_cache *cache)
{
unsigned long flags;
- struct qlist *q;
- struct qlist temp = QLIST_INIT;
+ struct qlist_head *q;
+ struct qlist_head temp = QLIST_INIT;

local_irq_save(flags);

q = this_cpu_ptr(&cpu_quarantine);
- qlist_put(q, (void **) &info->quarantine_link, cache->size);
+ qlist_put(q, &info->quarantine_link, cache->size);
if (unlikely(q->bytes > QUARANTINE_PERCPU_SIZE))
qlist_move_all(q, &temp);

@@ -197,21 +202,22 @@ void quarantine_reduce(void)
{
size_t new_quarantine_size;
unsigned long flags;
- struct qlist to_free = QLIST_INIT;
+ struct qlist_head to_free = QLIST_INIT;
size_t size_to_free = 0;
- void **last;
+ struct qlist_node *last;

/* smp_load_acquire() here pairs with smp_store_release() below. */
- if (likely(ACCESS_ONCE(global_quarantine.bytes) <=
+ if (likely(READ_ONCE(global_quarantine.bytes) <=
smp_load_acquire(&quarantine_size)))
return;

spin_lock_irqsave(&quarantine_lock, flags);

- /* Update quarantine size in case of hotplug. Allocate a fraction of
+ /*
+ * Update quarantine size in case of hotplug. Allocate a fraction of
* the installed memory to quarantine minus per-cpu queue limits.
*/
- new_quarantine_size = (ACCESS_ONCE(totalram_pages) << PAGE_SHIFT) /
+ new_quarantine_size = (READ_ONCE(totalram_pages) << PAGE_SHIFT) /
QUARANTINE_FRACTION;
new_quarantine_size -= QUARANTINE_PERCPU_SIZE * num_online_cpus();
/* Pairs with smp_load_acquire() above and in QUARANTINE_LOW_SIZE. */
@@ -222,10 +228,10 @@ void quarantine_reduce(void)
struct kmem_cache *cache = qlink_to_cache(last);

size_to_free += cache->size;
- if (!*last || size_to_free >
+ if (!last->next || size_to_free >
global_quarantine.bytes - QUARANTINE_LOW_SIZE)
break;
- last = (void **) *last;
+ last = last->next;
}
qlist_move(&global_quarantine, last, &to_free, size_to_free);

@@ -234,50 +240,46 @@ void quarantine_reduce(void)
qlist_free_all(&to_free, NULL);
}

-static void qlist_move_cache(struct qlist *from,
- struct qlist *to,
+static void qlist_move_cache(struct qlist_head *from,
+ struct qlist_head *to,
struct kmem_cache *cache)
{
- void ***prev;
+ struct qlist_node *prev;

if (unlikely(qlist_empty(from)))
return;

- prev = &from->head;
- while (*prev) {
- void **qlink = *prev;
+ prev = from->head;
+ while (prev) {
+ struct qlist_node *qlink = prev->next;
struct kmem_cache *obj_cache = qlink_to_cache(qlink);

if (obj_cache == cache) {
if (unlikely(from->tail == qlink))
- from->tail = (void **) prev;
- *prev = (void **) *qlink;
+ from->tail = prev;
+ prev = qlink->next;
from->bytes -= cache->size;
qlist_put(to, qlink, cache->size);
} else
- prev = (void ***) *prev;
+ prev = prev->next;
}
}

static void per_cpu_remove_cache(void *arg)
{
struct kmem_cache *cache = arg;
- struct qlist to_free = QLIST_INIT;
- struct qlist *q;
- unsigned long flags;
+ struct qlist_head to_free = QLIST_INIT;
+ struct qlist_head *q;

- local_irq_save(flags);
q = this_cpu_ptr(&cpu_quarantine);
qlist_move_cache(q, &to_free, cache);
- local_irq_restore(flags);
-
qlist_free_all(&to_free, cache);
}

void quarantine_remove_cache(struct kmem_cache *cache)
{
unsigned long flags;
- struct qlist to_free = QLIST_INIT;
+ struct qlist_head to_free = QLIST_INIT;

on_each_cpu(per_cpu_remove_cache, cache, 1);

diff --git a/mm/mempool.c b/mm/mempool.c
index 8655831..9e075f8 100644
--- a/mm/mempool.c
+++ b/mm/mempool.c
@@ -107,10 +107,9 @@ static void kasan_poison_element(mempool_t *pool, void *element)
if (pool->alloc == mempool_alloc_slab)
kasan_poison_slab_free(pool->pool_data, element);
if (pool->alloc == mempool_kmalloc)
- kasan_poison_kfree(element);
+ kasan_kfree(element);
if (pool->alloc == mempool_alloc_pages)
- kasan_poison_free_pages(element,
- (unsigned long)pool->pool_data);
+ kasan_free_pages(element, (unsigned long)pool->pool_data);
}

static void kasan_unpoison_element(mempool_t *pool, void *element, gfp_t flags)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 497befe..477d938 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -993,7 +993,7 @@ static __always_inline bool free_pages_prepare(struct page *page,

trace_mm_page_free(page, order);
kmemcheck_free_shadow(page, order);
- kasan_poison_free_pages(page, order);
+ kasan_free_pages(page, order);

/*
* Check tail pages before head page information is cleared to
diff --git a/mm/slab.c b/mm/slab.c
index 3f20800..cc8bbc1 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -3547,13 +3547,10 @@ free_done:
static inline void __cache_free(struct kmem_cache *cachep, void *objp,
unsigned long caller)
{
-#ifdef CONFIG_KASAN
+ /* Put the object into the quarantine, don't touch it for now. */
if (kasan_slab_free(cachep, objp))
- /* The object has been put into the quarantine, don't touch it
- * for now.
- */
return;
-#endif
+
___cache_free(cachep, objp, caller);
}

diff --git a/mm/slub.c b/mm/slub.c
index f41360e..538c858 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1319,7 +1319,7 @@ static inline void kmalloc_large_node_hook(void *ptr, size_t size, gfp_t flags)
static inline void kfree_hook(const void *x)
{
kmemleak_free(x);
- kasan_poison_kfree_large(x);
+ kasan_kfree_large(x);
}

static inline void slab_free_hook(struct kmem_cache *s, void *x)
@@ -1344,7 +1344,7 @@ static inline void slab_free_hook(struct kmem_cache *s, void *x)
if (!(s->flags & SLAB_DEBUG_OBJECTS))
debug_check_no_obj_freed(x, s->object_size);

- kasan_poison_slab_free(s, x);
+ kasan_slab_free(s, x);
}

static inline void slab_free_freelist_hook(struct kmem_cache *s,
--
2.7.3