From: Marco Elver <elver@google.com> To: andrey.konovalov@linux.dev Cc: Alexander Potapenko <glider@google.com>, Vincenzo Frascino <vincenzo.frascino@arm.com>, Catalin Marinas <catalin.marinas@arm.com>, Peter Collingbourne <pcc@google.com>, Andrey Konovalov <andreyknvl@gmail.com>, Dmitry Vyukov <dvyukov@google.com>, kasan-dev@googlegroups.com, Andrew Morton <akpm@linux-foundation.org>, linux-mm@kvack.org, Will Deacon <will@kernel.org>, linux-arm-kernel@lists.infradead.org, Evgenii Stepanov <eugenis@google.com>, linux-kernel@vger.kernel.org, Andrey Konovalov <andreyknvl@google.com> Subject: Re: [PATCH 20/31] kasan, vmalloc: reset tags in vmalloc functions Date: Thu, 2 Dec 2021 15:17:04 +0100 [thread overview] Message-ID: <YajVYNBDOyI3hTx1@elver.google.com> (raw) In-Reply-To: <f405e36b20bd5d79dffef3f70b523885dcc6b163.1638308023.git.andreyknvl@google.com> On Tue, Nov 30, 2021 at 11:07PM +0100, andrey.konovalov@linux.dev wrote: > From: Andrey Konovalov <andreyknvl@google.com> > > In preparation for adding vmalloc support to SW/HW_TAGS KASAN, > reset pointer tags in functions that use pointer values in > range checks. > > vread() is a special case here. Resetting the pointer tag in its > prologue could technically lead to missing bad accesses to virtual > mappings in its implementation. However, vread() doesn't access the > virtual mappings cirectly. Instead, it recovers the physical address s/cirectly/directly/ But this paragraph is a little confusing, because first you point out that vread() might miss bad accesses, but then say that it does checked accesses. I think to avoid confusing the reader, maybe just say that vread() is checked, but hypothetically, should its implementation change to directly access addr, invalid accesses might be missed. Did I get this right? Or am I still confused? > via page_address(vmalloc_to_page()) and acceses that. And as > page_address() recovers the pointer tag, the accesses are checked. > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > --- > mm/vmalloc.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c > index c5235e3e5857..a059b3100c0a 100644 > --- a/mm/vmalloc.c > +++ b/mm/vmalloc.c > @@ -72,7 +72,7 @@ static const bool vmap_allow_huge = false; > > bool is_vmalloc_addr(const void *x) > { > - unsigned long addr = (unsigned long)x; > + unsigned long addr = (unsigned long)kasan_reset_tag(x); > > return addr >= VMALLOC_START && addr < VMALLOC_END; > } > @@ -630,7 +630,7 @@ int is_vmalloc_or_module_addr(const void *x) > * just put it in the vmalloc space. > */ > #if defined(CONFIG_MODULES) && defined(MODULES_VADDR) > - unsigned long addr = (unsigned long)x; > + unsigned long addr = (unsigned long)kasan_reset_tag(x); > if (addr >= MODULES_VADDR && addr < MODULES_END) > return 1; > #endif > @@ -804,6 +804,8 @@ static struct vmap_area *find_vmap_area_exceed_addr(unsigned long addr) > struct vmap_area *va = NULL; > struct rb_node *n = vmap_area_root.rb_node; > > + addr = (unsigned long)kasan_reset_tag((void *)addr); > + > while (n) { > struct vmap_area *tmp; > > @@ -825,6 +827,8 @@ static struct vmap_area *__find_vmap_area(unsigned long addr) > { > struct rb_node *n = vmap_area_root.rb_node; > > + addr = (unsigned long)kasan_reset_tag((void *)addr); > + > while (n) { > struct vmap_area *va; > > @@ -2143,7 +2147,7 @@ EXPORT_SYMBOL_GPL(vm_unmap_aliases); > void vm_unmap_ram(const void *mem, unsigned int count) > { > unsigned long size = (unsigned long)count << PAGE_SHIFT; > - unsigned long addr = (unsigned long)mem; > + unsigned long addr = (unsigned long)kasan_reset_tag(mem); > struct vmap_area *va; > > might_sleep(); > @@ -3361,6 +3365,8 @@ long vread(char *buf, char *addr, unsigned long count) > unsigned long buflen = count; > unsigned long n; > > + addr = kasan_reset_tag(addr); > + > /* Don't allow overflow */ > if ((unsigned long) addr + count < count) > count = -(unsigned long) addr; > -- > 2.25.1 >
WARNING: multiple messages have this Message-ID (diff)
From: Marco Elver <elver@google.com> To: andrey.konovalov@linux.dev Cc: Alexander Potapenko <glider@google.com>, Vincenzo Frascino <vincenzo.frascino@arm.com>, Catalin Marinas <catalin.marinas@arm.com>, Peter Collingbourne <pcc@google.com>, Andrey Konovalov <andreyknvl@gmail.com>, Dmitry Vyukov <dvyukov@google.com>, kasan-dev@googlegroups.com, Andrew Morton <akpm@linux-foundation.org>, linux-mm@kvack.org, Will Deacon <will@kernel.org>, linux-arm-kernel@lists.infradead.org, Evgenii Stepanov <eugenis@google.com>, linux-kernel@vger.kernel.org, Andrey Konovalov <andreyknvl@google.com> Subject: Re: [PATCH 20/31] kasan, vmalloc: reset tags in vmalloc functions Date: Thu, 2 Dec 2021 15:17:04 +0100 [thread overview] Message-ID: <YajVYNBDOyI3hTx1@elver.google.com> (raw) In-Reply-To: <f405e36b20bd5d79dffef3f70b523885dcc6b163.1638308023.git.andreyknvl@google.com> On Tue, Nov 30, 2021 at 11:07PM +0100, andrey.konovalov@linux.dev wrote: > From: Andrey Konovalov <andreyknvl@google.com> > > In preparation for adding vmalloc support to SW/HW_TAGS KASAN, > reset pointer tags in functions that use pointer values in > range checks. > > vread() is a special case here. Resetting the pointer tag in its > prologue could technically lead to missing bad accesses to virtual > mappings in its implementation. However, vread() doesn't access the > virtual mappings cirectly. Instead, it recovers the physical address s/cirectly/directly/ But this paragraph is a little confusing, because first you point out that vread() might miss bad accesses, but then say that it does checked accesses. I think to avoid confusing the reader, maybe just say that vread() is checked, but hypothetically, should its implementation change to directly access addr, invalid accesses might be missed. Did I get this right? Or am I still confused? > via page_address(vmalloc_to_page()) and acceses that. And as > page_address() recovers the pointer tag, the accesses are checked. > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > --- > mm/vmalloc.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c > index c5235e3e5857..a059b3100c0a 100644 > --- a/mm/vmalloc.c > +++ b/mm/vmalloc.c > @@ -72,7 +72,7 @@ static const bool vmap_allow_huge = false; > > bool is_vmalloc_addr(const void *x) > { > - unsigned long addr = (unsigned long)x; > + unsigned long addr = (unsigned long)kasan_reset_tag(x); > > return addr >= VMALLOC_START && addr < VMALLOC_END; > } > @@ -630,7 +630,7 @@ int is_vmalloc_or_module_addr(const void *x) > * just put it in the vmalloc space. > */ > #if defined(CONFIG_MODULES) && defined(MODULES_VADDR) > - unsigned long addr = (unsigned long)x; > + unsigned long addr = (unsigned long)kasan_reset_tag(x); > if (addr >= MODULES_VADDR && addr < MODULES_END) > return 1; > #endif > @@ -804,6 +804,8 @@ static struct vmap_area *find_vmap_area_exceed_addr(unsigned long addr) > struct vmap_area *va = NULL; > struct rb_node *n = vmap_area_root.rb_node; > > + addr = (unsigned long)kasan_reset_tag((void *)addr); > + > while (n) { > struct vmap_area *tmp; > > @@ -825,6 +827,8 @@ static struct vmap_area *__find_vmap_area(unsigned long addr) > { > struct rb_node *n = vmap_area_root.rb_node; > > + addr = (unsigned long)kasan_reset_tag((void *)addr); > + > while (n) { > struct vmap_area *va; > > @@ -2143,7 +2147,7 @@ EXPORT_SYMBOL_GPL(vm_unmap_aliases); > void vm_unmap_ram(const void *mem, unsigned int count) > { > unsigned long size = (unsigned long)count << PAGE_SHIFT; > - unsigned long addr = (unsigned long)mem; > + unsigned long addr = (unsigned long)kasan_reset_tag(mem); > struct vmap_area *va; > > might_sleep(); > @@ -3361,6 +3365,8 @@ long vread(char *buf, char *addr, unsigned long count) > unsigned long buflen = count; > unsigned long n; > > + addr = kasan_reset_tag(addr); > + > /* Don't allow overflow */ > if ((unsigned long) addr + count < count) > count = -(unsigned long) addr; > -- > 2.25.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-12-02 14:17 UTC|newest] Thread overview: 112+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-11-30 21:39 [PATCH 00/31] kasan, vmalloc, arm64: add vmalloc tagging support for SW/HW_TAGS andrey.konovalov 2021-11-30 21:39 ` andrey.konovalov 2021-11-30 21:39 ` [PATCH 01/31] kasan, page_alloc: deduplicate should_skip_kasan_poison andrey.konovalov 2021-11-30 21:39 ` andrey.konovalov 2021-11-30 21:39 ` [PATCH 02/31] kasan, page_alloc: move tag_clear_highpage out of kernel_init_free_pages andrey.konovalov 2021-11-30 21:39 ` andrey.konovalov 2021-12-02 15:24 ` Alexander Potapenko 2021-12-02 15:24 ` Alexander Potapenko 2021-11-30 21:39 ` [PATCH 03/31] kasan, page_alloc: merge kasan_free_pages into free_pages_prepare andrey.konovalov 2021-11-30 21:39 ` andrey.konovalov 2021-12-02 15:32 ` Alexander Potapenko 2021-12-02 15:32 ` Alexander Potapenko 2021-11-30 21:39 ` [PATCH 04/31] kasan, page_alloc: simplify kasan_poison_pages call site andrey.konovalov 2021-11-30 21:39 ` andrey.konovalov 2021-12-01 14:09 ` Marco Elver 2021-12-01 14:09 ` Marco Elver 2021-12-06 21:07 ` Andrey Konovalov 2021-12-06 21:07 ` Andrey Konovalov 2021-11-30 21:39 ` [PATCH 05/31] kasan, page_alloc: init memory of skipped pages on free andrey.konovalov 2021-11-30 21:39 ` andrey.konovalov 2021-11-30 21:41 ` [PATCH 06/31] mm: clarify __GFP_ZEROTAGS comment andrey.konovalov 2021-11-30 21:41 ` andrey.konovalov 2021-11-30 21:41 ` [PATCH 07/31] kasan: only apply __GFP_ZEROTAGS when memory is zeroed andrey.konovalov 2021-11-30 21:41 ` andrey.konovalov 2021-12-02 15:40 ` Alexander Potapenko 2021-12-02 15:40 ` Alexander Potapenko 2021-11-30 21:41 ` [PATCH 08/31] kasan, page_alloc: refactor init checks in post_alloc_hook andrey.konovalov 2021-11-30 21:41 ` andrey.konovalov 2021-12-02 16:13 ` Alexander Potapenko 2021-12-02 16:13 ` Alexander Potapenko 2021-12-06 21:09 ` Andrey Konovalov 2021-12-06 21:09 ` Andrey Konovalov 2021-12-16 10:59 ` Alexander Potapenko 2021-12-16 10:59 ` Alexander Potapenko 2021-11-30 21:42 ` [PATCH 09/31] kasan, page_alloc: merge kasan_alloc_pages into post_alloc_hook andrey.konovalov 2021-11-30 21:42 ` andrey.konovalov 2021-11-30 21:52 ` [PATCH 10/31] kasan, page_alloc: combine tag_clear_highpage calls in post_alloc_hook andrey.konovalov 2021-11-30 21:52 ` andrey.konovalov 2021-11-30 22:05 ` [PATCH 11/31] kasan, page_alloc: move SetPageSkipKASanPoison " andrey.konovalov 2021-11-30 22:05 ` andrey.konovalov 2021-11-30 22:05 ` [PATCH 12/31] kasan, page_alloc: move kernel_init_free_pages " andrey.konovalov 2021-11-30 22:05 ` andrey.konovalov 2021-11-30 22:05 ` [PATCH 13/31] kasan, page_alloc: simplify kasan_unpoison_pages call site andrey.konovalov 2021-11-30 22:05 ` andrey.konovalov 2021-11-30 22:06 ` [PATCH 14/31] kasan: clean up metadata byte definitions andrey.konovalov 2021-11-30 22:06 ` andrey.konovalov 2021-11-30 22:06 ` [PATCH 15/31] kasan: define KASAN_VMALLOC_INVALID for SW_TAGS andrey.konovalov 2021-11-30 22:06 ` andrey.konovalov 2021-11-30 22:06 ` [PATCH 16/31] kasan, x86, arm64, s390: rename functions for modules shadow andrey.konovalov 2021-11-30 22:06 ` andrey.konovalov 2021-11-30 22:06 ` [PATCH 17/31] kasan, vmalloc: drop outdated VM_KASAN comment andrey.konovalov 2021-11-30 22:06 ` andrey.konovalov 2021-11-30 22:07 ` [PATCH 18/31] kasan: reorder vmalloc hooks andrey.konovalov 2021-11-30 22:07 ` andrey.konovalov 2021-11-30 22:07 ` [PATCH 20/31] kasan, vmalloc: reset tags in vmalloc functions andrey.konovalov 2021-11-30 22:07 ` andrey.konovalov 2021-12-02 14:17 ` Marco Elver [this message] 2021-12-02 14:17 ` Marco Elver 2021-12-06 21:08 ` Andrey Konovalov 2021-12-06 21:08 ` Andrey Konovalov 2021-11-30 22:07 ` [PATCH 19/31] kasan: add wrappers for vmalloc hooks andrey.konovalov 2021-11-30 22:07 ` andrey.konovalov 2021-11-30 22:07 ` [PATCH 21/31] kasan, fork: don't tag stacks allocated with vmalloc andrey.konovalov 2021-11-30 22:07 ` andrey.konovalov 2021-12-02 14:27 ` Marco Elver 2021-12-02 14:27 ` Marco Elver 2021-12-06 21:08 ` Andrey Konovalov 2021-12-06 21:08 ` Andrey Konovalov 2021-11-30 22:07 ` [PATCH 22/31] kasan, vmalloc: add vmalloc support to SW_TAGS andrey.konovalov 2021-11-30 22:07 ` andrey.konovalov 2021-11-30 22:07 ` [PATCH 23/31] kasan, arm64: allow KASAN_VMALLOC with SW_TAGS andrey.konovalov 2021-11-30 22:07 ` andrey.konovalov 2021-12-03 12:37 ` Marco Elver 2021-12-03 12:37 ` Marco Elver 2021-12-06 21:10 ` Andrey Konovalov 2021-12-06 21:10 ` Andrey Konovalov 2021-11-30 22:07 ` [PATCH 24/31] kasan, vmalloc, arm64: mark vmalloc mappings as pgprot_tagged andrey.konovalov 2021-11-30 22:07 ` andrey.konovalov 2021-12-03 12:42 ` Marco Elver 2021-12-03 12:42 ` Marco Elver 2021-12-06 21:12 ` Andrey Konovalov 2021-12-06 21:12 ` Andrey Konovalov 2021-11-30 22:08 ` [PATCH 25/31] kasan, vmalloc: don't unpoison VM_ALLOC pages before mapping andrey.konovalov 2021-11-30 22:08 ` andrey.konovalov 2021-11-30 22:08 ` [PATCH 26/31] kasan, page_alloc: allow skipping unpoisoning for HW_TAGS andrey.konovalov 2021-11-30 22:08 ` andrey.konovalov 2021-11-30 22:08 ` [PATCH 27/31] kasan, vmalloc: add vmalloc support to HW_TAGS andrey.konovalov 2021-11-30 22:08 ` andrey.konovalov 2021-12-03 12:41 ` Marco Elver 2021-12-03 12:41 ` Marco Elver 2021-12-06 21:12 ` Andrey Konovalov 2021-12-06 21:12 ` Andrey Konovalov 2021-11-30 22:08 ` [PATCH 28/31] kasan: add kasan.vmalloc command line flag andrey.konovalov 2021-11-30 22:08 ` andrey.konovalov 2021-12-03 12:09 ` Marco Elver 2021-12-03 12:09 ` Marco Elver 2021-12-06 21:09 ` Andrey Konovalov 2021-12-06 21:09 ` Andrey Konovalov 2021-11-30 22:08 ` [PATCH 29/31] kasan, arm64: allow KASAN_VMALLOC with HW_TAGS andrey.konovalov 2021-11-30 22:08 ` andrey.konovalov 2021-12-01 11:35 ` Marco Elver 2021-12-01 11:35 ` Marco Elver 2021-12-06 21:10 ` Andrey Konovalov 2021-12-06 21:10 ` Andrey Konovalov 2021-12-03 12:40 ` Marco Elver 2021-12-03 12:40 ` Marco Elver 2021-12-06 21:10 ` Andrey Konovalov 2021-12-06 21:10 ` Andrey Konovalov 2021-11-30 22:08 ` [PATCH 30/31] kasan: documentation updates andrey.konovalov 2021-11-30 22:08 ` andrey.konovalov 2021-11-30 22:08 ` [PATCH 31/31] kasan: improve vmalloc tests andrey.konovalov 2021-11-30 22:08 ` andrey.konovalov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=YajVYNBDOyI3hTx1@elver.google.com \ --to=elver@google.com \ --cc=akpm@linux-foundation.org \ --cc=andrey.konovalov@linux.dev \ --cc=andreyknvl@gmail.com \ --cc=andreyknvl@google.com \ --cc=catalin.marinas@arm.com \ --cc=dvyukov@google.com \ --cc=eugenis@google.com \ --cc=glider@google.com \ --cc=kasan-dev@googlegroups.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=pcc@google.com \ --cc=vincenzo.frascino@arm.com \ --cc=will@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.