All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: Eric Biggers <ebiggers3@gmail.com>
Cc: "kernel-hardening@lists.openwall.com" 
	<kernel-hardening@lists.openwall.com>,
	David Windsor <dave@nullcore.net>, Linux-MM <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [kernel-hardening] [PATCH 23/23] mm: Allow slab_nomerge to be set at build time
Date: Tue, 20 Jun 2017 16:09:40 -0700	[thread overview]
Message-ID: <CAGXu5j+9dpdMnjQWRGhogMwA+2K2HOF8_g8-ngHr=vf3r59XsQ@mail.gmail.com> (raw)
In-Reply-To: <20170620042936.GD610@zzz.localdomain>

On Mon, Jun 19, 2017 at 9:29 PM, Eric Biggers <ebiggers3@gmail.com> wrote:
> On Mon, Jun 19, 2017 at 04:36:37PM -0700, Kees Cook wrote:
>> Some hardened environments want to build kernels with slab_nomerge
>> already set (so that they do not depend on remembering to set the kernel
>> command line option). This is desired to reduce the risk of kernel heap
>> overflows being able to overwrite objects from merged caches, increasing
>> the difficulty of these attacks. By keeping caches unmerged, these kinds
>> of exploits can usually only damage objects in the same cache (though the
>> risk to metadata exploitation is unchanged).
>>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> ---
>>  mm/slab_common.c |  5 ++---
>>  security/Kconfig | 13 +++++++++++++
>>  2 files changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/mm/slab_common.c b/mm/slab_common.c
>> index 6c14d765379f..17a4c4b33283 100644
>> --- a/mm/slab_common.c
>> +++ b/mm/slab_common.c
>> @@ -47,13 +47,12 @@ static DECLARE_WORK(slab_caches_to_rcu_destroy_work,
>>
>>  /*
>>   * Merge control. If this is set then no merging of slab caches will occur.
>> - * (Could be removed. This was introduced to pacify the merge skeptics.)
>>   */
>> -static int slab_nomerge;
>> +static bool slab_nomerge = !IS_ENABLED(CONFIG_SLAB_MERGE_DEFAULT);
>>
>>  static int __init setup_slab_nomerge(char *str)
>>  {
>> -     slab_nomerge = 1;
>> +     slab_nomerge = true;
>>       return 1;
>>  }
>>
>> diff --git a/security/Kconfig b/security/Kconfig
>> index 0c181cebdb8a..e40bd2a260f8 100644
>> --- a/security/Kconfig
>> +++ b/security/Kconfig
>> @@ -166,6 +166,19 @@ config HARDENED_USERCOPY_SPLIT_KMALLOC
>>         confined to a separate cache, attackers must find other ways
>>         to prepare heap attacks that will be near their desired target.
>>
>> +config SLAB_MERGE_DEFAULT
>> +     bool "Allow slab caches to be merged"
>> +     default y
>> +     help
>> +       For reduced kernel memory fragmentation, slab caches can be
>> +       merged when they share the same size and other characteristics.
>> +       This carries a small risk of kernel heap overflows being able
>> +       to overwrite objects from merged caches, which reduces the
>> +       difficulty of such heap attacks. By keeping caches unmerged,
>> +       these kinds of exploits can usually only damage objects in the
>> +       same cache. To disable merging at runtime, "slab_nomerge" can be
>> +       passed on the kernel command line.
>> +
>
> It's good to at least have this option, but again it's logically separate and
> shouldn't just be hidden in patch 23/23.  And again, is it really just about
> heap overflows?
>
> Please also fix the documentation for slab_nomerge in
> Documentation/admin-guide/kernel-parameters.txt.

I've split it out and updated the docs, thanks!

-Kees

-- 
Kees Cook
Pixel Security

WARNING: multiple messages have this Message-ID (diff)
From: Kees Cook <keescook@chromium.org>
To: Eric Biggers <ebiggers3@gmail.com>
Cc: "kernel-hardening@lists.openwall.com"
	<kernel-hardening@lists.openwall.com>,
	David Windsor <dave@nullcore.net>, Linux-MM <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [kernel-hardening] [PATCH 23/23] mm: Allow slab_nomerge to be set at build time
Date: Tue, 20 Jun 2017 16:09:40 -0700	[thread overview]
Message-ID: <CAGXu5j+9dpdMnjQWRGhogMwA+2K2HOF8_g8-ngHr=vf3r59XsQ@mail.gmail.com> (raw)
In-Reply-To: <20170620042936.GD610@zzz.localdomain>

On Mon, Jun 19, 2017 at 9:29 PM, Eric Biggers <ebiggers3@gmail.com> wrote:
> On Mon, Jun 19, 2017 at 04:36:37PM -0700, Kees Cook wrote:
>> Some hardened environments want to build kernels with slab_nomerge
>> already set (so that they do not depend on remembering to set the kernel
>> command line option). This is desired to reduce the risk of kernel heap
>> overflows being able to overwrite objects from merged caches, increasing
>> the difficulty of these attacks. By keeping caches unmerged, these kinds
>> of exploits can usually only damage objects in the same cache (though the
>> risk to metadata exploitation is unchanged).
>>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> ---
>>  mm/slab_common.c |  5 ++---
>>  security/Kconfig | 13 +++++++++++++
>>  2 files changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/mm/slab_common.c b/mm/slab_common.c
>> index 6c14d765379f..17a4c4b33283 100644
>> --- a/mm/slab_common.c
>> +++ b/mm/slab_common.c
>> @@ -47,13 +47,12 @@ static DECLARE_WORK(slab_caches_to_rcu_destroy_work,
>>
>>  /*
>>   * Merge control. If this is set then no merging of slab caches will occur.
>> - * (Could be removed. This was introduced to pacify the merge skeptics.)
>>   */
>> -static int slab_nomerge;
>> +static bool slab_nomerge = !IS_ENABLED(CONFIG_SLAB_MERGE_DEFAULT);
>>
>>  static int __init setup_slab_nomerge(char *str)
>>  {
>> -     slab_nomerge = 1;
>> +     slab_nomerge = true;
>>       return 1;
>>  }
>>
>> diff --git a/security/Kconfig b/security/Kconfig
>> index 0c181cebdb8a..e40bd2a260f8 100644
>> --- a/security/Kconfig
>> +++ b/security/Kconfig
>> @@ -166,6 +166,19 @@ config HARDENED_USERCOPY_SPLIT_KMALLOC
>>         confined to a separate cache, attackers must find other ways
>>         to prepare heap attacks that will be near their desired target.
>>
>> +config SLAB_MERGE_DEFAULT
>> +     bool "Allow slab caches to be merged"
>> +     default y
>> +     help
>> +       For reduced kernel memory fragmentation, slab caches can be
>> +       merged when they share the same size and other characteristics.
>> +       This carries a small risk of kernel heap overflows being able
>> +       to overwrite objects from merged caches, which reduces the
>> +       difficulty of such heap attacks. By keeping caches unmerged,
>> +       these kinds of exploits can usually only damage objects in the
>> +       same cache. To disable merging at runtime, "slab_nomerge" can be
>> +       passed on the kernel command line.
>> +
>
> It's good to at least have this option, but again it's logically separate and
> shouldn't just be hidden in patch 23/23.  And again, is it really just about
> heap overflows?
>
> Please also fix the documentation for slab_nomerge in
> Documentation/admin-guide/kernel-parameters.txt.

I've split it out and updated the docs, thanks!

-Kees

-- 
Kees Cook
Pixel Security

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2017-06-20 23:09 UTC|newest]

Thread overview: 127+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-19 23:36 [PATCH 00/23] Hardened usercopy whitelisting Kees Cook
2017-06-19 23:36 ` [kernel-hardening] " Kees Cook
2017-06-19 23:36 ` Kees Cook
2017-06-19 23:36 ` [PATCH 01/23] usercopy: Prepare for " Kees Cook
2017-06-19 23:36   ` [kernel-hardening] " Kees Cook
2017-06-19 23:36   ` Kees Cook
2017-06-19 23:36 ` [PATCH 02/23] usercopy: Enforce slab cache usercopy region boundaries Kees Cook
2017-06-19 23:36   ` [kernel-hardening] " Kees Cook
2017-06-19 23:36   ` Kees Cook
2017-06-19 23:36 ` [PATCH 03/23] vfs: define usercopy region in names_cache slab caches Kees Cook
2017-06-19 23:36   ` [kernel-hardening] " Kees Cook
2017-06-19 23:36   ` Kees Cook
2017-06-19 23:36 ` [PATCH 04/23] vfs: copy struct mount.mnt_id to userspace using put_user() Kees Cook
2017-06-19 23:36   ` [kernel-hardening] " Kees Cook
2017-06-19 23:36   ` Kees Cook
2017-06-19 23:36 ` [PATCH 05/23] befs: define usercopy region in befs_inode_cache slab cache Kees Cook
2017-06-19 23:36   ` [kernel-hardening] " Kees Cook
2017-06-19 23:36   ` Kees Cook
2017-06-19 23:36 ` [PATCH 06/23] cifs: define usercopy region in cifs_request " Kees Cook
2017-06-19 23:36   ` [kernel-hardening] " Kees Cook
2017-06-19 23:36   ` Kees Cook
2017-06-19 23:36 ` [PATCH 07/23] exofs: define usercopy region in exofs_inode_cache " Kees Cook
2017-06-19 23:36   ` [kernel-hardening] " Kees Cook
2017-06-19 23:36   ` Kees Cook
2017-06-19 23:36 ` [PATCH 08/23] ext2: define usercopy region in ext2_inode_cache " Kees Cook
2017-06-19 23:36   ` [kernel-hardening] " Kees Cook
2017-06-19 23:36   ` Kees Cook
2017-06-19 23:36 ` [PATCH 09/23] ext4: define usercopy region in ext4_inode_cache " Kees Cook
2017-06-19 23:36   ` [kernel-hardening] " Kees Cook
2017-06-19 23:36   ` Kees Cook
2017-06-19 23:36 ` [PATCH 10/23] vxfs: define usercopy region in vxfs_inode " Kees Cook
2017-06-19 23:36   ` [kernel-hardening] " Kees Cook
2017-06-19 23:36   ` Kees Cook
2017-06-19 23:36 ` [PATCH 11/23] jfs: define usercopy region in jfs_ip " Kees Cook
2017-06-19 23:36   ` [kernel-hardening] " Kees Cook
2017-06-19 23:36   ` Kees Cook
2017-06-19 23:36 ` [PATCH 12/23] orangefs: define usercopy region in orangefs_inode_cache " Kees Cook
2017-06-19 23:36   ` [kernel-hardening] " Kees Cook
2017-06-19 23:36   ` Kees Cook
2017-06-19 23:36 ` [PATCH 13/23] ufs: define usercopy region in ufs_inode_cache " Kees Cook
2017-06-19 23:36   ` [kernel-hardening] " Kees Cook
2017-06-19 23:36   ` Kees Cook
2017-06-19 23:36 ` [PATCH 14/23] fork: define usercopy region in thread_stack, task_struct, mm_struct slab caches Kees Cook
2017-06-19 23:36   ` [kernel-hardening] " Kees Cook
2017-06-19 23:36   ` Kees Cook
2017-06-19 23:36 ` [PATCH 15/23] net: define usercopy region in struct proto slab cache Kees Cook
2017-06-19 23:36   ` [kernel-hardening] " Kees Cook
2017-06-19 23:36   ` Kees Cook
2017-06-19 23:36 ` [PATCH 16/23] net: copy struct sctp_sock.autoclose to userspace using put_user() Kees Cook
2017-06-19 23:36   ` [kernel-hardening] " Kees Cook
2017-06-19 23:36   ` Kees Cook
2017-06-19 23:36 ` [PATCH 17/23] dcache: define usercopy region in dentry_cache slab cache Kees Cook
2017-06-19 23:36   ` [kernel-hardening] " Kees Cook
2017-06-19 23:36   ` Kees Cook
2017-06-20  4:08   ` [kernel-hardening] " Eric Biggers
2017-06-20  4:08     ` Eric Biggers
2017-06-28 16:44     ` Kees Cook
2017-06-28 16:44       ` Kees Cook
2017-06-28 16:44       ` Kees Cook
2017-06-28 16:55       ` Eric Biggers
2017-06-28 16:55         ` Eric Biggers
2017-06-28 16:55         ` Eric Biggers
2017-06-19 23:36 ` [PATCH 18/23] scsi: define usercopy region in scsi_sense_cache " Kees Cook
2017-06-19 23:36   ` [kernel-hardening] " Kees Cook
2017-06-19 23:36   ` Kees Cook
2017-06-19 23:36 ` [PATCH 19/23] xfs: define usercopy region in xfs_inode " Kees Cook
2017-06-19 23:36   ` [kernel-hardening] " Kees Cook
2017-06-19 23:36   ` Kees Cook
2017-06-19 23:36 ` [PATCH 20/23] usercopy: convert kmalloc caches to usercopy caches Kees Cook
2017-06-19 23:36   ` [kernel-hardening] " Kees Cook
2017-06-19 23:36   ` Kees Cook
2017-06-19 23:36 ` [PATCH 21/23] usercopy: Restrict non-usercopy caches to size 0 Kees Cook
2017-06-19 23:36   ` [kernel-hardening] " Kees Cook
2017-06-19 23:36   ` Kees Cook
2017-06-20  4:04   ` [kernel-hardening] " Eric Biggers
2017-06-20  4:04     ` Eric Biggers
2017-06-28 17:03     ` Kees Cook
2017-06-28 17:03       ` Kees Cook
2017-06-28 17:03       ` Kees Cook
2017-06-19 23:36 ` [PATCH 22/23] usercopy: split user-controlled slabs to separate caches Kees Cook
2017-06-19 23:36   ` [kernel-hardening] " Kees Cook
2017-06-19 23:36   ` Kees Cook
2017-06-20  4:24   ` [kernel-hardening] " Eric Biggers
2017-06-20  4:24     ` Eric Biggers
2017-06-20  4:47   ` Eric Biggers
2017-06-20  4:47     ` Eric Biggers
2017-06-20 22:27     ` Kees Cook
2017-06-20 22:27       ` Kees Cook
2017-06-20 22:27       ` Kees Cook
2017-06-20 20:24   ` Laura Abbott
2017-06-20 20:24     ` [kernel-hardening] " Laura Abbott
2017-06-20 20:24     ` Laura Abbott
2017-06-20 22:22     ` Kees Cook
2017-06-20 22:22       ` [kernel-hardening] " Kees Cook
2017-06-20 22:22       ` Kees Cook
2017-06-27  7:31       ` Michal Hocko
2017-06-27  7:31         ` [kernel-hardening] " Michal Hocko
2017-06-27  7:31         ` Michal Hocko
2017-06-27 22:07         ` Kees Cook
2017-06-27 22:07           ` [kernel-hardening] " Kees Cook
2017-06-27 22:07           ` Kees Cook
2017-06-28  8:54           ` Michal Hocko
2017-06-28  8:54             ` [kernel-hardening] " Michal Hocko
2017-06-28  8:54             ` Michal Hocko
2017-06-19 23:36 ` [PATCH 23/23] mm: Allow slab_nomerge to be set at build time Kees Cook
2017-06-19 23:36   ` [kernel-hardening] " Kees Cook
2017-06-19 23:36   ` Kees Cook
2017-06-20  4:09   ` [kernel-hardening] " Daniel Micay
2017-06-20  4:09     ` Daniel Micay
2017-06-20 22:51     ` Kees Cook
2017-06-20 22:51       ` Kees Cook
2017-06-20 22:51       ` Kees Cook
2017-06-20  4:29   ` Eric Biggers
2017-06-20  4:29     ` Eric Biggers
2017-06-20 23:09     ` Kees Cook [this message]
2017-06-20 23:09       ` Kees Cook
2017-06-20 23:09       ` Kees Cook
2017-06-20 19:41 ` [kernel-hardening] [PATCH 00/23] Hardened usercopy whitelisting Rik van Riel
2017-10-20 22:40 ` Paolo Bonzini
2017-10-20 22:40   ` [kernel-hardening] " Paolo Bonzini
2017-10-20 22:40   ` Paolo Bonzini
2017-10-20 23:25   ` Paolo Bonzini
2017-10-20 23:25     ` [kernel-hardening] " Paolo Bonzini
2017-10-20 23:25     ` Paolo Bonzini
2017-10-21  3:04     ` Kees Cook
2017-10-21  3:04       ` [kernel-hardening] " Kees Cook
2017-10-21  3:04       ` Kees Cook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAGXu5j+9dpdMnjQWRGhogMwA+2K2HOF8_g8-ngHr=vf3r59XsQ@mail.gmail.com' \
    --to=keescook@chromium.org \
    --cc=dave@nullcore.net \
    --cc=ebiggers3@gmail.com \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.