Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752469AbdGEXkA (ORCPT ); Wed, 5 Jul 2017 19:40:00 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:38158 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751896AbdGEXj7 (ORCPT ); Wed, 5 Jul 2017 19:39:59 -0400 Date: Wed, 5 Jul 2017 16:39:57 -0700 From: Andrew Morton To: Kees Cook Cc: Christoph Lameter , Laura Abbott , Daniel Micay , Pekka Enberg , David Rientjes , Joonsoo Kim , "Paul E. McKenney" , Ingo Molnar , Josh Triplett , Andy Lutomirski , Nicolas Pitre , Tejun Heo , Daniel Mack , Sebastian Andrzej Siewior , Sergey Senozhatsky , Helge Deller , Rik van Riel , linux-kernel@vger.kernel.org, linux-mm@kvack.org, kernel-hardening@lists.openwall.com Subject: Re: [PATCH v2] mm: Add SLUB free list pointer obfuscation Message-Id: <20170705163957.90c7856f622a63666df4b5a6@linux-foundation.org> In-Reply-To: <20170623015010.GA137429@beast> References: <20170623015010.GA137429@beast> X-Mailer: Sylpheed 3.4.1 (GTK+ 2.24.23; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3129 Lines: 89 On Thu, 22 Jun 2017 18:50:10 -0700 Kees Cook wrote: > This SLUB free list pointer obfuscation code is modified from Brad > Spengler/PaX Team's code in the last public patch of grsecurity/PaX based > on my understanding of the code. Changes or omissions from the original > code are mine and don't reflect the original grsecurity/PaX code. > > This adds a per-cache random value to SLUB caches that is XORed with > their freelist pointers. This adds nearly zero overhead and frustrates the > very common heap overflow exploitation method of overwriting freelist > pointers. A recent example of the attack is written up here: > http://cyseclabs.com/blog/cve-2016-6187-heap-off-by-one-exploit > > This is based on patches by Daniel Micay, and refactored to avoid lots > of #ifdef code. > > ... > > --- a/init/Kconfig > +++ b/init/Kconfig > @@ -1900,6 +1900,15 @@ config SLAB_FREELIST_RANDOM > security feature reduces the predictability of the kernel slab > allocator against heap overflows. > > +config SLAB_FREELIST_HARDENED > + bool "Harden slab freelist metadata" > + depends on SLUB > + help > + Many kernel heap attacks try to target slab cache metadata and > + other infrastructure. This options makes minor performance > + sacrifies to harden the kernel slab allocator against common > + freelist exploit methods. > + Well, it is optable-outable. > config SLUB_CPU_PARTIAL > default y > depends on SLUB && SMP > diff --git a/mm/slub.c b/mm/slub.c > index 57e5156f02be..590e7830aaed 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -34,6 +34,7 @@ > #include > #include > #include > +#include > > #include > > @@ -238,30 +239,50 @@ static inline void stat(const struct kmem_cache *s, enum stat_item si) > * Core slab cache functions > *******************************************************************/ > > +#ifdef CONFIG_SLAB_FREELIST_HARDENED > +# define initialize_random(s) \ > + do { \ > + s->random = get_random_long(); \ > + } while (0) > +# define FREEPTR_VAL(ptr, ptr_addr, s) \ > + (void *)((unsigned long)(ptr) ^ s->random ^ (ptr_addr)) > +#else > +# define initialize_random(s) do { } while (0) > +# define FREEPTR_VAL(ptr, addr, s) ((void *)(ptr)) > +#endif > +#define FREELIST_ENTRY(ptr_addr, s) \ > + FREEPTR_VAL(*(unsigned long *)(ptr_addr), \ > + (unsigned long)ptr_addr, s) > + That's a bit of an eyesore. Is there any reason why we cannot implement all of the above in nice, conventional C functions? > > ... > > @@ -3536,6 +3557,7 @@ static int kmem_cache_open(struct kmem_cache *s, unsigned long flags) > { > s->flags = kmem_cache_flags(s->size, flags, s->name, s->ctor); > s->reserved = 0; > + initialize_random(s); > > if (need_reserve_slab_rcu && (s->flags & SLAB_TYPESAFE_BY_RCU)) > s->reserved = sizeof(struct rcu_head); We regularly have issues where the random system just isn't ready (enough) for clients to use it. Are you sure the above is actually useful for the boot-time caches?