* [PATCH 03/10] LSM: SafeSetID: refactor policy hash table
From: Micah Morton @ 2019-04-10 16:55 UTC (permalink / raw)
To: jmorris, keescook, casey, linux-security-module; +Cc: Jann Horn, Micah Morton
From: Jann Horn <jannh@google.com>
parent_kuid and child_kuid are kuids, there is no reason to make them
uint64_t. (And anyway, in the kernel, the normal name for that would be
u64, not uint64_t.)
check_setuid_policy_hashtable_key() and
check_setuid_policy_hashtable_key_value() are basically the same thing,
merge them.
Also fix the comment that claimed that (1<<8)==128.
Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Micah Morton <mortonm@chromium.org>
---
security/safesetid/lsm.c | 62 ++++++++++++----------------------------
security/safesetid/lsm.h | 19 ++++++++++++
2 files changed, 37 insertions(+), 44 deletions(-)
diff --git a/security/safesetid/lsm.c b/security/safesetid/lsm.c
index 5310fcf3052a..15cd13b5a211 100644
--- a/security/safesetid/lsm.c
+++ b/security/safesetid/lsm.c
@@ -14,67 +14,40 @@
#define pr_fmt(fmt) "SafeSetID: " fmt
-#include <linux/hashtable.h>
#include <linux/lsm_hooks.h>
#include <linux/module.h>
#include <linux/ptrace.h>
#include <linux/sched/task_stack.h>
#include <linux/security.h>
+#include "lsm.h"
/* Flag indicating whether initialization completed */
int safesetid_initialized;
-#define NUM_BITS 8 /* 128 buckets in hash table */
+#define NUM_BITS 8 /* 256 buckets in hash table */
static DEFINE_HASHTABLE(safesetid_whitelist_hashtable, NUM_BITS);
-/*
- * Hash table entry to store safesetid policy signifying that 'parent' user
- * can setid to 'child' user.
- */
-struct entry {
- struct hlist_node next;
- struct hlist_node dlist; /* for deletion cleanup */
- uint64_t parent_kuid;
- uint64_t child_kuid;
-};
-
static DEFINE_SPINLOCK(safesetid_whitelist_hashtable_spinlock);
-static bool check_setuid_policy_hashtable_key(kuid_t parent)
+static enum sid_policy_type setuid_policy_lookup(kuid_t src, kuid_t dst)
{
struct entry *entry;
+ enum sid_policy_type result = SIDPOL_DEFAULT;
rcu_read_lock();
hash_for_each_possible_rcu(safesetid_whitelist_hashtable,
- entry, next, __kuid_val(parent)) {
- if (entry->parent_kuid == __kuid_val(parent)) {
+ entry, next, __kuid_val(src)) {
+ if (!uid_eq(entry->src_uid, src))
+ continue;
+ if (uid_eq(entry->dst_uid, dst)) {
rcu_read_unlock();
- return true;
+ return SIDPOL_ALLOWED;
}
+ result = SIDPOL_CONSTRAINED;
}
rcu_read_unlock();
-
- return false;
-}
-
-static bool check_setuid_policy_hashtable_key_value(kuid_t parent,
- kuid_t child)
-{
- struct entry *entry;
-
- rcu_read_lock();
- hash_for_each_possible_rcu(safesetid_whitelist_hashtable,
- entry, next, __kuid_val(parent)) {
- if (entry->parent_kuid == __kuid_val(parent) &&
- entry->child_kuid == __kuid_val(child)) {
- rcu_read_unlock();
- return true;
- }
- }
- rcu_read_unlock();
-
- return false;
+ return result;
}
static int safesetid_security_capable(const struct cred *cred,
@@ -83,7 +56,7 @@ static int safesetid_security_capable(const struct cred *cred,
unsigned int opts)
{
if (cap == CAP_SETUID &&
- check_setuid_policy_hashtable_key(cred->uid)) {
+ setuid_policy_lookup(cred->uid, INVALID_UID) != SIDPOL_DEFAULT) {
if (!(opts & CAP_OPT_INSETID)) {
/*
* Deny if we're not in a set*uid() syscall to avoid
@@ -116,7 +89,8 @@ static bool uid_permitted_for_cred(const struct cred *old, kuid_t new_uid)
* Transitions to new UIDs require a check against the policy of the old
* RUID.
*/
- permitted = check_setuid_policy_hashtable_key_value(old->uid, new_uid);
+ permitted =
+ setuid_policy_lookup(old->uid, new_uid) != SIDPOL_CONSTRAINED;
if (!permitted) {
pr_warn("UID transition ((%d,%d,%d) -> %d) blocked\n",
__kuid_val(old->uid), __kuid_val(old->euid),
@@ -136,7 +110,7 @@ static int safesetid_task_fix_setuid(struct cred *new,
{
/* Do nothing if there are no setuid restrictions for our old RUID. */
- if (!check_setuid_policy_hashtable_key(old->uid))
+ if (setuid_policy_lookup(old->uid, INVALID_UID) == SIDPOL_DEFAULT)
return 0;
if (uid_permitted_for_cred(old, new->uid) &&
@@ -159,14 +133,14 @@ int add_safesetid_whitelist_entry(kuid_t parent, kuid_t child)
struct entry *new;
/* Return if entry already exists */
- if (check_setuid_policy_hashtable_key_value(parent, child))
+ if (setuid_policy_lookup(parent, child) == SIDPOL_ALLOWED)
return 0;
new = kzalloc(sizeof(struct entry), GFP_KERNEL);
if (!new)
return -ENOMEM;
- new->parent_kuid = __kuid_val(parent);
- new->child_kuid = __kuid_val(child);
+ new->src_uid = parent;
+ new->dst_uid = child;
spin_lock(&safesetid_whitelist_hashtable_spinlock);
hash_add_rcu(safesetid_whitelist_hashtable,
&new->next,
diff --git a/security/safesetid/lsm.h b/security/safesetid/lsm.h
index c1ea3c265fcf..6806f902794c 100644
--- a/security/safesetid/lsm.h
+++ b/security/safesetid/lsm.h
@@ -15,6 +15,8 @@
#define _SAFESETID_H
#include <linux/types.h>
+#include <linux/uidgid.h>
+#include <linux/hashtable.h>
/* Flag indicating whether initialization completed */
extern int safesetid_initialized;
@@ -25,6 +27,23 @@ enum safesetid_whitelist_file_write_type {
SAFESETID_WHITELIST_FLUSH, /* Flush whitelist policies. */
};
+enum sid_policy_type {
+ SIDPOL_DEFAULT, /* source ID is unaffected by policy */
+ SIDPOL_CONSTRAINED, /* source ID is affected by policy */
+ SIDPOL_ALLOWED /* target ID explicitly allowed */
+};
+
+/*
+ * Hash table entry to store safesetid policy signifying that 'src_uid'
+ * can setid to 'dst_uid'.
+ */
+struct entry {
+ struct hlist_node next;
+ struct hlist_node dlist; /* for deletion cleanup */
+ kuid_t src_uid;
+ kuid_t dst_uid;
+};
+
/* Add entry to safesetid whitelist to allow 'parent' to setid to 'child'. */
int add_safesetid_whitelist_entry(kuid_t parent, kuid_t child);
--
2.21.0.392.gf8f6787159e-goog
^ permalink raw reply related
* [PATCH 02/10] LSM: SafeSetID: fix check for setresuid(new1, new2, new3)
From: Micah Morton @ 2019-04-10 16:55 UTC (permalink / raw)
To: jmorris, keescook, casey, linux-security-module; +Cc: Jann Horn, Micah Morton
From: Jann Horn <jannh@google.com>
With the old code, when a process with the (real,effective,saved) UID set
(1,1,1) calls setresuid(2,3,4), safesetid_task_fix_setuid() only checks
whether the transition 1->2 is permitted; the transitions 1->3 and 1->4 are
not checked. Fix this.
This is also a good opportunity to refactor safesetid_task_fix_setuid() to
be less verbose - having one branch per set*uid() syscall is unnecessary.
Note that this slightly changes semantics: The UID transition check for
UIDs that were not in the old cred struct is now always performed against
the policy of the RUID. I think that's more consistent anyway, since the
RUID is also the one that decides whether any policy is enforced at all.
Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Micah Morton <mortonm@chromium.org>
---
security/safesetid/lsm.c | 125 +++++++++++----------------------------
1 file changed, 35 insertions(+), 90 deletions(-)
diff --git a/security/safesetid/lsm.c b/security/safesetid/lsm.c
index 2daecab3a4c0..5310fcf3052a 100644
--- a/security/safesetid/lsm.c
+++ b/security/safesetid/lsm.c
@@ -99,20 +99,30 @@ static int safesetid_security_capable(const struct cred *cred,
return 0;
}
-static int check_uid_transition(kuid_t parent, kuid_t child)
+/*
+ * Check whether a caller with old credentials @old is allowed to switch to
+ * credentials that contain @new_uid.
+ */
+static bool uid_permitted_for_cred(const struct cred *old, kuid_t new_uid)
{
- if (check_setuid_policy_hashtable_key_value(parent, child))
- return 0;
- pr_warn("UID transition (%d -> %d) blocked\n",
- __kuid_val(parent),
- __kuid_val(child));
+ bool permitted;
+
+ /* If our old creds already had this UID in it, it's fine. */
+ if (uid_eq(new_uid, old->uid) || uid_eq(new_uid, old->euid) ||
+ uid_eq(new_uid, old->suid))
+ return true;
+
/*
- * Kill this process to avoid potential security vulnerabilities
- * that could arise from a missing whitelist entry preventing a
- * privileged process from dropping to a lesser-privileged one.
+ * Transitions to new UIDs require a check against the policy of the old
+ * RUID.
*/
- force_sig(SIGKILL, current);
- return -EACCES;
+ permitted = check_setuid_policy_hashtable_key_value(old->uid, new_uid);
+ if (!permitted) {
+ pr_warn("UID transition ((%d,%d,%d) -> %d) blocked\n",
+ __kuid_val(old->uid), __kuid_val(old->euid),
+ __kuid_val(old->suid), __kuid_val(new_uid));
+ }
+ return permitted;
}
/*
@@ -125,88 +135,23 @@ static int safesetid_task_fix_setuid(struct cred *new,
int flags)
{
- /* Do nothing if there are no setuid restrictions for this UID. */
+ /* Do nothing if there are no setuid restrictions for our old RUID. */
if (!check_setuid_policy_hashtable_key(old->uid))
return 0;
- switch (flags) {
- case LSM_SETID_RE:
- /*
- * Users for which setuid restrictions exist can only set the
- * real UID to the real UID or the effective UID, unless an
- * explicit whitelist policy allows the transition.
- */
- if (!uid_eq(old->uid, new->uid) &&
- !uid_eq(old->euid, new->uid)) {
- return check_uid_transition(old->uid, new->uid);
- }
- /*
- * Users for which setuid restrictions exist can only set the
- * effective UID to the real UID, the effective UID, or the
- * saved set-UID, unless an explicit whitelist policy allows
- * the transition.
- */
- if (!uid_eq(old->uid, new->euid) &&
- !uid_eq(old->euid, new->euid) &&
- !uid_eq(old->suid, new->euid)) {
- return check_uid_transition(old->euid, new->euid);
- }
- break;
- case LSM_SETID_ID:
- /*
- * Users for which setuid restrictions exist cannot change the
- * real UID or saved set-UID unless an explicit whitelist
- * policy allows the transition.
- */
- if (!uid_eq(old->uid, new->uid))
- return check_uid_transition(old->uid, new->uid);
- if (!uid_eq(old->suid, new->suid))
- return check_uid_transition(old->suid, new->suid);
- break;
- case LSM_SETID_RES:
- /*
- * Users for which setuid restrictions exist cannot change the
- * real UID, effective UID, or saved set-UID to anything but
- * one of: the current real UID, the current effective UID or
- * the current saved set-user-ID unless an explicit whitelist
- * policy allows the transition.
- */
- if (!uid_eq(new->uid, old->uid) &&
- !uid_eq(new->uid, old->euid) &&
- !uid_eq(new->uid, old->suid)) {
- return check_uid_transition(old->uid, new->uid);
- }
- if (!uid_eq(new->euid, old->uid) &&
- !uid_eq(new->euid, old->euid) &&
- !uid_eq(new->euid, old->suid)) {
- return check_uid_transition(old->euid, new->euid);
- }
- if (!uid_eq(new->suid, old->uid) &&
- !uid_eq(new->suid, old->euid) &&
- !uid_eq(new->suid, old->suid)) {
- return check_uid_transition(old->suid, new->suid);
- }
- break;
- case LSM_SETID_FS:
- /*
- * Users for which setuid restrictions exist cannot change the
- * filesystem UID to anything but one of: the current real UID,
- * the current effective UID or the current saved set-UID
- * unless an explicit whitelist policy allows the transition.
- */
- if (!uid_eq(new->fsuid, old->uid) &&
- !uid_eq(new->fsuid, old->euid) &&
- !uid_eq(new->fsuid, old->suid) &&
- !uid_eq(new->fsuid, old->fsuid)) {
- return check_uid_transition(old->fsuid, new->fsuid);
- }
- break;
- default:
- pr_warn("Unknown setid state %d\n", flags);
- force_sig(SIGKILL, current);
- return -EINVAL;
- }
- return 0;
+ if (uid_permitted_for_cred(old, new->uid) &&
+ uid_permitted_for_cred(old, new->euid) &&
+ uid_permitted_for_cred(old, new->suid) &&
+ uid_permitted_for_cred(old, new->fsuid))
+ return 0;
+
+ /*
+ * Kill this process to avoid potential security vulnerabilities
+ * that could arise from a missing whitelist entry preventing a
+ * privileged process from dropping to a lesser-privileged one.
+ */
+ force_sig(SIGKILL, current);
+ return -EACCES;
}
int add_safesetid_whitelist_entry(kuid_t parent, kuid_t child)
--
2.21.0.392.gf8f6787159e-goog
^ permalink raw reply related
* [PATCH 01/10] LSM: SafeSetID: fix pr_warn() to include newline
From: Micah Morton @ 2019-04-10 16:54 UTC (permalink / raw)
To: jmorris, keescook, casey, linux-security-module; +Cc: Jann Horn, Micah Morton
From: Jann Horn <jannh@google.com>
Fix the pr_warn() calls in the SafeSetID LSM to have newlines at the end.
Without this, denial messages will be buffered as incomplete lines in
log_output(), and will then only show up once something else prints into
dmesg.
Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Micah Morton <mortonm@chromium.org>
---
security/safesetid/lsm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/security/safesetid/lsm.c b/security/safesetid/lsm.c
index cecd38e2ac80..2daecab3a4c0 100644
--- a/security/safesetid/lsm.c
+++ b/security/safesetid/lsm.c
@@ -91,7 +91,7 @@ static int safesetid_security_capable(const struct cred *cred,
* to functionality other than calling set*uid() (e.g.
* allowing user to set up userns uid mappings).
*/
- pr_warn("Operation requires CAP_SETUID, which is not available to UID %u for operations besides approved set*uid transitions",
+ pr_warn("Operation requires CAP_SETUID, which is not available to UID %u for operations besides approved set*uid transitions\n",
__kuid_val(cred->uid));
return -1;
}
@@ -103,7 +103,7 @@ static int check_uid_transition(kuid_t parent, kuid_t child)
{
if (check_setuid_policy_hashtable_key_value(parent, child))
return 0;
- pr_warn("UID transition (%d -> %d) blocked",
+ pr_warn("UID transition (%d -> %d) blocked\n",
__kuid_val(parent),
__kuid_val(child));
/*
--
2.21.0.392.gf8f6787159e-goog
^ permalink raw reply related
* Re: [GIT PULL] apparmor regression fix for v5.1-rc5
From: pr-tracker-bot @ 2019-04-10 16:50 UTC (permalink / raw)
To: John Johansen
Cc: Linus Torvalds, LKLM, open list:SECURITY SUBSYSTEM,
David Rheinsberg, Kees Cook
In-Reply-To: <234e0edf-0dd2-e74a-019a-4ce536af3358@canonical.com>
The pull request you sent on Wed, 10 Apr 2019 04:32:14 -0700:
> git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor 5.1-regression-fix
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/ed79cc87302bf7fbc87f05d655b998f866b4fed8
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
^ permalink raw reply
* [PATCH 0/3] Kconfig: Refactor memory initialization hardening
From: Kees Cook @ 2019-04-10 16:16 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Kees Cook, Alexander Potapenko, Nick Desaulniers,
Kostya Serebryany, Dmitry Vyukov, Sandeep Patil, Laura Abbott,
Randy Dunlap, Alexander Popov, Michal Marek, Emese Revfy,
James Morris, Serge E. Hallyn, linux-kbuild, linux-kernel,
linux-security-module, kernel-hardening
This is a proposed alternative for the memory initialization series,
which refactoring the existing gcc plugins into a separate Kconfig
file and collects all the related options together with some more
language to describe their differences. The last patch adds the
Clang auto init option, as done by Alexander Potapenko.
Since there isn't really a good way to "select" with dependencies,
I've left out CONFIG_INIT_ALL_MEMORY for the moment...
-Kees
Kees Cook (3):
Kconfig: Create "kernel hardening" config area
kbuild: Move stackleak config to Kconfig.hardening
kbuild: Implement Clang's stack initialization
Makefile | 5 ++
scripts/gcc-plugins/Kconfig | 121 +-------------------------
security/Kconfig | 2 +
security/Kconfig.hardening | 165 ++++++++++++++++++++++++++++++++++++
4 files changed, 175 insertions(+), 118 deletions(-)
create mode 100644 security/Kconfig.hardening
--
2.17.1
^ permalink raw reply
* [PATCH 3/3] kbuild: Implement Clang's stack initialization
From: Kees Cook @ 2019-04-10 16:16 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Kees Cook, Alexander Potapenko, Nick Desaulniers,
Kostya Serebryany, Dmitry Vyukov, Sandeep Patil, Laura Abbott,
Randy Dunlap, Alexander Popov, Michal Marek, Emese Revfy,
James Morris, Serge E. Hallyn, linux-kbuild, linux-kernel,
linux-security-module, kernel-hardening
In-Reply-To: <20190410161612.18545-1-keescook@chromium.org>
CONFIG_INIT_STACK_ALL turns on stack initialization based on
-ftrivial-auto-var-init in Clang builds and on
-fplugin-arg-structleak_plugin-byref-all in GCC builds.
-ftrivial-auto-var-init is a Clang flag that provides trivial
initializers for uninitialized local variables, variable fields and
padding.
It has three possible values:
pattern - uninitialized locals are filled with a fixed pattern
(mostly 0xAA on 64-bit platforms, see https://reviews.llvm.org/D54604
for more details) likely to cause crashes when uninitialized value is
used;
zero (it's still debated whether this flag makes it to the official
Clang release) - uninitialized locals are filled with zeroes;
uninitialized (default) - uninitialized locals are left intact.
The proposed config builds the kernel with
-ftrivial-auto-var-init=pattern when selected.
Developers have the possibility to opt-out of this feature on a
per-variable basis by using __attribute__((uninitialized)).
Co-developed-by: Alexander Potapenko <glider@google.com>
Signed-off-by: Alexander Potapenko <glider@google.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
Makefile | 5 +++++
security/Kconfig.hardening | 14 ++++++++++++++
2 files changed, 19 insertions(+)
diff --git a/Makefile b/Makefile
index c0a34064c574..a7d9c6cd0267 100644
--- a/Makefile
+++ b/Makefile
@@ -745,6 +745,11 @@ KBUILD_CFLAGS += -fomit-frame-pointer
endif
endif
+# Initialize all stack variables with a pattern, if desired.
+ifdef CONFIG_INIT_STACK_ALL
+KBUILD_CFLAGS += -ftrivial-auto-var-init=pattern
+endif
+
DEBUG_CFLAGS := $(call cc-option, -fno-var-tracking-assignments)
ifdef CONFIG_DEBUG_INFO
diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
index 9942d9869864..d744e20140b4 100644
--- a/security/Kconfig.hardening
+++ b/security/Kconfig.hardening
@@ -19,9 +19,13 @@ config GCC_PLUGIN_STRUCTLEAK
menu "Memory initialization"
+config CC_HAS_AUTO_VAR_INIT
+ def_bool $(cc-option,-ftrivial-auto-var-init=pattern)
+
choice
prompt "Initialize kernel stack variables at function entry"
depends on CC_HAS_AUTO_VAR_INIT || GCC_PLUGINS
+ default INIT_STACK_ALL if CC_HAS_AUTO_VAR_INIT
default INIT_STACK_NONE
help
This option enables initialization of stack variables at
@@ -77,6 +81,16 @@ choice
of uninitialized stack variable exploits and information
exposures.
+ config INIT_STACK_ALL
+ bool "0xAA-init everything on the stack (strongest)"
+ depends on CC_HAS_AUTO_VAR_INIT
+ help
+ Initializes everything on the stack with a 0xAA
+ pattern. This is intended to eliminate all classes
+ of uninitialized stack variable exploits and information
+ exposures, even variables that were warned to have been
+ left uninitialized.
+
endchoice
config GCC_PLUGIN_STRUCTLEAK_VERBOSE
--
2.17.1
^ permalink raw reply related
* [PATCH 1/3] Kconfig: Create "kernel hardening" config area
From: Kees Cook @ 2019-04-10 16:16 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Kees Cook, Alexander Potapenko, Nick Desaulniers,
Kostya Serebryany, Dmitry Vyukov, Sandeep Patil, Laura Abbott,
Randy Dunlap, Alexander Popov, Michal Marek, Emese Revfy,
James Morris, Serge E. Hallyn, linux-kbuild, linux-kernel,
linux-security-module, kernel-hardening
In-Reply-To: <20190410161612.18545-1-keescook@chromium.org>
Right now kernel hardening options are scattered around various Kconfig
files. This can be a central place to collect these kinds of options
going forward.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
scripts/gcc-plugins/Kconfig | 70 ++-------------------------
security/Kconfig | 2 +
security/Kconfig.hardening | 94 +++++++++++++++++++++++++++++++++++++
3 files changed, 99 insertions(+), 67 deletions(-)
create mode 100644 security/Kconfig.hardening
diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
index 74271dba4f94..01874ef0f883 100644
--- a/scripts/gcc-plugins/Kconfig
+++ b/scripts/gcc-plugins/Kconfig
@@ -13,10 +13,11 @@ config HAVE_GCC_PLUGINS
An arch should select this symbol if it supports building with
GCC plugins.
-menuconfig GCC_PLUGINS
- bool "GCC plugins"
+config GCC_PLUGINS
+ bool
depends on HAVE_GCC_PLUGINS
depends on PLUGIN_HOSTCC != ""
+ default y
help
GCC plugins are loadable modules that provide extra features to the
compiler. They are useful for runtime instrumentation and static analysis.
@@ -66,71 +67,6 @@ config GCC_PLUGIN_LATENT_ENTROPY
* https://grsecurity.net/
* https://pax.grsecurity.net/
-config GCC_PLUGIN_STRUCTLEAK
- bool "Zero initialize stack variables"
- help
- While the kernel is built with warnings enabled for any missed
- stack variable initializations, this warning is silenced for
- anything passed by reference to another function, under the
- occasionally misguided assumption that the function will do
- the initialization. As this regularly leads to exploitable
- flaws, this plugin is available to identify and zero-initialize
- such variables, depending on the chosen level of coverage.
-
- This plugin was originally ported from grsecurity/PaX. More
- information at:
- * https://grsecurity.net/
- * https://pax.grsecurity.net/
-
-choice
- prompt "Coverage"
- depends on GCC_PLUGIN_STRUCTLEAK
- default GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
- help
- This chooses the level of coverage over classes of potentially
- uninitialized variables. The selected class will be
- zero-initialized before use.
-
- config GCC_PLUGIN_STRUCTLEAK_USER
- bool "structs marked for userspace"
- help
- Zero-initialize any structures on the stack containing
- a __user attribute. This can prevent some classes of
- uninitialized stack variable exploits and information
- exposures, like CVE-2013-2141:
- https://git.kernel.org/linus/b9e146d8eb3b9eca
-
- config GCC_PLUGIN_STRUCTLEAK_BYREF
- bool "structs passed by reference"
- help
- Zero-initialize any structures on the stack that may
- be passed by reference and had not already been
- explicitly initialized. This can prevent most classes
- of uninitialized stack variable exploits and information
- exposures, like CVE-2017-1000410:
- https://git.kernel.org/linus/06e7e776ca4d3654
-
- config GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
- bool "anything passed by reference"
- help
- Zero-initialize any stack variables that may be passed
- by reference and had not already been explicitly
- initialized. This is intended to eliminate all classes
- of uninitialized stack variable exploits and information
- exposures.
-
-endchoice
-
-config GCC_PLUGIN_STRUCTLEAK_VERBOSE
- bool "Report forcefully initialized variables"
- depends on GCC_PLUGIN_STRUCTLEAK
- depends on !COMPILE_TEST # too noisy
- help
- This option will cause a warning to be printed each time the
- structleak plugin finds a variable it thinks needs to be
- initialized. Since not all existing initializers are detected
- by the plugin, this can produce false positive warnings.
-
config GCC_PLUGIN_RANDSTRUCT
bool "Randomize layout of sensitive kernel structures"
select MODVERSIONS if MODULES
diff --git a/security/Kconfig b/security/Kconfig
index 1d6463fb1450..7aec8d094ce2 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -249,5 +249,7 @@ config LSM
If unsure, leave this as the default.
+source "security/Kconfig.hardening"
+
endmenu
diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
new file mode 100644
index 000000000000..8223a8ab1a12
--- /dev/null
+++ b/security/Kconfig.hardening
@@ -0,0 +1,94 @@
+menu "Kernel hardening options"
+
+config GCC_PLUGIN_STRUCTLEAK
+ bool
+ depends on GCC_PLUGIN_STRUCTLEAK_USER || GCC_PLUGIN_STRUCTLEAK_BYREF || GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
+ help
+ While the kernel is built with warnings enabled for any missed
+ stack variable initializations, this warning is silenced for
+ anything passed by reference to another function, under the
+ occasionally misguided assumption that the function will do
+ the initialization. As this regularly leads to exploitable
+ flaws, this plugin is available to identify and zero-initialize
+ such variables, depending on the chosen level of coverage.
+
+ This plugin was originally ported from grsecurity/PaX. More
+ information at:
+ * https://grsecurity.net/
+ * https://pax.grsecurity.net/
+
+menu "Memory initialization"
+
+choice
+ prompt "Initialize kernel stack variables at function entry"
+ depends on CC_HAS_AUTO_VAR_INIT || GCC_PLUGINS
+ default INIT_STACK_NONE
+ help
+ This option enables initialization of stack variables at
+ function entry time. This has the possibility to have the
+ greatest coverage (since all functions can have their
+ variables initialized), but the performance impact depends
+ on the function calling complexity of a given workload's
+ syscalls.
+
+ This chooses the level of coverage over classes of potentially
+ uninitialized variables. The selected class will be
+ initialized before use in a function.
+
+ config INIT_STACK_NONE
+ bool "no automatic initialization (weakest)"
+ help
+ Disable automatic stack variable initialization.
+ This leaves the kernel vulnerable to the standard
+ classes of uninitialized stack variable exploits
+ and information exposures.
+
+ config GCC_PLUGIN_STRUCTLEAK_USER
+ bool "zero-init structs marked for userspace (weak)"
+ depends on GCC_PLUGINS
+ select GCC_PLUGIN_STRUCTLEAK
+ help
+ Zero-initialize any structures on the stack containing
+ a __user attribute. This can prevent some classes of
+ uninitialized stack variable exploits and information
+ exposures, like CVE-2013-2141:
+ https://git.kernel.org/linus/b9e146d8eb3b9eca
+
+ config GCC_PLUGIN_STRUCTLEAK_BYREF
+ bool "zero-init structs passed by reference (strong)"
+ depends on GCC_PLUGINS
+ select GCC_PLUGIN_STRUCTLEAK
+ help
+ Zero-initialize any structures on the stack that may
+ be passed by reference and had not already been
+ explicitly initialized. This can prevent most classes
+ of uninitialized stack variable exploits and information
+ exposures, like CVE-2017-1000410:
+ https://git.kernel.org/linus/06e7e776ca4d3654
+
+ config GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
+ bool "zero-init anything passed by reference (very strong)"
+ depends on GCC_PLUGINS
+ select GCC_PLUGIN_STRUCTLEAK
+ help
+ Zero-initialize any stack variables that may be passed
+ by reference and had not already been explicitly
+ initialized. This is intended to eliminate all classes
+ of uninitialized stack variable exploits and information
+ exposures.
+
+endchoice
+
+config GCC_PLUGIN_STRUCTLEAK_VERBOSE
+ bool "Report forcefully initialized variables"
+ depends on GCC_PLUGIN_STRUCTLEAK
+ depends on !COMPILE_TEST # too noisy
+ help
+ This option will cause a warning to be printed each time the
+ structleak plugin finds a variable it thinks needs to be
+ initialized. Since not all existing initializers are detected
+ by the plugin, this can produce false positive warnings.
+
+endmenu
+
+endmenu
--
2.17.1
^ permalink raw reply related
* [PATCH 2/3] kbuild: Move stackleak config to Kconfig.hardening
From: Kees Cook @ 2019-04-10 16:16 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Kees Cook, Alexander Potapenko, Nick Desaulniers,
Kostya Serebryany, Dmitry Vyukov, Sandeep Patil, Laura Abbott,
Randy Dunlap, Alexander Popov, Michal Marek, Emese Revfy,
James Morris, Serge E. Hallyn, linux-kbuild, linux-kernel,
linux-security-module, kernel-hardening
In-Reply-To: <20190410161612.18545-1-keescook@chromium.org>
This moves the stackleak plugin options to Kconfig.hardening's memory
initialization menu.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
scripts/gcc-plugins/Kconfig | 51 ---------------------------------
security/Kconfig.hardening | 57 +++++++++++++++++++++++++++++++++++++
2 files changed, 57 insertions(+), 51 deletions(-)
diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
index 01874ef0f883..50cfcf1ed979 100644
--- a/scripts/gcc-plugins/Kconfig
+++ b/scripts/gcc-plugins/Kconfig
@@ -107,57 +107,6 @@ config GCC_PLUGIN_RANDSTRUCT_PERFORMANCE
in structures. This reduces the performance hit of RANDSTRUCT
at the cost of weakened randomization.
-config GCC_PLUGIN_STACKLEAK
- bool "Erase the kernel stack before returning from syscalls"
- depends on GCC_PLUGINS
- depends on HAVE_ARCH_STACKLEAK
- help
- This option makes the kernel erase the kernel stack before
- returning from system calls. That reduces the information which
- kernel stack leak bugs can reveal and blocks some uninitialized
- stack variable attacks.
-
- The tradeoff is the performance impact: on a single CPU system kernel
- compilation sees a 1% slowdown, other systems and workloads may vary
- and you are advised to test this feature on your expected workload
- before deploying it.
-
- This plugin was ported from grsecurity/PaX. More information at:
- * https://grsecurity.net/
- * https://pax.grsecurity.net/
-
-config STACKLEAK_TRACK_MIN_SIZE
- int "Minimum stack frame size of functions tracked by STACKLEAK"
- default 100
- range 0 4096
- depends on GCC_PLUGIN_STACKLEAK
- help
- The STACKLEAK gcc plugin instruments the kernel code for tracking
- the lowest border of the kernel stack (and for some other purposes).
- It inserts the stackleak_track_stack() call for the functions with
- a stack frame size greater than or equal to this parameter.
- If unsure, leave the default value 100.
-
-config STACKLEAK_METRICS
- bool "Show STACKLEAK metrics in the /proc file system"
- depends on GCC_PLUGIN_STACKLEAK
- depends on PROC_FS
- help
- If this is set, STACKLEAK metrics for every task are available in
- the /proc file system. In particular, /proc/<pid>/stack_depth
- shows the maximum kernel stack consumption for the current and
- previous syscalls. Although this information is not precise, it
- can be useful for estimating the STACKLEAK performance impact for
- your workloads.
-
-config STACKLEAK_RUNTIME_DISABLE
- bool "Allow runtime disabling of kernel stack erasing"
- depends on GCC_PLUGIN_STACKLEAK
- help
- This option provides 'stack_erasing' sysctl, which can be used in
- runtime to control kernel stack erasing for kernels built with
- CONFIG_GCC_PLUGIN_STACKLEAK.
-
config GCC_PLUGIN_ARM_SSP_PER_TASK
bool
depends on GCC_PLUGINS && ARM
diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
index 8223a8ab1a12..9942d9869864 100644
--- a/security/Kconfig.hardening
+++ b/security/Kconfig.hardening
@@ -89,6 +89,63 @@ config GCC_PLUGIN_STRUCTLEAK_VERBOSE
initialized. Since not all existing initializers are detected
by the plugin, this can produce false positive warnings.
+config GCC_PLUGIN_STACKLEAK
+ bool "Poison kernel stack before returning from syscalls"
+ depends on GCC_PLUGINS
+ depends on HAVE_ARCH_STACKLEAK
+ help
+ This option makes the kernel erase the kernel stack before
+ returning from system calls. This has the effect of leaving
+ the stack initialized to the poison value, which both reduces
+ the lifetime of any sensitive stack contents and reduces
+ potential for uninitialized stack variable exploits or information
+ exposures (it does not cover functions reaching the same stack
+ depth as prior functions during the same syscall). This blocks
+ most uninitialized stack variable attacks, with the performance
+ impact being driven by the depth of the stack usage, rather than
+ the function calling complexity.
+
+ The performance impact on a single CPU system kernel compilation
+ sees a 1% slowdown, other systems and workloads may vary and you
+ are advised to test this feature on your expected workload before
+ deploying it.
+
+ This plugin was ported from grsecurity/PaX. More information at:
+ * https://grsecurity.net/
+ * https://pax.grsecurity.net/
+
+config STACKLEAK_TRACK_MIN_SIZE
+ int "Minimum stack frame size of functions tracked by STACKLEAK"
+ default 100
+ range 0 4096
+ depends on GCC_PLUGIN_STACKLEAK
+ help
+ The STACKLEAK gcc plugin instruments the kernel code for tracking
+ the lowest border of the kernel stack (and for some other purposes).
+ It inserts the stackleak_track_stack() call for the functions with
+ a stack frame size greater than or equal to this parameter.
+ If unsure, leave the default value 100.
+
+config STACKLEAK_METRICS
+ bool "Show STACKLEAK metrics in the /proc file system"
+ depends on GCC_PLUGIN_STACKLEAK
+ depends on PROC_FS
+ help
+ If this is set, STACKLEAK metrics for every task are available in
+ the /proc file system. In particular, /proc/<pid>/stack_depth
+ shows the maximum kernel stack consumption for the current and
+ previous syscalls. Although this information is not precise, it
+ can be useful for estimating the STACKLEAK performance impact for
+ your workloads.
+
+config STACKLEAK_RUNTIME_DISABLE
+ bool "Allow runtime disabling of kernel stack erasing"
+ depends on GCC_PLUGIN_STACKLEAK
+ help
+ This option provides 'stack_erasing' sysctl, which can be used in
+ runtime to control kernel stack erasing for kernels built with
+ CONFIG_GCC_PLUGIN_STACKLEAK.
+
endmenu
endmenu
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v4 2/3] initmem: introduce CONFIG_INIT_ALL_HEAP
From: Kees Cook @ 2019-04-10 16:09 UTC (permalink / raw)
To: Alexander Potapenko
Cc: Masahiro Yamada, James Morris, Serge E. Hallyn,
linux-security-module, linux-kbuild, Nick Desaulniers,
Kostya Serebryany, Dmitry Vyukov, Kees Cook, Sandeep Patil,
Laura Abbott, Kernel Hardening
In-Reply-To: <20190410131726.250295-3-glider@google.com>
On Wed, Apr 10, 2019 at 6:18 AM Alexander Potapenko <glider@google.com> wrote:
>
> This config option adds the possibility to initialize newly allocated
> pages and heap objects with a 0xAA pattern.
> There's already a number of places where allocations are initialized
> based on the presence of __GFP_ZERO flag. We just change this code so
> that under CONFIG_INIT_ALL_HEAP these allocations are always initialized
> with either 0x00 or 0xAA depending on the __GFP_ZERO.
Why not just make __GFP_ZERO unconditional instead? This looks like
it'd be simpler and not need arch-specific implementation?
--
Kees Cook
^ permalink raw reply
* Re: [PATCH 00/59] LSM: Module stacking for AppArmor
From: Casey Schaufler @ 2019-04-10 15:36 UTC (permalink / raw)
To: Stephen Smalley
Cc: Schaufler, Casey, James Morris, linux-security-module, selinux,
casey
In-Reply-To: <CAB9W1A2yUNcdsJbXC_n8xhmnQBcZvHkFzHtk1jBeTS1+cv_VVw@mail.gmail.com>
On 4/10/2019 5:52 AM, Stephen Smalley wrote:
> On Tue, Apr 9, 2019 at 5:40 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> This patchset provides the changes required for
>> the AppArmor security module to stack safely with
>> "exclusive" security modules, those being SELinux and
>> Smack.
> What's the use case? Who would use such support?
A device uses a Smack three domain policy for system
protection. It Uses AppArmor policy to maintain application
isolation.
-------------------------------------------------------------------
| Smack floor domain |
-------------------------------------------------------------------
| Smack System domain |
-------------------------------------------------------------------
| Smack User domain |
| ---------- ---------- --------- ---------- ---------- |
| |AppArmor| |AppArmor| |AppArmor| |AppArmor| |AppArmor| |
| | Fred | | Wilma | |Barney | | Betty | | Dino | |
| ---------- ---------- ---------- ---------- ---------- |
-------------------------------------------------------------------
Each of the security modules is used in the way it was designed. Neither
has to be stretched beyond its original goals. Yes, you can implement the
system using either Smack or AppArmor (or maybe even SELinux) but by using
each for what it is best at you make it much easier.
^ permalink raw reply
* Re: [PATCH v5 1/2] LSM: SafeSetID: gate setgid transitions
From: Micah Morton @ 2019-04-10 15:14 UTC (permalink / raw)
To: Casey Schaufler
Cc: James Morris, Serge E. Hallyn, Kees Cook, Stephen Smalley,
linux-security-module
In-Reply-To: <ac7638c0-911d-5c0c-014d-730e763454f3@schaufler-ca.com>
Lets hold off on merging this for now. We have some fixes that will be
going in for the existing LSM code and we can circle back to this once
those have been merged.
On Fri, Mar 29, 2019 at 12:44 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 3/29/2019 11:06 AM, James Morris wrote:
> > On Tue, 5 Mar 2019, mortonm@chromium.org wrote:
> >
> >> From: Micah Morton <mortonm@chromium.org>
> >>
> >> This patch generalizes the 'task_fix_setuid' LSM hook to enable hooking
> >> setgid transitions as well as setuid transitions. The hook is renamed to
> >> 'task_fix_setid'. The patch introduces calls to this hook from the
> >> setgid functions in kernel/sys.c. This will allow the SafeSetID LSM to
> >> govern setgid transitions in addition to setuid transitions. This patch
> >> also makes sure the setgid functions in kernel/sys.c call
> >> security_capable_setid rather than the ordinary security_capable
> >> function, so that the security_capable hook in the SafeSetID LSM knows
> >> it is being invoked from a setid function.
> >>
> >> Signed-off-by: Micah Morton <mortonm@chromium.org>
> > Wondering if there are any further comments or reviews for this before it
> > is merged?
>
> My comments have been addressed.
>
>
^ permalink raw reply
* Re: [PATCH 58/59] LSM: Specify which LSM to display with /proc/self/attr/display
From: Stephen Smalley @ 2019-04-10 14:09 UTC (permalink / raw)
To: Casey Schaufler
Cc: Schaufler, Casey, James Morris, linux-security-module, selinux
In-Reply-To: <CAB9W1A0ZVUHAtAEhYw+HSyOnRdekg=6WQ2RP8PqNAirmDxBkZw@mail.gmail.com>
On Wed, Apr 10, 2019 at 8:43 AM Stephen Smalley
<stephen.smalley@gmail.com> wrote:
>
> On Tue, Apr 9, 2019 at 5:42 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >
> > Create a new entry "display" in /proc/.../attr for controlling
> > which LSM security information is displayed for a process.
> > The name of an active LSM that supplies hooks for human readable
> > data may be written to "display" to set the value. The name of
> > the LSM currently in use can be read from "display".
> >
> > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> > ---
> > fs/proc/base.c | 1 +
> > security/security.c | 123 ++++++++++++++++++++++++++++++++++++++++++--
> > 2 files changed, 121 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/proc/base.c b/fs/proc/base.c
> > index ddef482f1334..7bf70e041315 100644
> > --- a/fs/proc/base.c
> > +++ b/fs/proc/base.c
> > @@ -2618,6 +2618,7 @@ static const struct pid_entry attr_dir_stuff[] = {
> > ATTR(NULL, "fscreate", 0666),
> > ATTR(NULL, "keycreate", 0666),
> > ATTR(NULL, "sockcreate", 0666),
> > + ATTR(NULL, "display", 0666),
> > #ifdef CONFIG_SECURITY_SMACK
> > DIR("smack", 0555,
> > proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops),
> > diff --git a/security/security.c b/security/security.c
> > index 29149db3f78a..6e304aa796f9 100644
> > --- a/security/security.c
> > +++ b/security/security.c
> > @@ -47,9 +47,13 @@ static struct kmem_cache *lsm_inode_cache;
> >
> > char *lsm_names;
> >
> > -/* Socket blobs include infrastructure managed data */
> > +/*
> > + * Socket blobs include infrastructure managed data
> > + * Cred blobs include context display instructions
> > + */
> > static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init = {
> > .lbs_sock = sizeof(struct lsm_export),
> > + .lbs_cred = sizeof(struct lsm_one_hooks),
> > };
> >
> > /**
> > @@ -751,7 +755,10 @@ int lsm_superblock_alloc(struct super_block *sb)
> >
> > #define call_one_int_hook(FUNC, IRC, ...) ({ \
> > int RC = IRC; \
> > - if (lsm_base_one.FUNC.FUNC) \
> > + struct lsm_one_hooks *LOH = current_cred()->security; \
> > + if (LOH->FUNC.FUNC) \
> > + RC = LOH->FUNC.FUNC(__VA_ARGS__); \
> > + else if (LOH->lsm == NULL && lsm_base_one.FUNC.FUNC) \
> > RC = lsm_base_one.FUNC.FUNC(__VA_ARGS__); \
> > RC; \
> > })
> > @@ -1617,6 +1624,7 @@ int security_cred_alloc_blank(struct cred *cred, gfp_t gfp)
> >
> > void security_cred_free(struct cred *cred)
> > {
> > + struct lsm_one_hooks *loh;
> > /*
> > * There is a failure case in prepare_creds() that
> > * may result in a call here with ->security being NULL.
> > @@ -1626,26 +1634,44 @@ void security_cred_free(struct cred *cred)
> >
> > call_void_hook(cred_free, cred);
> >
> > + loh = cred->security;
> > + kfree(loh->lsm);
> > kfree(cred->security);
> > cred->security = NULL;
> > }
> >
> > +static int copy_loh(struct lsm_one_hooks *new, struct lsm_one_hooks *old,
> > + gfp_t gfp)
> > +{
> > + *new = *old;
> > + if (old->lsm) {
> > + new->lsm = kstrdup(old->lsm, gfp);
> > + if (unlikely(new->lsm == NULL))
> > + return -ENOMEM;
> > + }
> > + return 0;
> > +}
> > +
> > int security_prepare_creds(struct cred *new, const struct cred *old, gfp_t gfp)
> > {
> > int rc = lsm_cred_alloc(new, gfp);
> >
> > - if (rc)
> > + if (unlikely(rc))
> > return rc;
> >
> > rc = call_int_hook(cred_prepare, 0, new, old, gfp);
> > if (unlikely(rc))
> > security_cred_free(new);
> > + else
> > + rc = copy_loh(new->security, old->security, gfp);
> > +
> > return rc;
> > }
> >
> > void security_transfer_creds(struct cred *new, const struct cred *old)
> > {
> > call_void_hook(cred_transfer, new, old);
> > + WARN_ON(copy_loh(new->security, old->security, GFP_KERNEL));
> > }
> >
> > void security_cred_getsecid(const struct cred *c, struct lsm_export *l)
> > @@ -1960,10 +1986,28 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
> > char **value)
> > {
> > struct security_hook_list *hp;
> > + struct lsm_one_hooks *loh = current_cred()->security;
> > + char *s;
> > +
> > + if (!strcmp(name, "display")) {
> > + if (loh->lsm)
> > + s = loh->lsm;
> > + else if (lsm_base_one.lsm)
> > + s = lsm_base_one.lsm;
> > + else
> > + return -EINVAL;
> > +
> > + *value = kstrdup(s, GFP_KERNEL);
> > + if (*value)
> > + return strlen(s);
> > + return -ENOMEM;
> > + }
> >
> > hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) {
> > if (lsm != NULL && strcmp(lsm, hp->lsm))
> > continue;
> > + if (lsm == NULL && loh->lsm && strcmp(loh->lsm, hp->lsm))
> > + continue;
> > return hp->hook.getprocattr(p, name, value);
> > }
> > return -EINVAL;
> > @@ -1973,10 +2017,83 @@ int security_setprocattr(const char *lsm, const char *name, void *value,
> > size_t size)
> > {
> > struct security_hook_list *hp;
> > + struct lsm_one_hooks *loh = current_cred()->security;
> > + bool found = false;
> > + char *s;
> > +
> > + /*
> > + * End the passed name at a newline.
> > + */
> > + s = strnchr(value, size, '\n');
> > + if (s)
> > + *s = '\0';
> > +
> > + if (!strcmp(name, "display")) {
> > + union security_list_options secid_to_secctx;
> > + union security_list_options secctx_to_secid;
> > + union security_list_options socket_getpeersec_stream;
> > +
> > + if (size == 0 || size >= 100)
> > + return -EINVAL;
> > +
> > + secid_to_secctx.secid_to_secctx = NULL;
> > + hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx,
> > + list) {
> > + if (size >= strlen(hp->lsm) &&
> > + !strncmp(value, hp->lsm, size)) {
> > + secid_to_secctx = hp->hook;
> > + found = true;
> > + break;
> > + }
> > + }
> > + secctx_to_secid.secctx_to_secid = NULL;
> > + hlist_for_each_entry(hp, &security_hook_heads.secctx_to_secid,
> > + list) {
> > + if (size >= strlen(hp->lsm) &&
> > + !strncmp(value, hp->lsm, size)) {
> > + secctx_to_secid = hp->hook;
> > + found = true;
> > + break;
> > + }
> > + }
> > + socket_getpeersec_stream.socket_getpeersec_stream = NULL;
> > + hlist_for_each_entry(hp,
> > + &security_hook_heads.socket_getpeersec_stream,
> > + list) {
> > + if (size >= strlen(hp->lsm) &&
> > + !strncmp(value, hp->lsm, size)) {
> > + socket_getpeersec_stream = hp->hook;
> > + found = true;
> > + break;
> > + }
> > + }
> > + if (!found)
> > + return -EINVAL;
> > +
> > + /*
> > + * The named lsm is active and supplies one or more
> > + * of the relevant hooks. Switch to it.
> > + */
> > + s = kmemdup(value, size + 1, GFP_KERNEL);
> > + if (s == NULL)
> > + return -ENOMEM;
> > + s[size] = '\0';
> > +
> > + if (loh->lsm)
> > + kfree(loh->lsm);
> > + loh->lsm = s;
> > + loh->secid_to_secctx = secid_to_secctx;
> > + loh->secctx_to_secid = secctx_to_secid;
> > + loh->socket_getpeersec_stream = socket_getpeersec_stream;
>
> You can't just write to the cred security blob like this; it is a
> shared data structure, not per-task.
To be clear, you either need to perform a new = prepare_creds(); /*
modify new->security as desired */; commit_creds(new); sequence here,
or use the task security blob instead of the cred security blob. The
latter seems more amenable to your needs.
>
> > +
> > + return size;
> > + }
> >
> > hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) {
> > if (lsm != NULL && strcmp(lsm, hp->lsm))
> > continue;
> > + if (lsm == NULL && loh->lsm && strcmp(loh->lsm, hp->lsm))
> > + continue;
> > return hp->hook.setprocattr(name, value, size);
> > }
> > return -EINVAL;
> > --
> > 2.19.1
> >
^ permalink raw reply
* [PATCH 3/3] net: make sk_prot_alloc() work with CONFIG_INIT_ALL_HEAP
From: Alexander Potapenko @ 2019-04-10 13:17 UTC (permalink / raw)
To: yamada.masahiro, jmorris, serge
Cc: linux-security-module, linux-kbuild, ndesaulniers, kcc, dvyukov,
keescook, sspatil, labbott, kernel-hardening
In-Reply-To: <20190410131726.250295-1-glider@google.com>
Rename sk_prot_clear_nulls() to sk_prot_clear() and introduce an extra
init_byte parameter to be passed to memset() when initializing struct sock.
In the case CONFIG_INIT_ALL_HEAP is on, initialize newly created struct
sock with 0xAA.
Signed-off-by: Alexander Potapenko <glider@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Kostya Serebryany <kcc@google.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Sandeep Patil <sspatil@android.com>
Cc: Laura Abbott <labbott@redhat.com>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Jann Horn <jannh@google.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: linux-security-module@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: linux-kbuild@vger.kernel.org
Cc: kernel-hardening@lists.openwall.com
---
include/net/sock.h | 8 ++++----
net/core/sock.c | 5 +++--
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index 8de5ee258b93..a49c1f1c71c1 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1044,13 +1044,13 @@ struct module;
/*
* caches using SLAB_TYPESAFE_BY_RCU should let .next pointer from nulls nodes
- * un-modified. Special care is taken when initializing object to zero.
+ * un-modified. Special care is taken when initializing object.
*/
-static inline void sk_prot_clear_nulls(struct sock *sk, int size)
+static inline void sk_prot_clear(struct sock *sk, int size, int init_byte)
{
if (offsetof(struct sock, sk_node.next) != 0)
- memset(sk, 0, offsetof(struct sock, sk_node.next));
- memset(&sk->sk_node.pprev, 0,
+ memset(sk, init_byte, offsetof(struct sock, sk_node.next));
+ memset(&sk->sk_node.pprev, init_byte,
size - offsetof(struct sock, sk_node.pprev));
}
diff --git a/net/core/sock.c b/net/core/sock.c
index 782343bb925b..1ad855e99512 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1601,8 +1601,9 @@ static struct sock *sk_prot_alloc(struct proto *prot, gfp_t priority,
sk = kmem_cache_alloc(slab, priority & ~__GFP_ZERO);
if (!sk)
return sk;
- if (priority & __GFP_ZERO)
- sk_prot_clear_nulls(sk, prot->obj_size);
+ if (GFP_INIT_ALWAYS_ON || (priority & __GFP_ZERO))
+ sk_prot_clear(sk, prot->obj_size,
+ INITMEM_FILL_BYTE(priority));
} else
sk = kmalloc(prot->obj_size, priority);
--
2.21.0.392.gf8f6787159e-goog
^ permalink raw reply related
* [PATCH v4 2/3] initmem: introduce CONFIG_INIT_ALL_HEAP
From: Alexander Potapenko @ 2019-04-10 13:17 UTC (permalink / raw)
To: yamada.masahiro, jmorris, serge
Cc: linux-security-module, linux-kbuild, ndesaulniers, kcc, dvyukov,
keescook, sspatil, labbott, kernel-hardening
In-Reply-To: <20190410131726.250295-1-glider@google.com>
This config option adds the possibility to initialize newly allocated
pages and heap objects with a 0xAA pattern.
There's already a number of places where allocations are initialized
based on the presence of __GFP_ZERO flag. We just change this code so
that under CONFIG_INIT_ALL_HEAP these allocations are always initialized
with either 0x00 or 0xAA depending on the __GFP_ZERO.
No performance optimizations are done at the moment to reduce double
initialization of memory regions.
Signed-off-by: Alexander Potapenko <glider@google.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Kostya Serebryany <kcc@google.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Sandeep Patil <sspatil@android.com>
Cc: Laura Abbott <labbott@redhat.com>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Jann Horn <jannh@google.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: linux-security-module@vger.kernel.org
Cc: linux-kbuild@vger.kernel.org
Cc: kernel-hardening@lists.openwall.com
---
v3:
- addressed comments by Masahiro Yamada (Kconfig fixes)
v4:
- addressed Randy Dunlap's comments: remove redundant "depends on"
- replaced code wiring SLUB_DEBUG and page poisoning with a more
lightweight implementation (Laura Abbott mentioned turning on
debugging has serious performance issues)
---
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/page.h | 1 +
arch/x86/Kconfig | 1 +
arch/x86/include/asm/page_64.h | 10 ++++++++++
arch/x86/lib/clear_page_64.S | 24 ++++++++++++++++++++++++
drivers/infiniband/core/uverbs_ioctl.c | 4 ++--
include/linux/gfp.h | 10 ++++++++++
include/linux/highmem.h | 8 ++++++++
kernel/kexec_core.c | 8 ++++++--
mm/dmapool.c | 4 ++--
mm/page_alloc.c | 9 +++++++--
mm/slab.c | 19 +++++++++++++------
mm/slub.c | 12 ++++++++----
security/Kconfig.initmem | 15 +++++++++++++++
14 files changed, 108 insertions(+), 18 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 7e34b9eba5de..3548235752f6 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -110,6 +110,7 @@ config ARM64
select HAVE_ARCH_AUDITSYSCALL
select HAVE_ARCH_BITREVERSE
select HAVE_ARCH_HUGE_VMAP
+ select HAVE_ARCH_INIT_ALL_HEAP
select HAVE_ARCH_JUMP_LABEL
select HAVE_ARCH_JUMP_LABEL_RELATIVE
select HAVE_ARCH_KASAN if !(ARM64_16K_PAGES && ARM64_VA_BITS_48)
diff --git a/arch/arm64/include/asm/page.h b/arch/arm64/include/asm/page.h
index c88a3cb117a1..a7bea266c16d 100644
--- a/arch/arm64/include/asm/page.h
+++ b/arch/arm64/include/asm/page.h
@@ -33,6 +33,7 @@ extern void copy_page(void *to, const void *from);
extern void clear_page(void *to);
#define clear_user_page(addr,vaddr,pg) __cpu_clear_user_page(addr, vaddr)
+#define clear_page_pattern(addr) __memset((addr), GFP_POISON_BYTE, PAGE_SIZE)
#define copy_user_page(to,from,vaddr,pg) __cpu_copy_user_page(to, from, vaddr)
typedef struct page *pgtable_t;
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 5ad92419be19..2058beaf3582 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -116,6 +116,7 @@ config X86
select HAVE_ALIGNED_STRUCT_PAGE if SLUB
select HAVE_ARCH_AUDITSYSCALL
select HAVE_ARCH_HUGE_VMAP if X86_64 || X86_PAE
+ select HAVE_ARCH_INIT_ALL_HEAP
select HAVE_ARCH_JUMP_LABEL
select HAVE_ARCH_JUMP_LABEL_RELATIVE
select HAVE_ARCH_KASAN if X86_64
diff --git a/arch/x86/include/asm/page_64.h b/arch/x86/include/asm/page_64.h
index 939b1cff4a7b..8e6e0c00f8c8 100644
--- a/arch/x86/include/asm/page_64.h
+++ b/arch/x86/include/asm/page_64.h
@@ -54,6 +54,16 @@ static inline void clear_page(void *page)
: "cc", "memory", "rax", "rcx");
}
+#ifdef CONFIG_INIT_ALL_HEAP
+
+void clear_page_pattern_orig(void *page);
+// FIXME: add rep and erms.
+static inline void clear_page_pattern(void *page)
+{
+ clear_page_pattern_orig(page);
+}
+#endif
+
void copy_page(void *to, void *from);
#endif /* !__ASSEMBLY__ */
diff --git a/arch/x86/lib/clear_page_64.S b/arch/x86/lib/clear_page_64.S
index 88acd349911b..b64dab97d4ba 100644
--- a/arch/x86/lib/clear_page_64.S
+++ b/arch/x86/lib/clear_page_64.S
@@ -49,3 +49,27 @@ ENTRY(clear_page_erms)
ret
ENDPROC(clear_page_erms)
EXPORT_SYMBOL_GPL(clear_page_erms)
+
+#ifdef CONFIG_INIT_ALL_HEAP
+ENTRY(clear_page_pattern_orig)
+ movq $0xAAAAAAAAAAAAAAAA,%rax
+ movl $4096/64,%ecx
+ .p2align 4
+.Lloop_pat:
+ decl %ecx
+ movq %rax,(%rdi)
+ PUT(1)
+ PUT(2)
+ PUT(3)
+ PUT(4)
+ PUT(5)
+ PUT(6)
+ PUT(7)
+ leaq 64(%rdi),%rdi
+ jnz .Lloop_pat
+ nop
+ ret
+ENDPROC(clear_page_pattern_orig)
+EXPORT_SYMBOL_GPL(clear_page_pattern_orig)
+#endif
+
diff --git a/drivers/infiniband/core/uverbs_ioctl.c b/drivers/infiniband/core/uverbs_ioctl.c
index e1379949e663..1ad0eb28e651 100644
--- a/drivers/infiniband/core/uverbs_ioctl.c
+++ b/drivers/infiniband/core/uverbs_ioctl.c
@@ -127,8 +127,8 @@ __malloc void *_uverbs_alloc(struct uverbs_attr_bundle *bundle, size_t size,
res = (void *)pbundle->internal_buffer + pbundle->internal_used;
pbundle->internal_used =
ALIGN(new_used, sizeof(*pbundle->internal_buffer));
- if (flags & __GFP_ZERO)
- memset(res, 0, size);
+ if (GFP_INIT_ALWAYS_ON || (flags & __GFP_ZERO))
+ memset(res, INITMEM_FILL_BYTE(flags), size);
return res;
}
EXPORT_SYMBOL(_uverbs_alloc);
diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index fdab7de7490d..a0016357a91a 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -213,6 +213,16 @@ struct vm_area_struct;
#define __GFP_COMP ((__force gfp_t)___GFP_COMP)
#define __GFP_ZERO ((__force gfp_t)___GFP_ZERO)
+#define GFP_POISON_BYTE (0xAA)
+#ifdef CONFIG_INIT_ALL_HEAP
+#define GFP_INIT_ALWAYS_ON (1)
+#define INITMEM_FILL_BYTE(gfp_flags) \
+ (((gfp_flags) & __GFP_ZERO) ? (0) : GFP_POISON_BYTE)
+#else
+#define GFP_INIT_ALWAYS_ON (0)
+#define INITMEM_FILL_BYTE(gfp_flags) (0)
+#endif
+
/* Disable lockdep for GFP context tracking */
#define __GFP_NOLOCKDEP ((__force gfp_t)___GFP_NOLOCKDEP)
diff --git a/include/linux/highmem.h b/include/linux/highmem.h
index ea5cdbd8c2c3..b48a04c7b046 100644
--- a/include/linux/highmem.h
+++ b/include/linux/highmem.h
@@ -215,6 +215,14 @@ static inline void clear_highpage(struct page *page)
kunmap_atomic(kaddr);
}
+static inline void clear_highpage_pattern(struct page *page)
+{
+ void *kaddr = kmap_atomic(page);
+
+ clear_page_pattern(kaddr);
+ kunmap_atomic(kaddr);
+}
+
static inline void zero_user_segments(struct page *page,
unsigned start1, unsigned end1,
unsigned start2, unsigned end2)
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index d7140447be75..63104a71cf60 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -315,9 +315,13 @@ static struct page *kimage_alloc_pages(gfp_t gfp_mask, unsigned int order)
arch_kexec_post_alloc_pages(page_address(pages), count,
gfp_mask);
- if (gfp_mask & __GFP_ZERO)
+ if (GFP_INIT_ALWAYS_ON || (gfp_mask & __GFP_ZERO)) {
for (i = 0; i < count; i++)
- clear_highpage(pages + i);
+ if (!INITMEM_FILL_BYTE(gfp_mask))
+ clear_highpage(pages + i);
+ else
+ clear_highpage_pattern(pages + i);
+ }
}
return pages;
diff --git a/mm/dmapool.c b/mm/dmapool.c
index 76a160083506..f36dd9065f2b 100644
--- a/mm/dmapool.c
+++ b/mm/dmapool.c
@@ -381,8 +381,8 @@ void *dma_pool_alloc(struct dma_pool *pool, gfp_t mem_flags,
#endif
spin_unlock_irqrestore(&pool->lock, flags);
- if (mem_flags & __GFP_ZERO)
- memset(retval, 0, pool->size);
+ if (GFP_INIT_ALWAYS_ON || (mem_flags & __GFP_ZERO))
+ memset(retval, INITMEM_FILL_BYTE(mem_flags), pool->size);
return retval;
}
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index d96ca5bc555b..c8477db5ac94 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2014,9 +2014,14 @@ static void prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags
post_alloc_hook(page, order, gfp_flags);
- if (!free_pages_prezeroed() && (gfp_flags & __GFP_ZERO))
+ if (!free_pages_prezeroed() && (GFP_INIT_ALWAYS_ON ||
+ (gfp_flags & __GFP_ZERO))) {
for (i = 0; i < (1 << order); i++)
- clear_highpage(page + i);
+ if (!INITMEM_FILL_BYTE(gfp_flags))
+ clear_highpage(page + i);
+ else
+ clear_highpage_pattern(page + i);
+ }
if (order && (gfp_flags & __GFP_COMP))
prep_compound_page(page, order);
diff --git a/mm/slab.c b/mm/slab.c
index 47a380a486ee..6d30db845dbd 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -3331,8 +3331,11 @@ slab_alloc_node(struct kmem_cache *cachep, gfp_t flags, int nodeid,
local_irq_restore(save_flags);
ptr = cache_alloc_debugcheck_after(cachep, flags, ptr, caller);
- if (unlikely(flags & __GFP_ZERO) && ptr)
- memset(ptr, 0, cachep->object_size);
+ if ((unlikely(flags & __GFP_ZERO) ||
+ (GFP_INIT_ALWAYS_ON && !cachep->ctor &&
+ !(cachep->flags & SLAB_TYPESAFE_BY_RCU))) &&
+ ptr)
+ memset(ptr, INITMEM_FILL_BYTE(flags), cachep->object_size);
slab_post_alloc_hook(cachep, flags, 1, &ptr);
return ptr;
@@ -3388,8 +3391,10 @@ slab_alloc(struct kmem_cache *cachep, gfp_t flags, unsigned long caller)
objp = cache_alloc_debugcheck_after(cachep, flags, objp, caller);
prefetchw(objp);
- if (unlikely(flags & __GFP_ZERO) && objp)
- memset(objp, 0, cachep->object_size);
+ if (((GFP_INIT_ALWAYS_ON && !cachep->ctor &&
+ !(cachep->flags & SLAB_TYPESAFE_BY_RCU)) ||
+ unlikely(flags & __GFP_ZERO)) && objp)
+ memset(objp, INITMEM_FILL_BYTE(flags), cachep->object_size);
slab_post_alloc_hook(cachep, flags, 1, &objp);
return objp;
@@ -3596,9 +3601,11 @@ int kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t size,
cache_alloc_debugcheck_after_bulk(s, flags, size, p, _RET_IP_);
/* Clear memory outside IRQ disabled section */
- if (unlikely(flags & __GFP_ZERO))
+ if ((GFP_INIT_ALWAYS_ON && !s->ctor &&
+ !(flags & SLAB_TYPESAFE_BY_RCU)) ||
+ unlikely(flags & __GFP_ZERO))
for (i = 0; i < size; i++)
- memset(p[i], 0, s->object_size);
+ memset(p[i], INITMEM_FILL_BYTE(flags), s->object_size);
slab_post_alloc_hook(s, flags, size, p);
/* FIXME: Trace call missing. Christoph would like a bulk variant */
diff --git a/mm/slub.c b/mm/slub.c
index d30ede89f4a6..f89c39b01578 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2750,8 +2750,10 @@ static __always_inline void *slab_alloc_node(struct kmem_cache *s,
stat(s, ALLOC_FASTPATH);
}
- if (unlikely(gfpflags & __GFP_ZERO) && object)
- memset(object, 0, s->object_size);
+ if (((GFP_INIT_ALWAYS_ON && !s->ctor &&
+ !(s->flags & SLAB_TYPESAFE_BY_RCU)) ||
+ unlikely(gfpflags & __GFP_ZERO)) && object)
+ memset(object, INITMEM_FILL_BYTE(gfpflags), s->object_size);
slab_post_alloc_hook(s, gfpflags, 1, &object);
@@ -3172,11 +3174,13 @@ int kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t size,
local_irq_enable();
/* Clear memory outside IRQ disabled fastpath loop */
- if (unlikely(flags & __GFP_ZERO)) {
+ if ((GFP_INIT_ALWAYS_ON && !s->ctor &&
+ !(s->flags & SLAB_TYPESAFE_BY_RCU)) ||
+ unlikely(flags & __GFP_ZERO)) {
int j;
for (j = 0; j < i; j++)
- memset(p[j], 0, s->object_size);
+ memset(p[j], INITMEM_FILL_BYTE(flags), s->object_size);
}
/* memcg and kmem_cache debug support */
diff --git a/security/Kconfig.initmem b/security/Kconfig.initmem
index cdad1e185b10..93bc6fe32536 100644
--- a/security/Kconfig.initmem
+++ b/security/Kconfig.initmem
@@ -1,5 +1,8 @@
menu "Initialize all memory"
+config HAVE_ARCH_INIT_ALL_HEAP
+ bool
+
config CC_HAS_AUTO_VAR_INIT
def_bool $(cc-option,-ftrivial-auto-var-init=pattern)
@@ -24,5 +27,17 @@ config INIT_ALL_STACK
Initialize uninitialized stack data with a fixed pattern
(0x00 in GCC, 0xAA in Clang).
+if HAVE_ARCH_INIT_ALL_HEAP
+
+config INIT_ALL_HEAP
+ bool "Initialize all heap"
+ depends on SLAB || SLUB
+ default y
+ help
+ Initialize uninitialized pages and SL[AOU]B allocations with 0xAA
+ pattern.
+
+endif # HAVE_ARCH_INIT_ALL_HEAP
+
endif # INIT_ALL_MEMORY
endmenu
--
2.21.0.392.gf8f6787159e-goog
^ permalink raw reply related
* [PATCH v4 1/3] initmem: introduce CONFIG_INIT_ALL_MEMORY and CONFIG_INIT_ALL_STACK
From: Alexander Potapenko @ 2019-04-10 13:17 UTC (permalink / raw)
To: yamada.masahiro, jmorris, serge
Cc: linux-security-module, linux-kbuild, ndesaulniers, kcc, dvyukov,
keescook, sspatil, labbott, kernel-hardening
In-Reply-To: <20190410131726.250295-1-glider@google.com>
CONFIG_INIT_ALL_MEMORY is going to be an umbrella config for options
that force heap and stack initialization.
The rationale behind doing so is to reduce the severity of bugs caused
by using uninitialized memory.
CONFIG_INIT_ALL_STACK turns on stack initialization based on
-ftrivial-auto-var-init in Clang builds and on
-fplugin-arg-structleak_plugin-byref-all in GCC builds.
-ftrivial-auto-var-init is a Clang flag that provides trivial
initializers for uninitialized local variables, variable fields and
padding.
It has three possible values:
pattern - uninitialized locals are filled with a fixed pattern
(mostly 0xAA on 64-bit platforms, see https://reviews.llvm.org/D54604
for more details) likely to cause crashes when uninitialized value is
used;
zero (it's still debated whether this flag makes it to the official
Clang release) - uninitialized locals are filled with zeroes;
uninitialized (default) - uninitialized locals are left intact.
The proposed config builds the kernel with
-ftrivial-auto-var-init=pattern.
Developers have the possibility to opt-out of this feature on a
per-variable basis by using __attribute__((uninitialized)).
For GCC builds, CONFIG_INIT_ALL_STACK is simply wired up to
CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL. No opt-out is possible at the
moment.
Signed-off-by: Alexander Potapenko <glider@google.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Kostya Serebryany <kcc@google.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Sandeep Patil <sspatil@android.com>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: linux-security-module@vger.kernel.org
Cc: linux-kbuild@vger.kernel.org
Cc: kernel-hardening@lists.openwall.com
---
v2:
- addressed Kees Cook's comments: added GCC support
v3: addressed Masahiro Yamada's comments:
- dropped per-file opt-out mechanism
- fixed GCC_PLUGINS dependencies
v4:
- addressed Randy Dunlap's comments: remove redundant "depends on"
- addressed Masahiro Yamada's comments: drop Makefile.initmem
---
Makefile | 10 ++++++++++
security/Kconfig | 1 +
security/Kconfig.initmem | 28 ++++++++++++++++++++++++++++
3 files changed, 39 insertions(+)
create mode 100644 security/Kconfig.initmem
diff --git a/Makefile b/Makefile
index 15c8251d4d5e..02f4b9df0102 100644
--- a/Makefile
+++ b/Makefile
@@ -727,6 +727,16 @@ KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare)
# See modpost pattern 2
KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,)
KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior)
+
+ifdef CONFIG_INIT_ALL_STACK
+# Clang's -ftrivial-auto-var-init=pattern flag initializes the
+# uninitialized parts of local variables (including fields and padding)
+# with a fixed pattern (0xAA in most cases).
+ifdef CONFIG_CC_HAS_AUTO_VAR_INIT
+KBUILD_CFLAGS += -ftrivial-auto-var-init=pattern
+endif
+endif
+
else
# These warnings generated too much noise in a regular build.
diff --git a/security/Kconfig b/security/Kconfig
index 353cfef71d4e..4d27437c2eb8 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -229,6 +229,7 @@ config STATIC_USERMODEHELPER_PATH
If you wish for all usermode helper programs to be disabled,
specify an empty string here (i.e. "").
+source "security/Kconfig.initmem"
source "security/selinux/Kconfig"
source "security/smack/Kconfig"
source "security/tomoyo/Kconfig"
diff --git a/security/Kconfig.initmem b/security/Kconfig.initmem
new file mode 100644
index 000000000000..cdad1e185b10
--- /dev/null
+++ b/security/Kconfig.initmem
@@ -0,0 +1,28 @@
+menu "Initialize all memory"
+
+config CC_HAS_AUTO_VAR_INIT
+ def_bool $(cc-option,-ftrivial-auto-var-init=pattern)
+
+config INIT_ALL_MEMORY
+ bool "Initialize all memory"
+ default n
+ help
+ Enforce memory initialization to mitigate infoleaks and make
+ the control-flow bugs depending on uninitialized values more
+ deterministic.
+
+if INIT_ALL_MEMORY
+
+config INIT_ALL_STACK
+ bool "Initialize all stack"
+ depends on CC_HAS_AUTO_VAR_INIT || (HAVE_GCC_PLUGINS && PLUGIN_HOSTCC != "")
+ select GCC_PLUGINS if !CC_HAS_AUTO_VAR_INIT
+ select GCC_PLUGIN_STRUCTLEAK if !CC_HAS_AUTO_VAR_INIT
+ select GCC_PLUGIN_STRUCTLEAK_BYREF_ALL if !CC_HAS_AUTO_VAR_INIT
+ default y
+ help
+ Initialize uninitialized stack data with a fixed pattern
+ (0x00 in GCC, 0xAA in Clang).
+
+endif # INIT_ALL_MEMORY
+endmenu
--
2.21.0.392.gf8f6787159e-goog
^ permalink raw reply related
* [PATCH v4 0/3] RFC: introduce CONFIG_INIT_ALL_MEMORY
From: Alexander Potapenko @ 2019-04-10 13:17 UTC (permalink / raw)
To: yamada.masahiro, jmorris, serge
Cc: linux-security-module, linux-kbuild, ndesaulniers, kcc, dvyukov,
keescook, sspatil, labbott, kernel-hardening
This patch is a part of a bigger initiative to allow initializing
heap/stack memory in the Linux kernels by default.
The rationale behind doing so is to reduce the severity of bugs caused
by using uninitialized memory.
Over the last two years KMSAN (https://github.com/google/kmsan/) has
found more than a hundred bugs running in a really moderate setup (orders
of magnitude less CPU/months than KASAN). Some of those bugs led to
information leaks if uninitialized memory was copied to the userspace,
other could cause DoS because of subverted control flow.
A lot more bugs remain uncovered, so we want to provide the distros and OS
vendors with a last resort measure to mitigate such bugs.
Our plan is to introduce configuration flags to force initialization of
stack and heap variables with a fixed pattern.
This is going to render information leaks inefficient (as we'll only leak
pattern data) and make uses of uninitialized values in conditions more
deterministic and discoverable.
The stack instrumentation part is based on Clang's -ftrivial-auto-var-init
(see https://reviews.llvm.org/D54604 ; there's also a GCC feature request
for a similar flag: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87210)
or GCC's -fplugin-arg-structleak_plugin-byref-all
The heap initialization part is compiler-agnostic and is done in the
places that previously checked for __GFP_ZERO to initialize the newly
allocated memory.
Alexander Potapenko (3):
initmem: introduce CONFIG_INIT_ALL_MEMORY and CONFIG_INIT_ALL_STACK
initmem: introduce CONFIG_INIT_ALL_HEAP
net: make sk_prot_alloc() work with CONFIG_INIT_ALL_HEAP
Makefile | 10 ++++++
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/page.h | 1 +
arch/x86/Kconfig | 1 +
arch/x86/include/asm/page_64.h | 10 ++++++
arch/x86/lib/clear_page_64.S | 24 ++++++++++++++
drivers/infiniband/core/uverbs_ioctl.c | 4 +--
include/linux/gfp.h | 10 ++++++
include/linux/highmem.h | 8 +++++
include/net/sock.h | 8 ++---
kernel/kexec_core.c | 8 +++--
mm/dmapool.c | 4 +--
mm/page_alloc.c | 9 ++++--
mm/slab.c | 19 ++++++++----
mm/slub.c | 12 ++++---
net/core/sock.c | 5 +--
security/Kconfig | 1 +
security/Kconfig.initmem | 43 ++++++++++++++++++++++++++
18 files changed, 154 insertions(+), 24 deletions(-)
create mode 100644 security/Kconfig.initmem
--
2.21.0.392.gf8f6787159e-goog
^ permalink raw reply
* Re: [PATCH 00/59] LSM: Module stacking for AppArmor
From: Stephen Smalley @ 2019-04-10 12:52 UTC (permalink / raw)
To: Casey Schaufler
Cc: Schaufler, Casey, James Morris, linux-security-module, selinux
In-Reply-To: <20190409213946.1667-1-casey@schaufler-ca.com>
On Tue, Apr 9, 2019 at 5:40 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> This patchset provides the changes required for
> the AppArmor security module to stack safely with
> "exclusive" security modules, those being SELinux and
> Smack.
What's the use case? Who would use such support?
>
> Performance: Using a kernel compile benchmark indicates
> a performance impact of 0.15% for a Fedora 29 system
> with SELinux. Adding AppArmor has an additional 0.20%
> impact. Fedora does not include an AppArmor profile.
>
> A new process attribute identifies which security module
> information should be reported by SO_PEERSEC and the
> /proc/.../attr/current interface. This is provided by
> /proc/.../attr/display. Writing the name of the security
> module desired to this interface will set which LSM hooks
> will be called for this information. The first security
> module providing the hooks will be used by default.
>
> The use of integer based security tokens (secids) is
> generally (but not completely) replaced by a structure
> lsm_export. The lsm_export structure can contain information
> for each of the security modules that export information
> outside the LSM layer.
>
> The LSM interfaces that provide "secctx" text strings
> have been changed to use a structure "lsm_context"
> instead of a pointer/length pair. In some cases the
> interfaces used a "char *" pointer and in others a
> "void *". This was necessary to ensure that the correct
> release mechanism for the text is used. It also makes
> many of the interfaces cleaner.
>
> The security module stacking issues around netlabel
> not addressed here as they are beyond what is required
> to stack AppArmor with either SELinux or Smack.
>
> git://github.com/cschaufler/lsm-stacking.git#stack-5.1-rc2-apparmor
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> drivers/android/binder.c | 25 ++-
> fs/kernfs/dir.c | 6 +-
> fs/kernfs/inode.c | 31 ++-
> fs/kernfs/kernfs-internal.h | 3 +-
> fs/nfs/inode.c | 13 +-
> fs/nfs/internal.h | 8 +-
> fs/nfs/nfs4proc.c | 17 +-
> fs/nfs/nfs4xdr.c | 16 +-
> fs/nfsd/nfs4proc.c | 8 +-
> fs/nfsd/nfs4xdr.c | 14 +-
> fs/nfsd/vfs.c | 7 +-
> fs/proc/base.c | 1 +
> include/linux/cred.h | 3 +-
> include/linux/lsm_hooks.h | 93 ++++----
> include/linux/nfs4.h | 8 +-
> include/linux/security.h | 137 ++++++++----
> include/net/netlabel.h | 10 +-
> include/net/scm.h | 14 +-
> kernel/audit.c | 43 ++--
> kernel/audit.h | 9 +-
> kernel/auditfilter.c | 6 +-
> kernel/auditsc.c | 77 ++++---
> kernel/cred.c | 15 +-
> net/ipv4/cipso_ipv4.c | 13 +-
> net/ipv4/ip_sockglue.c | 12 +-
> net/netfilter/nf_conntrack_netlink.c | 29 ++-
> net/netfilter/nf_conntrack_standalone.c | 16 +-
> net/netfilter/nfnetlink_queue.c | 38 ++--
> net/netfilter/nft_meta.c | 13 +-
> net/netfilter/xt_SECMARK.c | 14 +-
> net/netlabel/netlabel_kapi.c | 5 +-
> net/netlabel/netlabel_unlabeled.c | 101 +++++----
> net/netlabel/netlabel_unlabeled.h | 2 +-
> net/netlabel/netlabel_user.c | 13 +-
> net/netlabel/netlabel_user.h | 2 +-
> net/unix/af_unix.c | 11 +-
> security/apparmor/audit.c | 4 +-
> security/apparmor/include/audit.h | 2 +-
> security/apparmor/include/net.h | 6 +-
> security/apparmor/include/secid.h | 9 +-
> security/apparmor/lsm.c | 64 ++----
> security/apparmor/secid.c | 42 ++--
> security/integrity/ima/ima.h | 14 +-
> security/integrity/ima/ima_api.c | 9 +-
> security/integrity/ima/ima_appraise.c | 6 +-
> security/integrity/ima/ima_main.c | 34 +--
> security/integrity/ima/ima_policy.c | 19 +-
> security/security.c | 366 ++++++++++++++++++++++++++++----
> security/selinux/hooks.c | 259 +++++++++++-----------
> security/selinux/include/audit.h | 5 +-
> security/selinux/include/objsec.h | 42 +++-
> security/selinux/netlabel.c | 25 +--
> security/selinux/ss/services.c | 18 +-
> security/smack/smack.h | 18 ++
> security/smack/smack_lsm.c | 238 +++++++++++----------
> security/smack/smack_netfilter.c | 8 +-
> security/smack/smackfs.c | 12 +-
> 57 files changed, 1252 insertions(+), 781 deletions(-)
^ permalink raw reply
* Re: [PATCH 58/59] LSM: Specify which LSM to display with /proc/self/attr/display
From: Stephen Smalley @ 2019-04-10 12:43 UTC (permalink / raw)
To: Casey Schaufler
Cc: Schaufler, Casey, James Morris, linux-security-module, selinux
In-Reply-To: <20190409213946.1667-59-casey@schaufler-ca.com>
On Tue, Apr 9, 2019 at 5:42 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Create a new entry "display" in /proc/.../attr for controlling
> which LSM security information is displayed for a process.
> The name of an active LSM that supplies hooks for human readable
> data may be written to "display" to set the value. The name of
> the LSM currently in use can be read from "display".
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> fs/proc/base.c | 1 +
> security/security.c | 123 ++++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 121 insertions(+), 3 deletions(-)
>
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index ddef482f1334..7bf70e041315 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -2618,6 +2618,7 @@ static const struct pid_entry attr_dir_stuff[] = {
> ATTR(NULL, "fscreate", 0666),
> ATTR(NULL, "keycreate", 0666),
> ATTR(NULL, "sockcreate", 0666),
> + ATTR(NULL, "display", 0666),
> #ifdef CONFIG_SECURITY_SMACK
> DIR("smack", 0555,
> proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops),
> diff --git a/security/security.c b/security/security.c
> index 29149db3f78a..6e304aa796f9 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -47,9 +47,13 @@ static struct kmem_cache *lsm_inode_cache;
>
> char *lsm_names;
>
> -/* Socket blobs include infrastructure managed data */
> +/*
> + * Socket blobs include infrastructure managed data
> + * Cred blobs include context display instructions
> + */
> static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init = {
> .lbs_sock = sizeof(struct lsm_export),
> + .lbs_cred = sizeof(struct lsm_one_hooks),
> };
>
> /**
> @@ -751,7 +755,10 @@ int lsm_superblock_alloc(struct super_block *sb)
>
> #define call_one_int_hook(FUNC, IRC, ...) ({ \
> int RC = IRC; \
> - if (lsm_base_one.FUNC.FUNC) \
> + struct lsm_one_hooks *LOH = current_cred()->security; \
> + if (LOH->FUNC.FUNC) \
> + RC = LOH->FUNC.FUNC(__VA_ARGS__); \
> + else if (LOH->lsm == NULL && lsm_base_one.FUNC.FUNC) \
> RC = lsm_base_one.FUNC.FUNC(__VA_ARGS__); \
> RC; \
> })
> @@ -1617,6 +1624,7 @@ int security_cred_alloc_blank(struct cred *cred, gfp_t gfp)
>
> void security_cred_free(struct cred *cred)
> {
> + struct lsm_one_hooks *loh;
> /*
> * There is a failure case in prepare_creds() that
> * may result in a call here with ->security being NULL.
> @@ -1626,26 +1634,44 @@ void security_cred_free(struct cred *cred)
>
> call_void_hook(cred_free, cred);
>
> + loh = cred->security;
> + kfree(loh->lsm);
> kfree(cred->security);
> cred->security = NULL;
> }
>
> +static int copy_loh(struct lsm_one_hooks *new, struct lsm_one_hooks *old,
> + gfp_t gfp)
> +{
> + *new = *old;
> + if (old->lsm) {
> + new->lsm = kstrdup(old->lsm, gfp);
> + if (unlikely(new->lsm == NULL))
> + return -ENOMEM;
> + }
> + return 0;
> +}
> +
> int security_prepare_creds(struct cred *new, const struct cred *old, gfp_t gfp)
> {
> int rc = lsm_cred_alloc(new, gfp);
>
> - if (rc)
> + if (unlikely(rc))
> return rc;
>
> rc = call_int_hook(cred_prepare, 0, new, old, gfp);
> if (unlikely(rc))
> security_cred_free(new);
> + else
> + rc = copy_loh(new->security, old->security, gfp);
> +
> return rc;
> }
>
> void security_transfer_creds(struct cred *new, const struct cred *old)
> {
> call_void_hook(cred_transfer, new, old);
> + WARN_ON(copy_loh(new->security, old->security, GFP_KERNEL));
> }
>
> void security_cred_getsecid(const struct cred *c, struct lsm_export *l)
> @@ -1960,10 +1986,28 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
> char **value)
> {
> struct security_hook_list *hp;
> + struct lsm_one_hooks *loh = current_cred()->security;
> + char *s;
> +
> + if (!strcmp(name, "display")) {
> + if (loh->lsm)
> + s = loh->lsm;
> + else if (lsm_base_one.lsm)
> + s = lsm_base_one.lsm;
> + else
> + return -EINVAL;
> +
> + *value = kstrdup(s, GFP_KERNEL);
> + if (*value)
> + return strlen(s);
> + return -ENOMEM;
> + }
>
> hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) {
> if (lsm != NULL && strcmp(lsm, hp->lsm))
> continue;
> + if (lsm == NULL && loh->lsm && strcmp(loh->lsm, hp->lsm))
> + continue;
> return hp->hook.getprocattr(p, name, value);
> }
> return -EINVAL;
> @@ -1973,10 +2017,83 @@ int security_setprocattr(const char *lsm, const char *name, void *value,
> size_t size)
> {
> struct security_hook_list *hp;
> + struct lsm_one_hooks *loh = current_cred()->security;
> + bool found = false;
> + char *s;
> +
> + /*
> + * End the passed name at a newline.
> + */
> + s = strnchr(value, size, '\n');
> + if (s)
> + *s = '\0';
> +
> + if (!strcmp(name, "display")) {
> + union security_list_options secid_to_secctx;
> + union security_list_options secctx_to_secid;
> + union security_list_options socket_getpeersec_stream;
> +
> + if (size == 0 || size >= 100)
> + return -EINVAL;
> +
> + secid_to_secctx.secid_to_secctx = NULL;
> + hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx,
> + list) {
> + if (size >= strlen(hp->lsm) &&
> + !strncmp(value, hp->lsm, size)) {
> + secid_to_secctx = hp->hook;
> + found = true;
> + break;
> + }
> + }
> + secctx_to_secid.secctx_to_secid = NULL;
> + hlist_for_each_entry(hp, &security_hook_heads.secctx_to_secid,
> + list) {
> + if (size >= strlen(hp->lsm) &&
> + !strncmp(value, hp->lsm, size)) {
> + secctx_to_secid = hp->hook;
> + found = true;
> + break;
> + }
> + }
> + socket_getpeersec_stream.socket_getpeersec_stream = NULL;
> + hlist_for_each_entry(hp,
> + &security_hook_heads.socket_getpeersec_stream,
> + list) {
> + if (size >= strlen(hp->lsm) &&
> + !strncmp(value, hp->lsm, size)) {
> + socket_getpeersec_stream = hp->hook;
> + found = true;
> + break;
> + }
> + }
> + if (!found)
> + return -EINVAL;
> +
> + /*
> + * The named lsm is active and supplies one or more
> + * of the relevant hooks. Switch to it.
> + */
> + s = kmemdup(value, size + 1, GFP_KERNEL);
> + if (s == NULL)
> + return -ENOMEM;
> + s[size] = '\0';
> +
> + if (loh->lsm)
> + kfree(loh->lsm);
> + loh->lsm = s;
> + loh->secid_to_secctx = secid_to_secctx;
> + loh->secctx_to_secid = secctx_to_secid;
> + loh->socket_getpeersec_stream = socket_getpeersec_stream;
You can't just write to the cred security blob like this; it is a
shared data structure, not per-task.
> +
> + return size;
> + }
>
> hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) {
> if (lsm != NULL && strcmp(lsm, hp->lsm))
> continue;
> + if (lsm == NULL && loh->lsm && strcmp(loh->lsm, hp->lsm))
> + continue;
> return hp->hook.setprocattr(name, value, size);
> }
> return -EINVAL;
> --
> 2.19.1
>
^ permalink raw reply
* Re: [PATCH 27/59] NET: Store LSM access information in the socket blob for UDS
From: Stephen Smalley @ 2019-04-10 12:28 UTC (permalink / raw)
To: Casey Schaufler
Cc: Schaufler, Casey, James Morris, linux-security-module, selinux
In-Reply-To: <20190409213946.1667-28-casey@schaufler-ca.com>
On Tue, Apr 9, 2019 at 5:42 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> UNIX domain socket connections don't have sufficient
> space in the socket buffer (skb) secmark for more than
> one Linux security module (LSM) to pass data. Expanding
> the secmark has been ruled out as an option. Store the
> necessary data in the socket security blob pointed to
> by the skb socket.
I don't believe this is correct. The secid in the unix_skb_parms is
not the same as the secmark in the sk_buff, and I don't know if we are
necessarily prohibited from expanding it. Also, I don't think you can
just store it in the socket security blob, especially without any form
of locking, as that can be shared across multiple sk_buffs.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> include/linux/security.h | 20 +++++++++++++++++++-
> net/unix/af_unix.c | 14 ++++++++------
> security/security.c | 17 ++++++++++++++++-
> 3 files changed, 43 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index e76d7a9dbe50..c413dcc1905a 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -71,6 +71,7 @@ struct ctl_table;
> struct audit_krule;
> struct user_namespace;
> struct timezone;
> +struct sk_buff;
>
> enum lsm_event {
> LSM_POLICY_CHANGE,
> @@ -100,6 +101,22 @@ static inline bool lsm_export_any(struct lsm_export *l)
> ((l->flags & LSM_EXPORT_APPARMOR) && l->apparmor));
> }
>
> +static inline bool lsm_export_equal(struct lsm_export *l, struct lsm_export *m)
> +{
> + if (l->flags != m->flags || l->flags == LSM_EXPORT_NONE)
> + return false;
> + if (l->flags & LSM_EXPORT_SELINUX &&
> + (l->selinux != m->selinux || l->selinux == 0))
> + return false;
> + if (l->flags & LSM_EXPORT_SMACK &&
> + (l->smack != m->smack || l->smack == 0))
> + return false;
> + if (l->flags & LSM_EXPORT_APPARMOR &&
> + (l->apparmor != m->apparmor || l->apparmor == 0))
> + return false;
> + return true;
> +}
> +
> /**
> * lsm_export_secid - pull the useful secid out of a lsm_export
> * @data: the containing data structure
> @@ -143,6 +160,8 @@ static inline void lsm_export_to_all(struct lsm_export *data, u32 secid)
> LSM_EXPORT_APPARMOR;
> }
>
> +extern struct lsm_export *lsm_export_skb(struct sk_buff *skb);
> +
> /* These functions are in security/commoncap.c */
> extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
> int cap, unsigned int opts);
> @@ -174,7 +193,6 @@ extern int cap_task_setnice(struct task_struct *p, int nice);
> extern int cap_vm_enough_memory(struct mm_struct *mm, long pages);
>
> struct msghdr;
> -struct sk_buff;
> struct sock;
> struct sockaddr;
> struct socket;
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index 4d4107927ba2..afe9c9f1adeb 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -143,21 +143,23 @@ static struct hlist_head *unix_sockets_unbound(void *addr)
> #ifdef CONFIG_SECURITY_NETWORK
> static void unix_get_secdata(struct scm_cookie *scm, struct sk_buff *skb)
> {
> - lsm_export_secid(&scm->le, &(UNIXCB(skb).secid));
> + struct lsm_export *ble = lsm_export_skb(skb);
> +
> + *ble = scm->le;
> }
>
> static inline void unix_set_secdata(struct scm_cookie *scm, struct sk_buff *skb)
> {
> - lsm_export_to_all(&scm->le, UNIXCB(skb).secid);
> + struct lsm_export *ble = lsm_export_skb(skb);
> +
> + scm->le = *ble;
> }
>
> static inline bool unix_secdata_eq(struct scm_cookie *scm, struct sk_buff *skb)
> {
> - u32 best_secid;
> -
> - lsm_export_secid(&scm->le, &best_secid);
> - return (best_secid == UNIXCB(skb).secid);
> + return lsm_export_equal(&scm->le, lsm_export_skb(skb));
> }
> +
> #else
> static inline void unix_get_secdata(struct scm_cookie *scm, struct sk_buff *skb)
> { }
> diff --git a/security/security.c b/security/security.c
> index 69983ad68233..015c38c882ba 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -46,7 +46,22 @@ static struct kmem_cache *lsm_file_cache;
> static struct kmem_cache *lsm_inode_cache;
>
> char *lsm_names;
> -static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init;
> +
> +/* Socket blobs include infrastructure managed data */
> +static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init = {
> + .lbs_sock = sizeof(struct lsm_export),
> +};
> +
> +/**
> + * lsm_export_skb - pointer to the lsm_export associated with the skb
> + * @skb: the socket buffer
> + *
> + * Returns a pointer to the LSM managed data.
> + */
> +struct lsm_export *lsm_export_skb(struct sk_buff *skb)
> +{
> + return skb->sk->sk_security;
> +}
>
> /* Boot-time LSM user choice */
> static __initdata const char *chosen_lsm_order;
> --
> 2.19.1
>
^ permalink raw reply
* [GIT PULL] apparmor regression fix for v5.1-rc5
From: John Johansen @ 2019-04-10 11:32 UTC (permalink / raw)
To: Linus Torvalds
Cc: LKLM, open list:SECURITY SUBSYSTEM, David Rheinsberg, Kees Cook
Hi Linus,
Can you please pull the following regression fix for apparmor
Thanks!
- John
The following changes since commit 771acc7e4a6e5dba779cb1a7fd851a164bc81033:
Bluetooth: btusb: request wake pin with NOAUTOEN (2019-04-09 17:38:24 -1000)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor 5.1-regression-fix
for you to fetch changes up to e33c1b9923775d17ad246946fe67fcb9be288677:
apparmor: Restore Y/N in /sys for apparmor's "enabled" (2019-04-10 04:24:48 -0700)
----------------------------------------------------------------
Kees Cook (1):
apparmor: Restore Y/N in /sys for apparmor's "enabled"
security/apparmor/lsm.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 48 insertions(+), 1 deletion(-)
^ permalink raw reply
* Re: crypto: Kernel memory overwrite attempt detected to spans multiple pages
From: Eric Biggers @ 2019-04-10 3:17 UTC (permalink / raw)
To: Kees Cook
Cc: Geert Uytterhoeven, Herbert Xu, linux-security-module, Linux ARM,
Linux Crypto Mailing List, Linux Kernel Mailing List
In-Reply-To: <20190321175122.GA1587@sol.localdomain>
On Thu, Mar 21, 2019 at 10:51:22AM -0700, Eric Biggers wrote:
> On Thu, Mar 21, 2019 at 10:45:31AM -0700, Kees Cook wrote:
> > On Wed, Mar 20, 2019 at 11:57 AM Eric Biggers <ebiggers@kernel.org> wrote:
> > >
> > > On Tue, Mar 19, 2019 at 10:09:13AM -0700, Eric Biggers wrote:
> > > > On Tue, Mar 19, 2019 at 12:54:23PM +0100, Geert Uytterhoeven wrote:
> > > > > When running the sha1-asm crypto selftest on arm with
> > > > > CONFIG_HARDENED_USERCOPY_PAGESPAN=y:
> > > > >
> > > > > usercopy: Kernel memory overwrite attempt detected to spans
> > > > > multiple pages (offset 0, size 42)!
> > > > > ------------[ cut here ]------------
> > > > > kernel BUG at mm/usercopy.c:102!
> > > > > Internal error: Oops - BUG: 0 [#1] SMP ARM
> > > > > Modules linked in:
> > > > > CPU: 0 PID: 35 Comm: cryptomgr_test Not tainted
> > > > > 5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
> > > > > Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> > > > > PC is at usercopy_abort+0x68/0x90
> > > > > LR is at usercopy_abort+0x68/0x90
> > > > > pc : [<c030fd60>] lr : [<c030fd60>] psr: 60000013
> > > > > sp : ea54bc60 ip : 00000010 fp : cccccccd
> > > > > r10: 00000000 r9 : c0e0ce04 r8 : ea54d009
> > > > > r7 : ea54d00a r6 : 00000000 r5 : 0000002a r4 : c09d1120
> > > > > r3 : dd6cd422 r2 : dd6cd422 r1 : 2abb4000 r0 : 0000005f
> > > > > Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> > > > > Control: 30c5387d Table: 40003000 DAC: fffffffd
> > > > > Process cryptomgr_test (pid: 35, stack limit = 0x(ptrval))
> > > > > Stack: (0xea54bc60 to 0xea54c000)
> > > > > bc60: c09d1120 c09d1120 c09d1120 00000000 0000002a 0000002a
> > > > > 00000000 c0310060
> > > > > bc80: 0000002a 00000000 000001c0 00000000 00000000 c0eb11e8
> > > > > ea54cfe0 ea538c00
> > > > > bca0: 00000000 ea54cfe0 ebef73e0 0000002a ea538c20 ea54bd84
> > > > > 0000003a c0427a30
> > > > > bcc0: ea54bdbc 00000000 00000000 c081cf70 eb074280 c081cf70
> > > > > 0000002a c081cf80
> > > > > bce0: 0000000e c07da138 ea54bd0c 00000000 c084061c c04248e8
> > > > > c0e0a408 eb074240
> > > > > bd00: eb074200 c04253c8 eb074280 ea550000 00000012 dd6cd422
> > > > > ebef7480 eb074200
> > > > > bd20: ea54bd84 c081cf64 ea537200 00000002 00000000 00000014
> > > > > c084061c c0428c38
> > > > > bd40: ea54bd84 ea54bdbc c081cd34 00000000 c0e4e4b4 ea538c40
> > > > > 00000002 eabe4e80
> > > > > bd60: ea538c00 00000400 ea4f7a00 ea4f7a60 eb074240 00000060
> > > > > 00000006 c09d544c
> > > > > bd80: 00000038 00000003 00000000 00000038 ea54bd7c 00000001
> > > > > eb074200 00000000
> > > > > bda0: 00000000 dead4ead ffffffff ffffffff ea54bdb0 ea54bdb0
> > > > > 00000000 c081cf70
> > > > > bdc0: c081ce68 c081ce78 ea4f7480 eb000780 00000dc0 eb000780
> > > > > c0e4ee80 443e9884
> > > > > bde0: 6ed23b1c a14aaeba e52951f9 f17046e5 fefefefe fefefefe
> > > > > fefefefe fefefefe
> > > > > be00: eb000780 c04292c4 c0e0a638 60000013 60000013 c0305298
> > > > > ea4f7a00 c03062bc
> > > > > be20: eb000780 00000cc0 ea4f7a00 dd6cd422 00000cc0 ea538c00
> > > > > 00000002 eabe4e40
> > > > > be40: ea537200 00000007 00000000 ea4f7a00 eb074200 c0429314
> > > > > eb074200 ea538c00
> > > > > be60: ea4f7a00 0000000a eabe4e80 c084061c c08405fc 00000006
> > > > > c04dace8 00000006
> > > > > be80: 00000000 c084065c ea537200 0000000e 00000400 eb04de08
> > > > > ea4f71a8 c0429420
> > > > > bea0: 00000400 ea537200 0000000e ea537200 0000000e c0429374
> > > > > 00000400 ffffffff
> > > > > bec0: 000000a2 c042a414 00000103 c0e0a408 00000000 c0e0a438
> > > > > c0e5a2a0 c0e5a2a0
> > > > > bee0: 00000001 00000001 00000017 ffffe000 00000000 60000013
> > > > > c0e5a2a0 c0269470
> > > > > bf00: c09c9ed0 ea54bf5c 00000103 00000000 00000000 c0e0a408
> > > > > ea537280 0000000e
> > > > > bf20: 00000400 c0426500 00000000 eb04de08 ea4f71a8 c02694f4
> > > > > c09c9ed0 ea54bf5c
> > > > > bf40: ea54bf28 c02699d0 ea54bf5c dd6cd422 ea537200 dd6cd422
> > > > > c09c9ed0 ea537200
> > > > > bf60: ea4af1c0 ea54a000 ea537200 c0426500 00000000 eb04de08
> > > > > ea4f71a8 c0426524
> > > > > bf80: ea4f7180 c023dcec ea54a000 ea4af1c0 c023dbb4 00000000
> > > > > 00000000 00000000
> > > > > bfa0: 00000000 00000000 00000000 c02010d8 00000000 00000000
> > > > > 00000000 00000000
> > > > > bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> > > > > 00000000 00000000
> > > > > bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > > > > 00000000 00000000
> > > > > [<c030fd60>] (usercopy_abort) from [<c0310060>]
> > > > > (__check_object_size+0x2d8/0x448)
> > > > > [<c0310060>] (__check_object_size) from [<c0427a30>]
> > > > > (build_test_sglist+0x268/0x2d8)
> > > > > [<c0427a30>] (build_test_sglist) from [<c0428c38>]
> > > > > (test_hash_vec_cfg+0x110/0x694)
> > > > > [<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
> > > > > (__alg_test_hash+0x158/0x1b8)
> > > > > [<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
> > > > > [<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
> > > > > [<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
> > > > > [<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
> > > > > [<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
> > > > > Exception stack(0xea54bfb0 to 0xea54bff8)
> > > > > bfa0: 00000000 00000000
> > > > > 00000000 00000000
> > > > > bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> > > > > 00000000 00000000
> > > > > bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > > > > Code: e58de000 e98d0012 e1a0100c ebfd6712 (e7f001f2)
> > > > > ---[ end trace 190b3cf48e720f78 ]---
> > > > > BUG: sleeping function called from invalid context at
> > > > > include/linux/percpu-rwsem.h:34
> > > > > in_atomic(): 0, irqs_disabled(): 128, pid: 35, name: cryptomgr_test
> > > > > CPU: 0 PID: 35 Comm: cryptomgr_test Tainted: G D
> > > > > 5.1.0-rc1-koelsch-01109-gbeb7d6376ecfbf07-dirty #397
> > > > > Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> > > > > [<c020ec74>] (unwind_backtrace) from [<c020ae58>] (show_stack+0x10/0x14)
> > > > > [<c020ae58>] (show_stack) from [<c07c3624>] (dump_stack+0x7c/0x9c)
> > > > > [<c07c3624>] (dump_stack) from [<c0242e14>] (___might_sleep+0xf4/0x158)
> > > > > [<c0242e14>] (___might_sleep) from [<c0230210>] (exit_signals+0x2c/0x258)
> > > > > [<c0230210>] (exit_signals) from [<c0223d6c>] (do_exit+0x114/0xa20)
> > > > > [<c0223d6c>] (do_exit) from [<c020b160>] (die+0x304/0x344)
> > > > > [<c020b160>] (die) from [<c020b388>] (do_undefinstr+0x80/0x190)
> > > > > [<c020b388>] (do_undefinstr) from [<c0201b24>] (__und_svc_finish+0x0/0x3c)
> > > > > Exception stack(0xea54bc10 to 0xea54bc58)
> > > > > bc00: 0000005f 2abb4000
> > > > > dd6cd422 dd6cd422
> > > > > bc20: c09d1120 0000002a 00000000 ea54d00a ea54d009 c0e0ce04
> > > > > 00000000 cccccccd
> > > > > bc40: 00000010 ea54bc60 c030fd60 c030fd60 60000013 ffffffff
> > > > > [<c0201b24>] (__und_svc_finish) from [<c030fd60>] (usercopy_abort+0x68/0x90)
> > > > > [<c030fd60>] (usercopy_abort) from [<c0310060>]
> > > > > (__check_object_size+0x2d8/0x448)
> > > > > [<c0310060>] (__check_object_size) from [<c0427a30>]
> > > > > (build_test_sglist+0x268/0x2d8)
> > > > > [<c0427a30>] (build_test_sglist) from [<c0428c38>]
> > > > > (test_hash_vec_cfg+0x110/0x694)
> > > > > [<c0428c38>] (test_hash_vec_cfg) from [<c0429314>]
> > > > > (__alg_test_hash+0x158/0x1b8)
> > > > > [<c0429314>] (__alg_test_hash) from [<c0429420>] (alg_test_hash+0xac/0xf4)
> > > > > [<c0429420>] (alg_test_hash) from [<c042a414>] (alg_test.part.4+0x264/0x2f8)
> > > > > [<c042a414>] (alg_test.part.4) from [<c0426524>] (cryptomgr_test+0x24/0x44)
> > > > > [<c0426524>] (cryptomgr_test) from [<c023dcec>] (kthread+0x138/0x150)
> > > > > [<c023dcec>] (kthread) from [<c02010d8>] (ret_from_fork+0x14/0x3c)
> > > > > Exception stack(0xea54bfb0 to 0xea54bff8)
> > > > > bfa0: 00000000 00000000
> > > > > 00000000 00000000
> > > > > bfc0: 00000000 00000000 00000000 00000000 00000000 00000000
> > > > > 00000000 00000000
> > > > > bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > > > >
> > > >
> > > > Well, this must happen with the new (in 5.1) crypto self-tests implementation
> > > > for any crypto algorithm when CONFIG_HARDENED_USERCOPY_PAGESPAN=y. I don't
> > > > understand why hardened usercopy considers it a bug though, as there's no buffer
> > > > overflow. The crypto tests use copy_from_iter() to copy data into a 2-page
> > > > buffer that was allocated with __get_free_pages():
> > > >
> > > > __get_free_pages(GFP_KERNEL, 1)
> > > >
> > > > ... where 1 means an order-1 allocation.
> > > >
> > > > If it copies to offset=4064 len=42, for example, then hardened usercopy
> > > > considers it a bug even though the buffer is 8192 bytes long. Why?
> > > >
> > > > It isn't actually copying anything to/from userspace, BTW; it's using iov_iter
> > > > with ITER_KVEC.
> > > >
> > > > - Eric
> > >
> > > Kees, any thoughts on why hardened usercopy rejects copies spanning a page
> > > boundary when they seem to be fine?
> >
> > This is due to missing the compound page marking, if I remember
> > correctly. However, I tend to leave the pagespan test disabled: it
> > really isn't ready for production use -- there are a lot of missing
> > annotations still.
> >
>
> So do I need to add __GFP_COMP? Is there any actual reason to do so?
> Why does hardened usercopy check for it?
>
> - Eric
Hi Kees, any answer to this question?
- Eric
^ permalink raw reply
* Re: [PATCH v3 bpf-next 00/21] bpf: Sysctl hook
From: Alexei Starovoitov @ 2019-04-09 23:34 UTC (permalink / raw)
To: Jann Horn
Cc: Andrey Ignatov, Network Development, Alexei Starovoitov,
Daniel Borkmann, Roman Gushchin, Kernel Team, Luis Chamberlain,
Kees Cook, Alexey Dobriyan, kernel list, linux-fsdevel,
linux-security-module
In-Reply-To: <CAG48ez0-B_m6ruMiMp+hVgxmw1dZVtiKGqPWtaFP5esOQTjr9A@mail.gmail.com>
On Tue, Apr 9, 2019 at 4:22 PM Jann Horn <jannh@google.com> wrote:
>
> But there's a reason why user namespaces are all-or-nothing on these
> things. If the kernel does not explicitly make a sysctl available to a
> container, the sysctl has global effects, and therefore probably
> shouldn't be exposed to anything other than someone with
> administrative privileges across the whole system. If the kernel does
> make it available to a container, the sysctl's effects are limited to
> the container (or otherwise it's a kernel bug).
>
> Can you give examples of sysctls that you want to permit using from
> containers, that wouldn't be accessible in a user namespace?
I think this discussion has started with incorrect
assumptions about the goal of the patch set.
There is no _security_ part here.
The sysctl hook is to prevent silly things to be done by chef
and apps. Most interesting sysctls need root anyway.
The root can detach all progs and do its thing.
Consider tcp_mem sysctl. We've seen it's been misconfigured
and caused performance issues. bpf prog can track what is
being written, alarm, etc.
User namespaces are not applicable here.
^ permalink raw reply
* Re: [PATCH 1/1] Smack: Create smack_rule cache to optimize memory usage
From: Casey Schaufler @ 2019-04-09 23:30 UTC (permalink / raw)
To: Vishal Goel, linux-security-module, linux-kernel; +Cc: pankaj.m, a.sahrawat
In-Reply-To: <1552554393-11691-1-git-send-email-vishal.goel@samsung.com>
On 3/14/2019 2:06 AM, Vishal Goel wrote:
> This patch allows for small memory optimization by creating the
> kmem cache for "struct smack_rule" instead of using kzalloc.
> For adding new smack rule, kzalloc is used to allocate the memory
> for "struct smack_rule". kzalloc will always allocate 32 or 64 bytes
> for 1 structure depending upon the kzalloc cache sizes available in
> system. Although the size of structure is 20 bytes only, resulting
> in memory wastage per object in the default pool.
>
> For e.g., if there are 20000 rules, then it will save 240KB(20000*12)
> which is crucial for small memory targets.
>
> Signed-off-by: Vishal Goel <vishal.goel@samsung.com>
> Signed-off-by: Amit Sahrawat <a.sahrawat@samsung.com>
I have taken this patch in for
https://github.com/cschaufler/next-smack.git#smack-for-5.2
> ---
> security/smack/smack.h | 1 +
> security/smack/smack_lsm.c | 12 ++++++++++--
> security/smack/smackfs.c | 2 +-
> 3 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index 6a71fc7..a5d7461 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -354,6 +354,7 @@ int smk_tskacc(struct task_smack *, struct smack_known *,
>
> #define SMACK_HASH_SLOTS 16
> extern struct hlist_head smack_known_hash[SMACK_HASH_SLOTS];
> +extern struct kmem_cache *smack_rule_cache;
>
> /*
> * Is the directory transmuting?
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 319add3..16b6cf5 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -56,6 +56,7 @@
> static LIST_HEAD(smk_ipv6_port_list);
> #endif
> static struct kmem_cache *smack_inode_cache;
> +struct kmem_cache *smack_rule_cache;
> int smack_enabled;
>
> static const match_table_t smk_mount_tokens = {
> @@ -349,7 +350,7 @@ static int smk_copy_rules(struct list_head *nhead, struct list_head *ohead,
> int rc = 0;
>
> list_for_each_entry_rcu(orp, ohead, list) {
> - nrp = kzalloc(sizeof(struct smack_rule), gfp);
> + nrp = kmem_cache_zalloc(smack_rule_cache, gfp);
> if (nrp == NULL) {
> rc = -ENOMEM;
> break;
> @@ -1995,7 +1996,7 @@ static void smack_cred_free(struct cred *cred)
> list_for_each_safe(l, n, &tsp->smk_rules) {
> rp = list_entry(l, struct smack_rule, list);
> list_del(&rp->list);
> - kfree(rp);
> + kmem_cache_free(smack_rule_cache, rp);
> }
> kfree(tsp);
> }
> @@ -4788,10 +4789,17 @@ static __init int smack_init(void)
> if (!smack_inode_cache)
> return -ENOMEM;
>
> + smack_rule_cache = KMEM_CACHE(smack_rule, 0);
> + if (!smack_rule_cache) {
> + kmem_cache_destroy(smack_inode_cache);
> + return -ENOMEM;
> + }
> +
> tsp = new_task_smack(&smack_known_floor, &smack_known_floor,
> GFP_KERNEL);
> if (tsp == NULL) {
> kmem_cache_destroy(smack_inode_cache);
> + kmem_cache_destroy(smack_rule_cache);
> return -ENOMEM;
> }
>
> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index 2a8a1f5..d8a0e25 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c
> @@ -236,7 +236,7 @@ static int smk_set_access(struct smack_parsed_rule *srp,
> }
>
> if (found == 0) {
> - sp = kzalloc(sizeof(*sp), GFP_KERNEL);
> + sp = kmem_cache_zalloc(smack_rule_cache, GFP_KERNEL);
> if (sp == NULL) {
> rc = -ENOMEM;
> goto out;
^ permalink raw reply
* Re: [PATCH 1/1] Smack :- In this patch, global rule list has been removed. Now all smack rules will be read using "smack_known_list". This list contains all the smack labels and internally each smack label structure maintains the list of smack rules corresponding to that smack label. So there is no need to maintain extra list.
From: Casey Schaufler @ 2019-04-09 23:29 UTC (permalink / raw)
To: Vishal Goel, linux-security-module, linux-kernel; +Cc: pankaj.m, a.sahrawat
In-Reply-To: <1551950603-15570-1-git-send-email-vishal.goel@samsung.com>
On 3/7/2019 1:23 AM, Vishal Goel wrote:
> 1) Small Memory Optimization
> For eg. if there are 20000 rules, then it will save 625KB(20000*32),
> which is critical for small embedded systems.
> 2) Reducing the time taken in writing rules on load/load2 interface
> 3) Since global rule list is just used to read the rules, so there
> will be no performance impact on system
>
> Signed-off-by: Vishal Goel <vishal.goel@samsung.com>
> Signed-off-by: Amit Sahrawat <a.sahrawat@samsung.com>
I have taken this patch in for
https://github.com/cschaufler/next-smack.git#smack-for-5.2
> ---
> security/smack/smackfs.c | 53 ++++++++++++++----------------------------------
> 1 file changed, 15 insertions(+), 38 deletions(-)
>
> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index f6482e5..2a8a1f5 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c
> @@ -67,7 +67,6 @@ enum smk_inos {
> /*
> * List locks
> */
> -static DEFINE_MUTEX(smack_master_list_lock);
> static DEFINE_MUTEX(smack_cipso_lock);
> static DEFINE_MUTEX(smack_ambient_lock);
> static DEFINE_MUTEX(smk_net4addr_lock);
> @@ -134,15 +133,7 @@ enum smk_inos {
>
> /*
> * Rule lists are maintained for each label.
> - * This master list is just for reading /smack/load and /smack/load2.
> */
> -struct smack_master_list {
> - struct list_head list;
> - struct smack_rule *smk_rule;
> -};
> -
> -static LIST_HEAD(smack_rule_list);
> -
> struct smack_parsed_rule {
> struct smack_known *smk_subject;
> struct smack_known *smk_object;
> @@ -211,7 +202,6 @@ static void smk_netlabel_audit_set(struct netlbl_audit *nap)
> * @srp: the rule to add or replace
> * @rule_list: the list of rules
> * @rule_lock: the rule list lock
> - * @global: if non-zero, indicates a global rule
> *
> * Looks through the current subject/object/access list for
> * the subject/object pair and replaces the access that was
> @@ -223,10 +213,9 @@ static void smk_netlabel_audit_set(struct netlbl_audit *nap)
> */
> static int smk_set_access(struct smack_parsed_rule *srp,
> struct list_head *rule_list,
> - struct mutex *rule_lock, int global)
> + struct mutex *rule_lock)
> {
> struct smack_rule *sp;
> - struct smack_master_list *smlp;
> int found = 0;
> int rc = 0;
>
> @@ -258,22 +247,6 @@ static int smk_set_access(struct smack_parsed_rule *srp,
> sp->smk_access = srp->smk_access1 & ~srp->smk_access2;
>
> list_add_rcu(&sp->list, rule_list);
> - /*
> - * If this is a global as opposed to self and a new rule
> - * it needs to get added for reporting.
> - */
> - if (global) {
> - mutex_unlock(rule_lock);
> - smlp = kzalloc(sizeof(*smlp), GFP_KERNEL);
> - if (smlp != NULL) {
> - smlp->smk_rule = sp;
> - mutex_lock(&smack_master_list_lock);
> - list_add_rcu(&smlp->list, &smack_rule_list);
> - mutex_unlock(&smack_master_list_lock);
> - } else
> - rc = -ENOMEM;
> - return rc;
> - }
> }
>
> out:
> @@ -540,9 +513,9 @@ static ssize_t smk_write_rules_list(struct file *file, const char __user *buf,
>
> if (rule_list == NULL)
> rc = smk_set_access(&rule, &rule.smk_subject->smk_rules,
> - &rule.smk_subject->smk_rules_lock, 1);
> + &rule.smk_subject->smk_rules_lock);
> else
> - rc = smk_set_access(&rule, rule_list, rule_lock, 0);
> + rc = smk_set_access(&rule, rule_list, rule_lock);
>
> if (rc)
> goto out;
> @@ -636,21 +609,23 @@ static void smk_rule_show(struct seq_file *s, struct smack_rule *srp, int max)
>
> static void *load2_seq_start(struct seq_file *s, loff_t *pos)
> {
> - return smk_seq_start(s, pos, &smack_rule_list);
> + return smk_seq_start(s, pos, &smack_known_list);
> }
>
> static void *load2_seq_next(struct seq_file *s, void *v, loff_t *pos)
> {
> - return smk_seq_next(s, v, pos, &smack_rule_list);
> + return smk_seq_next(s, v, pos, &smack_known_list);
> }
>
> static int load_seq_show(struct seq_file *s, void *v)
> {
> struct list_head *list = v;
> - struct smack_master_list *smlp =
> - list_entry_rcu(list, struct smack_master_list, list);
> + struct smack_rule *srp;
> + struct smack_known *skp =
> + list_entry_rcu(list, struct smack_known, list);
>
> - smk_rule_show(s, smlp->smk_rule, SMK_LABELLEN);
> + list_for_each_entry_rcu(srp, &skp->smk_rules, list)
> + smk_rule_show(s, srp, SMK_LABELLEN);
>
> return 0;
> }
> @@ -2352,10 +2327,12 @@ static ssize_t smk_write_access(struct file *file, const char __user *buf,
> static int load2_seq_show(struct seq_file *s, void *v)
> {
> struct list_head *list = v;
> - struct smack_master_list *smlp =
> - list_entry_rcu(list, struct smack_master_list, list);
> + struct smack_rule *srp;
> + struct smack_known *skp =
> + list_entry_rcu(list, struct smack_known, list);
>
> - smk_rule_show(s, smlp->smk_rule, SMK_LONGLABEL);
> + list_for_each_entry_rcu(srp, &skp->smk_rules, list)
> + smk_rule_show(s, srp, SMK_LONGLABEL);
>
> return 0;
> }
^ permalink raw reply
* Re: [PATCH v3 bpf-next 00/21] bpf: Sysctl hook
From: Jann Horn @ 2019-04-09 23:22 UTC (permalink / raw)
To: Andrey Ignatov
Cc: Network Development, Alexei Starovoitov, Daniel Borkmann,
Roman Gushchin, Kernel Team, Luis Chamberlain, Kees Cook,
Alexey Dobriyan, kernel list, linux-fsdevel,
linux-security-module
In-Reply-To: <20190409230432.GA59615@rdna-mbp>
On Wed, Apr 10, 2019 at 1:04 AM Andrey Ignatov <rdna@fb.com> wrote:
> Jann Horn <jannh@google.com> [Tue, 2019-04-09 13:42 -0700]:
> > On Tue, Apr 9, 2019 at 10:26 PM Andrey Ignatov <rdna@fb.com> wrote:
> > > The patch set introduces new BPF hook for sysctl.
> > >
> > > It adds new program type BPF_PROG_TYPE_CGROUP_SYSCTL and attach type
> > > BPF_CGROUP_SYSCTL.
> > >
> > > BPF_CGROUP_SYSCTL hook is placed before calling to sysctl's proc_handler so
> > > that accesses (read/write) to sysctl can be controlled for specific cgroup
> > > and either allowed or denied, or traced.
> >
> > Don't look at the credentials of "current" in a read or write handler.
> > Consider what happens if, for example, someone inside a cgroup opens a
> > sysctl file and passes the file descriptor to another process outside
> > the cgroup over a unix domain socket, and that other process then
> > writes to it. Either do your access check on open, or use the
> > credentials that were saved during open() in the read/write handler.
>
> This way this someone inside cgroup should already have control over
> something running as root [1] outside of this cgroup, i.e. the game is
> already lost, even without this hook.
>
> [1] Since proc_sys_read() / proc_sys_write() check sysctl_perm() before
> execution reaches the hook.
You don't need to have _control_ over something running as root. You
only need to be able to communicate with something that expects to be
passed in file descriptors for some purpose.
> This patch set doesn't look at credentials at all and relies on what
> checks were already done at sys_open time or in proc_sys_call_handler()
> before execution reaches the hook.
You're looking at the cgroup though.
> > > The hook has access to sysctl name, current sysctl value and (on write
> > > only) to new sysctl value via corresponding helpers. New sysctl value can
> > > be overridden by program. Both name and values (current/new) are
> > > represented as strings same way they're visible in /proc/sys/. It is up to
> > > program to parse these strings.
> >
> > But even if a filter is installed that prevents all access to a
> > sysctl, you can still read it by installing your own filter that, when
> > a read is attempted the next time, dumps the value into a map or
> > something like that, right?
>
> No. This can be controlled by cgroup hierarchy and appropriate attach
> flags, same way as with any other cgroup-bpf hook.
>
> E.g. imagine there is a cgroup hierarchy:
> root/slice/container/
>
> and container application runs in root/slice/container/ in a cgroup
> namespace (CLONE_NEWCGROUP) that makes visible only "container/" part of
> the hierarchy, i.e. from inside container application can't even see
> "root/slice/".
>
> Administrator can then attach sysctl hook to "root/slice/" with attach
> flag NONE (bpf_attr.attach_flags = 0) what means nobody down the
> hierarchy can override the program attached by administrator.
Ah, okay.
> > > To help with parsing the most common kind of sysctl value, vector of
> > > integers, two new helpers are provided: bpf_strtol and bpf_strtoul with
> > > semantic similar to user space strtol(3) and strtoul(3).
> > >
> > > The hook also provides bpf_sysctl context with two fields:
> > > * @write indicates whether sysctl is being read (= 0) or written (= 1);
> > > * @file_pos is sysctl file position to read from or write to, can be
> > > overridden.
> > >
> > > The hook allows to make better isolation for containerized applications
> > > that are run as root so that one container can't change a sysctl and affect
> > > all other containers on a host, make changes to allowed sysctl in a safer
> > > way and simplify sysctl tracing for cgroups.
> >
> > Why can't you use a user namespace and isolate things properly that
> > way? That would be much cleaner, wouldn't it?
>
> I'm not sure I understand how user namespace helps here. From my
> understanding it can only completely deny access to sysctl and can't do
> fine-grained control for specific sysctl knobs. It also can't make
> allow/deny decision based on sysctl value being written.
>
> Basically user namespace is all or nothing. This sysctl hook provides a
> way to implement fine-grained access control for sysctl knobs based on
> sysctl name or value being written or whatever else policy administrator
> can come up with.
But there's a reason why user namespaces are all-or-nothing on these
things. If the kernel does not explicitly make a sysctl available to a
container, the sysctl has global effects, and therefore probably
shouldn't be exposed to anything other than someone with
administrative privileges across the whole system. If the kernel does
make it available to a container, the sysctl's effects are limited to
the container (or otherwise it's a kernel bug).
Can you give examples of sysctls that you want to permit using from
containers, that wouldn't be accessible in a user namespace?
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox