On Fri, Mar 30, 2018 at 7:47 PM, Andrey Ryabinin
<aryabinin@xxxxxxxxxxxxx> wrote:
> On 03/23/2018 09:05 PM, Andrey Konovalov wrote:
>> This commit adds KHWASAN hooks implementation.
>>
>> 1. When a new slab cache is created, KHWASAN rounds up the size of the
>> objects in this cache to KASAN_SHADOW_SCALE_SIZE (== 16).
>>
>> 2. On each kmalloc KHWASAN generates a random tag, sets the shadow memory,
>> that corresponds to this object to this tag, and embeds this tag value
>> into the top byte of the returned pointer.
>>
>> 3. On each kfree KHWASAN poisons the shadow memory with a random tag to
>> allow detection of use-after-free bugs.
>>
>> The rest of the logic of the hook implementation is very much similar to
>> the one provided by KASAN. KHWASAN saves allocation and free stack metadata
>> to the slab object the same was KASAN does this.
>>
>> Signed-off-by: Andrey Konovalov <andreyknvl@xxxxxxxxxx>
>> ---
>> mm/kasan/khwasan.c | 200 ++++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 197 insertions(+), 3 deletions(-)
>>
>> diff --git a/mm/kasan/khwasan.c b/mm/kasan/khwasan.c
>> index da4b17997c71..e8bed5a078c7 100644
>> --- a/mm/kasan/khwasan.c
>> +++ b/mm/kasan/khwasan.c
>> @@ -90,69 +90,260 @@ void *khwasan_reset_tag(const void *addr)
>> return reset_tag(addr);
>> }
>>
>> +void kasan_poison_shadow(const void *address, size_t size, u8 value)
>> +{
>> + void *shadow_start, *shadow_end;
>> +
>> + /* Perform shadow offset calculation based on untagged address */
>> + address = reset_tag(address);
>> +
>> + shadow_start = kasan_mem_to_shadow(address);
>> + shadow_end = kasan_mem_to_shadow(address + size);
>> +
>> + memset(shadow_start, value, shadow_end - shadow_start);
>> +}
>> +
>> void kasan_unpoison_shadow(const void *address, size_t size)
>> {
>> + /* KHWASAN only allows 16-byte granularity */
>> + size = round_up(size, KASAN_SHADOW_SCALE_SIZE);
>> + kasan_poison_shadow(address, size, get_tag(address));
>> }
>>
>
>
> This is way too much of copy-paste/code duplication. Ideally, you should have only
> check_memory_region() stuff separated, the rest (poisoning/unpoisoning, slabs management) should be
> in common.c code.
>
> So it should be something like this:
>
> in kasan.h
> ...
> #ifdef CONFIG_KASAN_CLASSIC
> #define KASAN_FREE_PAGE 0xFF /* page was freed */
> #define KASAN_PAGE_REDZONE 0xFE /* redzone for kmalloc_large allocations */
> #define KASAN_KMALLOC_REDZONE 0xFC /* redzone inside slub object */
> #define KASAN_KMALLOC_FREE 0xFB /* object was freed (kmem_cache_free/kfree) */
> #else
> #define KASAN_FREE_PAGE 0xFE
> #define KASAN_PAGE_REDZONE 0xFE
> #define KASAN_KMALLOC_REDZONE 0xFE
> #define KASAN_KMALLOC_FREE 0xFE
> #endif
>
> ...
>
> #ifdef CONFIG_KASAN_CLASSIC
> static inline void *reset_tag(const void *addr)
> {
> return (void *)addr;
> }
> static inline u8 get_tag(const void *addr)
> {
> return 0;
> }
> #else
> static inline u8 get_tag(const void *addr)
> {
> return (u8)((u64)addr >> KHWASAN_TAG_SHIFT);
> }
>
> static inline void *reset_tag(const void *addr)
> {
> return set_tag(addr, KHWASAN_TAG_KERNEL);
> }
> #endif
>
>
> in kasan/common.c:
>
>
> void kasan_poison_shadow(const void *address, size_t size, u8 value)
> {
> void *shadow_start, *shadow_end;
>
> address = reset_tag(address);
>
> shadow_start = kasan_mem_to_shadow(address);
> shadow_end = kasan_mem_to_shadow(address + size);
>
> memset(shadow_start, value, shadow_end - shadow_start);
> }
>
> void kasan_unpoison_shadow(const void *address, size_t size)
> {
>
> kasan_poison_shadow(address, size, get_tag(address));
>
> if (size & KASAN_SHADOW_MASK) {
> u8 *shadow = (u8 *)kasan_mem_to_shadow(address + size);
>
> if (IS_ENABLED(CONFIG_KASAN_TAGS)
> *shadow = get_tag(address);
> else
> *shadow = size & KASAN_SHADOW_MASK;
> }
> }
>
> void kasan_free_pages(struct page *page, unsigned int order)
> {
> if (likely(!PageHighMem(page)))
> kasan_poison_shadow(page_address(page),
> PAGE_SIZE << order,
> KASAN_FREE_PAGE);
> }
>
> etc.
OK, I'll rework this part.
>
>
>
>> void check_memory_region(unsigned long addr, size_t size, bool write,
>> unsigned long ret_ip)
>> {
>> + u8 tag;
>> + u8 *shadow_first, *shadow_last, *shadow;
>> + void *untagged_addr;
>> +
>> + tag = get_tag((const void *)addr);
>> +
>> + /* Ignore accesses for pointers tagged with 0xff (native kernel
>> + * pointer tag) to suppress false positives caused by kmap.
>> + *
>> + * Some kernel code was written to account for archs that don't keep
>> + * high memory mapped all the time, but rather map and unmap particular
>> + * pages when needed. Instead of storing a pointer to the kernel memory,
>> + * this code saves the address of the page structure and offset within
>> + * that page for later use. Those pages are then mapped and unmapped
>> + * with kmap/kunmap when necessary and virt_to_page is used to get the
>> + * virtual address of the page. For arm64 (that keeps the high memory
>> + * mapped all the time), kmap is turned into a page_address call.
>> +
>> + * The issue is that with use of the page_address + virt_to_page
>> + * sequence the top byte value of the original pointer gets lost (gets
>> + * set to 0xff.
>> + */
>> + if (tag == 0xff)
>> + return;
>
> You can save tag somewhere in page struct and make page_address() return tagged address.
>
> I'm not sure it might be even possible to squeeze the tag into page->flags on some configurations,
> see include/linux/page-flags-layout.h
One page can contain multiple objects with different tags, so we would
need to save the tag for each of them.
>
>
>> void *kasan_slab_alloc(struct kmem_cache *cache, void *object, gfp_t flags)
>> {
>> + if (!READ_ONCE(khwasan_enabled))
>> + return object;
>
> ...
>
>> void *kasan_kmalloc(struct kmem_cache *cache, const void *object,
>> size_t size, gfp_t flags)
>> {
>
>> + if (!READ_ONCE(khwasan_enabled))
>> + return (void *)object;
>> +
>
> ...
>
>> void *kasan_kmalloc_large(const void *ptr, size_t size, gfp_t flags)
>> {
>
> ...
>
>> +
>> + if (!READ_ONCE(khwasan_enabled))
>> + return (void *)ptr;
>> +
>
> I don't see any possible way of khwasan_enabled being 0 here.
Can't kmem_cache_alloc be called for the temporary caches that are
used before the slab allocator and kasan are initialized?
>
> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+unsubscribe@xxxxxxxxxxxxxxxx.
> To post to this group, send email to kasan-dev@xxxxxxxxxxxxxxxx.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/805d1e85-2d3c-2327-6e6c-f14a56dc0b67%40virtuozzo.com.
> For more options, visit https://groups.google.com/d/optout.
