Re: [PATCH] mm/slub: support left red zone

From: Joonsoo Kim
Date: Thu Feb 04 2016 - 01:21:41 EST

Next message: Viresh Kumar: "Re: [PATCH V2 0/7] cpufreq: governors: Fix ABBA lockups"
Previous message: Joonsoo Kim: "[PATCH v2 3/3] mm/compaction: speed up pageblock_pfn_to_page() when zone is contiguous"
In reply to: Joonsoo Kim: "[PATCH] mm/slub: support left red zone"
Next in thread: Christoph Lameter: "Re: [PATCH] mm/slub: support left red zone"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Thu, Feb 04, 2016 at 03:15:50PM +0900, Joonsoo Kim wrote:
> SLUB already has red zone debugging feature. But, it is only positioned
> at the end of object(aka right red zone) so it cannot catch left oob.
> Although current object's right red zone acts as left red zone of
> previous object, first object in a slab cannot take advantage of

Oops... s/previous/next.

> this effect. This patch explicitly add left red zone to each objects
> to detect left oob more precisely.
>
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@xxxxxxx>
> ---
> include/linux/slub_def.h | 1 +
> mm/slub.c | 158 ++++++++++++++++++++++++++++++++++-------------
> 2 files changed, 115 insertions(+), 44 deletions(-)
>
> diff --git a/include/linux/slub_def.h b/include/linux/slub_def.h
> index b7e57927..a33869b 100644
> --- a/include/linux/slub_def.h
> +++ b/include/linux/slub_def.h
> @@ -77,6 +77,7 @@ struct kmem_cache {
> int refcount; /* Refcount for slab cache destroy */
> void (*ctor)(void *);
> int inuse; /* Offset to metadata */
> + int red_left_pad; /* Left redzone padding size */
> int align; /* Alignment */
> int reserved; /* Reserved bytes at the end of slabs */
> const char *name; /* Name (only for display!) */
> diff --git a/mm/slub.c b/mm/slub.c
> index 7b5a965..7216769 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -39,6 +39,10 @@
>
> #include "internal.h"
>
> +#ifdef CONFIG_KASAN
> +#include "kasan/kasan.h"
> +#endif
> +
> /*
> * Lock order:
> * 1. slab_mutex (Global Mutex)
> @@ -124,6 +128,14 @@ static inline int kmem_cache_debug(struct kmem_cache *s)
> #endif
> }
>
> +static inline void *fixup_red_left(struct kmem_cache *s, void *p)
> +{
> + if (kmem_cache_debug(s) && s->flags & SLAB_RED_ZONE)
> + p += s->red_left_pad;
> +
> + return p;
> +}
> +
> static inline bool kmem_cache_has_cpu_partial(struct kmem_cache *s)
> {
> #ifdef CONFIG_SLUB_CPU_PARTIAL
> @@ -224,24 +236,6 @@ static inline void stat(const struct kmem_cache *s, enum stat_item si)
> * Core slab cache functions
> *******************************************************************/
>
> -/* Verify that a pointer has an address that is valid within a slab page */
> -static inline int check_valid_pointer(struct kmem_cache *s,
> - struct page *page, const void *object)
> -{
> - void *base;
> -
> - if (!object)
> - return 1;
> -
> - base = page_address(page);
> - if (object < base || object >= base + page->objects * s->size ||
> - (object - base) % s->size) {
> - return 0;
> - }
> -
> - return 1;
> -}
> -
> static inline void *get_freepointer(struct kmem_cache *s, void *object)
> {
> return *(void **)(object + s->offset);
> @@ -435,6 +429,22 @@ static void get_map(struct kmem_cache *s, struct page *page, unsigned long *map)
> set_bit(slab_index(p, s, addr), map);
> }
>
> +static inline int size_from_object(struct kmem_cache *s)
> +{
> + if (s->flags & SLAB_RED_ZONE)
> + return s->size - s->red_left_pad;
> +
> + return s->size;
> +}
> +
> +static inline void *restore_red_left(struct kmem_cache *s, void *p)
> +{
> + if (s->flags & SLAB_RED_ZONE)
> + p -= s->red_left_pad;
> +
> + return p;
> +}
> +
> /*
> * Debug settings:
> */
> @@ -468,6 +478,26 @@ static inline void metadata_access_disable(void)
> /*
> * Object debugging
> */
> +
> +/* Verify that a pointer has an address that is valid within a slab page */
> +static inline int check_valid_pointer(struct kmem_cache *s,
> + struct page *page, void *object)
> +{
> + void *base;
> +
> + if (!object)
> + return 1;
> +
> + base = page_address(page);
> + object = restore_red_left(s, object);
> + if (object < base || object >= base + page->objects * s->size ||
> + (object - base) % s->size) {
> + return 0;
> + }
> +
> + return 1;
> +}
> +
> static void print_section(char *text, u8 *addr, unsigned int length)
> {
> metadata_access_enable();
> @@ -607,7 +637,9 @@ static void print_trailer(struct kmem_cache *s, struct page *page, u8 *p)
> pr_err("INFO: Object 0x%p @offset=%tu fp=0x%p\n\n",
> p, p - addr, get_freepointer(s, p));
>
> - if (p > addr + 16)
> + if (s->flags & SLAB_RED_ZONE)
> + print_section("Redzone ", p - s->red_left_pad, s->red_left_pad);
> + else if (p > addr + 16)
> print_section("Bytes b4 ", p - 16, 16);
>
> print_section("Object ", p, min_t(unsigned long, s->object_size,
> @@ -624,9 +656,9 @@ static void print_trailer(struct kmem_cache *s, struct page *page, u8 *p)
> if (s->flags & SLAB_STORE_USER)
> off += 2 * sizeof(struct track);
>
> - if (off != s->size)
> + if (off != size_from_object(s))
> /* Beginning of the filler is the free pointer */
> - print_section("Padding ", p + off, s->size - off);
> + print_section("Padding ", p + off, size_from_object(s) - off);
>
> dump_stack();
> }
> @@ -656,6 +688,9 @@ static void init_object(struct kmem_cache *s, void *object, u8 val)
> {
> u8 *p = object;
>
> + if (s->flags & SLAB_RED_ZONE)
> + memset(p - s->red_left_pad, val, s->red_left_pad);
> +
> if (s->flags & __OBJECT_POISON) {
> memset(p, POISON_FREE, s->object_size - 1);
> p[s->object_size - 1] = POISON_END;
> @@ -748,11 +783,11 @@ static int check_pad_bytes(struct kmem_cache *s, struct page *page, u8 *p)
> /* We also have user information there */
> off += 2 * sizeof(struct track);
>
> - if (s->size == off)
> + if (size_from_object(s) == off)
> return 1;
>
> return check_bytes_and_report(s, page, p, "Object padding",
> - p + off, POISON_INUSE, s->size - off);
> + p + off, POISON_INUSE, size_from_object(s) - off);
> }
>
> /* Check the pad bytes at the end of a slab page */
> @@ -797,6 +832,10 @@ static int check_object(struct kmem_cache *s, struct page *page,
>
> if (s->flags & SLAB_RED_ZONE) {
> if (!check_bytes_and_report(s, page, object, "Redzone",
> + object - s->red_left_pad, val, s->red_left_pad))
> + return 0;
> +
> + if (!check_bytes_and_report(s, page, object, "Redzone",
> endobject, val, s->inuse - s->object_size))
> return 0;
> } else {
> @@ -998,14 +1037,17 @@ static inline void dec_slabs_node(struct kmem_cache *s, int node, int objects)
> }
>
> /* Object debug checks for alloc/free paths */
> -static void setup_object_debug(struct kmem_cache *s, struct page *page,
> +static void *setup_object_debug(struct kmem_cache *s, struct page *page,
> void *object)
> {
> if (!(s->flags & (SLAB_STORE_USER|SLAB_RED_ZONE|__OBJECT_POISON)))
> - return;
> + return object;
>
> + object = fixup_red_left(s, object);
> init_object(s, object, SLUB_RED_INACTIVE);
> init_tracking(s, object);
> +
> + return object;
> }
>
> static noinline int alloc_debug_processing(struct kmem_cache *s,
> @@ -1202,8 +1244,8 @@ unsigned long kmem_cache_flags(unsigned long object_size,
> return flags;
> }
> #else /* !CONFIG_SLUB_DEBUG */
> -static inline void setup_object_debug(struct kmem_cache *s,
> - struct page *page, void *object) {}
> +static inline void *setup_object_debug(struct kmem_cache *s,
> + struct page *page, void *object) { return object; }
>
> static inline int alloc_debug_processing(struct kmem_cache *s,
> struct page *page, void *object, unsigned long addr) { return 0; }
> @@ -1306,15 +1348,17 @@ static inline void slab_free_freelist_hook(struct kmem_cache *s,
> #endif
> }
>
> -static void setup_object(struct kmem_cache *s, struct page *page,
> +static void *setup_object(struct kmem_cache *s, struct page *page,
> void *object)
> {
> - setup_object_debug(s, page, object);
> + object = setup_object_debug(s, page, object);
> if (unlikely(s->ctor)) {
> kasan_unpoison_object_data(s, object);
> s->ctor(object);
> kasan_poison_object_data(s, object);
> }
> +
> + return object;
> }
>
> /*
> @@ -1410,14 +1454,16 @@ static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node)
> kasan_poison_slab(page);
>
> for_each_object_idx(p, idx, s, start, page->objects) {
> - setup_object(s, page, p);
> - if (likely(idx < page->objects))
> - set_freepointer(s, p, p + s->size);
> - else
> - set_freepointer(s, p, NULL);
> + void *object = setup_object(s, page, p);
> +
> + if (likely(idx < page->objects)) {
> + set_freepointer(s, object,
> + fixup_red_left(s, p + s->size));
> + } else
> + set_freepointer(s, object, NULL);
> }
>
> - page->freelist = start;
> + page->freelist = fixup_red_left(s, start);
> page->inuse = page->objects;
> page->frozen = 1;
>
> @@ -1458,8 +1504,11 @@ static void __free_slab(struct kmem_cache *s, struct page *page)
>
> slab_pad_check(s, page);
> for_each_object(p, s, page_address(page),
> - page->objects)
> - check_object(s, page, p, SLUB_RED_INACTIVE);
> + page->objects) {
> + void *object = fixup_red_left(s, p);
> +
> + check_object(s, page, object, SLUB_RED_INACTIVE);
> + }
> }
>
> kmemcheck_free_shadow(page, compound_order(page));
> @@ -3256,6 +3305,16 @@ static int calculate_sizes(struct kmem_cache *s, int forced_order)
> * of the object.
> */
> size += sizeof(void *);
> +
> + if (flags & SLAB_RED_ZONE) {
> + s->red_left_pad = sizeof(void *);
> +#ifdef CONFIG_KASAN
> + s->red_left_pad = min_t(int, s->red_left_pad,
> + KASAN_SHADOW_SCALE_SIZE);
> +#endif
> + s->red_left_pad = ALIGN(s->red_left_pad, s->align);
> + size += s->red_left_pad;
> + }
> #endif
>
> /*
> @@ -3392,10 +3451,12 @@ static void list_slab_objects(struct kmem_cache *s, struct page *page,
>
> get_map(s, page, map);
> for_each_object(p, s, addr, page->objects) {
> + void *object = fixup_red_left(s, p);
>
> if (!test_bit(slab_index(p, s, addr), map)) {
> - pr_err("INFO: Object 0x%p @offset=%tu\n", p, p - addr);
> - print_tracking(s, p);
> + pr_err("INFO: Object 0x%p @offset=%tu\n",
> + object, object - addr);
> + print_tracking(s, object);
> }
> }
> slab_unlock(page);
> @@ -4064,15 +4125,21 @@ static int validate_slab(struct kmem_cache *s, struct page *page,
>
> get_map(s, page, map);
> for_each_object(p, s, addr, page->objects) {
> + void *object = fixup_red_left(s, p);
> +
> if (test_bit(slab_index(p, s, addr), map))
> - if (!check_object(s, page, p, SLUB_RED_INACTIVE))
> + if (!check_object(s, page, object, SLUB_RED_INACTIVE))
> return 0;
> }
>
> - for_each_object(p, s, addr, page->objects)
> + for_each_object(p, s, addr, page->objects) {
> + void *object = fixup_red_left(s, p);
> +
> if (!test_bit(slab_index(p, s, addr), map))
> - if (!check_object(s, page, p, SLUB_RED_ACTIVE))
> + if (!check_object(s, page, object, SLUB_RED_ACTIVE))
> return 0;
> + }
> +
> return 1;
> }
>
> @@ -4270,9 +4337,12 @@ static void process_slab(struct loc_track *t, struct kmem_cache *s,
> bitmap_zero(map, page->objects);
> get_map(s, page, map);
>
> - for_each_object(p, s, addr, page->objects)
> + for_each_object(p, s, addr, page->objects) {
> + void *object = fixup_red_left(s, p);
> +
> if (!test_bit(slab_index(p, s, addr), map))
> - add_location(t, s, get_track(s, p, alloc));
> + add_location(t, s, get_track(s, object, alloc));
> + }
> }
>
> static int list_locations(struct kmem_cache *s, char *buf,
> --
> 1.9.1
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@xxxxxxxxxx For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>

Next message: Viresh Kumar: "Re: [PATCH V2 0/7] cpufreq: governors: Fix ABBA lockups"
Previous message: Joonsoo Kim: "[PATCH v2 3/3] mm/compaction: speed up pageblock_pfn_to_page() when zone is contiguous"
In reply to: Joonsoo Kim: "[PATCH] mm/slub: support left red zone"
Next in thread: Christoph Lameter: "Re: [PATCH] mm/slub: support left red zone"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]