Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1296518 > unrolled thread

[RFC][PATCH 0/7] Sanitization of slabs based on grsecurity/PaX

Back to article view | Back to linux.kernel

Contents

  [RFC][PATCH 0/7] Sanitization of slabs based on grsecurity/PaX Laura Abbott <laura@labbott.name> - 2015-12-22 04:50 +0100
    [RFC][PATCH 6/7] mm: Add Kconfig option for slab sanitization Laura Abbott <laura@labbott.name> - 2015-12-22 04:50 +0100
      Re: [kernel-hardening] [RFC][PATCH 6/7] mm: Add Kconfig option for  slab sanitization Mathias Krause <minipli@googlemail.com> - 2015-12-22 10:40 +0100
        Re: [kernel-hardening] [RFC][PATCH 6/7] mm: Add Kconfig option for  slab sanitization Laura Abbott <laura@labbott.name> - 2015-12-22 19:00 +0100
          Re: [kernel-hardening] [RFC][PATCH 6/7] mm: Add Kconfig option for  slab sanitization Mathias Krause <minipli@googlemail.com> - 2015-12-22 19:40 +0100
            Re: [kernel-hardening] [RFC][PATCH 6/7] mm: Add Kconfig option for  slab sanitization Laura Abbott <laura@labbott.name> - 2015-12-22 20:20 +0100
            Re: [kernel-hardening] [RFC][PATCH 6/7] mm: Add Kconfig option for  slab sanitization Christoph Lameter <cl@linux.com> - 2015-12-22 21:10 +0100
              Re: [kernel-hardening] [RFC][PATCH 6/7] mm: Add Kconfig option for  slab sanitization Mathias Krause <minipli@googlemail.com> - 2015-12-22 21:10 +0100
      Re: [kernel-hardening] [RFC][PATCH 6/7] mm: Add Kconfig option for  slab sanitization Dave Hansen <dave.hansen@intel.com> - 2015-12-22 16:00 +0100
        Re: [kernel-hardening] [RFC][PATCH 6/7] mm: Add Kconfig option for  slab sanitization Christoph Lameter <cl@linux.com> - 2015-12-22 17:30 +0100
          Re: [kernel-hardening] [RFC][PATCH 6/7] mm: Add Kconfig option for  slab sanitization Dave Hansen <dave.hansen@intel.com> - 2015-12-22 18:30 +0100
            Re: [kernel-hardening] [RFC][PATCH 6/7] mm: Add Kconfig option for  slab sanitization Christoph Lameter <cl@linux.com> - 2015-12-22 19:10 +0100
              Re: [kernel-hardening] [RFC][PATCH 6/7] mm: Add Kconfig option for  slab sanitization Dave Hansen <dave.hansen@intel.com> - 2015-12-22 19:20 +0100
                Re: [kernel-hardening] [RFC][PATCH 6/7] mm: Add Kconfig option for  slab sanitization Laura Abbott <laura@labbott.name> - 2015-12-22 20:20 +0100
                  Re: [kernel-hardening] [RFC][PATCH 6/7] mm: Add Kconfig option for  slab sanitization Dave Hansen <dave.hansen@intel.com> - 2015-12-22 20:40 +0100
          Re: [kernel-hardening] [RFC][PATCH 6/7] mm: Add Kconfig option for  slab sanitization Christoph Lameter <cl@linux.com> - 2015-12-22 18:30 +0100
          Re: [kernel-hardening] [RFC][PATCH 6/7] mm: Add Kconfig option for  slab sanitization Dave Hansen <dave.hansen@intel.com> - 2015-12-22 18:30 +0100
    [RFC][PATCH 4/7] slob: Add support for sanitization Laura Abbott <laura@labbott.name> - 2015-12-22 04:50 +0100
    [RFC][PATCH 3/7] slab: Add support for sanitization Laura Abbott <laura@labbott.name> - 2015-12-22 04:50 +0100
    [RFC][PATCH 7/7] lkdtm: Add READ_AFTER_FREE test Laura Abbott <laura@labbott.name> - 2015-12-22 04:50 +0100
    [RFC][PATCH 5/7] mm: Mark several cases as SLAB_NO_SANITIZE Laura Abbott <laura@labbott.name> - 2015-12-22 04:50 +0100
    [RFC][PATCH 1/7] mm/slab_common.c: Add common support for slab saniziation Laura Abbott <laura@labbott.name> - 2015-12-22 04:50 +0100
      Re: [RFC][PATCH 1/7] mm/slab_common.c: Add common support for slab  saniziation Vlastimil Babka <vbabka@suse.cz> - 2015-12-22 21:50 +0100
    [RFC][PATCH 2/7] slub: Add support for sanitization Laura Abbott <laura@labbott.name> - 2015-12-22 04:50 +0100
    Re: [RFC][PATCH 0/7] Sanitization of slabs based on grsecurity/PaX Christoph Lameter <cl@linux.com> - 2015-12-22 17:10 +0100
      Re: [kernel-hardening] Re: [RFC][PATCH 0/7] Sanitization of slabs  based on grsecurity/PaX Dave Hansen <dave.hansen@intel.com> - 2015-12-22 17:20 +0100
      Re: [kernel-hardening] Re: [RFC][PATCH 0/7] Sanitization of slabs  based on grsecurity/PaX Daniel Micay <danielmicay@gmail.com> - 2015-12-22 17:40 +0100
      Re: [RFC][PATCH 0/7] Sanitization of slabs based on grsecurity/PaX Laura Abbott <laura@labbott.name> - 2015-12-22 21:10 +0100

