On 09/26/2014 08:48 AM, Dmitry Vyukov wrote:
> On Wed, Sep 24, 2014 at 5:44 AM, Andrey Ryabinin <a.ryabinin@xxxxxxxxxxx> wrote:
>> --- a/lib/Kconfig.kasan
>> +++ b/lib/Kconfig.kasan
>> @@ -6,6 +6,7 @@ if HAVE_ARCH_KASAN
>> config KASAN
>> bool "AddressSanitizer: runtime memory debugger"
>> depends on !MEMORY_HOTPLUG
>> + depends on SLUB_DEBUG
>
>
> What does SLUB_DEBUG do? I think that generally we don't want any
> other *heavy* debug checks to be required for kasan.
>
SLUB_DEBUG enables support for different debugging features.
It doesn't enables this debugging features by default, it only allows
you to switch them on/off in runtime.
Generally SLUB_DEBUG option is enabled in most kernels. SLUB_DEBUG disabled
only with intention to get minimal kernel.
Without SLUB_DEBUG there will be no redzones, no user tracking info (allocation/free stacktraces).
KASAN won't be so usefull without SLUB_DEBUG.
[...]
>> --- a/mm/kasan/kasan.c
>> +++ b/mm/kasan/kasan.c
>> @@ -30,6 +30,7 @@
>> #include <linux/kasan.h>
>>
>> #include "kasan.h"
>> +#include "../slab.h"
>>
>> /*
>> * Poisons the shadow memory for 'size' bytes starting from 'addr'.
>> @@ -265,6 +266,102 @@ void kasan_free_pages(struct page *page, unsigned int order)
>> KASAN_FREE_PAGE);
>> }
>>
>> +void kasan_free_slab_pages(struct page *page, int order)
>
> Doesn't this callback followed by actually freeing the pages, and so
> kasan_free_pages callback that will poison the range? If so, I would
> prefer to not double poison.
>
Yes, this could be removed.
>
>> +{
>> + kasan_poison_shadow(page_address(page),
>> + PAGE_SIZE << order, KASAN_SLAB_FREE);
>> +}
>> +
>> +void kasan_mark_slab_padding(struct kmem_cache *s, void *object)
>> +{
>> + unsigned long object_end = (unsigned long)object + s->size;
>> + unsigned long padding_end = round_up(object_end, PAGE_SIZE);
>> + unsigned long padding_start = round_up(object_end,
>> + KASAN_SHADOW_SCALE_SIZE);
>> + size_t size = padding_end - padding_start;
>> +
>> + if (size)
>> + kasan_poison_shadow((void *)padding_start,
>> + size, KASAN_SLAB_PADDING);
>> +}
>> +
>> +void kasan_slab_alloc(struct kmem_cache *cache, void *object)
>> +{
>> + kasan_kmalloc(cache, object, cache->object_size);
>> +}
>> +
>> +void kasan_slab_free(struct kmem_cache *cache, void *object)
>> +{
>> + unsigned long size = cache->size;
>> + unsigned long rounded_up_size = round_up(size, KASAN_SHADOW_SCALE_SIZE);
>> +
>
> Add a comment saying that SLAB_DESTROY_BY_RCU objects can be "legally"
> used after free.
>
Ok.
>> + if (unlikely(cache->flags & SLAB_DESTROY_BY_RCU))
>> + return;
>> +
>> + kasan_poison_shadow(object, rounded_up_size, KASAN_KMALLOC_FREE);
>> +}
>> +
>> +void kasan_kmalloc(struct kmem_cache *cache, const void *object, size_t size)
>> +{
>> + unsigned long redzone_start;
>> + unsigned long redzone_end;
>> +
>> + if (unlikely(object == NULL))
>> + return;
>> +
>> + redzone_start = round_up((unsigned long)(object + size),
>> + KASAN_SHADOW_SCALE_SIZE);
>> + redzone_end = (unsigned long)object + cache->size;
>> +
>> + kasan_unpoison_shadow(object, size);
>> + kasan_poison_shadow((void *)redzone_start, redzone_end - redzone_start,
>> + KASAN_KMALLOC_REDZONE);
>> +
>> +}
>> +EXPORT_SYMBOL(kasan_kmalloc);
>> +
>> +void kasan_kmalloc_large(const void *ptr, size_t size)
>> +{
>> + struct page *page;
>> + unsigned long redzone_start;
>> + unsigned long redzone_end;
>> +
>> + if (unlikely(ptr == NULL))
>> + return;
>> +
>> + page = virt_to_page(ptr);
>> + redzone_start = round_up((unsigned long)(ptr + size),
>> + KASAN_SHADOW_SCALE_SIZE);
>> + redzone_end = (unsigned long)ptr + (PAGE_SIZE << compound_order(page));
>
> If size == N*PAGE_SIZE - KASAN_SHADOW_SCALE_SIZE - 1, the object does
> not receive any redzone at all.
If size == N*PAGE_SIZE - KASAN_SHADOW_SCALE_SIZE - 1, there will be redzone
KASAN_SHADOW_SCALE_SIZE + 1 bytes. There will be no readzone if and only if
(size == PAGE_SIZE << compound_order(page))
> Can we pass full memory block size
> from above to fix it? Will compound_order(page) do?
>
What is full memory block size?
PAGE_SIZE << compound_order(page) is how much was really allocated.
[..]
>>
>> static struct page *new_slab(struct kmem_cache *s, gfp_t flags, int node)
>> @@ -1416,8 +1426,10 @@ static struct page *new_slab(struct kmem_cache *s, gfp_t flags, int node)
>> setup_object(s, page, p);
>> if (likely(idx < page->objects))
>> set_freepointer(s, p, p + s->size);
>
> Sorry, I don't fully follow this code, so I will just ask some questions.
> Can we have some slab padding after last object in this case as well?
>
This case is for not the last object. Padding is the place after the last object.
The last object initialized bellow in else case.
>> - else
>> + else {
>> set_freepointer(s, p, NULL);
>> + kasan_mark_slab_padding(s, p);
>
> kasan_mark_slab_padding poisons only up to end of the page. Can there
> be multiple pages that we need to poison?
>
Yep, that's a good catch.
Thanks.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>