From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932247AbbBPEKv (ORCPT ); Sun, 15 Feb 2015 23:10:51 -0500 Received: from ozlabs.org ([103.22.144.67]:49290 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752565AbbBPEKs (ORCPT ); Sun, 15 Feb 2015 23:10:48 -0500 From: Rusty Russell To: Andrey Ryabinin , linux-kernel@vger.kernel.org Cc: Andrey Ryabinin , Dmitry Vyukov , Konstantin Serebryany , Dmitry Chernenkov , Andrey Konovalov , Yuri Gribov , Konstantin Khlebnikov , Sasha Levin , Christoph Lameter , Joonsoo Kim , Andrew Morton , Dave Hansen , Andi Kleen , x86@kernel.org, linux-mm@kvack.org, Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , Michal Marek , "open list\:KERNEL BUILD + fi..." Subject: Re: [PATCH v11 19/19] kasan: enable instrumentation of global variables In-Reply-To: <1422985392-28652-20-git-send-email-a.ryabinin@samsung.com> References: <1404905415-9046-1-git-send-email-a.ryabinin@samsung.com> <1422985392-28652-1-git-send-email-a.ryabinin@samsung.com> <1422985392-28652-20-git-send-email-a.ryabinin@samsung.com> User-Agent: Notmuch/0.17 (http://notmuchmail.org) Emacs/24.3.1 (x86_64-pc-linux-gnu) Date: Mon, 16 Feb 2015 13:28:09 +1030 Message-ID: <87a90ea7ge.fsf@rustcorp.com.au> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Andrey Ryabinin writes: > This feature let us to detect accesses out of bounds of > global variables. This will work as for globals in kernel > image, so for globals in modules. Currently this won't work > for symbols in user-specified sections (e.g. __init, __read_mostly, ...) > > The idea of this is simple. Compiler increases each global variable > by redzone size and add constructors invoking __asan_register_globals() > function. Information about global variable (address, size, > size with redzone ...) passed to __asan_register_globals() so we could > poison variable's redzone. > > This patch also forces module_alloc() to return 8*PAGE_SIZE aligned > address making shadow memory handling ( kasan_module_alloc()/kasan_module_free() ) > more simple. Such alignment guarantees that each shadow page backing > modules address space correspond to only one module_alloc() allocation. Hmm, I understand why you only fixed x86, but it's weird. I think MODULE_ALIGN belongs in linux/moduleloader.h, and every arch should be fixed up to use it (though you could leave that for later). Might as well fix the default implementation at least. > @@ -49,8 +49,15 @@ void kasan_krealloc(const void *object, size_t new_size); > void kasan_slab_alloc(struct kmem_cache *s, void *object); > void kasan_slab_free(struct kmem_cache *s, void *object); > > +#define MODULE_ALIGN (PAGE_SIZE << KASAN_SHADOW_SCALE_SHIFT) > + > +int kasan_module_alloc(void *addr, size_t size); > +void kasan_module_free(void *addr); > + > #else /* CONFIG_KASAN */ > > +#define MODULE_ALIGN 1 Hmm, that should be PAGE_SIZE (we assume that in several places). > @@ -1807,6 +1808,7 @@ static void unset_module_init_ro_nx(struct module *mod) { } > void __weak module_memfree(void *module_region) > { > vfree(module_region); > + kasan_module_free(module_region); > } This looks racy (memory reuse?). Perhaps try other order? Cheers, Rusty. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ozlabs.org ([103.22.144.67]:49290 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752565AbbBPEKs (ORCPT ); Sun, 15 Feb 2015 23:10:48 -0500 From: Rusty Russell Subject: Re: [PATCH v11 19/19] kasan: enable instrumentation of global variables In-Reply-To: <1422985392-28652-20-git-send-email-a.ryabinin@samsung.com> References: <1404905415-9046-1-git-send-email-a.ryabinin@samsung.com> <1422985392-28652-1-git-send-email-a.ryabinin@samsung.com> <1422985392-28652-20-git-send-email-a.ryabinin@samsung.com> Date: Mon, 16 Feb 2015 13:28:09 +1030 Message-ID: <87a90ea7ge.fsf@rustcorp.com.au> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kbuild-owner@vger.kernel.org List-ID: To: Andrey Ryabinin , linux-kernel@vger.kernel.org Cc: Dmitry Vyukov , Konstantin Serebryany , Dmitry Chernenkov , Andrey Konovalov , Yuri Gribov , Konstantin Khlebnikov , Sasha Levin , Christoph Lameter , Joonsoo Kim , Andrew Morton , Dave Hansen , Andi Kleen , x86@kernel.org, linux-mm@kvack.org, Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , Michal Marek , "open list:KERNEL BUILD + fi..." Andrey Ryabinin writes: > This feature let us to detect accesses out of bounds of > global variables. This will work as for globals in kernel > image, so for globals in modules. Currently this won't work > for symbols in user-specified sections (e.g. __init, __read_mostly, ...) > > The idea of this is simple. Compiler increases each global variable > by redzone size and add constructors invoking __asan_register_globals() > function. Information about global variable (address, size, > size with redzone ...) passed to __asan_register_globals() so we could > poison variable's redzone. > > This patch also forces module_alloc() to return 8*PAGE_SIZE aligned > address making shadow memory handling ( kasan_module_alloc()/kasan_module_free() ) > more simple. Such alignment guarantees that each shadow page backing > modules address space correspond to only one module_alloc() allocation. Hmm, I understand why you only fixed x86, but it's weird. I think MODULE_ALIGN belongs in linux/moduleloader.h, and every arch should be fixed up to use it (though you could leave that for later). Might as well fix the default implementation at least. > @@ -49,8 +49,15 @@ void kasan_krealloc(const void *object, size_t new_size); > void kasan_slab_alloc(struct kmem_cache *s, void *object); > void kasan_slab_free(struct kmem_cache *s, void *object); > > +#define MODULE_ALIGN (PAGE_SIZE << KASAN_SHADOW_SCALE_SHIFT) > + > +int kasan_module_alloc(void *addr, size_t size); > +void kasan_module_free(void *addr); > + > #else /* CONFIG_KASAN */ > > +#define MODULE_ALIGN 1 Hmm, that should be PAGE_SIZE (we assume that in several places). > @@ -1807,6 +1808,7 @@ static void unset_module_init_ro_nx(struct module *mod) { } > void __weak module_memfree(void *module_region) > { > vfree(module_region); > + kasan_module_free(module_region); > } This looks racy (memory reuse?). Perhaps try other order? Cheers, Rusty. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pa0-f54.google.com (mail-pa0-f54.google.com [209.85.220.54]) by kanga.kvack.org (Postfix) with ESMTP id 5D7526B0032 for ; Sun, 15 Feb 2015 23:10:52 -0500 (EST) Received: by mail-pa0-f54.google.com with SMTP id kx10so32353897pab.13 for ; Sun, 15 Feb 2015 20:10:52 -0800 (PST) Received: from ozlabs.org (ozlabs.org. [2401:3900:2:1::2]) by mx.google.com with ESMTPS id jq1si136295pbc.53.2015.02.15.20.10.50 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 15 Feb 2015 20:10:51 -0800 (PST) From: Rusty Russell Subject: Re: [PATCH v11 19/19] kasan: enable instrumentation of global variables In-Reply-To: <1422985392-28652-20-git-send-email-a.ryabinin@samsung.com> References: <1404905415-9046-1-git-send-email-a.ryabinin@samsung.com> <1422985392-28652-1-git-send-email-a.ryabinin@samsung.com> <1422985392-28652-20-git-send-email-a.ryabinin@samsung.com> Date: Mon, 16 Feb 2015 13:28:09 +1030 Message-ID: <87a90ea7ge.fsf@rustcorp.com.au> MIME-Version: 1.0 Content-Type: text/plain Sender: owner-linux-mm@kvack.org List-ID: To: Andrey Ryabinin , linux-kernel@vger.kernel.org Cc: Dmitry Vyukov , Konstantin Serebryany , Dmitry Chernenkov , Andrey Konovalov , Yuri Gribov , Konstantin Khlebnikov , Sasha Levin , Christoph Lameter , Joonsoo Kim , Andrew Morton , Dave Hansen , Andi Kleen , x86@kernel.org, linux-mm@kvack.org, Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , Michal Marek , "open list:KERNEL BUILD + fi..." Andrey Ryabinin writes: > This feature let us to detect accesses out of bounds of > global variables. This will work as for globals in kernel > image, so for globals in modules. Currently this won't work > for symbols in user-specified sections (e.g. __init, __read_mostly, ...) > > The idea of this is simple. Compiler increases each global variable > by redzone size and add constructors invoking __asan_register_globals() > function. Information about global variable (address, size, > size with redzone ...) passed to __asan_register_globals() so we could > poison variable's redzone. > > This patch also forces module_alloc() to return 8*PAGE_SIZE aligned > address making shadow memory handling ( kasan_module_alloc()/kasan_module_free() ) > more simple. Such alignment guarantees that each shadow page backing > modules address space correspond to only one module_alloc() allocation. Hmm, I understand why you only fixed x86, but it's weird. I think MODULE_ALIGN belongs in linux/moduleloader.h, and every arch should be fixed up to use it (though you could leave that for later). Might as well fix the default implementation at least. > @@ -49,8 +49,15 @@ void kasan_krealloc(const void *object, size_t new_size); > void kasan_slab_alloc(struct kmem_cache *s, void *object); > void kasan_slab_free(struct kmem_cache *s, void *object); > > +#define MODULE_ALIGN (PAGE_SIZE << KASAN_SHADOW_SCALE_SHIFT) > + > +int kasan_module_alloc(void *addr, size_t size); > +void kasan_module_free(void *addr); > + > #else /* CONFIG_KASAN */ > > +#define MODULE_ALIGN 1 Hmm, that should be PAGE_SIZE (we assume that in several places). > @@ -1807,6 +1808,7 @@ static void unset_module_init_ro_nx(struct module *mod) { } > void __weak module_memfree(void *module_region) > { > vfree(module_region); > + kasan_module_free(module_region); > } This looks racy (memory reuse?). Perhaps try other order? Cheers, Rusty. -- 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: email@kvack.org