Page 1 of 2  [1] 2  Next page →

#1296518 — [RFC][PATCH 0/7] Sanitization of slabs based on grsecurity/PaX

Hi,

This is a partial port of the PAX_MEMORY_SANITIZE feature. The concept is
fairly simple: when memory is freed, existing data should be erased. This
helps to reduce the impact of problems
(e.g. 45a22f4 inotify: Fix reporting of cookies for inotify events
e4514cb RDMA/cxgb3: Fix information leak in send_abort()
your favorite use after free bug)

The biggest change from PAX_MEMORY_SANTIIZE is that this feature sanitizes
the SL[AOU]B allocators only. My plan is to work on the buddy allocator
santization after this series gets picked up. A side effect of this is
that allocations which go directly to the buddy allocator (i.e. large
allocations) aren't sanitized. I'd like feedback about whether it's worth
it to add sanitization on that path directly or just use the page
allocator sanitization when that comes in.

I also expanded the command line options, mostly for SLUB. Since SLUB
has had so much tuning work done for performance, I added an option
to only sanitize on the slow path. Freeing on only fast vs. slow path
was most noticable in the bulk test cases. Overall, I saw impacts of
3% to 20% on various benchmarks when this feature was enabled. The
overall impact of sanitize_slab=off seemed to be pretty negligable.

This feature is similar to the debug feature of SLAB_POISON. I did
consider trying to make that feature not related to debug. Ultimately,
I concluded there was too much extra debug overhead and other features
to make it worth it.

Bike shed whatever you like. The Kconfig will probably end up in
a separate sanitization Kconfig.

All credit for the original work should be given to Brad Spengler and
the PaX Team. 

Thanks,
Laura

Laura Abbott (7):
  mm/slab_common.c: Add common support for slab saniziation
  slub: Add support for sanitization
  slab: Add support for sanitization
  slob: Add support for sanitization
  mm: Mark several cases as SLAB_NO_SANITIZE
  mm: Add Kconfig option for slab sanitization
  lkdtm: Add READ_AFTER_FREE test

 drivers/misc/lkdtm.c     | 29 ++++++++++++++++
 fs/buffer.c              |  2 +-
 fs/dcache.c              |  2 +-
 include/linux/slab.h     |  7 ++++
 include/linux/slab_def.h |  4 +++
 init/Kconfig             | 48 ++++++++++++++++++++++++++
 kernel/fork.c            |  2 +-
 mm/rmap.c                |  4 +--
 mm/slab.c                | 35 +++++++++++++++++++
 mm/slab.h                | 24 ++++++++++++-
 mm/slab_common.c         | 53 ++++++++++++++++++++++++++++
 mm/slob.c                | 27 +++++++++++----
 mm/slub.c                | 90 +++++++++++++++++++++++++++++++++++++++++++++++-
 net/core/skbuff.c        |  4 +--
 14 files changed, 316 insertions(+), 15 deletions(-)

-- 
2.5.0

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

[toc] | [next] | [standalone]

