lkml.org 
[lkml]   [2021]   [Dec]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 20/31] kasan, vmalloc: reset tags in vmalloc functions
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
>

\
 
 \ /
  Last update: 2021-12-02 15:18    [W:0.234 / U:1.312 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site