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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <56798D8F.9090402@labbott.name>
Date:	Tue, 22 Dec 2015 09:51:11 -0800
From:	Laura Abbott <laura@...bott.name>
To:	Mathias Krause <minipli@...glemail.com>,
	kernel-hardening@...ts.openwall.com
Cc:	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>, linux-mm@...ck.org,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Kees Cook <keescook@...omium.org>
Subject: Re: [kernel-hardening] [RFC][PATCH 6/7] mm: Add Kconfig option for
 slab sanitization

On 12/22/15 1:33 AM, Mathias Krause wrote:
> On 22 December 2015 at 04:40, Laura Abbott <laura@...bott.name> wrote:
>>
>> +config SLAB_MEMORY_SANITIZE
>> +       bool "Sanitize all freed memory"
>> +       help
>> +         By saying Y here the kernel will erase slab objects as soon as they
>> +         are freed.  This in turn reduces the lifetime of data
>> +         stored in them, making it less likely that sensitive information such
>> +         as passwords, cryptographic secrets, etc stay in memory for too long.
>> +
>
>> +         This is especially useful for programs whose runtime is short, long
>> +         lived processes and the kernel itself benefit from this as long as
>> +         they ensure timely freeing of memory that may hold sensitive
>> +         information.
>
> This part is not true. The code is handling SLAB objects only, so
> talking about processes in this context is misleading. Freeing memory
> in userland containing secrets cannot be covered by this feature as
> is. It needs a counter-part in the userland memory allocator as well
> as handling page sanitization in the buddy allocator.
>
> I guess you've just copy+pasted that Kconfig description from the PaX
> feature PAX_MEMORY_SANITIZE that also covers the buddy allocator,
> therefore fits that description while this patch set does not. So
> please adapt the text or implement the fully featured version.
>

I was thinking of secrets that may be stored in the slab allocator. While
certainly not as common they would exist. I'll clarify the text though
to make it obvious this is for kernel slab memory only.
  
>> +
>> +         A nice side effect of the sanitization of slab objects is the
>> +         reduction of possible info leaks caused by padding bytes within the
>> +         leaky structures.  Use-after-free bugs for structures containing
>> +         pointers can also be detected as dereferencing the sanitized pointer
>> +         will generate an access violation.
>> +
>> +         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
>> +
>
>> +         The slab sanitization feature excludes a few slab caches per default
>> +         for performance reasons. The level of sanitization can be adjusted
>> +         with the sanitize_slab commandline option:
>> +               sanitize_slab=off: No sanitization will occur
>> +               santiize_slab=slow: Sanitization occurs only on the slow path
>> +               for all but the excluded slabs
>> +               (relevant for SLUB allocator only)
>> +               sanitize_slab=partial: Sanitization occurs on all path for all
>> +               but the excluded slabs
>> +               sanitize_slab=full: All slabs are sanitize
>
> This should probably be moved to Documentation/kernel-parameters.txt,
> as can be found in the PaX patch[1]?
>

Yes, I missed that. I'll fix that.
  
>> +
>> +         If unsure, say Y here.
>
> Really? It has an unknown performance impact, depending on the
> workload, which might make "unsure users" preferably say No, if they
> don't care about info leaks.

This is getting to the argument about security vs. performance and
what should be default. I think this deserves more advice than just
"If unsure, do X" so I'll add some more description about the trade
offs.

>
> Related to this, have you checked that the sanitization doesn't
> interfere with the various slab handling schemes, namely RCU related
> specialties? Not all caches are marked SLAB_DESTROY_BY_RCU, some use
> call_rcu() instead, implicitly relying on the semantics RCU'ed slabs
> permit, namely allowing a "use-after-free" access to be legitimate
> within the RCU grace period. Scrubbing the object during that period
> would break that assumption.

I haven't looked into that. I was working off the assumption that
if the regular SLAB debug poisoning worked so would the sanitization.
The regular debug poisoning only checks for SLAB_DESTROY_BY_RCU so
how does that work then?

>
> Speaking of RCU, do you have a plan to support RCU'ed slabs as well?
>

My only plan was to get the base support in. I didn't have a plan to
support RCU slabs but that's certainly something to be done in the
future.

Thanks,
Laura

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/