#1296519 — [RFC][PATCH 6/7] mm: Add Kconfig option for slab sanitization

The SL[AOU]B allocators all behave differently w.r.t. to what happen
an object is freed. CONFIG_SLAB_SANITIZATION introduces a common
mechanism to control what happens on free. When this option is
enabled, objects may be poisoned according to a combination of
slab_sanitization command line option and whether SLAB_NO_SANITIZE
is set on a cache.

All credit for the original work should be given to Brad Spengler and
the PaX Team.

Signed-off-by: Laura Abbott <laura@labbott.name>
---
 init/Kconfig | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/init/Kconfig b/init/Kconfig
index 235c7a2..37857f3 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1755,6 +1755,42 @@ config SLUB_CPU_PARTIAL
 	  which requires the taking of locks that may cause latency spikes.
 	  Typically one would choose no for a realtime system.
 
+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.
+
+	  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
+
+	  If unsure, say Y here.
+
 config MMAP_ALLOW_UNINITIALIZED
 	bool "Allow mmapped anonymous memory to be uninitialized"
 	depends on EXPERT && !MMU
-- 
2.5.0

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

[toc] | [prev] | [next] | [standalone]

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

On 22 December 2015 at 04:40, Laura Abbott <laura@labbott.name> wrote:
>
> The SL[AOU]B allocators all behave differently w.r.t. to what happen
> an object is freed. CONFIG_SLAB_SANITIZATION introduces a common
> mechanism to control what happens on free. When this option is
> enabled, objects may be poisoned according to a combination of
> slab_sanitization command line option and whether SLAB_NO_SANITIZE
> is set on a cache.
>
> All credit for the original work should be given to Brad Spengler and
> the PaX Team.
>
> Signed-off-by: Laura Abbott <laura@labbott.name>
> ---
>  init/Kconfig | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 235c7a2..37857f3 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1755,6 +1755,42 @@ config SLUB_CPU_PARTIAL
>           which requires the taking of locks that may cause latency spikes.
>           Typically one would choose no for a realtime system.
>
> +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.

> +
> +         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]?

> +
> +         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.

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.

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

> +
>  config MMAP_ALLOW_UNINITIALIZED
>         bool "Allow mmapped anonymous memory to be uninitialized"
>         depends on EXPERT && !MMU
> --
> 2.5.0
>

Regards,
Mathias

[1] https://github.com/minipli/linux-grsec/blob/v4.3.3-pax/Documentation/kernel-parameters.txt#L2689-L2696
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1296944 — 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@labbott.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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

On 22 December 2015 at 18:51, Laura Abbott <laura@labbott.name> wrote:
>> [snip]
>>
>> 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?

Maybe it doesn't? ;)

How many systems, do you think, are running with enabled DEBUG_SLAB /
SLUB_DEBUG in production? Not so many, I'd guess. And the ones running
into issues probably just disable DEBUG_SLAB / SLUB_DEBUG.

Btw, SLUB not only looks for SLAB_DESTROY_BY_RCU but also excludes
"call_rcu slabs" via other mechanisms. As SLUB is the default SLAB
allocator for quite some time now, even with enabled SLUB_DEBUG one
wouldn't be able to trigger RCU related sanitization issues.

>> 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.

"Base support", in my opinion, includes covering the buddy allocator
as well. Otherwise this feature is incomplete.

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

[toc] | [prev] | [next] | [standalone]

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

On 12/22/15 10:37 AM, Mathias Krause wrote:
> On 22 December 2015 at 18:51, Laura Abbott <laura@labbott.name> wrote:
>>> [snip]
>>>
>>> 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?
>
> Maybe it doesn't? ;)
>
> How many systems, do you think, are running with enabled DEBUG_SLAB /
> SLUB_DEBUG in production? Not so many, I'd guess. And the ones running
> into issues probably just disable DEBUG_SLAB / SLUB_DEBUG.
>
> Btw, SLUB not only looks for SLAB_DESTROY_BY_RCU but also excludes
> "call_rcu slabs" via other mechanisms. As SLUB is the default SLAB
> allocator for quite some time now, even with enabled SLUB_DEBUG one
> wouldn't be able to trigger RCU related sanitization issues.
>

I've seen SLUB_DEBUG used in stress testing situations but you're
right about production and giving up if there are issues. I'll take
a closer look at this.
  
