Re: [kernel-hardening] [RFC][PATCH 6/7] mm: Add Kconfig option for slab sanitization

[Dave Hansen <dave.hansen@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 1078 bytes --]

On 12/22/2015 08:25 AM, Christoph Lameter wrote:
> On Tue, 22 Dec 2015, Dave Hansen wrote:
>> On 12/21/2015 07:40 PM, Laura Abbott wrote:
>>> +	  The tradeoff is performance impact. The noticible impact can vary
>>> +	  and you are advised to test this feature on your expected workload
>>> +	  before deploying it
>>
>> What if instead of writing SLAB_MEMORY_SANITIZE_VALUE, we wrote 0's?
>> That still destroys the information, but it has the positive effect of
>> allowing a kzalloc() call to avoid zeroing the slab object.  It might
>> mitigate some of the performance impact.
> 
> We already write zeros in many cases or the object is initialized in a
> different. No one really wants an uninitialized object. The problem may be
> that a freed object is having its old content until reused. Which is
> something that poisoning deals with.

Or are you just saying that we should use the poisoning *code* that we
already have in slub?  Using the _code_ looks like a really good idea,
whether we're using it to write POISON_FREE, or 0's.  Something like the
attached patch?



[-- Attachment #2: slub-poison-zeros.patch --]
[-- Type: text/x-patch, Size: 1546 bytes --]



---

 b/mm/slub.c |   12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff -puN mm/slub.c~slub-poison-zeros mm/slub.c
--- a/mm/slub.c~slub-poison-zeros	2015-12-22 09:18:30.585371985 -0800
+++ b/mm/slub.c	2015-12-22 09:21:23.754174731 -0800
@@ -177,6 +177,7 @@ static inline bool kmem_cache_has_cpu_pa
 /* Internal SLUB flags */
 #define __OBJECT_POISON		0x80000000UL /* Poison object */
 #define __CMPXCHG_DOUBLE	0x40000000UL /* Use cmpxchg_double */
+#define __OBJECT_POISON_ZERO	0x20000000UL /* Poison with zeroes */
 
 #ifdef CONFIG_SMP
 static struct notifier_block slab_notifier;
@@ -678,7 +679,10 @@ static void init_object(struct kmem_cach
 	u8 *p = object;
 
 	if (s->flags & __OBJECT_POISON) {
-		memset(p, POISON_FREE, s->object_size - 1);
+		if (s->flags & __OBJECT_POISON_ZERO) {
+			memset(p, POISON_FREE, s->object_size - 1);
+		else
+			memset(p, 0, s->object_size - 1);
 		p[s->object_size - 1] = POISON_END;
 	}
 
@@ -2495,7 +2499,8 @@ redo:
 		stat(s, ALLOC_FASTPATH);
 	}
 
-	if (unlikely(gfpflags & __GFP_ZERO) && object)
+	if (unlikely(gfpflags & __GFP_ZERO) && object &&
+	    !(s->flags & __OBJECT_POISON_ZERO)) {
 		memset(object, 0, s->object_size);
 
 	slab_post_alloc_hook(s, gfpflags, object);
@@ -2839,7 +2844,8 @@ bool kmem_cache_alloc_bulk(struct kmem_c
 	local_irq_enable();
 
 	/* Clear memory outside IRQ disabled fastpath loop */
-	if (unlikely(flags & __GFP_ZERO)) {
+	if (unlikely(flags & __GFP_ZERO) &&
+	    !(s->flags & __OBJECT_POISON_ZERO)) {
 		int j;
 
 		for (j = 0; j < i; j++)
_