Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87oa6aysje.fsf@@concordia.ellerman.id.au>
Date: Thu, 07 Jul 2016 14:35:17 +1000
From: Michael Ellerman <mpe@...erman.id.au>
To: Kees Cook <keescook@...omium.org>, linux-kernel@...r.kernel.org
Cc: Kees Cook <keescook@...omium.org>, Rik van Riel <riel@...hat.com>, Casey Schaufler <casey@...aufler-ca.com>, PaX Team <pageexec@...email.hu>, Brad Spengler <spender@...ecurity.net>, Russell King <linux@...linux.org.uk>, Catalin Marinas <catalin.marinas@....com>, Will Deacon <will.deacon@....com>, Ard Biesheuvel <ard.biesheuvel@...aro.org>, Benjamin Herrenschmidt <benh@...nel.crashing.org>, Tony Luck <tony.luck@...el.com>, Fenghua Yu <fenghua.yu@...el.com>, "David S. Miller" <davem@...emloft.net>, x86@...nel.org, Christoph Lameter <cl@...ux.com>, Pekka Enberg <penberg@...nel.org>, David Rientjes <rientjes@...gle.com>, Joonsoo Kim <iamjoonsoo.kim@....com>, Andrew Morton <akpm@...ux-foundation.org>, Andy Lutomirski <luto@...nel.org>, Borislav Petkov <bp@...e.de>, Mathias Krause <minipli@...glemail.com>, Jan Kara <jack@...e.cz>, Vitaly Wool <vitalywool@...il.com>, Andrea Arcangeli <aarcange@...hat.com>, Dmitry Vyukov <dvyukov@...gle.com>, Laura Abbott <labbott@...oraproject.org>, lin
 ux-arm-kernel@...ts.infradead.org, linux-ia64@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org, sparclinux@...r.kernel.org, linux-arch@...r.kernel.org, linux-mm@...ck.org, kernel-hardening@...ts.openwall.com
Subject: Re: [PATCH 9/9] mm: SLUB hardened usercopy support

Kees Cook <keescook@...omium.org> writes:

> Under CONFIG_HARDENED_USERCOPY, this adds object size checking to the
> SLUB allocator to catch any copies that may span objects.
>
> Based on code from PaX and grsecurity.
>
> Signed-off-by: Kees Cook <keescook@...omium.org>

> diff --git a/mm/slub.c b/mm/slub.c
> index 825ff4505336..0c8ace04f075 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -3614,6 +3614,33 @@ void *__kmalloc_node(size_t size, gfp_t flags, int node)
>  EXPORT_SYMBOL(__kmalloc_node);
>  #endif
>  
> +#ifdef CONFIG_HARDENED_USERCOPY
> +/*
> + * Rejects objects that are incorrectly sized.
> + *
> + * Returns NULL if check passes, otherwise const char * to name of cache
> + * to indicate an error.
> + */
> +const char *__check_heap_object(const void *ptr, unsigned long n,
> +				struct page *page)
> +{
> +	struct kmem_cache *s;
> +	unsigned long offset;
> +
> +	/* Find object. */
> +	s = page->slab_cache;
> +
> +	/* Find offset within object. */
> +	offset = (ptr - page_address(page)) % s->size;
> +
> +	/* Allow address range falling entirely within object size. */
> +	if (offset <= s->object_size && n <= s->object_size - offset)
> +		return NULL;
> +
> +	return s->name;
> +}

I gave this a quick spin on powerpc, it blew up immediately :)

  Brought up 16 CPUs
  usercopy: kernel memory overwrite attempt detected to c0000001fe023868 (kmalloc-16) (9 bytes)
  CPU: 8 PID: 103 Comm: kdevtmpfs Not tainted 4.7.0-rc3-00098-g09d9556ae5d1 #55
  Call Trace:
  [c0000001fa0cfb40] [c0000000009bdbe8] dump_stack+0xb0/0xf0 (unreliable)
  [c0000001fa0cfb80] [c00000000029cf44] __check_object_size+0x74/0x320
  [c0000001fa0cfc00] [c00000000005d4d0] copy_from_user+0x60/0xd4
  [c0000001fa0cfc40] [c00000000022b6cc] memdup_user+0x5c/0xf0
  [c0000001fa0cfc80] [c00000000022b90c] strndup_user+0x7c/0x110
  [c0000001fa0cfcc0] [c0000000002d6c28] SyS_mount+0x58/0x180
  [c0000001fa0cfd10] [c0000000005ee908] devtmpfsd+0x98/0x210
  [c0000001fa0cfd80] [c0000000000df810] kthread+0x110/0x130
  [c0000001fa0cfe30] [c0000000000095e8] ret_from_kernel_thread+0x5c/0x74

SLUB tracing says:

  TRACE kmalloc-16 alloc 0xc0000001fe023868 inuse=186 fp=0x          (null)

Which is not 16-byte aligned, which seems to be caused by the red zone?
The following patch fixes it for me, but I don't know SLUB enough to say
if it's always correct.


diff --git a/mm/slub.c b/mm/slub.c
index 0c8ace04f075..66191ea4545a 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3630,6 +3630,9 @@ const char *__check_heap_object(const void *ptr, unsigned long n,
 	/* Find object. */
 	s = page->slab_cache;
 
+	/* Subtract red zone if enabled */
+	ptr = restore_red_left(s, ptr);
+
 	/* Find offset within object. */
 	offset = (ptr - page_address(page)) % s->size;
 
cheers

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.