>>> 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.
>
> "Base support", in my opinion, includes covering the buddy allocator
> as well. Otherwise this feature is incomplete.

Point taken. I'll look at the buddy allocator post-holidays.

It was also pointed out I should be giving you full credit for this
feature originally. I apologize for not doing that. Thanks for
doing the original work and taking the time to review this series.

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

[toc] | [prev] | [next] | [standalone]

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

On Tue, 22 Dec 2015, Mathias Krause wrote:

> How many systems, do you think, are running with enabled DEBUG_SLAB /
> SLUB_DEBUG in production? Not so many, I'd guess. And the ones running
> into issues probably just disable DEBUG_SLAB / SLUB_DEBUG.

All systems run with SLUB_DEBUG in production. SLUB_DEBUG causes the code
for debugging to be compiled in. Then it can be enabled later with a
command line parameter.

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

[toc] | [prev] | [next] | [standalone]

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

On 22 December 2015 at 21:01, Christoph Lameter <cl@linux.com> wrote:
> On Tue, 22 Dec 2015, Mathias Krause wrote:
>
>> How many systems, do you think, are running with enabled DEBUG_SLAB /
>> SLUB_DEBUG in production? Not so many, I'd guess. And the ones running
>> into issues probably just disable DEBUG_SLAB / SLUB_DEBUG.
>
> All systems run with SLUB_DEBUG in production. SLUB_DEBUG causes the code
> for debugging to be compiled in. Then it can be enabled later with a
> command line parameter.

Indeed, I meant CONFIG_SLUB_DEBUG_ON, i.e. compiled in and enabled
SLAB cache debugging including poisoning.

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

[toc] | [prev] | [next] | [standalone]

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

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.

If this is on at compile time, but booted with sanitize_slab=off, is
there a performance impact?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

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.

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

[toc] | [prev] | [next] | [standalone]

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

On 12/22/2015 09:24 AM, Christoph Lameter wrote:
> On Tue, 22 Dec 2015, Dave Hansen wrote:
>> 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?
> 
> Why would you use zeros? The point is just to clear the information right?
> The regular poisoning does that.

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

[toc] | [prev] | [next] | [standalone]

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

On Tue, 22 Dec 2015, Dave Hansen wrote:

> > Why would you use zeros? The point is just to clear the information right?
> > The regular poisoning does that.
>
> It then allows you to avoid the zeroing at allocation time.

Well much of the code is expecting a zeroed object from the allocator and
its zeroed at that time. Zeroing makes the object cache hot which is an
important performance aspect.

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

[toc] | [prev] | [next] | [standalone]

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

On 12/22/2015 10:08 AM, Christoph Lameter wrote:
> On Tue, 22 Dec 2015, Dave Hansen wrote:
>>> Why would you use zeros? The point is just to clear the information right?
>>> The regular poisoning does that.
>>
>> It then allows you to avoid the zeroing at allocation time.
> 
> Well much of the code is expecting a zeroed object from the allocator and
> its zeroed at that time. Zeroing makes the object cache hot which is an
> important performance aspect.

Yes, modifying this behavior has a performance impact.  It absolutely
needs to be evaluated, and I wouldn't want to speculate too much on how
good or bad any of the choices are.

Just to reiterate, I think we have 3 real choices here:

1. Zero at alloc, only when __GFP_ZERO
   (behavior today)
2. Poison at free, also Zero at alloc (when __GFP_ZERO)
   (this patch's proposed behavior, also what current poisoning does,
    doubles writes)
3. Zero at free, *don't* Zero at alloc (when __GFP_ZERO)
   (what I'm suggesting, possibly less perf impact vs. #2)


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

[toc] | [prev] | [next] | [standalone]

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

On 12/22/15 10:19 AM, Dave Hansen wrote:
> On 12/22/2015 10:08 AM, Christoph Lameter wrote:
>> On Tue, 22 Dec 2015, Dave Hansen wrote:
>>>> Why would you use zeros? The point is just to clear the information right?
>>>> The regular poisoning does that.
>>>
>>> It then allows you to avoid the zeroing at allocation time.
>>
>> Well much of the code is expecting a zeroed object from the allocator and
>> its zeroed at that time. Zeroing makes the object cache hot which is an
>> important performance aspect.
>
> Yes, modifying this behavior has a performance impact.  It absolutely
> needs to be evaluated, and I wouldn't want to speculate too much on how
> good or bad any of the choices are.
>
> Just to reiterate, I think we have 3 real choices here:
>
> 1. Zero at alloc, only when __GFP_ZERO
>     (behavior today)
> 2. Poison at free, also Zero at alloc (when __GFP_ZERO)
>     (this patch's proposed behavior, also what current poisoning does,
>      doubles writes)
> 3. Zero at free, *don't* Zero at alloc (when __GFP_ZERO)
>     (what I'm suggesting, possibly less perf impact vs. #2)
>
>

poisoning with non-zero memory makes it easier to determine that the error
came from accessing the sanitized memory vs. some other case. I don't think
the feature would be as strong if the memory was only zeroed vs. some other
data value.

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

[toc] | [prev] | [next] | [standalone]

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

On 12/22/2015 11:13 AM, Laura Abbott wrote:
>> 3. Zero at free, *don't* Zero at alloc (when __GFP_ZERO)
>>     (what I'm suggesting, possibly less perf impact vs. #2)
> 
> poisoning with non-zero memory makes it easier to determine that the error
> came from accessing the sanitized memory vs. some other case. I don't think
> the feature would be as strong if the memory was only zeroed vs. some other
> data value.

How does that scenario work?  Your patch description says:

> +	  Use-after-free bugs for structures containing
> +	  pointers can also be detected as dereferencing the sanitized pointer
> +	  will generate an access violation.

In the case that we wrote all zeros, we'd be accessing userspace at a
known place that we don't generally allow memory to be mapped anyway.
Could you elaborate on a scenario where zeros are weaker than a random
poison value?

In any case (if a poison value is superior to 0's), it's a balance
between performance vs. the likelihood of the poisoned value being
tripped over.

I think the performance impact of this feature is going to be *the*
major thing that keeps folks from using it in practice.  I'm trying to
suggest a way that you _might_ preserve some performance, and get more
folks to use it.

1. Keep information from leaking (doesn't matter which value we write)
2. Detect use-after-free bugs (0's are less likely to be detected???)
3. Preserve performance (0's are likely to preserve more performance)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

[Multipart message — attachments visible in raw view] — view raw

On Tue, 22 Dec 2015, Dave Hansen wrote:

> 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?

Why would you use zeros? The point is just to clear the information right?
The regular poisoning does that.

[toc] | [prev] | [next] | [standalone]

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

[Multipart message — attachments visible in raw view] — view raw

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?

[toc] | [prev] | [next] | [standalone]

#1296522 — [RFC][PATCH 4/7] slob: Add support for sanitization

The SLOB allocator does not clear objects on free. This is a security
risk since sensitive data may exist long past its expected life
span. Add support for clearing objects on free.

All credit for the original work should be given to Brad Spengler and
the PaX Team.

Signed-off-by: Laura Abbott <laura@labbott.name>
---
 mm/slob.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/mm/slob.c b/mm/slob.c
index 17e8f8c..37a4ecb 100644
--- a/mm/slob.c
+++ b/mm/slob.c
@@ -334,10 +334,21 @@ static void *slob_alloc(size_t size, gfp_t gfp, int align, int node)
 	return b;
 }
 
+static void slob_sanitize(struct kmem_cache *c, slob_t *b, int size)
+{
+#ifdef CONFIG_SLAB_MEMORY_SANITIZE
+	if (c && (c->flags & SLAB_NO_SANITIZE))
+		return;
+
+	if (sanitize_slab)
+		memset(b, SLAB_MEMORY_SANITIZE_VALUE, size);
+#endif
+}
+
 /*
  * slob_free: entry point into the slob allocator.
  */
-static void slob_free(void *block, int size)
+static void slob_free(struct kmem_cache *c, void *block, int size)
 {
 	struct page *sp;
 	slob_t *prev, *next, *b = (slob_t *)block;
@@ -365,6 +376,8 @@ static void slob_free(void *block, int size)
 		return;
 	}
 
+	slob_sanitize(c, block, size);
+
 	if (!slob_page_free(sp)) {
 		/* This slob page is about to become partially free. Easy! */
 		sp->units = units;
@@ -495,7 +508,7 @@ void kfree(const void *block)
 	if (PageSlab(sp)) {
 		int align = max_t(size_t, ARCH_KMALLOC_MINALIGN, ARCH_SLAB_MINALIGN);
 		unsigned int *m = (unsigned int *)(block - align);
-		slob_free(m, *m + align);
+		slob_free(NULL, m, *m + align);
 	} else
 		__free_pages(sp, compound_order(sp));
 }
@@ -579,10 +592,10 @@ void *kmem_cache_alloc_node(struct kmem_cache *cachep, gfp_t gfp, int node)
 EXPORT_SYMBOL(kmem_cache_alloc_node);
 #endif
 
-static void __kmem_cache_free(void *b, int size)
+static void __kmem_cache_free(struct kmem_cache *c, void *b, int size)
 {
 	if (size < PAGE_SIZE)
-		slob_free(b, size);
+		slob_free(c, b, size);
 	else
 		slob_free_pages(b, get_order(size));
 }
@@ -592,7 +605,7 @@ static void kmem_rcu_free(struct rcu_head *head)
 	struct slob_rcu *slob_rcu = (struct slob_rcu *)head;
 	void *b = (void *)slob_rcu - (slob_rcu->size - sizeof(struct slob_rcu));
 
-	__kmem_cache_free(b, slob_rcu->size);
+	__kmem_cache_free(NULL, b, slob_rcu->size);
 }
 
 void kmem_cache_free(struct kmem_cache *c, void *b)
@@ -604,7 +617,7 @@ void kmem_cache_free(struct kmem_cache *c, void *b)
 		slob_rcu->size = c->size;
 		call_rcu(&slob_rcu->head, kmem_rcu_free);
 	} else {
-		__kmem_cache_free(b, c->size);
+		__kmem_cache_free(NULL, b, c->size);
 	}
 
 	trace_kmem_cache_free(_RET_IP_, b);
-- 
2.5.0

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

[toc] | [prev] | [next] | [standalone]

#1296523 — [RFC][PATCH 3/7] slab: Add support for sanitization

Clearing of objects on free only happens on debug paths. This is a
security risk since sensative data may exist long past it's life
span. Add unconditional clearing of objects on free.

All credit for the original work should be given to Brad Spengler and
the PaX Team.

Signed-off-by: Laura Abbott <laura@labbott.name>
---
 mm/slab.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/mm/slab.c b/mm/slab.c
index 4765c97..0ca92d8 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -319,6 +319,8 @@ static void kmem_cache_node_init(struct kmem_cache_node *parent)
 #define STATS_INC_ALLOCMISS(x)	atomic_inc(&(x)->allocmiss)
 #define STATS_INC_FREEHIT(x)	atomic_inc(&(x)->freehit)
 #define STATS_INC_FREEMISS(x)	atomic_inc(&(x)->freemiss)
+#define STATS_INC_SANITIZED(x)	atomic_inc(&(x)->sanitized)
+#define STATS_INC_NOT_SANITIZED(x) atomic_inc(&(x)->not_sanitized)
 #else
 #define	STATS_INC_ACTIVE(x)	do { } while (0)
 #define	STATS_DEC_ACTIVE(x)	do { } while (0)
@@ -335,6 +337,8 @@ static void kmem_cache_node_init(struct kmem_cache_node *parent)
 #define STATS_INC_ALLOCMISS(x)	do { } while (0)
 #define STATS_INC_FREEHIT(x)	do { } while (0)
 #define STATS_INC_FREEMISS(x)	do { } while (0)
+#define STATS_INC_SANITIZED(x)  do { } while (0)
+#define STATS_INC_NOT_SANITIZED(x) do { } while (0)
 #endif
 
 #if DEBUG
@@ -3359,6 +3363,27 @@ free_done:
 	memmove(ac->entry, &(ac->entry[batchcount]), sizeof(void *)*ac->avail);
 }
 
+#ifdef CONFIG_SLAB_MEMORY_SANITIZE
+static void slab_sanitize(struct kmem_cache *cachep, void *objp)
+{
+	if (cachep->flags & (SLAB_POISON | SLAB_NO_SANITIZE)) {
+		STATS_INC_NOT_SANITIZED(cachep);
+	} else {
+		memset(objp, SLAB_MEMORY_SANITIZE_VALUE, cachep->object_size);
+
+		if (cachep->ctor)
+			cachep->ctor(objp);
+
+		STATS_INC_SANITIZED(cachep);
+	}
+}
+#else
+static void slab_sanitize(struct kmem_cache *cachep, void *objp)
+{
+	return;
+}
+#endif
+
 /*
  * Release an obj back to its cache. If the obj has a constructed state, it must
  * be in this state _before_ it is released.  Called with disabled ints.
@@ -3369,6 +3394,8 @@ static inline void __cache_free(struct kmem_cache *cachep, void *objp,
 	struct array_cache *ac = cpu_cache_get(cachep);
 
 	check_irq_off();
+
+	slab_sanitize(cachep, objp);
 	kmemleak_free_recursive(objp, cachep->flags);
 	objp = cache_free_debugcheck(cachep, objp, caller);
 
@@ -4014,6 +4041,14 @@ void slabinfo_show_stats(struct seq_file *m, struct kmem_cache *cachep)
 		seq_printf(m, " : cpustat %6lu %6lu %6lu %6lu",
 			   allochit, allocmiss, freehit, freemiss);
 	}
+#ifdef CONFIG_SLAB_MEMORY_SANITIZE
+	{
+		unsigned long sanitized = atomic_read(&cachep->sanitized);
+		unsigned long not_sanitized = atomic_read(&cachep->not_sanitized);
+
+		seq_printf(m, " : sanitized %6lu %6lu", sanitized, not_sanitized);
+	}
+#endif
 #endif
 }
 
-- 
2.5.0

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

[toc] | [prev] | [next] | [standalone]

#1296524 — [RFC][PATCH 7/7] lkdtm: Add READ_AFTER_FREE test

In a similar manner to WRITE_AFTER_FREE, add a READ_AFTER_FREE
test to test free poisoning features. Sample output when
no poison is present:

[   20.222501] lkdtm: Performing direct entry READ_AFTER_FREE
[   20.226163] lkdtm: Freed val: 12345678

with poison:

[   24.203748] lkdtm: Performing direct entry READ_AFTER_FREE
[   24.207261] general protection fault: 0000 [#1] SMP
[   24.208193] Modules linked in:
[   24.208193] CPU: 0 PID: 866 Comm: sh Not tainted 4.4.0-rc5-work+ #108

Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Laura Abbott <laura@labbott.name>
---
 drivers/misc/lkdtm.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/misc/lkdtm.c b/drivers/misc/lkdtm.c
index 11fdadc..c641fb7 100644
--- a/drivers/misc/lkdtm.c
+++ b/drivers/misc/lkdtm.c
@@ -92,6 +92,7 @@ enum ctype {
 	CT_UNALIGNED_LOAD_STORE_WRITE,
 	CT_OVERWRITE_ALLOCATION,
 	CT_WRITE_AFTER_FREE,
+	CT_READ_AFTER_FREE,
 	CT_SOFTLOCKUP,
 	CT_HARDLOCKUP,
 	CT_SPINLOCKUP,
@@ -129,6 +130,7 @@ static char* cp_type[] = {
 	"UNALIGNED_LOAD_STORE_WRITE",
 	"OVERWRITE_ALLOCATION",
 	"WRITE_AFTER_FREE",
+	"READ_AFTER_FREE",
 	"SOFTLOCKUP",
 	"HARDLOCKUP",
 	"SPINLOCKUP",
@@ -417,6 +419,33 @@ static void lkdtm_do_action(enum ctype which)
 		memset(data, 0x78, len);
 		break;
 	}
+	case CT_READ_AFTER_FREE: {
+		int **base;
+		int *val, *tmp;
+
+		base = kmalloc(1024, GFP_KERNEL);
+		if (!base)
+			return;
+
+		val = kmalloc(1024, GFP_KERNEL);
+		if (!val)
+			return;
+
+		*val = 0x12345678;
+
+		/*
+		 * Don't just use the first entry since that's where the
+		 * freelist goes for the slab allocator
+		 */
+		base[1] = val;
+		kfree(base);
+
+		tmp = base[1];
+		pr_info("Freed val: %x\n", *tmp);
+
+		kfree(val);
+		break;
+	}
 	case CT_SOFTLOCKUP:
 		preempt_disable();
 		for (;;)
-- 
2.5.0

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

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web