Linux Security Modules development
 help / color / mirror / Atom feed
* [RFC PATCH v6 03/11] security: add ipe lsm policy parser and policy loading
From: Deven Bowers @ 2020-07-30  0:31 UTC (permalink / raw)
  To: agk, axboe, snitzer, jmorris, serge, zohar, viro, paul, eparis,
	jannh, dm-devel, linux-integrity, linux-security-module,
	linux-fsdevel, linux-block, linux-audit
  Cc: tyhicks, linux-kernel, corbet, sashal, jaskarankhurana, mdsakib,
	nramas, pasha.tatashin
In-Reply-To: <20200730003113.2561644-1-deven.desai@linux.microsoft.com>

Adds the policy parser and the policy loading to IPE, along with the
related securityfs entries and audit events.

Signed-off-by: Deven Bowers <deven.desai@linux.microsoft.com>
---
 security/ipe/Kconfig             |    2 +
 security/ipe/Makefile            |    3 +
 security/ipe/ipe-audit.c         |   74 +-
 security/ipe/ipe-audit.h         |    6 +
 security/ipe/ipe-parse.c         |  889 ++++++++++++++++++++
 security/ipe/ipe-parse.h         |   17 +
 security/ipe/ipe-policy.c        |  149 ++++
 security/ipe/ipe-policy.h        |   13 +-
 security/ipe/ipe-prop-internal.h |   18 +-
 security/ipe/ipe-property.c      |    9 +-
 security/ipe/ipe-property.h      |    1 +
 security/ipe/ipe-secfs.c         | 1309 ++++++++++++++++++++++++++++++
 security/ipe/ipe-secfs.h         |   14 +
 13 files changed, 2493 insertions(+), 11 deletions(-)
 create mode 100644 security/ipe/ipe-parse.c
 create mode 100644 security/ipe/ipe-parse.h
 create mode 100644 security/ipe/ipe-policy.c
 create mode 100644 security/ipe/ipe-secfs.c
 create mode 100644 security/ipe/ipe-secfs.h

diff --git a/security/ipe/Kconfig b/security/ipe/Kconfig
index 7615109a19ca..665524fc3ca4 100644
--- a/security/ipe/Kconfig
+++ b/security/ipe/Kconfig
@@ -7,6 +7,8 @@ menuconfig SECURITY_IPE
 	bool "Integrity Policy Enforcement (IPE)"
 	depends on SECURITY && AUDIT
 	select SYSTEM_DATA_VERIFICATION
+	select SECURITYFS
+	select CRYPTO_SHA1
 	help
 	  This option enables the Integrity Policy Enforcement subsystem
 	  allowing systems to enforce integrity having no dependencies
diff --git a/security/ipe/Makefile b/security/ipe/Makefile
index 40565b73fac2..7d6da33dd0c4 100644
--- a/security/ipe/Makefile
+++ b/security/ipe/Makefile
@@ -19,7 +19,10 @@ obj-$(CONFIG_SECURITY_IPE) += \
 	ipe-audit.o \
 	ipe-bp.o \
 	ipe-engine.o \
+	ipe-parse.o \
+	ipe-policy.o \
 	ipe-property.o \
 	ipe-hooks.o \
+	ipe-secfs.o \
 
 clean-files := ipe-bp.c
diff --git a/security/ipe/ipe-audit.c b/security/ipe/ipe-audit.c
index 2c754851bd40..0e731025f2f5 100644
--- a/security/ipe/ipe-audit.c
+++ b/security/ipe/ipe-audit.c
@@ -17,7 +17,8 @@
 #include <crypto/sha1_base.h>
 
 #define ACTION_STR(a) ((a) == ipe_action_allow ? "ALLOW" : "DENY")
-
+#define POLICY_LOAD_FSTR	"IPE policy_name=\"%s\" policy_version=%hu.%hu.%hu sha1="
+#define POLICY_ACTIVATE_STR	"IPE policy_name=\"%s\" policy_version=%hu.%hu.%hu"
 #define IPE_UNKNOWN		"UNKNOWN"
 
 /* Keep in sync with ipe_op in ipe-hooks.h */
@@ -229,3 +230,74 @@ void ipe_audit_match(const struct ipe_engine_ctx *ctx,
 
 	audit_log_end(ab);
 }
+
+/**
+ * ipe_audit_policy_load: Emit an audit event that an IPE policy has been
+ *			  loaded, with the name of the policy, the policy
+ *			  version triple, and a flat hash of the content.
+ * @pol: The parsed policy to derive the policy_name and policy_version
+ *	 triple.
+ * @raw: The raw content that was passed to the ipe.policy sysctl to derive
+ *	 the sha1 hash.
+ * @raw_size: the length of @raw.
+ * @tfm: shash structure allocated by the caller, used to fingerprint the
+ *	 policy being deployed
+ */
+void ipe_audit_policy_load(const struct ipe_policy *pol, const uint8_t *raw,
+			   size_t raw_size, struct crypto_shash *tfm)
+{
+	int rc = 0;
+	struct audit_buffer *ab;
+	u8 digest[SHA1_DIGEST_SIZE];
+	SHASH_DESC_ON_STACK(desc, tfm);
+
+	ab = audit_log_start(audit_context(), GFP_KERNEL,
+			     AUDIT_INTEGRITY_POLICY_LOAD);
+	if (!ab)
+		return;
+
+	audit_log_format(ab, POLICY_LOAD_FSTR, pol->policy_name,
+			 pol->policy_version.major, pol->policy_version.minor,
+			 pol->policy_version.rev);
+
+	desc->tfm = tfm;
+
+	if (crypto_shash_init(desc) != 0)
+		goto err;
+
+	if (crypto_shash_update(desc, raw, raw_size) != 0)
+		goto err;
+
+	if (crypto_shash_final(desc, digest) != 0)
+		goto err;
+
+	audit_log_n_hex(ab, digest, crypto_shash_digestsize(tfm));
+
+err:
+	if (rc != 0)
+		audit_log_format(ab, "ERR(%d)", rc);
+
+	audit_log_end(ab);
+}
+
+/**
+ * ipe_audit_policy_activation: Emit an audit event that a specific policy
+ *				was activated as the active policy.
+ * @pol: policy that is being activated
+ */
+void ipe_audit_policy_activation(const struct ipe_policy *pol)
+{
+	struct audit_buffer *ab;
+
+	ab = audit_log_start(audit_context(), GFP_KERNEL,
+			     AUDIT_INTEGRITY_POLICY_ACTIVATE);
+
+	if (!ab)
+		return;
+
+	audit_log_format(ab, POLICY_ACTIVATE_STR, pol->policy_name,
+			 pol->policy_version.major, pol->policy_version.minor,
+			 pol->policy_version.rev);
+
+	audit_log_end(ab);
+}
diff --git a/security/ipe/ipe-audit.h b/security/ipe/ipe-audit.h
index e00f415bed2c..350818d5c47f 100644
--- a/security/ipe/ipe-audit.h
+++ b/security/ipe/ipe-audit.h
@@ -3,6 +3,7 @@
  * Copyright (C) Microsoft Corporation. All rights reserved.
  */
 
+#include "ipe-prop-internal.h"
 #include "ipe-engine.h"
 #include "ipe-policy.h"
 
@@ -15,4 +16,9 @@ void ipe_audit_match(const struct ipe_engine_ctx *ctx,
 		     enum ipe_match match_type, enum ipe_action action,
 		     const struct ipe_rule *rule);
 
+void ipe_audit_policy_load(const struct ipe_policy *pol, const uint8_t *raw,
+			   size_t raw_size, struct crypto_shash *tfm);
+
+void ipe_audit_policy_activation(const struct ipe_policy *pol);
+
 #endif /* IPE_AUDIT_H */
diff --git a/security/ipe/ipe-parse.c b/security/ipe/ipe-parse.c
new file mode 100644
index 000000000000..edc3f52617c5
--- /dev/null
+++ b/security/ipe/ipe-parse.c
@@ -0,0 +1,889 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) Microsoft Corporation. All rights reserved.
+ */
+
+#include "ipe.h"
+#include "ipe-prop-internal.h"
+#include "ipe-hooks.h"
+#include "ipe-parse.h"
+#include "ipe-property.h"
+#include "ipe-audit.h"
+
+#include <linux/types.h>
+#include <linux/list.h>
+#include <linux/list_sort.h>
+#include <linux/slab.h>
+#include <linux/ctype.h>
+#include <linux/parser.h>
+#include <linux/errno.h>
+#include <linux/err.h>
+
+#define ALLOW_ACTION	"ALLOW"
+#define DENY_ACTION	"DENY"
+#define COMMENT_CHAR	'#'
+#define VER_FSTR	"%hu.%hu.%hu"
+
+/* Internal Type Definitions */
+enum property_priority {
+	other = 0,
+	action = 1,
+	op = 2,
+	default_action = 3,
+	policy_ver = 4,
+	policy_name = 5,
+};
+
+struct token {
+	struct list_head	next_tok;
+	const char		*key;
+	enum property_priority	key_priority;
+	const char		*val;
+};
+
+/* Utility Functions */
+static inline bool is_quote(char c)
+{
+	return c == '"' || c == '\'';
+}
+
+static inline bool valid_token(char *s)
+{
+	return !s || !strpbrk(s, "\"\'");
+}
+
+static inline bool is_default(const struct token *t)
+{
+	return !t->val &&  t->key_priority == default_action;
+}
+
+static inline bool is_operation(const struct token *t)
+{
+	return t->val && t->key_priority == op;
+}
+
+static inline bool is_action(const struct token *t)
+{
+	return t->val && t->key_priority == action;
+}
+
+static inline bool is_name(const struct token *t)
+{
+	return t->val && t->key_priority == policy_name;
+}
+
+static inline bool is_ver(const struct token *t)
+{
+	return t->val && t->key_priority == policy_ver;
+}
+
+static int cmp_pri(void *priv, struct list_head *a, struct list_head *b)
+{
+	struct token *t_a = container_of(a, struct token, next_tok);
+	struct token *t_b = container_of(b, struct token, next_tok);
+
+	return t_b->key_priority - t_a->key_priority;
+}
+
+static char *trim_quotes(char *str)
+{
+	char s;
+	size_t len;
+
+	if (!str)
+		return str;
+
+	s = *str;
+
+	if (is_quote(s)) {
+		len = strlen(str) - 1;
+
+		if (str[len] != s)
+			return NULL;
+
+		str[len] = '\0';
+		++str;
+	}
+
+	return str;
+}
+
+/**
+ * ipe_set_action: Set an action with error checking.
+ * @src: Valid pointer to the source location to set wih the result
+ * @set: Value to apply to @src, if valid
+ *
+ * Return:
+ * 0 - OK
+ * -EBADMSG - Attempting to set something that is already set
+ */
+static int ipe_set_action(enum ipe_action *src, enum ipe_action set)
+{
+	if (*src != ipe_action_unset)
+		return -EBADMSG;
+
+	*src = set;
+
+	return 0;
+}
+
+/**
+ * ipe_insert_token: Allocate and append the key=value pair indicated by @val,
+ *		     to the list represented by @head.
+ * @val: Token to parse, of form "key=val".
+ * @head: Head of the list to insert the token structure into.
+ *
+ * If "=val" is omitted, this function will succeed, and the value set will be
+ * NULL.
+ *
+ * Return:
+ * 0 - OK
+ * -EBADMSG - Invalid policy syntax
+ * -ENOMEM - No Memory
+ */
+static int ipe_insert_token(char *val, struct list_head *head)
+{
+	char *key;
+	substring_t match[MAX_OPT_ARGS];
+	struct token *tok;
+	const match_table_t prop_priorities = {
+		{ policy_name,		IPE_HEADER_POLICY_NAME },
+		{ policy_ver,		IPE_HEADER_POLICY_VERSION},
+		{ op,			IPE_PROPERTY_OPERATION },
+		{ default_action,	IPE_PROPERTY_DEFAULT },
+		{ action,		IPE_PROPERTY_ACTION },
+		{ other, NULL },
+	};
+
+	key = strsep(&val, "=");
+	if (!key)
+		return -EBADMSG;
+
+	tok = kzalloc(sizeof(*tok), GFP_KERNEL);
+	if (!tok)
+		return -ENOMEM;
+
+	tok->key = key;
+	tok->val = trim_quotes(val);
+
+	/* remap empty string */
+	if (tok->val && !strlen(tok->val))
+		tok->val = NULL;
+
+	tok->key_priority = match_token(key, prop_priorities, match);
+	INIT_LIST_HEAD(&tok->next_tok);
+
+	list_add_tail(&tok->next_tok, head);
+
+	return 0;
+}
+
+/**
+ * ipe_tokenize_line: Parse a line of text into a list of token structures.
+ * @line: Line to parse.
+ * @list: Head of the list to insert the token structure into.
+ *
+ * The final result will be sorted in the priority order definted by
+ * enum property_priorities to enforce policy structure.
+ *
+ * Return:
+ * 0 - OK
+ * -EBADMSG - Invalid policy syntax
+ * -ENOMEM - No Memory
+ * -ENOENT - No tokens were parsed
+ */
+static int ipe_tokenize_line(char *line, struct list_head *list)
+{
+	int rc = 0;
+	size_t i = 0;
+	size_t len = 0;
+	char *tok = NULL;
+	char quote = '\0';
+
+	len = strlen(line);
+
+	for (i = 0; i < len; ++i) {
+		if (quote == '\0' && is_quote(line[i])) {
+			quote = line[i];
+			continue;
+		}
+
+		if (quote != '\0' && line[i] == quote) {
+			quote = '\0';
+			continue;
+		}
+
+		if (quote == '\0' && line[i] == COMMENT_CHAR) {
+			tok = NULL;
+			break;
+		}
+
+		if (isgraph(line[i]) && !tok)
+			tok = &line[i];
+
+		if (quote == '\0' && isspace(line[i])) {
+			line[i] = '\0';
+
+			if (!tok)
+				continue;
+
+			rc = ipe_insert_token(tok, list);
+			if (rc != 0)
+				return rc;
+
+			tok = NULL;
+		}
+	}
+
+	if (quote != '\0')
+		return -EBADMSG;
+
+	if (tok)
+		ipe_insert_token(tok, list);
+
+	if (list_empty(list))
+		return -ENOENT;
+
+	list_sort(NULL, list, cmp_pri);
+
+	return 0;
+}
+
+static inline int ipe_parse_version(const char *val, struct ipe_pol_ver *ver)
+{
+	if (sscanf(val, VER_FSTR, &ver->major, &ver->minor, &ver->rev) != 3)
+		return -EBADMSG;
+
+	return 0;
+}
+
+/**
+ * ipe_parse_action: Given a token, parse the value as if it were an 'action'
+ *		     token.
+ * @action: Token to parse to determine the action.
+ *
+ * Action tokens are of the form: action=(ALLOW|DENY) for more information
+ * about IPE policy, please see the documentation.
+ *
+ * Return:
+ * ipe_action_allow - OK
+ * ipe_action_deny - OK
+ * ipe_action_unset - ERR
+ */
+static enum ipe_action ipe_parse_action(struct token *action)
+{
+	if (!action->val)
+		return ipe_action_unset;
+	else if (!strcmp(action->val, ALLOW_ACTION))
+		return ipe_action_allow;
+	else if (!strcmp(action->val, DENY_ACTION))
+		return ipe_action_deny;
+
+	return ipe_action_unset;
+}
+
+/**
+ * ipe_parse_op: Given a token, parse the value as if it were an 'op' token.
+ * @op: Token to parse to determine the operation.
+ *
+ * "op" tokens are of the form: op=(EXECUTE|FIRMWARE|KEXEC_IMAGE|...)
+ * for more information about IPE policy, please see the documentation.
+ *
+ * Return:
+ * ipe_op_max - ERR
+ * otherwise - OK
+ */
+static enum ipe_op ipe_parse_op(struct token *op)
+{
+	substring_t match[MAX_OPT_ARGS];
+	const match_table_t ops = {
+		{ ipe_op_execute,		IPE_OP_EXECUTE },
+		{ ipe_op_firmware,		IPE_OP_FIRMWARE },
+		{ ipe_op_kexec_image,		IPE_OP_KEXEC_IMAGE },
+		{ ipe_op_kexec_initramfs,	IPE_OP_KEXEC_INITRAMFS },
+		{ ipe_op_x509,			IPE_OP_X509_CERTIFICATE },
+		{ ipe_op_policy,		IPE_OP_POLICY },
+		{ ipe_op_kmodule,		IPE_OP_KMODULE },
+		{ ipe_op_kernel_read,		IPE_OP_KERNEL_READ },
+		{ ipe_op_max,			NULL },
+	};
+
+	return match_token((char *)op->val, ops, match);
+}
+
+/**
+ * ipe_set_default: Set the default of the policy, at various scope levels
+ *		    depending on the value of op.
+ * @op: Operation that was parsed.
+ * @pol: Policy to modify with the newly-parsed default action.
+ * @a: Action token (see parse_action) to parse to determine
+ *     the default.
+ *
+ * Return:
+ * 0 - OK
+ * -EBADMSG - Invalid policy format
+ */
+static int ipe_set_default(enum ipe_op op, struct ipe_policy *pol,
+			   struct token *a)
+{
+	int rc = 0;
+	size_t i = 0;
+	enum ipe_action act = ipe_parse_action(a);
+
+	if (act == ipe_action_unset)
+		return -EBADMSG;
+
+	if (op == ipe_op_max)
+		return ipe_set_action(&pol->def, act);
+
+	if (op == ipe_op_kernel_read) {
+		for (i = ipe_op_firmware; i <= ipe_op_kmodule; ++i) {
+			rc = ipe_set_action(&pol->ops[i].def, act);
+			if (rc != 0)
+				return rc;
+		}
+		return 0;
+	}
+
+	return ipe_set_action(&pol->ops[op].def, act);
+}
+
+/**
+ * ipe_parse_default: Parse a default statement of an IPE policy modify @pol
+ *		      with the proper changes
+ * @tokens: List of tokens parsed from the line
+ * @pol: Policy to modify with the newly-parsed default action
+ *
+ *
+ * Return:
+ * 0 - OK
+ * -EBADMSG - Invalid policy format
+ * -ENOENT - Unknown policy structure
+ */
+static int ipe_parse_default(struct list_head *tokens,
+			     struct ipe_policy *pol)
+{
+	struct token *f = NULL;
+	struct token *s = NULL;
+	struct token *t = NULL;
+	enum ipe_op i = ipe_op_max;
+
+	f = list_first_entry(tokens, struct token, next_tok);
+	s = list_next_entry(f, next_tok);
+	if (is_action(s))
+		return ipe_set_default(ipe_op_max, pol, s);
+
+	i = ipe_parse_op(s);
+	if (i == ipe_op_max)
+		return -ENOENT;
+
+	t = list_next_entry(s, next_tok);
+	if (is_action(t)) {
+		t = list_next_entry(s, next_tok);
+		return ipe_set_default(i, pol, t);
+	}
+
+	return -ENOENT;
+}
+
+/**
+ * ipe_free_token_list - Free a list of tokens, and then reinitialize @list
+ *			 dropping all tokens.
+ * @list: List to be freed.
+ */
+static void ipe_free_token_list(struct list_head *list)
+{
+	struct token *ptr, *next;
+
+	list_for_each_entry_safe(ptr, next, list, next_tok)
+		kfree(ptr);
+
+	INIT_LIST_HEAD(list);
+}
+
+/**
+ * ipe_free_prop - Deallocator for an ipe_prop_container structure.
+ * @cont: Object to free.
+ */
+static void ipe_free_prop(struct ipe_prop_container *cont)
+{
+	if (IS_ERR_OR_NULL(cont))
+		return;
+
+	if (cont->prop->free_val)
+		cont->prop->free_val(&cont->value);
+	kfree(cont);
+}
+
+/**
+ * ipe_alloc_prop: Allocator for a ipe_prop_container structure.
+ * @tok: Token structure representing the "key=value" pair of the property.
+ *
+ * Return:
+ * Pointer to ipe_rule - OK
+ * ERR_PTR(-ENOMEM) - Allocation failed
+ */
+static struct ipe_prop_container *ipe_alloc_prop(const struct token *tok)
+{
+	int rc = 0;
+	const struct ipe_property *prop = NULL;
+	struct ipe_prop_container *cont = NULL;
+
+	prop = ipe_lookup_prop(tok->key);
+	if (!prop) {
+		rc = -ENOENT;
+		goto err;
+	}
+
+	cont = kzalloc(sizeof(*cont), GFP_KERNEL);
+	if (!cont) {
+		rc = -ENOMEM;
+		goto err;
+	}
+
+	INIT_LIST_HEAD(&cont->next);
+
+	rc = prop->parse(tok->val, &cont->value);
+	if (rc != 0)
+		goto err;
+
+	cont->prop = prop;
+
+	return cont;
+err:
+	ipe_free_prop(cont);
+	return ERR_PTR(rc);
+}
+
+/**
+ * ipe_free_rule: Deallocator for an ipe_rule structure.
+ * @rule: Object to free.
+ */
+static void ipe_free_rule(struct ipe_rule *rule)
+{
+	struct ipe_prop_container *ptr;
+	struct list_head *l_ptr, *l_next;
+
+	if (IS_ERR_OR_NULL(rule))
+		return;
+
+	list_for_each_safe(l_ptr, l_next, &rule->props) {
+		ptr = container_of(l_ptr, struct ipe_prop_container, next);
+		list_del(l_ptr);
+		ipe_free_prop(ptr);
+	}
+
+	kfree(rule);
+}
+
+/**
+ * ipe_alloc_rule: Allocate a ipe_rule structure, for operation @op, parsed
+ *		   from the first token in list @head.
+ * @op: Operation parsed from the first token in @head.
+ * @t: The first token in @head that was parsed.
+ * @head: List of remaining tokens to parse.
+ *
+ * Return:
+ * Valid ipe_rule pointer - OK
+ * ERR_PTR(-EBADMSG) - Invalid syntax
+ * ERR_PTR(-ENOMEM) - Out of memory
+ */
+static struct ipe_rule *ipe_alloc_rule(enum ipe_op op, struct token *t,
+				       struct list_head *head)
+{
+	int rc = 0;
+	struct token *ptr;
+	enum ipe_action act;
+	struct ipe_rule *rule = NULL;
+	struct ipe_prop_container *prop = NULL;
+
+	ptr = list_next_entry(t, next_tok);
+	if (!is_action(ptr)) {
+		rc = -EBADMSG;
+		goto err;
+	}
+
+	act = ipe_parse_action(ptr);
+	if (act == ipe_action_unset) {
+		rc = -EBADMSG;
+		goto err;
+	}
+
+	rule = kzalloc(sizeof(*rule), GFP_KERNEL);
+	if (!rule) {
+		rc = -ENOMEM;
+		goto err;
+	}
+
+	INIT_LIST_HEAD(&rule->props);
+	INIT_LIST_HEAD(&rule->next);
+	rule->action = act;
+	rule->op = op;
+
+	list_for_each_entry_continue(ptr, head, next_tok) {
+		prop = ipe_alloc_prop(ptr);
+
+		if (IS_ERR(prop)) {
+			rc = PTR_ERR(prop);
+			goto err;
+		}
+
+		list_add_tail(&prop->next, &rule->props);
+	}
+
+	return rule;
+err:
+	ipe_free_prop(prop);
+	ipe_free_rule(rule);
+	return ERR_PTR(rc);
+}
+
+/**
+ * ipe_dup_prop: Duplicate an ipe_prop_container structure
+ * @p: Container to duplicate.
+ *
+ * This function is used to duplicate individual properties within a rule.
+ * It should only be called in operations that actually map to one or more
+ * operations.
+ *
+ * Return:
+ * Valid ipe_prop_container - OK
+ * ERR_PTR(-ENOMEM) - Out of memory
+ * Other Errors - see various property duplicator functions
+ */
+static
+struct ipe_prop_container *ipe_dup_prop(const struct ipe_prop_container *p)
+{
+	int rc = 0;
+	struct ipe_prop_container *dup;
+
+	dup = kzalloc(sizeof(*dup), GFP_KERNEL);
+	if (!dup) {
+		rc = -ENOMEM;
+		goto err;
+	}
+
+	dup->prop = p->prop;
+	INIT_LIST_HEAD(&dup->next);
+
+	rc = p->prop->dup(p->value, &dup->value);
+	if (rc != 0)
+		goto err;
+
+	return dup;
+err:
+	ipe_free_prop(dup);
+	return ERR_PTR(rc);
+}
+
+/**
+ * ipe_dup_rule: Duplicate a policy rule, used for pseudo hooks like
+ *		 KERNEL_READ to map a policy rule across all hooks.
+ * @r: Rule to duplicate.
+ *
+ * Return:
+ * valid ipe_rule - OK
+ * ERR_PTR(-ENOMEM) - Out of memory
+ * Other Errors - See ipe_dup_prop
+ */
+static struct ipe_rule *ipe_dup_rule(const struct ipe_rule *r)
+{
+	int rc = 0;
+	struct ipe_rule *dup;
+	struct ipe_prop_container *ptr;
+
+	dup = kzalloc(sizeof(*dup), GFP_KERNEL);
+	if (!dup) {
+		rc = -ENOMEM;
+		goto err;
+	}
+
+	dup->op = r->op;
+	dup->action = r->action;
+	INIT_LIST_HEAD(&dup->props);
+	INIT_LIST_HEAD(&dup->next);
+
+	list_for_each_entry(ptr, &r->props, next) {
+		struct ipe_prop_container *prop2;
+
+		prop2 = ipe_dup_prop(ptr);
+		if (IS_ERR(prop2)) {
+			rc = PTR_ERR(prop2);
+			goto err;
+		}
+
+		list_add_tail(&prop2->next, &dup->props);
+	}
+
+	return dup;
+err:
+	ipe_free_rule(dup);
+	return ERR_PTR(rc);
+}
+
+/**
+ * ipe_free_policy: Deallocate an ipe_policy structure.
+ * @pol: Policy to free.
+ */
+void ipe_free_policy(struct ipe_policy *pol)
+{
+	size_t i;
+	struct ipe_rule *ptr;
+	struct ipe_rule_table *op;
+	struct list_head *l_ptr, *l_next;
+
+	if (IS_ERR_OR_NULL(pol))
+		return;
+
+	for (i = 0; i < ARRAY_SIZE(pol->ops); ++i) {
+		op = &pol->ops[i];
+
+		list_for_each_safe(l_ptr, l_next, &op->rules) {
+			ptr = list_entry(l_ptr, struct ipe_rule, next);
+			list_del(l_ptr);
+			ipe_free_rule(ptr);
+		}
+	}
+
+	kfree(pol->policy_name);
+	kfree(pol);
+}
+
+/**
+ * ipe_alloc_policy: Give a list of tokens representing the first line of the
+ *		     token, attempt to parse it as an IPE policy header, and
+ *		     allocate a policy structure based on those values.
+ * @tokens: List of tokens parsed from the first line of the policy
+ *
+ * Return:
+ * Valid ipe_policy pointer - OK
+ * ERR_PTR(-ENOMEM) - Out of memory
+ * ERR_PTR(-EBADMSG) - Invalid policy syntax
+ */
+static struct ipe_policy *ipe_alloc_policy(struct list_head *tokens)
+{
+	size_t i;
+	int rc = 0;
+	struct token *name = NULL;
+	struct token *ver = NULL;
+	struct ipe_policy *lp = NULL;
+
+	name = list_first_entry(tokens, struct token, next_tok);
+	if (!is_name(name)) {
+		rc = -EBADMSG;
+		goto err;
+	}
+
+	if (list_is_singular(tokens)) {
+		rc = -EBADMSG;
+		goto err;
+	}
+
+	ver = list_next_entry(name, next_tok);
+	if (!is_ver(ver)) {
+		rc = -EBADMSG;
+		goto err;
+	}
+
+	lp = kzalloc(sizeof(*lp), GFP_KERNEL);
+	if (!lp) {
+		rc = -ENOMEM;
+		goto err;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(lp->ops); ++i) {
+		lp->ops[i].def = ipe_action_unset;
+		INIT_LIST_HEAD(&lp->ops[i].rules);
+	}
+
+	lp->policy_name = kstrdup(name->val, GFP_KERNEL);
+	if (!lp->policy_name) {
+		rc = -ENOMEM;
+		goto err;
+	}
+
+	rc = ipe_parse_version(ver->val, &lp->policy_version);
+	if (rc != 0)
+		goto err;
+
+	lp->def = ipe_action_unset;
+
+	return lp;
+err:
+	ipe_free_policy(lp);
+	return ERR_PTR(rc);
+}
+
+/**
+ * ipe_add_rule_for_range: Given a ipe_rule @r, duplicate @r and add the rule
+ *			   to @pol for the operation range @start to @end.
+ * @start: The starting point of the range to add the rule to.
+ * @end: The ending point of the range to add the rule to.
+ * @r: The rule to copy.
+ * @pol: Policy structure to modify with the result.
+ *
+ * This is @start to @end, inclusive. @r is still valid after this function,
+ * and should be freed if appropriate.
+ *
+ * Return:
+ * 0 - OK
+ * Other Errors - See ipe_dup_prop
+ */
+static int ipe_add_rule_for_range(enum ipe_op start, enum ipe_op end,
+				  struct ipe_rule *r, struct ipe_policy *pol)
+{
+	enum ipe_op i;
+	struct ipe_rule *cpy = NULL;
+
+	for (i = start; i <= end; ++i) {
+		cpy = ipe_dup_rule(r);
+		if (IS_ERR(cpy))
+			return PTR_ERR(cpy);
+
+		list_add_tail(&cpy->next, &pol->ops[i].rules);
+	}
+
+	return 0;
+}
+
+/**
+ * ipe_parse_line: Given a list of tokens, attempt to parse it into a rule
+ *		   structure, and add it to the passed-in ipe_policy structure.
+ * @tokens: List of tokens that were parsed.
+ * @pol: Policy structure to modify with the result.
+ *
+ * Return:
+ * 0 - OK
+ * -ENOENT - Unrecognized property
+ * -ENOMEM - Out of memory
+ * Other Errors - See ipe_dup_prop
+ */
+static int ipe_parse_line(struct list_head *tokens,
+			  struct ipe_policy *pol)
+{
+	int rc = 0;
+	struct token *f;
+	enum ipe_op i = ipe_op_max;
+	struct ipe_rule *rule = NULL;
+
+	f = list_first_entry(tokens, struct token, next_tok);
+
+	switch (f->key_priority) {
+	case default_action:
+		rc = ipe_parse_default(tokens, pol);
+		break;
+	case op:
+		i = ipe_parse_op(f);
+		if (i == ipe_op_max)
+			return -ENOENT;
+
+		if (list_is_singular(tokens))
+			return -EBADMSG;
+
+		rule = ipe_alloc_rule(i, f, tokens);
+		if (IS_ERR(rule)) {
+			rc = PTR_ERR(rule);
+			goto cleanup;
+		}
+
+		if (i == ipe_op_kernel_read) {
+			rc = ipe_add_rule_for_range(ipe_op_firmware,
+						    ipe_op_kmodule, rule, pol);
+			if (rc != 0)
+				goto cleanup;
+		} else {
+			list_add_tail(&rule->next, &pol->ops[i].rules);
+			rule = NULL;
+		}
+		break;
+	default:
+		return -ENOENT;
+	}
+
+cleanup:
+	ipe_free_rule(rule);
+	return rc;
+}
+
+/**
+ * ipe_check_policy_defaults: Ensure all defaults in policy are set
+ *	for every operation known to IPE.
+ *
+ * @p: Policy to check the defaults.
+ *
+ * Return:
+ * 0 - OK
+ * -EBADMSG - A default was left unset.
+ */
+static int ipe_check_policy_defaults(const struct ipe_policy *p)
+{
+	size_t i;
+
+	if (p->def == ipe_action_unset) {
+		for (i = 0; i < ARRAY_SIZE(p->ops); ++i) {
+			if (p->ops[i].def == ipe_action_unset)
+				return -EBADMSG;
+		}
+	}
+
+	return 0;
+}
+
+/**
+ * ipe_parse_policy: Given a string, parse the string into an IPE policy
+ *		     structure.
+ * @policy: NULL terminated string to parse.
+ *
+ * This function will modify @policy, callers should pass a copy if this
+ * value is needed later.
+ *
+ * Return:
+ * Valid ipe_policy structure - OK
+ * ERR_PTR(-EBADMSG) - Invalid Policy Syntax (Unrecoverable)
+ * ERR_PTR(-ENOMEM) - Out of Memory
+ */
+struct ipe_policy *ipe_parse_policy(char *policy)
+{
+	int rc = 0;
+	size_t i = 1;
+	char *p = NULL;
+	LIST_HEAD(t_list);
+	struct ipe_policy *local_p = NULL;
+
+	while ((p = strsep(&policy, "\n\0")) != NULL) {
+		rc = ipe_tokenize_line(p, &t_list);
+		if (rc == -ENOENT) {
+			++i;
+			continue;
+		}
+		if (rc != 0)
+			goto err;
+
+		if (!local_p) {
+			local_p = ipe_alloc_policy(&t_list);
+			if (IS_ERR(local_p)) {
+				rc = PTR_ERR(local_p);
+				goto err;
+			}
+		} else {
+			rc = ipe_parse_line(&t_list, local_p);
+			if (rc) {
+				pr_warn("failed to parse line %zu", i);
+				goto err;
+			}
+		}
+
+		ipe_free_token_list(&t_list);
+		++i;
+	}
+
+	rc = ipe_check_policy_defaults(local_p);
+	if (rc != 0)
+		goto err;
+
+	return local_p;
+err:
+	ipe_free_token_list(&t_list);
+	ipe_free_policy(local_p);
+	return ERR_PTR(rc);
+}
diff --git a/security/ipe/ipe-parse.h b/security/ipe/ipe-parse.h
new file mode 100644
index 000000000000..b1a9bbd97534
--- /dev/null
+++ b/security/ipe/ipe-parse.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) Microsoft Corporation. All rights reserved.
+ */
+
+#include "ipe-policy.h"
+
+#include <linux/types.h>
+
+#ifndef IPE_PARSE_H
+#define IPE_PARSE_H
+
+struct ipe_policy *ipe_parse_policy(char *policy);
+
+void ipe_free_policy(struct ipe_policy *pol);
+
+#endif /* IPE_AUDIT_H */
diff --git a/security/ipe/ipe-policy.c b/security/ipe/ipe-policy.c
new file mode 100644
index 000000000000..74535fb03666
--- /dev/null
+++ b/security/ipe/ipe-policy.c
@@ -0,0 +1,149 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) Microsoft Corporation. All rights reserved.
+ */
+
+#include "ipe.h"
+#include "ipe-secfs.h"
+#include "ipe-policy.h"
+#include "ipe-parse.h"
+#include "ipe-audit.h"
+
+#include <linux/types.h>
+#include <linux/errno.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/lockdep.h>
+#include <linux/fs.h>
+#include <linux/security.h>
+#include <linux/rcupdate.h>
+
+#define VER_TO_UINT64(_major, _minor, _rev) \
+	((((((u64)(_major)) << 16) | ((u64)(_minor))) << 16) | ((u64)(_rev)))
+
+/**
+ * ipe_is_version_allowed: Determine if @new has a greater or equal
+ *			   policy version than @old.
+ * @old: The policy to compare against.
+ * @new: The policy staged to replace @old.
+ *
+ * Return:
+ * true - @new has a policy version >= than @old
+ * false - @new does not have a policy version >= than @old
+ */
+static bool ipe_is_version_allowed(const struct ipe_pol_ver *old,
+				   const struct ipe_pol_ver *new)
+{
+	u64 old_ver = VER_TO_UINT64(old->major, old->minor, old->rev);
+	u64 new_ver = VER_TO_UINT64(new->major, new->minor, new->rev);
+
+	return new_ver >= old_ver;
+}
+
+/**
+ * ipe_is_valid_policy: determine if @old is allowed to replace @new.
+ * @old: policy that the @new is supposed to replace. Can be NULL.
+ * @new: the policy that is supposed to replace @new.
+ *
+ * Return:
+ * true - @new can replace @old
+ * false - @new cannot replace @old
+ */
+bool ipe_is_valid_policy(const struct ipe_policy *old,
+			 const struct ipe_policy *new)
+{
+	if (old)
+		return ipe_is_version_allowed(&old->policy_version,
+					      &new->policy_version);
+	return true;
+}
+
+/**
+ * ipe_is_active_policy: Determine if @policy is the currently active policy.
+ * @policy: Policy to check if it's the active policy.
+ *
+ * Return:
+ * true - @policy is the active policy
+ * false - @policy is not the active policy
+ */
+bool ipe_is_active_policy(const struct ipe_policy *policy)
+{
+	return rcu_access_pointer(ipe_active_policy) == policy;
+}
+
+/**
+ * ipe_update_active_policy: Determine if @old is the active policy, and update
+ *			     the active policy if necessary.
+ * @old: The previous policy that the update is trying to replace.
+ * @new: The new policy attempting to replace @old.
+ *
+ * If @old is not the active policy, nothing will be done.
+ *
+ * Return:
+ * 0 - OK
+ * -EBADMSG - Invalid Policy
+ */
+int ipe_update_active_policy(const struct ipe_policy *old,
+			     const struct ipe_policy *new)
+{
+	const struct ipe_policy *curr = NULL;
+
+	lockdep_assert_held(&ipe_policy_lock);
+
+	/* no active policy, safe to ignore */
+	if (!rcu_access_pointer(ipe_active_policy))
+		return 0;
+
+	curr = rcu_dereference_protected(ipe_active_policy,
+					 lockdep_is_held(&ipe_policy_lock));
+
+	if (curr == old) {
+		if (!ipe_is_valid_policy(curr, new))
+			return -EINVAL;
+
+		ipe_audit_policy_activation(new);
+
+		(void)rcu_replace_pointer(ipe_active_policy, new,
+					  lockdep_is_held(&ipe_policy_lock));
+	}
+
+	return 0;
+}
+
+/**
+ * ipe_activate_policy: Set a specific policy as the active policy.
+ * @pol: The policy to set as the active policy.
+ *
+ * This is only called by the securityfs entry,
+ *	"$securityfs/ipe/policies/$policy_name/active".
+ *
+ * Return:
+ * 0 - OK
+ * -EINVAL - Policy that is being activated is lower in version than
+ *	     currently running policy.
+ */
+int ipe_activate_policy(const struct ipe_policy *pol)
+{
+	const struct ipe_policy *curr = NULL;
+
+	lockdep_assert_held(&ipe_policy_lock);
+
+	curr = rcu_dereference_protected(ipe_active_policy,
+					 lockdep_is_held(&ipe_policy_lock));
+
+	/*
+	 * User-set policies must be >= to current running policy.
+	 */
+	if (!ipe_is_valid_policy(curr, pol))
+		return -EINVAL;
+
+	ipe_audit_policy_activation(pol);
+
+	/* cleanup of this pointer is handled by the secfs removal */
+	(void)rcu_replace_pointer(ipe_active_policy, pol,
+				  lockdep_is_held(&ipe_policy_lock));
+
+	return 0;
+}
diff --git a/security/ipe/ipe-policy.h b/security/ipe/ipe-policy.h
index c0c9f2962c92..b3ef3d3d2e7f 100644
--- a/security/ipe/ipe-policy.h
+++ b/security/ipe/ipe-policy.h
@@ -14,9 +14,6 @@
 #ifndef IPE_POLICY_H
 #define IPE_POLICY_H
 
-#define IPE_HEADER_POLICY_NAME		"policy_name"
-#define IPE_HEADER_POLICY_VERSION	"policy_version"
-
 extern const char *const ipe_boot_policy;
 extern const struct ipe_policy *ipe_active_policy;
 
@@ -59,4 +56,14 @@ struct ipe_policy {
 	struct ipe_rule_table ops[ipe_op_max - 1];
 };
 
+bool ipe_is_valid_policy(const struct ipe_policy *old,
+			 const struct ipe_policy *new);
+
+bool ipe_is_active_policy(const struct ipe_policy *policy);
+
+int ipe_update_active_policy(const struct ipe_policy *old,
+			     const struct ipe_policy *new);
+
+int ipe_activate_policy(const struct ipe_policy *policy);
+
 #endif /* IPE_POLICY_H */
diff --git a/security/ipe/ipe-prop-internal.h b/security/ipe/ipe-prop-internal.h
index 95a2081e77ee..7c14b204be13 100644
--- a/security/ipe/ipe-prop-internal.h
+++ b/security/ipe/ipe-prop-internal.h
@@ -10,9 +10,19 @@
 #ifndef IPE_PROPERTY_INTERNAL_H
 #define IPE_PROPERTY_INTERNAL_H
 
-#define IPE_PROPERTY_OPERATION	"op"
-#define IPE_PROPERTY_DEFAULT	"DEFAULT"
-#define IPE_PROPERTY_ACTION	"action"
+/* built-in tokens */
+#define IPE_HEADER_POLICY_NAME		"policy_name"
+#define IPE_HEADER_POLICY_VERSION	"policy_version"
+#define IPE_PROPERTY_OPERATION		"op"
+#define IPE_PROPERTY_DEFAULT		"DEFAULT"
+#define IPE_PROPERTY_ACTION		"action"
+
+/* Version strings for built-in tokens */
+#define IPE_PROPERTY_OPERATION_VER	IPE_PROPERTY_OPERATION		"=1"
+#define IPE_PROPERTY_ACTION_VER		IPE_PROPERTY_ACTION		"=1"
+#define IPE_PROPERTY_DEFAULT_VER	IPE_PROPERTY_DEFAULT		"=1"
+#define IPE_HEADER_POLICY_NAME_VER	IPE_HEADER_POLICY_NAME		"=1"
+#define IPE_HEADER_POLICY_VERSION_VER	IPE_HEADER_POLICY_VERSION	"=1"
 
 #define IPE_OP_EXECUTE		"EXECUTE"
 #define IPE_OP_FIRMWARE		"FIRMWARE"
@@ -23,6 +33,8 @@
 #define IPE_OP_KMODULE		"KMODULE"
 #define IPE_OP_KERNEL_READ	"KERNEL_READ"
 
+#define IPE_UNKNOWN		"UNKNOWN"
+
 struct ipe_prop_reg {
 	struct rb_node node;
 	const struct ipe_property *prop;
diff --git a/security/ipe/ipe-property.c b/security/ipe/ipe-property.c
index d4b0283f86bd..262da9f622d6 100644
--- a/security/ipe/ipe-property.c
+++ b/security/ipe/ipe-property.c
@@ -12,7 +12,7 @@
 #include <linux/slab.h>
 
 /* global root containing all registered properties */
-struct rb_root ipe_registry_root = RB_ROOT;
+static struct rb_root ipe_registry_root = RB_ROOT;
 
 /**
  * reg_lookup: Attempt to find a `prop_reg` structure with property_name @key.
@@ -70,7 +70,8 @@ const struct ipe_property *ipe_lookup_prop(const char *key)
  * the system, after calling ipe_register_property.
  *
  * All necessary properties need to be loaded via this method before
- * loading a policy, otherwise the properties will be ignored as unknown.
+ * loading a policy, otherwise the marked as unknown, and cause parsing to
+ * fail.
  *
  * Return:
  * 0 - OK
@@ -113,9 +114,9 @@ int ipe_register_property(const struct ipe_property *prop)
 
 /**
  * ipe_for_each_prop: Iterate over all currently-registered properties
- *	calling @fn on the values, and providing @view @ctx.
+ *		      calling @fn on the values, and providing @view @ctx.
  * @view: The function to call for each property. This is given the property
- *	structure as the first argument, and @ctx as the second.
+ *	  structure as the first argument, and @ctx as the second.
  * @ctx: caller-specified context that is passed to the function. Can be NULL.
  *
  * Return:
diff --git a/security/ipe/ipe-property.h b/security/ipe/ipe-property.h
index cf570d52d0d2..8bb2e2c1619c 100644
--- a/security/ipe/ipe-property.h
+++ b/security/ipe/ipe-property.h
@@ -86,6 +86,7 @@ typedef void (*ipe_free_value)(void **value);
 
 struct ipe_property {
 	const char			*const property_name;
+	u16				version;
 	ipe_property_evaluator		eval;
 	ipe_property_audit		rule_audit;
 	ipe_ctx_audit			ctx_audit;
diff --git a/security/ipe/ipe-secfs.c b/security/ipe/ipe-secfs.c
new file mode 100644
index 000000000000..006619598d57
--- /dev/null
+++ b/security/ipe/ipe-secfs.c
@@ -0,0 +1,1309 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) Microsoft Corporation. All rights reserved.
+ */
+
+#include "ipe.h"
+#include "ipe-parse.h"
+#include "ipe-secfs.h"
+#include "ipe-policy.h"
+#include "ipe-audit.h"
+
+#include <linux/types.h>
+#include <linux/security.h>
+#include <linux/fs.h>
+#include <linux/rcupdate.h>
+#include <linux/mutex.h>
+#include <linux/init.h>
+#include <linux/dcache.h>
+#include <linux/namei.h>
+#include <linux/verification.h>
+#include <linux/capability.h>
+
+#define IPE_ROOT		"ipe"
+#define IPE_POLICIES		"policies"
+#define NEW_POLICY		"new_policy"
+#define IPE_PROPERTY_CFG	"property_config"
+#define IPE_SUCCESS_AUDIT	"success_audit"
+#define IPE_ENFORCE		"enforce"
+
+#define IPE_FULL_CONTENT	"raw"
+#define IPE_INNER_CONTENT	"content"
+#define IPE_ACTIVE_POLICY	"active"
+#define IPE_DELETE_POLICY	"delete"
+
+struct ipe_policy_node {
+	u8		*data;
+	size_t		data_len;
+	const u8	*content;
+	size_t		content_size;
+
+	struct ipe_policy *parsed;
+};
+
+/* root directory */
+static struct dentry *securityfs_root __ro_after_init;
+
+/* subdirectory containing policies */
+static struct dentry *policies_root __ro_after_init;
+
+/* boot policy */
+static struct dentry *boot_policy_node __ro_after_init;
+
+/* top-level IPE commands */
+static struct dentry *new_policy_node __ro_after_init;
+static struct dentry *property_cfg_node __ro_after_init;
+static struct dentry *enforce_node __ro_after_init;
+static struct dentry *success_audit_node __ro_after_init;
+
+/* lock for synchronizing writers across ipe policy */
+DEFINE_MUTEX(ipe_policy_lock);
+
+/**
+ * get_int_user - retrieve a single integer from a string located in userspace.
+ * @data: usespace address to parse for an integer
+ * @len: length of @data
+ * @offset: offset into @data. Unused.
+ * @value: pointer to a value to propagate with the result
+ *
+ * Return:
+ * 0 - OK
+ * -ENOMEM - allocation failed
+ * -EINVAL - more than 1 integer was present
+ * Other - see strnpy_from_user
+ */
+static int get_int_user(const char __user *data, size_t len, loff_t *offset,
+			int *value)
+{
+	int rc = 0;
+	char *buffer = NULL;
+
+	buffer = kzalloc(len + 1, GFP_KERNEL);
+	if (!buffer)
+		return -ENOMEM;
+
+	rc = strncpy_from_user(buffer, data, len + 1);
+	if (rc < 0)
+		goto out;
+
+	rc = kstrtoint(buffer, 10, value);
+out:
+	kfree(buffer);
+	return rc;
+}
+
+/**
+ * ipe_get_audit_mode - retrieve the current value of the success_audit flag
+ *			as a string representation.
+ * @f: The file structure representing the securityfs entry. Unused.
+ * @data: userspace buffer to place the result
+ * @len: length of @data
+ * @offset: offset into @data
+ *
+ * This is the handler for the 'read' syscall on the securityfs node,
+ * ipe/success_audit
+ *
+ * Return:
+ * > 0 - OK
+ * < 0 - Error, see simple_read_from_buffer
+ */
+static ssize_t ipe_get_audit_mode(struct file *f, char __user *data, size_t len,
+				  loff_t *offset)
+{
+	char tmp[3] = { 0 };
+
+	snprintf(tmp, ARRAY_SIZE(tmp), "%c\n", (ipe_success_audit) ? '1' : '0');
+
+	return simple_read_from_buffer(data, len, offset, tmp,
+				       ARRAY_SIZE(tmp));
+}
+
+/**
+ * ipe_set_audit_mode - change the value of the ipe_success_audit flag.
+ * @f: The file structure representing the securityfs entry
+ * @data: userspace buffer containing value to be set. Should be "1" or "0".
+ * @len: length of @data
+ * @offset: offset into @data
+ *
+ * Return:
+ * > 0 - OK
+ * -EPERM - if MAC system available, missing CAP_MAC_ADMIN.
+ * -EINVAL - value written was not "1" or "0".
+ */
+static ssize_t ipe_set_audit_mode(struct file *f, const char __user *data, size_t len,
+				  loff_t *offset)
+{
+	int v = 0;
+	int rc = 0;
+
+	if (!file_ns_capable(f, &init_user_ns, CAP_MAC_ADMIN))
+		return -EPERM;
+
+	rc = get_int_user(data, len, offset, &v);
+	if (rc)
+		return rc;
+
+	if (v != 0 && v != 1)
+		return -EINVAL;
+
+	ipe_success_audit = v == 1;
+
+	return len;
+}
+
+static const struct file_operations audit_ops = {
+	.read = ipe_get_audit_mode,
+	.write = ipe_set_audit_mode
+};
+
+#ifdef CONFIG_SECURITY_IPE_PERMISSIVE_SWITCH
+
+/**
+ * ipe_get_enforce - retrieve the current value of the ipe_enforce flag
+ *		     as a string representation.
+ * @f: The file structure representing the securityfs entry. Unused.
+ * @data: userspace buffer to place the result
+ * @len: length of @data
+ * @offset: offset into @data
+ *
+ * This is the handler for the 'read' syscall on the securityfs node,
+ * ipe/enforce
+ *
+ * Return:
+ * > 0 - OK
+ * < 0 - Error, see simple_read_from_buffer
+ */
+static ssize_t ipe_get_enforce(struct file *f, char __user *data, size_t len,
+			       loff_t *offset)
+{
+	char tmp[3] = { 0 };
+
+	snprintf(tmp, ARRAY_SIZE(tmp), "%c\n", (ipe_enforce) ? '1' : '0');
+
+	return simple_read_from_buffer(data, len, offset, tmp,
+				       ARRAY_SIZE(tmp));
+}
+
+/**
+ * ipe_set_enforce - change the value of the ipe_enforce flag.
+ * @f: The file structure representing the securityfs entry
+ * @data: userspace buffer containing value to be set. Should be "1" or "0".
+ * @len: length of @data
+ * @offset: offset into @data
+ *
+ * Return:
+ * > 0 - OK
+ * -EPERM - if MAC system available, missing CAP_MAC_ADMIN.
+ * -EINVAL - value written was not "1" or "0".
+ */
+static ssize_t ipe_set_enforce(struct file *f, const char __user *data, size_t len,
+			       loff_t *offset)
+{
+	int v = 0;
+	int rc = 0;
+	bool ret = 0;
+
+	if (!file_ns_capable(f, &init_user_ns, CAP_MAC_ADMIN))
+		return -EPERM;
+
+	rc = get_int_user(data, len, offset, &v);
+	if (rc)
+		return rc;
+
+	if (v != 0 && v != 1)
+		return -EINVAL;
+
+	ret = v == 1;
+
+	if (ret != ipe_enforce)
+		ipe_audit_mode(ret);
+
+	ipe_enforce = ret;
+
+	return len;
+}
+
+static const struct file_operations enforce_ops = {
+	.read = ipe_get_enforce,
+	.write = ipe_set_enforce
+};
+
+/**
+ * ipe_init_enforce_node - Wrapper around securityfs_create_file for the
+ *			   ipe/enforce securityfs node.
+ * @root: securityfs node that is the parent of the new node to be created
+ *
+ * This allows this function to be no-op'd when the permissive switch is
+ * disabled.
+ *
+ * Return:
+ * See securityfs_create_file.
+ */
+static inline struct dentry *ipe_init_enforce_node(struct dentry *root)
+{
+	return securityfs_create_file(IPE_ENFORCE, 0644, root, NULL,
+				      &enforce_ops);
+}
+
+#else
+
+/**
+ * ipe_init_enforce_node - Wrapper around securityfs_create_file for the
+ *			   ipe/enforce securityfs node.
+ * @root: Unused
+ *
+ * This allows this function to be no-op'd when the permissive switch is
+ * disabled.
+ *
+ * Return:
+ * NULL.
+ */
+static inline struct dentry *ipe_init_enforce_node(struct dentry *root)
+{
+	return NULL;
+}
+
+#endif /* CONFIG_SECURITY_IPE_PERMISSIVE_SWITCH */
+
+/**
+ * retrieve_backed_dentry: Retrieve a dentry with a backing inode, identified
+ *			   by @name, under @parent.
+ * @name: Name of the dentry under @parent.
+ * @parent: The parent dentry to search under for @name.
+ * @size: Length of @name.
+ *
+ * This takes a reference to the returned dentry. Caller needs to call dput
+ * to drop the reference.
+ *
+ * Return:
+ * valid dentry - OK
+ * ERR_PTR - Error, see lookup_one_len_unlocked
+ * NULL - No backing inode was found
+ */
+static struct dentry *retrieve_backed_dentry(const char *name,
+					     struct dentry *parent,
+					     size_t size)
+{
+	struct dentry *tmp = NULL;
+
+	tmp = lookup_one_len_unlocked(name, parent, size);
+	if (IS_ERR(tmp))
+		return tmp;
+
+	if (!d_really_is_positive(tmp))
+		return NULL;
+
+	return tmp;
+}
+
+/**
+ * alloc_size_cb: Callback for determining the allocation size of the grammar
+ *		  buffer
+ * @prop: ipe_property structure to determine allocation size
+ * @ctx: void* representing a size_t* to add the allocation size to.
+ *
+ * Return:
+ * 0 - Always
+ */
+static int alloc_size_cb(const struct ipe_property *prop, void *ctx)
+{
+	size_t *ref = ctx;
+	char tmp[6] = { 0 };
+
+	snprintf(tmp, ARRAY_SIZE(tmp), "%d", prop->version);
+
+	/* property_name=u16\n */
+	*ref += strlen(prop->property_name) + strlen(tmp) + 2;
+
+	return 0;
+}
+
+/**
+ * build_cfg_str: Callback to populate the previously-allocated string
+ *		  buffer for ipe's grammar version with the content.
+ * @prop: ipe_property structure to determine allocation size
+ * @ctx: void* representing a char* to append the population to.
+ *
+ * Return:
+ * 0 - Always
+ */
+static int build_cfg_str(const struct ipe_property *prop, void *ctx)
+{
+	char *ref = (char *)ctx;
+	char tmp[6] = { 0 };
+
+	snprintf(tmp, ARRAY_SIZE(tmp), "%d", prop->version);
+	strcat(ref, prop->property_name);
+	strcat(ref, "=");
+	strcat(ref, tmp);
+	strcat(ref, "\n");
+
+	return 0;
+}
+
+/**
+ * create_new_prop_cfg: create a new property configuration string for consumers
+ *			of IPE policy.
+ *
+ * This function will iterate over all currently registered properties, and
+ * return a string of form:
+ *
+ *	property1=version1\n
+ *	property2=version2\n
+ *	...
+ *	propertyN=versionN
+ *
+ * Where propertyX is the property_name and versionX is the version associated.
+ *
+ * Return:
+ * !ERR_PTR - Success
+ * ERR_PTR(-ENOMEM) - Allocation Failed
+ */
+static char *create_new_prop_cfg(void)
+{
+	size_t i;
+	ssize_t rc = 0;
+	size_t alloc = 0;
+	char *ret = NULL;
+	const char *const built_ins[] = {
+		IPE_PROPERTY_OPERATION_VER,
+		IPE_PROPERTY_ACTION_VER,
+		IPE_PROPERTY_DEFAULT_VER,
+		IPE_HEADER_POLICY_NAME_VER,
+		IPE_HEADER_POLICY_VERSION_VER
+	};
+
+	for (i = 0; i < ARRAY_SIZE(built_ins); ++i)
+		alloc += strlen(built_ins[i]) + 1; /* \n */
+
+	(void)ipe_for_each_prop(alloc_size_cb, (void *)&alloc);
+	++alloc; /* null for strcat */
+
+	ret = kzalloc(alloc, GFP_KERNEL);
+	if (!ret)
+		return ERR_PTR(-ENOMEM);
+
+	for (i = 0; i < ARRAY_SIZE(built_ins); ++i) {
+		strcat(ret, built_ins[i]);
+		strcat(ret,  "\n");
+	}
+
+	rc = ipe_for_each_prop(build_cfg_str, (void *)ret);
+	if (rc)
+		goto err;
+
+	return ret;
+err:
+	kfree(ret);
+	return ERR_PTR(rc);
+}
+
+/**
+ * ipe_get_prop_cfg: Get (or allocate if one does not exist) the property
+ *		     configuration string for IPE.
+ *
+ * @f: File representing the securityfs entry.
+ * @data: User mode buffer to place the configuration string.
+ * @len: Length of @data.
+ * @offset: Offset into @data.
+ *
+ * As this string can only change on a new kernel build, this string
+ * is cached in the i_private field of @f's inode for subsequent calls.
+ *
+ * Return:
+ * < 0 - Error
+ * > 0 - Success, bytes written to @data
+ */
+static ssize_t ipe_get_prop_cfg(struct file *f, char __user *data, size_t size,
+				loff_t *offset)
+{
+	ssize_t rc = 0;
+	const char *cfg = NULL;
+	struct inode *grammar = d_inode(property_cfg_node);
+
+	inode_lock(grammar);
+
+	/*
+	 * This can only change with a new kernel build,
+	 * so cache the result in i->private
+	 */
+	if (IS_ERR_OR_NULL(grammar->i_private)) {
+		grammar->i_private = create_new_prop_cfg();
+		if (IS_ERR(grammar->i_private)) {
+			rc = PTR_ERR(grammar->i_private);
+			goto out;
+		}
+	}
+	cfg = (const char *)grammar->i_private;
+
+	rc = simple_read_from_buffer(data, size, offset, cfg, strlen(cfg));
+
+out:
+	inode_unlock(grammar);
+	return rc;
+}
+
+static const struct file_operations prop_cfg_ops = {
+	.read = ipe_get_prop_cfg
+};
+
+/**
+ * ipe_free_policy_node: Free an ipe_policy_node structure allocated by
+ *			 ipe_alloc_policy_node.
+ * @n: ipe_policy_node to free
+ */
+static void ipe_free_policy_node(struct ipe_policy_node *n)
+{
+	if (IS_ERR_OR_NULL(n))
+		return;
+
+	ipe_free_policy(n->parsed);
+	kfree(n->data);
+
+	kfree(n);
+}
+
+/**
+ * alloc_callback: Callback given to verify_pkcs7_signature function to set
+ *		   the inner content reference and parse the policy.
+ * @ctx: "ipe_policy_node" to set inner content, size and parsed policy of.
+ * @data: Start of PKCS#7 inner content.
+ * @len: Length of @data.
+ * @asn1hdrlen: Unused.
+ *
+ * Return:
+ * 0 - OK
+ * ERR_PTR(-EBADMSG) - Invalid policy syntax
+ * ERR_PTR(-ENOMEM) - Out of memory
+ */
+static int alloc_callback(void *ctx, const void *data, size_t len,
+			  size_t asn1hdrlen)
+{
+	char *cpy = NULL;
+	struct ipe_policy *pol = NULL;
+	struct ipe_policy_node *n = (struct ipe_policy_node *)ctx;
+
+	n->content = (const u8 *)data;
+	n->content_size = len;
+
+	if (len == 0)
+		return -EBADMSG;
+
+	cpy = kzalloc(len + 1, GFP_KERNEL);
+	if (!cpy)
+		return -ENOMEM;
+
+	(void)memcpy(cpy, data, len);
+
+	pol = ipe_parse_policy(cpy);
+	if (IS_ERR(pol)) {
+		kfree(cpy);
+		return PTR_ERR(pol);
+	}
+
+	n->parsed = pol;
+	kfree(cpy);
+	return 0;
+}
+
+/**
+ * ipe_delete_policy_tree - delete the policy subtree under
+ *			    $securityfs/ipe/policies.
+ * @policy_root: the policy root directory, i.e.
+ *		 $securityfs/ipe/policies/$policy_name
+ *
+ * Return:
+ * 0 - OK
+ * -EPERM - Tree being deleted is the active policy
+ * -ENOENT - A subnode is missing under the tree.
+ * Other - see lookup_one_len_unlocked.
+ */
+static int ipe_delete_policy_tree(struct dentry *policy_root)
+{
+	int rc = 0;
+	struct dentry *raw = NULL;
+	struct dentry *active = NULL;
+	struct dentry *content = NULL;
+	struct dentry *delete = NULL;
+	const struct ipe_policy_node *target = NULL;
+
+	/* ensure the active policy cannot be changed */
+	lockdep_assert_held(&ipe_policy_lock);
+
+	/* fail if it's the active policy */
+	target = (const struct ipe_policy_node *)d_inode(policy_root)->i_private;
+	if (ipe_is_active_policy(target->parsed)) {
+		rc = -EPERM;
+		goto out;
+	}
+
+	raw = retrieve_backed_dentry(IPE_FULL_CONTENT, policy_root,
+				     strlen(IPE_FULL_CONTENT));
+	if (IS_ERR_OR_NULL(raw)) {
+		rc = IS_ERR(raw) ? PTR_ERR(raw) : -ENOENT;
+		goto out;
+	}
+
+	content = retrieve_backed_dentry(IPE_INNER_CONTENT, policy_root,
+					 strlen(IPE_INNER_CONTENT));
+	if (IS_ERR_OR_NULL(content)) {
+		rc = IS_ERR(content) ? PTR_ERR(content) : -ENOENT;
+		goto out_free_raw;
+	}
+
+	active = retrieve_backed_dentry(IPE_ACTIVE_POLICY, policy_root,
+					strlen(IPE_ACTIVE_POLICY));
+	if (IS_ERR_OR_NULL(active)) {
+		rc = IS_ERR(active) ? PTR_ERR(active) : -ENOENT;
+		goto out_free_content;
+	}
+
+	delete = retrieve_backed_dentry(IPE_DELETE_POLICY, policy_root,
+					strlen(IPE_DELETE_POLICY));
+	if (IS_ERR_OR_NULL(active)) {
+		rc = IS_ERR(active) ? PTR_ERR(active) : -ENOENT;
+		goto out_free_active;
+	}
+
+	inode_lock(d_inode(policy_root));
+	ipe_free_policy_node(d_inode(policy_root)->i_private);
+	d_inode(policy_root)->i_private = NULL;
+	inode_unlock(d_inode(policy_root));
+
+	/* drop references from acquired in this function */
+	dput(raw);
+	dput(content);
+	dput(policy_root);
+	dput(active);
+	dput(delete);
+
+	/* drop securityfs' references */
+	securityfs_remove(raw);
+	securityfs_remove(content);
+	securityfs_remove(policy_root);
+	securityfs_remove(active);
+	securityfs_remove(delete);
+
+	return rc;
+
+out_free_active:
+	dput(active);
+out_free_content:
+	dput(content);
+out_free_raw:
+	dput(raw);
+out:
+	return rc;
+}
+
+/**
+ * ipe_delete_policy: Delete a policy, which is stored in this file's parent
+ *		      dentry's inode.
+ * @f: File representing the securityfs entry.
+ * @data: Buffer containing the value 1.
+ * @len: sizeof(u8).
+ * @offset: Offset into @data.
+ *
+ * Return:
+ * > 0 - OK
+ * -ENOMEM - Out of memory
+ * -EINVAL - Incorrect parameter
+ * -EPERM - Policy is active
+ * -ENOENT - A policy subnode does not exist
+ * -EPERM - if a MAC subsystem is enabled, missing CAP_MAC_ADMIN
+ * Other - See retrieve_backed_dentry
+ */
+static ssize_t ipe_delete_policy(struct file *f, const char __user *data,
+				 size_t len, loff_t *offset)
+{
+	int v = 0;
+	ssize_t rc = 0;
+	struct inode *policy_i = NULL;
+	struct dentry *policy_root = NULL;
+
+	if (!file_ns_capable(f, &init_user_ns, CAP_MAC_ADMIN))
+		return -EPERM;
+
+	rc = get_int_user(data, len, offset, &v);
+	if (rc)
+		return rc;
+
+	if (v != 1)
+		return -EINVAL;
+
+	policy_root = f->f_path.dentry->d_parent;
+	policy_i = d_inode(policy_root);
+
+	if (!policy_i->i_private)
+		return -ENOENT;
+
+	/* guarantee active policy cannot change */
+	mutex_lock(&ipe_policy_lock);
+
+	rc = ipe_delete_policy_tree(policy_root);
+	if (rc)
+		goto out_unlock;
+
+	mutex_unlock(&ipe_policy_lock);
+	synchronize_rcu();
+
+	return len;
+
+out_unlock:
+	mutex_unlock(&ipe_policy_lock);
+	return rc;
+}
+
+static const struct file_operations policy_delete_ops = {
+	.write = ipe_delete_policy
+};
+
+/**
+ * ipe_alloc_policy_node: Allocate a new ipe_policy_node structure.
+ * @data: Raw enveloped PKCS#7 data that represents the policy.
+ * @len: Length of @data.
+ *
+ * Return:
+ * valid ipe_policy_node - OK
+ * ERR_PTR(-EBADMSG) - Invalid policy syntax
+ * ERR_PTR(-ENOMEM) - Out of memory
+ */
+static struct ipe_policy_node *ipe_alloc_policy_node(const u8 *data,
+						     size_t len)
+{
+	int rc = 0;
+	struct ipe_policy_node *node = NULL;
+
+	node = kzalloc(sizeof(*node), GFP_KERNEL);
+	if (!node) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
+	node->data_len = len;
+	node->data = kmemdup(data, len, GFP_KERNEL);
+	if (!node->data) {
+		rc = -ENOMEM;
+		goto out2;
+	}
+
+	rc = verify_pkcs7_signature(node->content, node->content_size,
+				    node->data, node->data_len, NULL,
+				    VERIFYING_UNSPECIFIED_SIGNATURE,
+				    alloc_callback, node);
+	if (rc != 0)
+		goto out2;
+
+	return node;
+out2:
+	ipe_free_policy_node(node);
+out:
+	return ERR_PTR(rc);
+}
+
+/**
+ * ipe_read_policy: Read the raw content (full enveloped PKCS7) data of
+ *			the policy stored within the file's parent inode.
+ * @f: File representing the securityfs entry.
+ * @data: User mode buffer to place the raw pkcs7.
+ * @len: Length of @data.
+ * @offset: Offset into @data.
+ *
+ * Return:
+ * > 0 - OK
+ * -ENOMEM - Out of memory
+ */
+static ssize_t ipe_read_policy(struct file *f, char __user *data,
+			       size_t size, loff_t *offset)
+{
+	ssize_t rc = 0;
+	size_t avail = 0;
+	struct inode *root = NULL;
+	const struct ipe_policy_node *node = NULL;
+
+	root = d_inode(f->f_path.dentry->d_parent);
+
+	inode_lock_shared(root);
+	node = (const struct ipe_policy_node *)root->i_private;
+
+	avail = node->data_len;
+	rc = simple_read_from_buffer(data, size, offset, node->data, avail);
+
+	inode_unlock_shared(root);
+	return rc;
+}
+
+/**
+ * ipe_update_policy: Update a policy in place with a new PKCS7 policy.
+ * @f: File representing the securityfs entry.
+ * @data: Buffer user mode to place the raw pkcs7.
+ * @len: Length of @data.
+ * @offset: Offset into @data.
+ *
+ * Return:
+ * 0 - OK
+ * -EBADMSG - Invalid policy format
+ * -ENOMEM - Out of memory
+ * -EPERM - if a MAC subsystem is enabled, missing CAP_MAC_ADMIN
+ * -EINVAL - Incorrect policy name for this node, or version is < current
+ */
+static ssize_t ipe_update_policy(struct file *f, const char __user *data,
+				 size_t len, loff_t *offset)
+{
+	ssize_t rc = 0;
+	u8 *cpy = NULL;
+	struct inode *root = NULL;
+	struct crypto_shash *tfm = NULL;
+	struct ipe_policy_node *new = NULL;
+	struct ipe_policy_node *old = NULL;
+
+	if (!file_ns_capable(f, &init_user_ns, CAP_MAC_ADMIN))
+		return -EPERM;
+
+	cpy = memdup_user(data, len);
+	if (IS_ERR(cpy))
+		return PTR_ERR(cpy);
+
+	new = ipe_alloc_policy_node(cpy, len);
+	if (IS_ERR(new)) {
+		rc = PTR_ERR(new);
+		goto out_free_cpy;
+	}
+
+	tfm = crypto_alloc_shash("sha1", 0, 0);
+	if (IS_ERR(tfm))
+		goto out_free_cpy;
+
+	root = d_inode(f->f_path.dentry->d_parent);
+	inode_lock(root);
+	mutex_lock(&ipe_policy_lock);
+
+	old = (struct ipe_policy_node *)root->i_private;
+
+	if (strcmp(old->parsed->policy_name, new->parsed->policy_name)) {
+		rc = -EINVAL;
+		goto out_unlock_inode;
+	}
+
+	if (!ipe_is_valid_policy(old->parsed, new->parsed)) {
+		rc = -EINVAL;
+		goto out_unlock_inode;
+	}
+
+	rc = ipe_update_active_policy(old->parsed, new->parsed);
+	if (rc != 0)
+		goto out_unlock_inode;
+
+	ipe_audit_policy_load(new->parsed, new->data, new->data_len, tfm);
+	swap(root->i_private, new);
+
+	mutex_unlock(&ipe_policy_lock);
+	synchronize_rcu();
+
+	inode_unlock(root);
+	kfree(cpy);
+	ipe_free_policy_node(new);
+	crypto_free_shash(tfm);
+
+	return len;
+
+out_unlock_inode:
+	mutex_unlock(&ipe_policy_lock);
+	inode_unlock(root);
+	ipe_free_policy_node(new);
+	crypto_free_shash(tfm);
+out_free_cpy:
+	kfree(cpy);
+	return rc;
+}
+
+static const struct file_operations policy_raw_ops = {
+	.read = ipe_read_policy,
+	.write = ipe_update_policy
+};
+
+/**
+ * ipe_read_content: Read the inner content of the enveloped PKCS7 data,
+ *			 representing the IPE policy.
+ * @f: File representing the securityfs entry.
+ * @data: User mode buffer to place the inner content of the pkcs7 data.
+ * @len: Length of @data.
+ * @offset: Offset into @data.
+ *
+ * Return:
+ * > 0 - OK
+ * -ENOMEM - Out of memory
+ */
+static ssize_t ipe_read_content(struct file *f, char __user *data,
+				size_t size, loff_t *offset)
+{
+	ssize_t rc = 0;
+	size_t avail = 0;
+	struct inode *root = NULL;
+	const struct ipe_policy_node *node = NULL;
+
+	root = d_inode(f->f_path.dentry->d_parent);
+
+	inode_lock_shared(root);
+	node = (const struct ipe_policy_node *)root->i_private;
+
+	avail = node->content_size;
+	rc = simple_read_from_buffer(data, size, offset, node->content, avail);
+
+	inode_unlock_shared(root);
+	return rc;
+}
+
+static const struct file_operations policy_content_ops = {
+	.read = ipe_read_content
+};
+
+/**
+ * ipe_get_active - return a string representation of whether a policy
+ *		    is active.
+ * @f: File struct representing the securityfs node. Unused.
+ * @data: buffer to place the result.
+ * @len: length of @data.
+ * @offset: offset into @data.
+ *
+ * This is the 'read' syscall handler for
+ * $securityfs/ipe/policies/$policy_name/active
+ *
+ * Return:
+ * > 0 - OK
+ * < 0 - see simple_read_from_buffer.
+ */
+static ssize_t ipe_get_active(struct file *f, char __user *data, size_t len,
+			      loff_t *offset)
+{
+	ssize_t rc = 0;
+	char tmp[3] = { 0 };
+	struct inode *root = NULL;
+	const struct ipe_policy_node *node = NULL;
+
+	root = d_inode(f->f_path.dentry->d_parent);
+	inode_lock_shared(root);
+
+	node = (const struct ipe_policy_node *)root->i_private;
+
+	snprintf(tmp, ARRAY_SIZE(tmp), "%c\n",
+		 ipe_is_active_policy(node->parsed) ? '1' : '0');
+
+	rc = simple_read_from_buffer(data, len, offset, tmp,
+				     ARRAY_SIZE(tmp));
+
+	inode_unlock_shared(root);
+
+	return rc;
+}
+
+/**
+ * ipe_set_active - mark a policy as active, causing IPE to start enforcing
+ *		    this policy.
+ * @f: File struct representing the securityfs node.
+ * @data: buffer containing data written to the securityfs node..
+ * @len: length of @data.
+ * @offset: offset into @data.
+ *
+ * This is the 'write' syscall handler for
+ * $securityfs/ipe/policies/$policy_name/active
+ *
+ * Return:
+ * > 0 - OK
+ * -EINVAL - Value written is not "1".
+ * -EPERM - if MAC system is enabled, missing CAP_MAC_ADMIN.
+ * Other - see ipe_activate_policy, get_int_user
+ */
+static ssize_t ipe_set_active(struct file *f, const char __user *data, size_t len,
+			      loff_t *offset)
+{
+	int v = 0;
+	ssize_t rc = 0;
+	struct inode *root = NULL;
+	const struct ipe_policy_node *node = NULL;
+
+	if (!file_ns_capable(f, &init_user_ns, CAP_MAC_ADMIN))
+		return -EPERM;
+
+	rc = get_int_user(data, len, offset, &v);
+	if (rc)
+		return rc;
+
+	if (v != 1)
+		return -EINVAL;
+
+	root = d_inode(f->f_path.dentry->d_parent);
+	mutex_lock(&ipe_policy_lock);
+	inode_lock_shared(root);
+
+	node = (const struct ipe_policy_node *)root->i_private;
+	rc = ipe_activate_policy(node->parsed);
+
+	inode_unlock_shared(root);
+	mutex_unlock(&ipe_policy_lock);
+	synchronize_rcu();
+
+	return len;
+}
+
+static const struct file_operations policy_active_ops = {
+	.read = ipe_get_active,
+	.write = ipe_set_active
+};
+
+/**
+ * ipe_alloc_policy_tree - allocate the proper subnodes for a policy under
+ *			   securityfs.
+ * @parent: The parent directory that these securityfs files should be created
+ *	    under.
+ *
+ * Return:
+ * 0 - OK
+ * !0 - See securityfs_create_file
+ */
+static int ipe_alloc_policy_tree(struct dentry *parent)
+{
+	int rc = 0;
+	struct dentry *raw = NULL;
+	struct dentry *delete = NULL;
+	struct dentry *active = NULL;
+	struct dentry *content = NULL;
+
+	raw = securityfs_create_file(IPE_FULL_CONTENT, 0644, parent, NULL,
+				     &policy_raw_ops);
+	if (IS_ERR(raw))
+		return PTR_ERR(raw);
+
+	content = securityfs_create_file(IPE_INNER_CONTENT, 0444, parent,
+					 NULL, &policy_content_ops);
+	if (IS_ERR(raw)) {
+		rc = PTR_ERR(raw);
+		goto free_raw;
+	}
+
+	active = securityfs_create_file(IPE_ACTIVE_POLICY, 0644, parent, NULL,
+					&policy_active_ops);
+	if (IS_ERR(active)) {
+		rc = PTR_ERR(active);
+		goto free_content;
+	}
+
+	delete = securityfs_create_file(IPE_DELETE_POLICY, 0644, parent, NULL,
+					&policy_delete_ops);
+	if (IS_ERR(delete)) {
+		rc = PTR_ERR(delete);
+		goto free_active;
+	}
+
+	return rc;
+
+free_active:
+	securityfs_remove(active);
+free_content:
+	securityfs_remove(content);
+free_raw:
+	securityfs_remove(raw);
+
+	return rc;
+}
+
+/**
+ * ipe_build_policy_node: Build a new securityfs node for IPE policies.
+ * @data: Raw enveloped PKCS#7 data that represents the policy.
+ * @len: Length of @data.
+ *
+ * Return:
+ * 0 - OK
+ * -EEXIST - Policy already exists
+ * -EBADMSG - Invalid policy syntax
+ * -ENOMEM - Out of memory
+ */
+static int ipe_build_policy_node(const u8 *data, size_t len)
+{
+	int rc = 0;
+	struct dentry *root = NULL;
+	struct inode *root_i = NULL;
+	struct crypto_shash *tfm = NULL;
+	struct ipe_policy_node *node = NULL;
+
+	tfm = crypto_alloc_shash("sha1", 0, 0);
+	if (IS_ERR(tfm)) {
+		rc = PTR_ERR(tfm);
+		goto out;
+	}
+
+	node = ipe_alloc_policy_node(data, len);
+	if (IS_ERR(node)) {
+		rc = PTR_ERR(node);
+		goto free_hash;
+	}
+
+	root = securityfs_create_dir(node->parsed->policy_name,
+				     policies_root);
+	if (IS_ERR(root)) {
+		rc = PTR_ERR(root);
+		goto free_private;
+	}
+
+	root_i = d_inode(root);
+
+	inode_lock(root_i);
+	root_i->i_private = node;
+	ipe_audit_policy_load(node->parsed, node->data, node->data_len, tfm);
+	inode_unlock(root_i);
+
+	rc = ipe_alloc_policy_tree(root);
+	if (rc)
+		goto free_secfs;
+
+	crypto_free_shash(tfm);
+	return rc;
+
+free_secfs:
+	securityfs_remove(root);
+free_private:
+	ipe_free_policy_node(node);
+free_hash:
+	crypto_free_shash(tfm);
+out:
+	return rc;
+}
+
+/**
+ * ipe_new_policy: Entry point of the securityfs node, "ipe/new_policy".
+ * @f: File representing the securityfs entry.
+ * @data: Raw enveloped PKCS#7 data that represents the policy.
+ * @len: Length of @data.
+ * @offset: Offset for @data.
+ *
+ * Return:
+ * > 0 - OK
+ * -EEXIST - Policy already exists
+ * -EBADMSG - Invalid policy syntax
+ * -ENOMEM - Out of memory
+ * -EPERM - if a MAC subsystem is enabled, missing CAP_MAC_ADMIN
+ */
+static ssize_t ipe_new_policy(struct file *f, const char __user *data,
+			      size_t len, loff_t *offset)
+{
+	ssize_t rc = 0;
+	u8 *cpy = NULL;
+
+	if (!file_ns_capable(f, &init_user_ns, CAP_MAC_ADMIN))
+		return -EPERM;
+
+	cpy = memdup_user(data, len);
+	if (IS_ERR(cpy))
+		return PTR_ERR(cpy);
+
+	rc = ipe_build_policy_node(cpy, len);
+
+	kfree(cpy);
+	return rc < 0 ? rc : len;
+}
+
+static const struct file_operations new_policy_ops = {
+	.write = ipe_new_policy
+};
+
+/**
+ * ipe_build_secfs_root: Build the root of securityfs for IPE.
+ *
+ * Return:
+ * 0 - OK
+ * !0 - See securityfs_create_dir and securityfs_create_file
+ */
+static int __init ipe_build_secfs_root(void)
+{
+	int rc = 0;
+	struct dentry *new = NULL;
+	struct dentry *cfg = NULL;
+	struct dentry *root = NULL;
+	struct dentry *audit = NULL;
+	struct dentry *enforce = NULL;
+	struct dentry *policies = NULL;
+
+	root = securityfs_create_dir(IPE_ROOT, NULL);
+	if (IS_ERR(root)) {
+		rc = PTR_ERR(root);
+		goto out;
+	}
+
+	new = securityfs_create_file(NEW_POLICY, 0644, root, NULL,
+				     &new_policy_ops);
+	if (IS_ERR(new)) {
+		rc = PTR_ERR(new);
+		goto out_free_root;
+	}
+
+	policies = securityfs_create_dir(IPE_POLICIES, root);
+	if (IS_ERR(policies)) {
+		rc = PTR_ERR(policies);
+		goto out_free_new;
+	}
+
+	cfg = securityfs_create_file(IPE_PROPERTY_CFG, 0444, root, NULL,
+				     &prop_cfg_ops);
+	if (IS_ERR(cfg)) {
+		rc = PTR_ERR(cfg);
+		goto out_free_policies;
+	}
+
+	audit = securityfs_create_file(IPE_SUCCESS_AUDIT, 0644, root, NULL,
+				       &audit_ops);
+	if (IS_ERR(cfg)) {
+		rc = PTR_ERR(audit);
+		goto out_free_cfg;
+	}
+
+	enforce = ipe_init_enforce_node(root);
+	if (IS_ERR(enforce)) {
+		rc = PTR_ERR(audit);
+		goto out_free_audit;
+	}
+
+	securityfs_root = root;
+	new_policy_node = new;
+	policies_root = policies;
+	property_cfg_node = cfg;
+	success_audit_node = audit;
+	enforce_node = enforce;
+
+	return rc;
+
+out_free_audit:
+	securityfs_remove(audit);
+out_free_cfg:
+	securityfs_remove(cfg);
+out_free_policies:
+	securityfs_remove(policies);
+out_free_new:
+	securityfs_remove(new);
+out_free_root:
+	securityfs_remove(root);
+out:
+	return rc;
+}
+
+/**
+ * ipe_build_boot_node: Build a policy node for IPE's boot policy.
+ *
+ * This differs from the normal policy nodes, as the IPE boot policy is
+ * read only, and only has the 'content' and 'active' nodes (as it is
+ * unsigned).
+ *
+ * Return:
+ * 0 - OK
+ * !0 - See securityfs_create_dir and securityfs_create_file
+ */
+static int __init ipe_build_boot_node(void)
+{
+	int rc = 0;
+	char *cpy = NULL;
+	struct inode *root_i = NULL;
+	struct dentry *root = NULL;
+	struct dentry *active = NULL;
+	struct dentry *content = NULL;
+	struct ipe_policy *parsed = NULL;
+	struct ipe_policy_node *node = NULL;
+
+	if (!ipe_boot_policy)
+		return 0;
+
+	node = kzalloc(sizeof(*node), GFP_KERNEL);
+	if (!node) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
+	cpy = kstrdup(ipe_boot_policy, GFP_KERNEL);
+	if (!cpy) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
+	parsed = ipe_parse_policy(cpy);
+	if (IS_ERR(parsed)) {
+		rc = PTR_ERR(parsed);
+		goto out_free_policy;
+	}
+
+	node->content = ipe_boot_policy;
+	node->content_size = strlen(ipe_boot_policy);
+	node->parsed = parsed;
+
+	root = securityfs_create_dir(node->parsed->policy_name,
+				     policies_root);
+	if (IS_ERR(root)) {
+		rc = PTR_ERR(root);
+		goto out_free_policy;
+	}
+
+	content = securityfs_create_file(IPE_INNER_CONTENT, 0444, root, NULL,
+					 &policy_content_ops);
+	if (IS_ERR(content)) {
+		rc = PTR_ERR(content);
+		goto out_free_root;
+	}
+
+	active = securityfs_create_file(IPE_ACTIVE_POLICY, 0644, root, NULL,
+					&policy_active_ops);
+	if (IS_ERR(active)) {
+		rc = PTR_ERR(active);
+		goto out_free_content;
+	}
+
+	root_i = d_inode(root);
+
+	inode_lock(root_i);
+	root_i->i_private = node;
+	inode_unlock(root_i);
+
+	boot_policy_node = root;
+	mutex_lock(&ipe_policy_lock);
+	rc = ipe_activate_policy(node->parsed);
+	mutex_unlock(&ipe_policy_lock);
+	synchronize_rcu();
+
+	return rc;
+
+out_free_content:
+	securityfs_remove(active);
+out_free_root:
+	securityfs_remove(root);
+out_free_policy:
+	ipe_free_policy(parsed);
+out:
+	kfree(cpy);
+	kfree(node);
+	return rc;
+}
+
+/**
+ * ipe_securityfs_init: Initialize IPE's securityfs entries.
+ *
+ * This is called after the lsm initialization.
+ *
+ * Return:
+ * 0 - OK
+ * !0 - Error
+ */
+static int __init ipe_securityfs_init(void)
+{
+	int rc = 0;
+
+	rc = ipe_build_secfs_root();
+	if (rc != 0)
+		goto err;
+
+	rc = ipe_build_boot_node();
+	if (rc != 0)
+		panic("IPE failed to initialize the boot policy: %d", rc);
+
+	return rc;
+err:
+	pr_err("failed to initialize secfs: %d", -rc);
+	return rc;
+}
+
+core_initcall(ipe_securityfs_init);
diff --git a/security/ipe/ipe-secfs.h b/security/ipe/ipe-secfs.h
new file mode 100644
index 000000000000..7732a8dcf11f
--- /dev/null
+++ b/security/ipe/ipe-secfs.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) Microsoft Corporation. All rights reserved.
+ */
+#include <linux/types.h>
+
+#include "ipe-policy.h"
+
+#ifndef IPE_SECFS_H
+#define IPE_SECFS_H
+
+extern struct mutex ipe_policy_lock;
+
+#endif /* IPE_SECFS_H */
-- 
2.27.0


^ permalink raw reply related

* [RFC PATCH v6 08/11] dm-verity: add bdev_setsecurity hook for root-hash
From: Deven Bowers @ 2020-07-30  0:31 UTC (permalink / raw)
  To: agk, axboe, snitzer, jmorris, serge, zohar, viro, paul, eparis,
	jannh, dm-devel, linux-integrity, linux-security-module,
	linux-fsdevel, linux-block, linux-audit
  Cc: tyhicks, linux-kernel, corbet, sashal, jaskarankhurana, mdsakib,
	nramas, pasha.tatashin
In-Reply-To: <20200730003113.2561644-1-deven.desai@linux.microsoft.com>

Add a security hook call to set a security property of a block_device
in dm-verity with the root-hash that was passed to device-mapper.

Signed-off-by: Deven Bowers <deven.desai@linux.microsoft.com>
---
 drivers/md/dm-verity-target.c | 8 ++++++++
 include/linux/device-mapper.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
index 9970488e67ed..44914668398d 100644
--- a/drivers/md/dm-verity-target.c
+++ b/drivers/md/dm-verity-target.c
@@ -16,8 +16,10 @@
 #include "dm-verity.h"
 #include "dm-verity-fec.h"
 #include "dm-verity-verify-sig.h"
+#include "dm-core.h"
 #include <linux/module.h>
 #include <linux/reboot.h>
+#include <linux/security.h>
 
 #define DM_MSG_PREFIX			"verity"
 
@@ -1207,6 +1209,12 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv)
 	ti->per_io_data_size = roundup(ti->per_io_data_size,
 				       __alignof__(struct dm_verity_io));
 
+	r = security_bdev_setsecurity(dm_table_get_md(v->ti->table)->bdev,
+				      DM_VERITY_ROOTHASH_SEC_NAME,
+				      v->root_digest, v->digest_size);
+	if (r)
+		goto bad;
+
 	verity_verify_sig_opts_cleanup(&verify_args);
 
 	return 0;
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index ab6b8eb0a150..e8ef887e4bae 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -626,5 +626,6 @@ static inline unsigned long to_bytes(sector_t n)
 }
 
 #define DM_VERITY_SIGNATURE_SEC_NAME DM_NAME	".verity-sig"
+#define DM_VERITY_ROOTHASH_SEC_NAME DM_NAME	".verity-rh"
 
 #endif	/* _LINUX_DEVICE_MAPPER_H */
-- 
2.27.0


^ permalink raw reply related

* [RFC PATCH v6 09/11] ipe: add property for dmverity roothash
From: Deven Bowers @ 2020-07-30  0:31 UTC (permalink / raw)
  To: agk, axboe, snitzer, jmorris, serge, zohar, viro, paul, eparis,
	jannh, dm-devel, linux-integrity, linux-security-module,
	linux-fsdevel, linux-block, linux-audit
  Cc: tyhicks, linux-kernel, corbet, sashal, jaskarankhurana, mdsakib,
	nramas, pasha.tatashin
In-Reply-To: <20200730003113.2561644-1-deven.desai@linux.microsoft.com>

Add a property to allow IPE policy to express rules around a specific
root-hash of a dm-verity volume.

This can be used for revocation, (when combined with the previous dm-verity
property) or the authorization of a single dm-verity volume.

Signed-off-by: Deven Bowers <deven.desai@linux.microsoft.com>
---
 security/ipe/ipe-blobs.c                    |  11 ++
 security/ipe/ipe-engine.h                   |   3 +
 security/ipe/ipe.c                          |   4 +
 security/ipe/properties/Kconfig             |  13 +-
 security/ipe/properties/Makefile            |   1 +
 security/ipe/properties/dmverity-roothash.c | 153 ++++++++++++++++++++
 security/ipe/properties/prop-entry.h        |   9 ++
 7 files changed, 193 insertions(+), 1 deletion(-)
 create mode 100644 security/ipe/properties/dmverity-roothash.c

diff --git a/security/ipe/ipe-blobs.c b/security/ipe/ipe-blobs.c
index 041d7d47b723..6a09d5c6dea8 100644
--- a/security/ipe/ipe-blobs.c
+++ b/security/ipe/ipe-blobs.c
@@ -46,6 +46,7 @@ void ipe_bdev_free_security(struct block_device *bdev)
 	struct ipe_bdev_blob *bdev_sec = ipe_bdev(bdev);
 
 	kfree(bdev_sec->dmverity_rh_sig);
+	kfree(bdev_sec->dmverity_rh);
 
 	memset(bdev_sec, 0x0, sizeof(*bdev_sec));
 }
@@ -80,5 +81,15 @@ int ipe_bdev_setsecurity(struct block_device *bdev, const char *key,
 		return 0;
 	}
 
+	if (!strcmp(key, DM_VERITY_ROOTHASH_SEC_NAME)) {
+		bdev_sec->dmverity_rh = kmemdup(value, len, GFP_KERNEL);
+		if (!bdev_sec->dmverity_rh)
+			return -ENOMEM;
+
+		bdev_sec->rh_size = len;
+
+		return 0;
+	}
+
 	return -ENOSYS;
 }
diff --git a/security/ipe/ipe-engine.h b/security/ipe/ipe-engine.h
index 038c39a8973e..696baaa423ff 100644
--- a/security/ipe/ipe-engine.h
+++ b/security/ipe/ipe-engine.h
@@ -18,6 +18,9 @@
 struct ipe_bdev_blob {
 	u8	*dmverity_rh_sig;
 	size_t	dmv_rh_sig_len;
+
+	u8 *dmverity_rh;
+	size_t rh_size;
 };
 
 struct ipe_engine_ctx {
diff --git a/security/ipe/ipe.c b/security/ipe/ipe.c
index 8a612eb62879..8f4dfb8c547f 100644
--- a/security/ipe/ipe.c
+++ b/security/ipe/ipe.c
@@ -47,6 +47,10 @@ static int __init ipe_load_properties(void)
 	if (rc != 0)
 		return rc;
 
+	rc = ipe_init_dm_verity_rh();
+	if (rc != 0)
+		return rc;
+
 	return rc;
 }
 
diff --git a/security/ipe/properties/Kconfig b/security/ipe/properties/Kconfig
index 4046f7e5eaef..4f09092522d9 100644
--- a/security/ipe/properties/Kconfig
+++ b/security/ipe/properties/Kconfig
@@ -14,8 +14,19 @@ config IPE_BOOT_PROP
 
 	  if unsure, answer N.
 
+config IPE_DM_VERITY_ROOTHASH
+	bool "Enable property for authorizing dm-verity volumes via root-hash"
+	depends on DM_VERITY
+	help
+	  This option enables IPE's integration with Device-Mapper Verity.
+	  This enables the usage of the property "dmverity_roothash" in IPE's
+	  policy. This property allows authorization or revocation via a
+	  a hex-string representing the roothash of a dmverity volume.
+
+	  if unsure, answer Y.
+
 config IPE_DM_VERITY_SIGNATURE
-	bool "Enable property for signature verified dm-verity volumes"
+	bool "Enable property for verified dm-verity volumes"
 	depends on DM_VERITY_VERIFY_ROOTHASH_SIG
 	help
 	  This option enables IPE's integration with Device-Mapper Verity's
diff --git a/security/ipe/properties/Makefile b/security/ipe/properties/Makefile
index 6b67cbe36e31..d9a3807797f4 100644
--- a/security/ipe/properties/Makefile
+++ b/security/ipe/properties/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_SECURITY_IPE) += properties.o
 
 properties-$(CONFIG_IPE_BOOT_PROP) += boot-verified.o
 properties-$(CONFIG_IPE_DM_VERITY_SIGNATURE) += dmverity-signature.o
+properties-$(CONFIG_IPE_DM_VERITY_ROOTHASH) += dmverity-roothash.o
diff --git a/security/ipe/properties/dmverity-roothash.c b/security/ipe/properties/dmverity-roothash.c
new file mode 100644
index 000000000000..09112e1af753
--- /dev/null
+++ b/security/ipe/properties/dmverity-roothash.c
@@ -0,0 +1,153 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) Microsoft Corporation. All rights reserved.
+ */
+
+#include "../ipe.h"
+#include "../ipe-pin.h"
+#include "../ipe-property.h"
+#include "../utility.h"
+
+#include <linux/types.h>
+#include <linux/slab.h>
+#include <linux/fs.h>
+#include <linux/mount.h>
+#include <linux/audit.h>
+#include <linux/kernel.h>
+
+#define PROPERTY_NAME "dmverity_roothash"
+
+struct counted_array {
+	u8 *arr;
+	size_t len;
+};
+
+static void audit(struct audit_buffer *ab, const void *value)
+{
+	const struct counted_array *a = (const struct counted_array *)value;
+
+	if (!a || a->len == 0)
+		audit_log_format(ab, "NULL");
+	else
+		audit_log_n_hex(ab, a->arr, a->len);
+}
+
+static inline void audit_rule_value(struct audit_buffer *ab,
+				    const void *value)
+{
+	audit(ab, value);
+}
+
+static inline void audit_ctx(struct audit_buffer *ab,
+			     const struct ipe_engine_ctx *ctx)
+{
+	struct counted_array a;
+
+	if (!has_bdev(ctx->file))
+		return audit(ab, NULL);
+
+	a.arr = ctx->sec_bdev->dmverity_rh;
+	a.len = ctx->sec_bdev->rh_size;
+
+	return audit(ab, &a);
+}
+
+static bool evaluate(const struct ipe_engine_ctx *ctx,
+		     const void *value)
+{
+	const struct counted_array *a = (const struct counted_array *)value;
+
+	if (!has_bdev(ctx->file))
+		return false;
+
+	if (a->len != ctx->sec_bdev->rh_size)
+		return false;
+
+	return memcmp(a->arr, ctx->sec_bdev->dmverity_rh, a->len) == 0;
+}
+
+static int parse(const char *val_str, void **value)
+{
+	struct counted_array *arr = NULL;
+	int rv = 0;
+
+	arr = kzalloc(sizeof(*arr), GFP_KERNEL);
+	if (!arr) {
+		rv = -ENOMEM;
+		goto err;
+	}
+
+	arr->len = strlen(val_str) / 2;
+
+	arr->arr = kzalloc(arr->len, GFP_KERNEL);
+	if (!arr->arr) {
+		rv = -ENOMEM;
+		goto err;
+	}
+
+	rv = hex2bin(arr->arr, val_str, arr->len);
+	if (rv != 0)
+		goto err;
+
+	*value = arr;
+	return rv;
+err:
+	if (arr)
+		kfree(arr->arr);
+	kfree(arr);
+	return rv;
+}
+
+static int duplicate(const void *src, void **dest)
+{
+	struct counted_array *arr = NULL;
+	const struct counted_array *src_arr = src;
+	int rv = 0;
+
+	arr = kmemdup(src_arr, sizeof(*arr), GFP_KERNEL);
+	if (!arr) {
+		rv = -ENOMEM;
+		goto err;
+	}
+
+	arr->arr = kmemdup(src_arr->arr, src_arr->len, GFP_KERNEL);
+	if (!arr->arr) {
+		rv = -ENOMEM;
+		goto err;
+	}
+
+	*dest = arr;
+	return rv;
+err:
+	if (arr)
+		kfree(arr->arr);
+	kfree(arr);
+
+	return rv;
+}
+
+static void free_val(void **value)
+{
+	struct counted_array *a = (struct counted_array *)*value;
+
+	if (a)
+		kfree(a->arr);
+	kfree(a);
+	*value = NULL;
+}
+
+static const struct ipe_property dmv_roothash = {
+	.property_name = PROPERTY_NAME,
+	.version = 1,
+	.eval = evaluate,
+	.parse = parse,
+	.rule_audit = audit_rule_value,
+	.ctx_audit = audit_ctx,
+	.dup = duplicate,
+	.free_val = free_val,
+};
+
+int ipe_init_dm_verity_rh(void)
+{
+	return ipe_register_property(&dmv_roothash);
+}
diff --git a/security/ipe/properties/prop-entry.h b/security/ipe/properties/prop-entry.h
index 85366366ff0d..86a360570f3b 100644
--- a/security/ipe/properties/prop-entry.h
+++ b/security/ipe/properties/prop-entry.h
@@ -26,4 +26,13 @@ static inline int __init ipe_init_dm_verity_signature(void)
 int __init ipe_init_dm_verity_signature(void);
 #endif /* CONFIG_IPE_DM_VERITY_SIGNATURE */
 
+#ifndef CONFIG_IPE_DM_VERITY_ROOTHASH
+static inline int __init ipe_init_dm_verity_rh(void)
+{
+	return 0;
+}
+#else
+int __init ipe_init_dm_verity_rh(void);
+#endif /* CONFIG_IPE_DM_VERITY_ROOTHASH */
+
 #endif /* IPE_PROP_ENTRY_H */
-- 
2.27.0


^ permalink raw reply related

* [RFC PATCH v6 11/11] cleanup: uapi/linux/audit.h
From: Deven Bowers @ 2020-07-30  0:31 UTC (permalink / raw)
  To: agk, axboe, snitzer, jmorris, serge, zohar, viro, paul, eparis,
	jannh, dm-devel, linux-integrity, linux-security-module,
	linux-fsdevel, linux-block, linux-audit
  Cc: tyhicks, linux-kernel, corbet, sashal, jaskarankhurana, mdsakib,
	nramas, pasha.tatashin
In-Reply-To: <20200730003113.2561644-1-deven.desai@linux.microsoft.com>

Remove trailing whitespaces and align the integrity #defines in
linux/uapi/audit.h

Signed-off-by: Deven Bowers <deven.desai@linux.microsoft.com>
---
 include/uapi/linux/audit.h | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 5a634cca1d42..609b4a5e8a80 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -48,7 +48,7 @@
  * 2500 - 2999 future user space (maybe integrity labels and related events)
  *
  * Messages from 1000-1199 are bi-directional. 1200-1299 & 2100 - 2999 are
- * exclusively user space. 1300-2099 is kernel --> user space 
+ * exclusively user space. 1300-2099 is kernel --> user space
  * communication.
  */
 #define AUDIT_GET		1000	/* Get status */
@@ -78,7 +78,7 @@
 #define AUDIT_LAST_USER_MSG	1199
 #define AUDIT_FIRST_USER_MSG2	2100	/* More user space messages */
 #define AUDIT_LAST_USER_MSG2	2999
- 
+
 #define AUDIT_DAEMON_START      1200    /* Daemon startup record */
 #define AUDIT_DAEMON_END        1201    /* Daemon normal stop record */
 #define AUDIT_DAEMON_ABORT      1202    /* Daemon error stop record */
@@ -140,20 +140,20 @@
 #define AUDIT_MAC_CALIPSO_ADD	1418	/* NetLabel: add CALIPSO DOI entry */
 #define AUDIT_MAC_CALIPSO_DEL	1419	/* NetLabel: del CALIPSO DOI entry */
 
-#define AUDIT_FIRST_KERN_ANOM_MSG   1700
-#define AUDIT_LAST_KERN_ANOM_MSG    1799
-#define AUDIT_ANOM_PROMISCUOUS      1700 /* Device changed promiscuous mode */
-#define AUDIT_ANOM_ABEND            1701 /* Process ended abnormally */
-#define AUDIT_ANOM_LINK		    1702 /* Suspicious use of file links */
-#define AUDIT_ANOM_CREAT	    1703 /* Suspicious file creation */
-#define AUDIT_INTEGRITY_DATA	    1800 /* Data integrity verification */
-#define AUDIT_INTEGRITY_METADATA    1801 /* Metadata integrity verification */
-#define AUDIT_INTEGRITY_STATUS	    1802 /* Integrity enable status */
-#define AUDIT_INTEGRITY_HASH	    1803 /* Integrity HASH type */
-#define AUDIT_INTEGRITY_PCR	    1804 /* PCR invalidation msgs */
-#define AUDIT_INTEGRITY_RULE	    1805 /* policy rule */
-#define AUDIT_INTEGRITY_EVM_XATTR   1806 /* New EVM-covered xattr */
-#define AUDIT_INTEGRITY_POLICY_RULE 1807 /* IMA policy rules */
+#define AUDIT_FIRST_KERN_ANOM_MSG	1700
+#define AUDIT_LAST_KERN_ANOM_MSG	1799
+#define AUDIT_ANOM_PROMISCUOUS		1700 /* Device changed promiscuous mode */
+#define AUDIT_ANOM_ABEND		1701 /* Process ended abnormally */
+#define AUDIT_ANOM_LINK			1702 /* Suspicious use of file links */
+#define AUDIT_ANOM_CREAT		1703 /* Suspicious file creation */
+#define AUDIT_INTEGRITY_DATA		1800 /* Data integrity verification */
+#define AUDIT_INTEGRITY_METADATA	1801 /* Metadata integrity verification */
+#define AUDIT_INTEGRITY_STATUS		1802 /* Integrity enable status */
+#define AUDIT_INTEGRITY_HASH		1803 /* Integrity HASH type */
+#define AUDIT_INTEGRITY_PCR		1804 /* PCR invalidation msgs */
+#define AUDIT_INTEGRITY_RULE		1805 /* policy rule */
+#define AUDIT_INTEGRITY_EVM_XATTR	1806 /* New EVM-covered xattr */
+#define AUDIT_INTEGRITY_POLICY_RULE	1807 /* IMA policy rules */
 #define AUDIT_INTEGRITY_POLICY_LOAD	1808 /* IPE Policy Load */
 #define AUDIT_INTEGRITY_POLICY_ACTIVATE	1809 /* IPE Policy Activation */
 #define AUDIT_INTEGRITY_EVENT		1810 /* IPE Evaluation Event */
-- 
2.27.0


^ permalink raw reply related

* [RFC PATCH v6 00/11] Integrity Policy Enforcement LSM (IPE)
From: Deven Bowers @ 2020-07-30  0:31 UTC (permalink / raw)
  To: agk, axboe, snitzer, jmorris, serge, zohar, viro, paul, eparis,
	jannh, dm-devel, linux-integrity, linux-security-module,
	linux-fsdevel, linux-block, linux-audit
  Cc: tyhicks, linux-kernel, corbet, sashal, jaskarankhurana, mdsakib,
	nramas, pasha.tatashin

Overview:
------------------------------------

IPE is a Linux Security Module which allows for a configurable
policy to enforce integrity requirements on the whole system. It
attempts to solve the issue of Code Integrity: that any code being
executed (or files being read), are identical to the version that
was built by a trusted source.

The type of system for which IPE is designed for use is an embedded device
with a specific purpose (e.g. network firewall device in a data center),
where all software and configuration is built and provisioned by the owner.

Specifically, a system which leverages IPE is not intended for general
purpose computing and does not utilize any software or configuration
built by a third party. An ideal system to leverage IPE has both mutable
and immutable components, however, all binary executable code is immutable.

The scope of IPE is constrained to the OS. It is assumed that platform
firmware verifies the the kernel and optionally the root filesystem (e.g.
via U-Boot verified boot). IPE then utilizes LSM hooks to enforce a
flexible, kernel-resident integrity verification policy.

IPE differs from other LSMs which provide integrity checking (for instance,
IMA), as it has no dependency on the filesystem metadata itself. The
attributes that IPE checks are deterministic properties that exist solely
in the kernel. Additionally, IPE provides no additional mechanisms of
verifying these files (e.g. IMA Signatures) - all of the attributes of
verifying files are existing features within the kernel, such as dm-verity
or fsverity.

IPE provides a policy that allows owners of the system to easily specify
integrity requirements and uses dm-verity signatures to simplify the
authentication of allowed objects like authorized code and data.

IPE supports two modes, permissive (similar to SELinux's permissive mode)
and enforce. Permissive mode performs the same checks, and logs policy
violations as enforce mode, but will not enforce the policy. This allows
users to test policies before enforcing them.

The default mode is enforce, and can be changed via the kernel commandline
parameter `ipe.enforce=(0|1)`, or the securityfs node
`/sys/kernel/security/ipe/enforce`. The ability to switch modes can be
compiled out of the LSM via setting the config
CONFIG_SECURITY_IPE_PERMISSIVE_SWITCH to N.

IPE additionally supports success auditing. When enabled, all events
that pass IPE policy and are not blocked will emit an audit event. This
is disabled by default, and can be enabled via the kernel commandline
`ipe.success_audit=(0|1)` or the securityfs node
`/sys/kernel/security/ipe/success_audit`.

Policies can be staged at runtime through securityfs and activated through
sysfs. Please see the Deploying Policies section of this cover letter for
more information.

The IPE LSM is compiled under CONFIG_SECURITY_IPE.

Policy:
------------------------------------

IPE policy is designed to be both forward compatible and backwards
compatible. There is one required line, at the top of the policy,
indicating the policy name, and the policy version, for instance:

  policy_name="Ex Policy" policy_version=0.0.0

The policy version indicates the current version of the policy (NOT the
policy syntax version). This is used to prevent roll-back of policy to
potentially insecure previous versions of the policy.

The next portion of IPE policy, are rules. Rules are formed by key=value
pairs, known as properties. IPE rules require two properties: "action",
which determines what IPE does when it encounters a match against the
policy, and "op", which determines when that rule should be evaluated.
Thus, a minimal rule is:

  op=EXECUTE action=ALLOW

This example will allow any execution. Additional properties are used to
restrict attributes about the files being evaluated. These properties are
intended to be deterministic attributes that are resident in the kernel.
Available properties for IPE described in the properties section of this
cover-letter, the repository available in Appendix A, and the kernel
documentation page.

Order does not matter for the rule's properties - they can be listed in
any order, however it is encouraged to have the "op" property be first,
and the "action" property be last, for readability.

Additionally, rules are evaluated top-to-bottom. As a result, any
revocation rules, or denies should be placed early in the file to ensure
that these rules are evaluated before a rule with "action=ALLOW" is hit.

Any unknown syntax in IPE policy will result in a fatal error to parse
the policy. User mode can interrogate the kernel to understand what
properties and the associated versions through the securityfs node,
$securityfs/ipe/property_config, which will return a string of form:

  key1=version1
  key2=version2
  .
  .
  .
  keyN=versionN

User-mode should correlate these versions with the supported values
identified in the documentation to determine whether a policy should
be accepted by the system.

Additionally, a DEFAULT operation must be set for all understood
operations within IPE. For policies to remain completely forwards
compatible, it is recommended that users add a "DEFAULT action=ALLOW"
and override the defaults on a per-operation basis.

For more information about the policy syntax, please see Appendix A or
the kernel documentation page.

Early Usermode Protection:
--------------------------

IPE can be provided with a policy at startup to load and enforce.
This is intended to be a minimal policy to get the system to a state
where userland is setup and ready to receive commands, at which
point a policy can be deployed via securityfs. This "boot policy" can be
specified via the config, SECURITY_IPE_BOOT_POLICY, which accepts a path
to a plain-text version of the IPE policy to apply. This policy will be
compiled into the kernel. If not specified, IPE will be disabled until a
policy is deployed and activated through the method above.

Policy Examples:
------------------------------------

Allow all:

  policy_name="Allow All" policy_version=0.0.0
  DEFAULT action=ALLOW

Allow only initial superblock:

  policy_name="Allow All Initial SB" policy_version=0.0.0
  DEFAULT action=DENY

  op=EXECUTE boot_verified=TRUE action=ALLOW

Allow any signed dm-verity volume and the initial superblock:

  policy_name="AllowSignedAndInitial" policy_version=0.0.0
  DEFAULT action=DENY

  op=EXECUTE boot_verified=TRUE action=ALLOW
  op=EXECUTE dmverity_signature=TRUE action=ALLOW

Prohibit execution from a specific dm-verity volume:

  policy_name="AllowSignedAndInitial" policy_version=0.0.0
  DEFAULT action=DENY

  op=EXECUTE dmverity_roothash=401fcec5944823ae12f62726e8184407a5fa9599783f030dec146938 action=DENY
  op=EXECUTE boot_verified=TRUE action=ALLOW
  op=EXECUTE dmverity_signature=TRUE action=ALLOW

Allow only a specific dm-verity volume:

  policy_name="AllowSignedAndInitial" policy_version=0.0.0
  DEFAULT action=DENY

  op=EXECUTE dmverity_roothash=401fcec5944823ae12f62726e8184407a5fa9599783f030dec146938 action=ALLOW

Deploying Policies:
-------------------

Deploying policies is simple. First sign a plain text policy, with a
certificate that is present in the SYSTEM_TRUSTED_KEYRING of your test
machine. Through openssl, the signing can be done via:

  openssl smime -sign -in "$MY_POLICY" -signer "$MY_CERTIFICATE" \
    -inkey "$MY_PRIVATE_KEY" -binary -outform der -noattr -nodetach \
    -out "$MY_POLICY.p7s"

Then, simply cat the file into the IPE's "new_policy" securityfs node:

  cat "$MY_POLICY.p7s" > /sys/kernel/security/ipe/new_policy

The policy should now be present under the policies/ subdirectory, under
its "policy_name" attribute.

The policy is now present in the kernel and can be marked as active,
via the sysctl "ipe.active_policy":

  echo -n 1 > "/sys/kernel/security/ipe/$MY_POLICY_NAME/active"

This will now mark the policy as active and the system will be enforcing
$MY_POLICY_NAME. At any point the policy can be updated on the provision
that the policy version to be deployed is greater than or equal to the
running version (to prevent roll-back attacks). This update can be done
by redirecting the file into the policy's "raw" node, under the policies
subdirectory:

  cat "$MY_UPDATED_POLICY.p7s" > \
    "/sys/kernel/security/ipe/policies/$MY_POLICY_NAME/raw"

Additionally, policies can be deleted via the "del_policy" securityfs
node. Simply write the name of the policy to be deleted to that node:

  echo -n 1 >
    "/sys/kernel/security/ipe/policies/$MY_POLICY_NAME/delete"

There are two requirements to delete policies:

1. The policy being deleted must not be the active policy.
2. The policy being deleted must not be the boot policy.

It's important to know above that the "echo" command will add a newline
to the end of the input, and this will be considered as part of the
filename. You can remove the newline via the -n parameter.

NOTE: If a MAC LSM is enabled, the securityfs commands will require
CAP_MAC_ADMIN. This is due to sysfs supporting fine-grained MAC
attributes, while securityfs at the current moment does not.

Properties:
------------------------------------

This initial patchset introducing IPE adds three properties:
'boot_verified', 'dmverity_signature' and 'dmverity_roothash'.

boot_verified (CONFIG_IPE_BOOT_PROP):
  This property can be utilized for authorization of the first
  super-block that is mounted on the system, where IPE attempts
  to evaluate a file. Typically this is used for systems with
  an initramfs or other initial disk, where this is unmounted before
  the system becomes available, and is not covered by any other property.
  The format of this property is:

    boot_verified=(TRUE|FALSE)

  WARNING: This property will trust any disk where the first IPE
  evaluation occurs. If you do not have a startup disk that is
  unpacked and unmounted (like initramfs), then it will automatically
  trust the root filesystem and potentially overauthorize the entire
  disk.

dmverity_roothash (CONFIG_IPE_DM_VERITY_ROOTHASH):
  This property can be utilized for authorization or revocation of
  specific dmverity volumes, identified via root hash. It has a
  dependency on the DM_VERITY module. The format of this property is:

    dmverity_roothash=<HashHexDigest>

dmverity_signature (CONFIG_IPE_DM_VERITY_SIGNATURE):
  This property can be utilized for authorization of all dm-verity
  volumes that have a signed roothash that chains to the system
  trusted keyring. It has a dependency on the
  DM_VERITY_VERIFY_ROOTHASH_SIG config. The format of this property is:

    dmverity_signature=(TRUE|FALSE)

Testing:
------------------------------------

A test suite is available (Appendix B) for ease of use. For manual
instructions:

Enable IPE through the following Kconfigs:

  CONFIG_SECURITY_IPE=y
  CONFIG_SECURITY_IPE_BOOT_POLICY="../AllowAllInitialSB.pol"
  CONFIG_SECURITY_IPE_PERMISSIVE_SWITCH=y
  CONFIG_IPE_BOOT_PROP=y
  CONFIG_IPE_DM_VERITY_ROOTHASH=y
  CONFIG_IPE_DM_VERITY_SIGNATURE=y
  CONFIG_DM_VERITY=y
  CONFIG_DM_VERITY_VERIFY_ROOTHASH_SIG=y
  CONFIG_SYSTEM_TRUSTED_KEYRING=y
  CONFIG_SYSTEM_TRUSTED_KEYS="/path/to/my/cert/list.pem"

Start a test system, that boots directly from the filesystem, without
an initrd. I recommend testing in permissive mode until all tests
pass, then switch to enforce to ensure behavior remains identical.

boot_verified:

  If booted correctly, the filesystem mounted on / should be marked as
  boot_verified. Verify by turning on success auditing (sysctl
  ipe.success_audit=1), and run a binary. In the audit output,
  `prop_boot_verified` should be `TRUE`.

  To test denials, mount a temporary filesystem (mount -t tmpfs -o
  size=4M tmp tmp), and copy a binary (e.g. ls) to this new
  filesystem. Disable success auditing and attempt to run the file.
  The file should have an audit event, but be allowed to execute in
  permissive mode, and prop_boot_verified should be FALSE.

dmverity_roothash:

  First, you must create a dm-verity volume. This can be done through
  squashfs-tools and veritysetup (provided by cryptsetup).

  Creating a squashfs volume:

    mksquashfs /path/to/directory/with/executable /path/to/output.squashfs

  Format the volume for use with dm-verity & save the root hash:

    output_rh=$(veritysetup format output.squashfs output.hashtree | \
      tee verity_out.txt | awk "/Root hash/" | \
      sed -E "s/Root hash:\s+//g")

    echo -n $output_rh > output.roothash

  Create a two policies, filling in the appropriate fields below:

    Policy 1:

      policy_name="roothash-denial" policy_version=0.0.0
      DEFAULT action=ALLOW
      op=EXECUTE dmverity_roothash=$output_rh action=DENY

    Policy 2:

      policy_name="roothash-allow" policy_version=0.0.0
      DEFAULT action=ALLOW
      DEFAULT op=EXECUTE action=DENY

      op=EXECUTE boot_verified=TRUE action=ALLOW
      op=EXECUTE dmverity_roothash=$output_rh action=ALLOW

  Deploy each policy, then mark the first, "roothash-denial" as active,
  per the "Deploying Policies" section of this cover letter. Mount the
  dm-verity volume:

    veritysetup open output.squashfs output.hashtree unverified \
      `cat output.roothash`

    mount /dev/mapper/unverified /my/mount/point

  Attempt to execute a binary in the mount point, and it should emit an
  audit event for a match against the rule:
  
    op=EXECUTE dmverity_roothash=$output_rh action=DENY

  To test the second policy, perform the same steps, but this time, enable
  success auditing before running the executable. The success audit event
  should be a match against this rule:

    op=EXECUTE dmverity_roothash=$output_rh action=ALLOW

dmverity_signature:

  Follow the setup steps for dmverity_roothash. Sign the roothash via:

    openssl smime -sign -in "output.roothash" -signer "$MY_CERTIFICATE" \
      -inkey "$MY_PRIVATE_KEY" -binary -outform der -noattr \
      -out "output.p7s"

    Create a policy:

      policy_name="verified" policy_version=0.0.0
      DEFAULT action=DENY

      op=EXECUTE boot_verified=TRUE action=ALLOW
      op=EXECUTE dmverity_verified=TRUE action=ALLOW

  Deploy the policy, and mark as active, per the "Deploying Policies"
  section of this cover letter. Mount the dm-verity volume with
  verification:

    veritysetup open output.squashfs output.hashtree unverified \
      `cat output.roothash` --root-hash-signature=output.p7s

    mount /dev/mapper/unverified /my/mount/point

  NOTE: The --root-hash-signature option was introduced in veritysetup
  2.3.0

  Turn on success auditing and attempt to execute a binary in the mount
  point, and it should emit an audit event for a match against the rule:

    op=EXECUTE dmverity_verified=TRUE action=ALLOW

  To test denials, mount the dm-verity volume the same way as the
  "dmverity_roothash" section, and attempt to execute a binary. Failure
  should occur.

Documentation:
------------------------------------

Full documentation is available on github in IPE's master repository
(Appendix A). This is intended to be an exhaustive source of documentation
around IPE.

Additionally, there is higher level documentation in the admin-guide.

Technical diagrams are available here:

  http://microsoft.github.io/ipe/technical/diagrams/

Known Gaps:
------------------------------------

IPE has two known gaps:

1. IPE cannot verify the integrity of anonymous executable memory, such as
  the trampolines created by gcc closures and libffi, or JIT'd code.
  Unfortunately, as this is dynamically generated code, there is no way for
  IPE to detect that this code has not been tampered with in transition
  from where it was built, to where it is running. As a result, IPE is
  incapable of tackling this problem for dynamically generated code.
  However, there is a patch series being prepared that addresses this
  problem for libffi and gcc closures by implemeting a safer kernel
  trampoline API. 

2. IPE cannot verify the integrity of interpreted languages' programs when
  these scripts invoked via `<interpreter> <file>`. This is because the way
  interpreters execute these files, the scripts themselves are not
  evaluated as executable code through one of IPE's hooks. Interpreters
  can be enlightened to the usage of IPE by trying to mmap a file into
  executable memory (+X), after opening the file and responding to the
  error code appropriately. This also applies to included files, or high
  value files, such as configuration files of critical system components.
  This specific gap is planned on being addressed within IPE. For more
  information on how we plan to address this gap, please see the Future
  Development section, below.

Future Development:
------------------------------------

Support for filtering signatures by specific certificates. In this case,
our "dmverity_signature" (or a separate property) can be set to a
specific certificate declared in IPE's policy, allowing for more
controlled use-cases determine by a user's PKI structure.

Support for integrity verification for general file reads. This addresses
the script interpreter issue indicated in the "Known Gaps" section, as
these script files are typically opened with O_RDONLY. We are evaluating
whether to do this by comparing the original userland filepath passed into
the open syscall, thereby allowing existing callers to take advantage
without any code changes; the alternate design is to extend the new
openat2(2) syscall, with an new flag, tentatively called "O_VERIFY". While
the second option requires a code change for all the interpreters,
frameworks and languages that wish to leverage it, it is a wholly cleaner
implementation in the kernel. For interpreters specifically, the O_MAYEXEC
patch series published by Mickaël Salaün[1] is a similar implementation
to the O_VERIFY idea described above.

Onboarding IPE's test suite to KernelCI. Currently we are developing a
test suite in the same vein as SELinux's test suite. Once development
of the test suite is complete, and provided IPE is accepted, we intend
to onboard this test suite onto KernelCI.

Hardened resistance against roll-back attacks. Currently there exists a
window of opportunity between user-mode setup and the user-policy being
deployed, where a prior user-policy can be loaded, that is potentially
insecure. However, with a kernel update, you can revise the boot policy's
version to be the same version as the latest policy, closing this window.
In the future, I would like to close this window of opportunity without
a kernel update, using some persistent storage mechanism.

Open Issues:
------------

For linux-audit/integrity folks:
1. Introduction of new audit definitions in the kernel integrity range - is
  this preferred, as opposed to reusing definitions with existing IMA
  definitions?

TODOs:
------

linux-audit changes to support the new audit events.


Appendix:
------------------------------------

A. IPE Github Repository: https://github.com/microsoft/ipe
   Hosted Documentation: https://microsoft.github.io/ipe
B. IPE Users' Guide: Documentation/admin-guide/LSM/ipe.rst
C. IPE Test Suite: *TBA* (under development)

References:
------------------------------------

1. https://lore.kernel.org/linux-integrity/20200505153156.925111-1-mic@digikod.net/

Changelog:
------------------------------------

v1: Introduced

v2:
  Split the second patch of the previous series into two.
  Minor corrections in the cover-letter and documentation
  comments regarding CAP_MAC_ADMIN checks in IPE.

v3:
  Address various comments by Jann Horn. Highlights:
    Switch various audit allocators to GFP_KERNEL.
    Utilize rcu_access_pointer() in various locations.
    Strip out the caching system for properties
    Strip comments from headers
    Move functions around in patches
    Remove kernel command line parameters
    Reconcile the race condition on the delete node for policy by
      expanding the policy critical section.

  Address a few comments by Jonathan Corbet around the documentation
    pages for IPE.

  Fix an issue with the initialization of IPE policy with a "-0"
    version, caused by not initializing the hlist entries before
    freeing.

v4:
  Address a concern around IPE's behavior with unknown syntax.
    Specifically, make any unknown syntax a fatal error instead of a
    warning, as suggested by Mickaël Salaün.
  Introduce a new securityfs node, $securityfs/ipe/property_config,
    which provides a listing of what properties are enabled by the
    kernel and their versions. This allows usermode to predict what
    policies should be allowed.
  Strip some comments from c files that I missed.
  Clarify some documentation comments around 'boot_verified'.
    While this currently does not functionally change the property
    itself, the distinction is important when IPE can enforce verified
    reads. Additionally, 'KERNEL_READ' was omitted from the documentation.
    This has been corrected.
  Change SecurityFS and SHA1 to a reverse dependency.
  Update the cover-letter with the updated behavior of unknown syntax.
  Remove all sysctls, making an equivalent function in securityfs.
  Rework the active/delete mechanism to be a node under the policy in
    $securityfs/ipe/policies.
  The kernel command line parameters ipe.enforce and ipe.success_audit
    have returned as this functionality is no longer exposed through
    sysfs.

v5:
  Correct some grammatical errors reported by Randy Dunlap.
  Fix some warnings reported by kernel test bot.
  Change convention around security_bdev_setsecurity. -ENOSYS
    is now expected if an LSM does not implement a particular @name,
    as suggested by Casey Schaufler.
  Minor string corrections related to the move from sysfs to securityfs
  Correct a spelling of an #ifdef for the permissive argument.
  Add the kernel parameters re-added to the documentation.
  Fix a minor bug where the mode being audited on permissive switch
    was the original mode, not the mode being swapped to.
  Cleanup doc comments, fix some whitespace alignment issues.

v6:
  Change if statement condition in security_bdev_setsecurity to be
    more concise, as suggested by Casey Schaufler and Al Viro
  Drop the 6th patch in the series, "dm-verity move signature check..."
    due to numerous issues, and it ultimately providing no real value.
  Fix the patch tree - the previous iteration appears to have been in a
    torn state (patches 8+9 were merged). This has since been corrected.

Deven Bowers (11):
  scripts: add ipe tooling to generate boot policy
  security: add ipe lsm evaluation loop and audit system
  security: add ipe lsm policy parser and policy loading
  ipe: add property for trust of boot volume
  fs: add security blob and hooks for block_device
  dm-verity: add bdev_setsecurity hook for dm-verity signature
  ipe: add property for signed dmverity volumes
  dm-verity: add bdev_setsecurity hook for root-hash
  ipe: add property for dmverity roothash
  documentation: add ipe documentation
  cleanup: uapi/linux/audit.h

 Documentation/admin-guide/LSM/index.rst       |    1 +
 Documentation/admin-guide/LSM/ipe.rst         |  508 +++++++
 .../admin-guide/kernel-parameters.txt         |   12 +
 MAINTAINERS                                   |    8 +
 drivers/md/dm-verity-target.c                 |   10 +-
 drivers/md/dm-verity-verify-sig.c             |   14 +-
 drivers/md/dm-verity-verify-sig.h             |   10 +-
 fs/block_dev.c                                |    8 +
 include/linux/device-mapper.h                 |    3 +
 include/linux/fs.h                            |    1 +
 include/linux/lsm_hook_defs.h                 |    5 +
 include/linux/lsm_hooks.h                     |   12 +
 include/linux/security.h                      |   22 +
 include/uapi/linux/audit.h                    |   36 +-
 scripts/Makefile                              |    1 +
 scripts/ipe/Makefile                          |    2 +
 scripts/ipe/polgen/.gitignore                 |    1 +
 scripts/ipe/polgen/Makefile                   |    7 +
 scripts/ipe/polgen/polgen.c                   |  136 ++
 security/Kconfig                              |   12 +-
 security/Makefile                             |    2 +
 security/ipe/.gitignore                       |    2 +
 security/ipe/Kconfig                          |   48 +
 security/ipe/Makefile                         |   33 +
 security/ipe/ipe-audit.c                      |  303 ++++
 security/ipe/ipe-audit.h                      |   24 +
 security/ipe/ipe-blobs.c                      |   95 ++
 security/ipe/ipe-blobs.h                      |   18 +
 security/ipe/ipe-engine.c                     |  213 +++
 security/ipe/ipe-engine.h                     |   49 +
 security/ipe/ipe-hooks.c                      |  169 +++
 security/ipe/ipe-hooks.h                      |   70 +
 security/ipe/ipe-parse.c                      |  889 +++++++++++
 security/ipe/ipe-parse.h                      |   17 +
 security/ipe/ipe-pin.c                        |   93 ++
 security/ipe/ipe-pin.h                        |   36 +
 security/ipe/ipe-policy.c                     |  149 ++
 security/ipe/ipe-policy.h                     |   69 +
 security/ipe/ipe-prop-internal.h              |   49 +
 security/ipe/ipe-property.c                   |  143 ++
 security/ipe/ipe-property.h                   |  100 ++
 security/ipe/ipe-secfs.c                      | 1309 +++++++++++++++++
 security/ipe/ipe-secfs.h                      |   14 +
 security/ipe/ipe.c                            |  115 ++
 security/ipe/ipe.h                            |   22 +
 security/ipe/properties/Kconfig               |   36 +
 security/ipe/properties/Makefile              |   13 +
 security/ipe/properties/boot-verified.c       |   82 ++
 security/ipe/properties/dmverity-roothash.c   |  153 ++
 security/ipe/properties/dmverity-signature.c  |   82 ++
 security/ipe/properties/prop-entry.h          |   38 +
 security/ipe/utility.h                        |   32 +
 security/security.c                           |   70 +
 53 files changed, 5316 insertions(+), 30 deletions(-)
 create mode 100644 Documentation/admin-guide/LSM/ipe.rst
 create mode 100644 scripts/ipe/Makefile
 create mode 100644 scripts/ipe/polgen/.gitignore
 create mode 100644 scripts/ipe/polgen/Makefile
 create mode 100644 scripts/ipe/polgen/polgen.c
 create mode 100644 security/ipe/.gitignore
 create mode 100644 security/ipe/Kconfig
 create mode 100644 security/ipe/Makefile
 create mode 100644 security/ipe/ipe-audit.c
 create mode 100644 security/ipe/ipe-audit.h
 create mode 100644 security/ipe/ipe-blobs.c
 create mode 100644 security/ipe/ipe-blobs.h
 create mode 100644 security/ipe/ipe-engine.c
 create mode 100644 security/ipe/ipe-engine.h
 create mode 100644 security/ipe/ipe-hooks.c
 create mode 100644 security/ipe/ipe-hooks.h
 create mode 100644 security/ipe/ipe-parse.c
 create mode 100644 security/ipe/ipe-parse.h
 create mode 100644 security/ipe/ipe-pin.c
 create mode 100644 security/ipe/ipe-pin.h
 create mode 100644 security/ipe/ipe-policy.c
 create mode 100644 security/ipe/ipe-policy.h
 create mode 100644 security/ipe/ipe-prop-internal.h
 create mode 100644 security/ipe/ipe-property.c
 create mode 100644 security/ipe/ipe-property.h
 create mode 100644 security/ipe/ipe-secfs.c
 create mode 100644 security/ipe/ipe-secfs.h
 create mode 100644 security/ipe/ipe.c
 create mode 100644 security/ipe/ipe.h
 create mode 100644 security/ipe/properties/Kconfig
 create mode 100644 security/ipe/properties/Makefile
 create mode 100644 security/ipe/properties/boot-verified.c
 create mode 100644 security/ipe/properties/dmverity-roothash.c
 create mode 100644 security/ipe/properties/dmverity-signature.c
 create mode 100644 security/ipe/properties/prop-entry.h
 create mode 100644 security/ipe/utility.h

-- 
2.27.0

^ permalink raw reply

* [RFC PATCH v6 01/11] scripts: add ipe tooling to generate boot policy
From: Deven Bowers @ 2020-07-30  0:31 UTC (permalink / raw)
  To: agk, axboe, snitzer, jmorris, serge, zohar, viro, paul, eparis,
	jannh, dm-devel, linux-integrity, linux-security-module,
	linux-fsdevel, linux-block, linux-audit
  Cc: tyhicks, linux-kernel, corbet, sashal, jaskarankhurana, mdsakib,
	nramas, pasha.tatashin
In-Reply-To: <20200730003113.2561644-1-deven.desai@linux.microsoft.com>

Add a tool for the generation of an IPE policy to be compiled into the
kernel. This policy will be enforced until userland deploys and activates
a new policy.

Signed-off-by: Deven Bowers <deven.desai@linux.microsoft.com>
---
 MAINTAINERS                   |   6 ++
 scripts/Makefile              |   1 +
 scripts/ipe/Makefile          |   2 +
 scripts/ipe/polgen/.gitignore |   1 +
 scripts/ipe/polgen/Makefile   |   7 ++
 scripts/ipe/polgen/polgen.c   | 136 ++++++++++++++++++++++++++++++++++
 6 files changed, 153 insertions(+)
 create mode 100644 scripts/ipe/Makefile
 create mode 100644 scripts/ipe/polgen/.gitignore
 create mode 100644 scripts/ipe/polgen/Makefile
 create mode 100644 scripts/ipe/polgen/polgen.c

diff --git a/MAINTAINERS b/MAINTAINERS
index f0569cf304ca..d29c2e4612e4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8580,6 +8580,12 @@ S:	Supported
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git
 F:	security/integrity/ima/
 
+INTEGRITY POLICY ENFORCEMENT (IPE)
+M:	Deven Bowers <deven.desai@linux.microsoft.com>
+L:	linux-integrity@vger.kernel.org
+S:	Supported
+F:	scripts/ipe/
+
 INTEL 810/815 FRAMEBUFFER DRIVER
 M:	Antonino Daplas <adaplas@gmail.com>
 L:	linux-fbdev@vger.kernel.org
diff --git a/scripts/Makefile b/scripts/Makefile
index 95ecf970c74c..b3c1882fd6dd 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -34,6 +34,7 @@ hostprogs += unifdef
 subdir-$(CONFIG_GCC_PLUGINS) += gcc-plugins
 subdir-$(CONFIG_MODVERSIONS) += genksyms
 subdir-$(CONFIG_SECURITY_SELINUX) += selinux
+subdir-$(CONFIG_SECURITY_IPE) += ipe
 
 # Let clean descend into subdirs
 subdir-	+= basic dtc gdb kconfig mod
diff --git a/scripts/ipe/Makefile b/scripts/ipe/Makefile
new file mode 100644
index 000000000000..e87553fbb8d6
--- /dev/null
+++ b/scripts/ipe/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0-only
+subdir-y := polgen
diff --git a/scripts/ipe/polgen/.gitignore b/scripts/ipe/polgen/.gitignore
new file mode 100644
index 000000000000..80f32f25d200
--- /dev/null
+++ b/scripts/ipe/polgen/.gitignore
@@ -0,0 +1 @@
+polgen
diff --git a/scripts/ipe/polgen/Makefile b/scripts/ipe/polgen/Makefile
new file mode 100644
index 000000000000..a519b594e13c
--- /dev/null
+++ b/scripts/ipe/polgen/Makefile
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0
+hostprogs-y	:= polgen
+HOST_EXTRACFLAGS += \
+	-I$(srctree)/include \
+	-I$(srctree)/include/uapi \
+
+always		:= $(hostprogs-y)
diff --git a/scripts/ipe/polgen/polgen.c b/scripts/ipe/polgen/polgen.c
new file mode 100644
index 000000000000..a80fffe1b27c
--- /dev/null
+++ b/scripts/ipe/polgen/polgen.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) Microsoft Corporation. All rights reserved.
+ */
+
+#include <stdlib.h>
+#include <stddef.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <errno.h>
+
+static void usage(const char *const name)
+{
+	printf("Usage: %s OutputFile (PolicyFile)\n", name);
+	exit(EINVAL);
+}
+
+static int policy_to_buffer(const char *pathname, char **buffer, size_t *size)
+{
+	int rc = 0;
+	FILE *fd;
+	char *lbuf;
+	size_t fsize;
+	size_t read;
+
+	fd = fopen(pathname, "r");
+	if (!fd) {
+		rc = errno;
+		goto out;
+	}
+
+	fseek(fd, 0, SEEK_END);
+	fsize = ftell(fd);
+	rewind(fd);
+
+	lbuf = malloc(fsize);
+	if (!lbuf) {
+		rc = ENOMEM;
+		goto out_close;
+	}
+
+	read = fread((void *)lbuf, sizeof(*lbuf), fsize, fd);
+	if (read != fsize) {
+		rc = -1;
+		goto out_free;
+	}
+
+	*buffer = lbuf;
+	*size = fsize;
+	fclose(fd);
+
+	return rc;
+
+out_free:
+	free(lbuf);
+out_close:
+	fclose(fd);
+out:
+	return rc;
+}
+
+static int write_boot_policy(const char *pathname, const char *buf, size_t size)
+{
+	FILE *fd;
+	size_t i;
+
+	fd = fopen(pathname, "w");
+	if (!fd)
+		goto err;
+
+	fprintf(fd, "/* This file is automatically generated.");
+	fprintf(fd, " Do not edit. */\n");
+	fprintf(fd, "#include <stddef.h>\n");
+	fprintf(fd, "const char *const ipe_boot_policy =\n");
+
+	if (!buf || size == 0) {
+		fprintf(fd, "\tNULL;\n");
+		fclose(fd);
+		return 0;
+	}
+
+	for (i = 0; i < size; ++i) {
+		if (i == 0)
+			fprintf(fd, "\t\"");
+
+		switch (buf[i]) {
+		case '"':
+			fprintf(fd, "\\\"");
+			break;
+		case '\'':
+			fprintf(fd, "'");
+			break;
+		case '\n':
+			fprintf(fd, "\\n\"\n\t\"");
+			break;
+		case '\\':
+			fprintf(fd, "\\\\");
+			break;
+		default:
+			fprintf(fd, "%c", buf[i]);
+		}
+	}
+	fprintf(fd, "\";\n");
+	fclose(fd);
+
+	return 0;
+
+err:
+	if (fd)
+		fclose(fd);
+	return errno;
+}
+
+int main(int argc, const char *argv[])
+{
+	int rc = 0;
+	size_t len = 0;
+	char *policy = NULL;
+
+	if (argc < 2)
+		usage(argv[0]);
+
+	if (argc > 2) {
+		rc = policy_to_buffer(argv[2], &policy, &len);
+		if (rc != 0)
+			goto cleanup;
+	}
+
+	rc = write_boot_policy(argv[1], policy, len);
+cleanup:
+	if (policy)
+		free(policy);
+	if (rc != 0)
+		perror("An error occurred during policy conversion: ");
+	return rc;
+}
-- 
2.27.0


^ permalink raw reply related

* Re: [PATCH v2] netfilter: Replace HTTP links with HTTPS ones
From: Pablo Neira Ayuso @ 2020-07-29 20:43 UTC (permalink / raw)
  To: Alexander A. Klimov
  Cc: kadlec, fw, davem, kuba, paul, netfilter-devel, coreteam,
	linux-kernel, linux-decnet-user, netdev, linux-security-module
In-Reply-To: <20200725170225.4505-1-grandmaster@al2klimov.de>

On Sat, Jul 25, 2020 at 07:02:25PM +0200, Alexander A. Klimov wrote:
> Rationale:
> Reduces attack surface on kernel devs opening the links for MITM
> as HTTPS traffic is much harder to manipulate.

Applied.

^ permalink raw reply

* general protection fault in security_inode_getattr
From: syzbot @ 2020-07-29 20:23 UTC (permalink / raw)
  To: andriin, ast, bpf, daniel, jmorris, john.fastabend, kafai,
	kpsingh, linux-kernel, linux-security-module, netdev, serge,
	songliubraving, syzkaller-bugs, yhs

Hello,

syzbot found the following issue on:

HEAD commit:    92ed3019 Linux 5.8-rc7
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=140003ac900000
kernel config:  https://syzkaller.appspot.com/x/.config?x=84f076779e989e69
dashboard link: https://syzkaller.appspot.com/bug?extid=f07cc9be8d1d226947ed
compiler:       gcc (GCC) 10.1.0-syz 20200507

Unfortunately, I don't have any reproducer for this issue yet.

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+f07cc9be8d1d226947ed@syzkaller.appspotmail.com

general protection fault, probably for non-canonical address 0xdffffc000000000c: 0000 [#1] PREEMPT SMP KASAN
KASAN: null-ptr-deref in range [0x0000000000000060-0x0000000000000067]
CPU: 0 PID: 9214 Comm: syz-executor.3 Not tainted 5.8.0-rc7-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:d_backing_inode include/linux/dcache.h:549 [inline]
RIP: 0010:security_inode_getattr+0x46/0x140 security/security.c:1276
Code: 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 04 01 00 00 48 b8 00 00 00 00 00 fc ff df 49 8b 5d 08 48 8d 7b 60 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 d7 00 00 00 48 b8 00 00 00 00 00 fc ff df 48 8b
RSP: 0018:ffffc9000d41f638 EFLAGS: 00010206
RAX: dffffc0000000000 RBX: 0000000000000000 RCX: ffffc9000f539000
RDX: 000000000000000c RSI: ffffffff8354f8ee RDI: 0000000000000060
RBP: ffffc9000d41f810 R08: 0000000000000001 R09: ffff88804edc2dc8
R10: 0000000000000000 R11: 00000000000ebc58 R12: ffff888089f10170
R13: ffffc9000d41f810 R14: 00000000000007ff R15: 0000000000000000
FS:  00007f3599717700(0000) GS:ffff8880ae600000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000001b2c12c000 CR3: 0000000099919000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 vfs_getattr+0x22/0x60 fs/stat.c:121
 ovl_copy_up_one+0x13b/0x1870 fs/overlayfs/copy_up.c:850
 ovl_copy_up_flags+0x14b/0x1d0 fs/overlayfs/copy_up.c:931
 ovl_maybe_copy_up+0x140/0x190 fs/overlayfs/copy_up.c:963
 ovl_open+0xba/0x270 fs/overlayfs/file.c:147
 do_dentry_open+0x501/0x1290 fs/open.c:828
 do_open fs/namei.c:3243 [inline]
 path_openat+0x1bb9/0x2750 fs/namei.c:3360
 do_filp_open+0x17e/0x3c0 fs/namei.c:3387
 file_open_name+0x290/0x400 fs/open.c:1124
 acct_on+0x78/0x770 kernel/acct.c:207
 __do_sys_acct kernel/acct.c:286 [inline]
 __se_sys_acct kernel/acct.c:273 [inline]
 __x64_sys_acct+0xab/0x1f0 kernel/acct.c:273
 do_syscall_64+0x60/0xe0 arch/x86/entry/common.c:384
 entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x45c369
Code: 8d b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 5b b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f3599716c78 EFLAGS: 00000246 ORIG_RAX: 00000000000000a3
RAX: ffffffffffffffda RBX: 0000000000000700 RCX: 000000000045c369
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000020000440
RBP: 000000000078bf30 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 000000000078bf0c
R13: 00007ffda41ffbef R14: 00007f35997179c0 R15: 000000000078bf0c
Modules linked in:
---[ end trace d1398a63985d3915 ]---
RIP: 0010:d_backing_inode include/linux/dcache.h:549 [inline]
RIP: 0010:security_inode_getattr+0x46/0x140 security/security.c:1276
Code: 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 04 01 00 00 48 b8 00 00 00 00 00 fc ff df 49 8b 5d 08 48 8d 7b 60 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 d7 00 00 00 48 b8 00 00 00 00 00 fc ff df 48 8b
RSP: 0018:ffffc9000d41f638 EFLAGS: 00010206
RAX: dffffc0000000000 RBX: 0000000000000000 RCX: ffffc9000f539000
RDX: 000000000000000c RSI: ffffffff8354f8ee RDI: 0000000000000060
RBP: ffffc9000d41f810 R08: 0000000000000001 R09: ffff88804edc2dc8
R10: 0000000000000000 R11: 00000000000ebc58 R12: ffff888089f10170
R13: ffffc9000d41f810 R14: 00000000000007ff R15: 0000000000000000
FS:  00007f3599717700(0000) GS:ffff8880ae600000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020000440 CR3: 0000000099919000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

^ permalink raw reply

* Re: [PATCH v3 12/19] firmware_loader: Use security_post_load_data()
From: Kees Cook @ 2020-07-29 19:13 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Greg Kroah-Hartman, Scott Branden, Luis Chamberlain, Jessica Yu,
	SeongJae Park, KP Singh, linux-efi, linux-security-module,
	linux-integrity, selinux, linux-kselftest, linux-kernel
In-Reply-To: <e6f7ce4aa506ff016dde9a75c607849587c1ca2c.camel@linux.ibm.com>

On Wed, Jul 29, 2020 at 02:10:18PM -0400, Mimi Zohar wrote:
> Actually, the partial firmware read should be calling
> security_kernel_read_file().

Yup, it does[1], and when "whole_file" is true, it will call
security_kernel_post_read_file() with the buffer contents at the end.

> The sysfs firmware fallback is calling security_kernel_load_data().

Correct[2]; it has no file associated with it (same as the EFI platform
source).

> Which firmware is calling security_kernel_post_load_data()?

sysfs and platform both call it[2], matched with their
security_kernel_load_data() calls.

-Kees


[1] v4 patch 14: "fs/kernel_file_read: Add "offset" arg for partial reads"
    https://lore.kernel.org/lkml/20200729175845.1745471-1-keescook@chromium.org/T/#iZ2e.:..:20200729175845.1745471-15-keescook::40chromium.org:0fs:kernel_read_file.c
[2] v4 patch 10: "firmware_loader: Use security_post_load_data()"
    https://lore.kernel.org/lkml/20200729175845.1745471-1-keescook@chromium.org/T/#iZ2e.:..:20200729175845.1745471-11-keescook::40chromium.org:0drivers:base:firmware_loader:fallback.c

-- 
Kees Cook

^ permalink raw reply

* Re: [PATCH v3 12/19] firmware_loader: Use security_post_load_data()
From: Mimi Zohar @ 2020-07-29 18:10 UTC (permalink / raw)
  To: Kees Cook
  Cc: Greg Kroah-Hartman, Scott Branden, Luis Chamberlain, Jessica Yu,
	SeongJae Park, KP Singh, linux-efi, linux-security-module,
	linux-integrity, selinux, linux-kselftest, linux-kernel
In-Reply-To: <e5ed8876b9907315c2a906ab248639ea8c6d2cd5.camel@linux.ibm.com>

On Wed, 2020-07-29 at 12:29 -0400, Mimi Zohar wrote:
> On Tue, 2020-07-28 at 12:43 -0700, Kees Cook wrote:
> > On Mon, Jul 27, 2020 at 06:57:45AM -0400, Mimi Zohar wrote:
> > > On Fri, 2020-07-24 at 14:36 -0700, Kees Cook wrote:
> > > > Now that security_post_load_data() is wired up, use it instead
> > > > of the NULL file argument style of security_post_read_file(),
> > > > and update the security_kernel_load_data() call to indicate that a
> > > > security_kernel_post_load_data() call is expected.
> > > > 
> > > > Wire up the IMA check to match earlier logic. Perhaps a generalized
> > > > change to ima_post_load_data() might look something like this:
> > > > 
> > > >     return process_buffer_measurement(buf, size,
> > > >                                       kernel_load_data_id_str(load_id),
> > > >                                       read_idmap[load_id] ?: FILE_CHECK,
> > > >                                       0, NULL);
> > > > 
> > > > Signed-off-by: Kees Cook <keescook@chromium.org>
> > > 
> > > process_measurement() measures, verifies a file signature -  both
> > > signatures stored as an xattr and as an appended buffer signature -
> > > and augments audit records with the file hash. (Support for measuring,
> > > augmenting audit records, and/or verifying fs-verity signatures has
> > > yet to be added.)
> > > 
> > > As explained in my response to 11/19, the file descriptor provides the
> > > file pathname associated with the buffer data.  In addition, IMA
> > > policy rules may be defined in terms of other file descriptor info -
> > > uid, euid, uuid, etc.
> > > 
> > > Recently support was added for measuring the kexec boot command line,
> > > certificates being loaded onto a keyring, and blacklisted file hashes
> > > (limited to appended signatures).  None of these buffers are signed.
> > >  process_buffer_measurement() was added for this reason and as a
> > > result is limited to just measuring the buffer data.
> > > 
> > > Whether process_measurement() or process_buffer_measurement() should
> > > be modified, needs to be determined.  In either case to support the
> > > init_module syscall, would at minimum require the associated file
> > > pathname.
> > 
> > Right -- I don't intend to make changes to the init_module() syscall
> > since it's deprecated, so this hook is more of a "fuller LSM coverage
> > for old syscalls" addition.
> > 
> > IMA can happily continue to ignore it, which is what I have here, but I
> > thought I'd at least show what it *might* look like. Perhaps BPF LSM is
> > a better example.
> > 
> > Does anything need to change for this patch?
> 
> I wasn't aware that init_syscall was deprecated.  From your original comments,
> it sounded like you wanted a new LSM for verifying kernel module signatures,
> as
> they're currently supported via init_module().
> 
> I was mistaken.  Without a file descriptor, security_post_load_data() will
> measure the firmware, as Scott confirmed, but won't be able to verify the
> signature, whether he signed it using evmctl or not,

Actually, the partial firmware read should be calling
security_kernel_read_file().  The sysfs firmware fallback is calling
security_kernel_load_data().  Which firmware is calling
security_kernel_post_load_data()?

thanks,

Mimi


^ permalink raw reply

* [PATCH v4 17/17] test_firmware: Test partial read support
From: Kees Cook @ 2020-07-29 17:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Kees Cook, Scott Branden, Mimi Zohar, Luis Chamberlain,
	Takashi Iwai, Jessica Yu, SeongJae Park, KP Singh, linux-efi,
	linux-security-module, linux-integrity, selinux, linux-kselftest,
	linux-kernel
In-Reply-To: <20200729175845.1745471-1-keescook@chromium.org>

From: Scott Branden <scott.branden@broadcom.com>

Add additional hooks to test_firmware to pass in support
for partial file read using request_firmware_into_buf():

	buf_size: size of buffer to request firmware into
	partial: indicates that a partial file request is being made
	file_offset: to indicate offset into file to request

Also update firmware selftests to use the new partial read test API.

Signed-off-by: Scott Branden <scott.branden@broadcom.com>
Co-developed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 lib/test_firmware.c                           | 154 ++++++++++++++++--
 .../selftests/firmware/fw_filesystem.sh       |  91 +++++++++++
 2 files changed, 233 insertions(+), 12 deletions(-)

diff --git a/lib/test_firmware.c b/lib/test_firmware.c
index 62af792e151c..387acb94eeea 100644
--- a/lib/test_firmware.c
+++ b/lib/test_firmware.c
@@ -50,6 +50,9 @@ struct test_batched_req {
  * @name: the name of the firmware file to look for
  * @into_buf: when the into_buf is used if this is true
  *	request_firmware_into_buf() will be used instead.
+ * @buf_size: size of buf to allocate when into_buf is true
+ * @file_offset: file offset to request when calling request_firmware_into_buf
+ * @partial: partial read opt when calling request_firmware_into_buf
  * @sync_direct: when the sync trigger is used if this is true
  *	request_firmware_direct() will be used instead.
  * @send_uevent: whether or not to send a uevent for async requests
@@ -89,6 +92,9 @@ struct test_batched_req {
 struct test_config {
 	char *name;
 	bool into_buf;
+	size_t buf_size;
+	size_t file_offset;
+	bool partial;
 	bool sync_direct;
 	bool send_uevent;
 	u8 num_requests;
@@ -183,6 +189,9 @@ static int __test_firmware_config_init(void)
 	test_fw_config->num_requests = TEST_FIRMWARE_NUM_REQS;
 	test_fw_config->send_uevent = true;
 	test_fw_config->into_buf = false;
+	test_fw_config->buf_size = TEST_FIRMWARE_BUF_SIZE;
+	test_fw_config->file_offset = 0;
+	test_fw_config->partial = false;
 	test_fw_config->sync_direct = false;
 	test_fw_config->req_firmware = request_firmware;
 	test_fw_config->test_result = 0;
@@ -236,28 +245,35 @@ static ssize_t config_show(struct device *dev,
 			dev_name(dev));
 
 	if (test_fw_config->name)
-		len += scnprintf(buf+len, PAGE_SIZE - len,
+		len += scnprintf(buf + len, PAGE_SIZE - len,
 				"name:\t%s\n",
 				test_fw_config->name);
 	else
-		len += scnprintf(buf+len, PAGE_SIZE - len,
+		len += scnprintf(buf + len, PAGE_SIZE - len,
 				"name:\tEMTPY\n");
 
-	len += scnprintf(buf+len, PAGE_SIZE - len,
+	len += scnprintf(buf + len, PAGE_SIZE - len,
 			"num_requests:\t%u\n", test_fw_config->num_requests);
 
-	len += scnprintf(buf+len, PAGE_SIZE - len,
+	len += scnprintf(buf + len, PAGE_SIZE - len,
 			"send_uevent:\t\t%s\n",
 			test_fw_config->send_uevent ?
 			"FW_ACTION_HOTPLUG" :
 			"FW_ACTION_NOHOTPLUG");
-	len += scnprintf(buf+len, PAGE_SIZE - len,
+	len += scnprintf(buf + len, PAGE_SIZE - len,
 			"into_buf:\t\t%s\n",
 			test_fw_config->into_buf ? "true" : "false");
-	len += scnprintf(buf+len, PAGE_SIZE - len,
+	len += scnprintf(buf + len, PAGE_SIZE - len,
+			"buf_size:\t%zu\n", test_fw_config->buf_size);
+	len += scnprintf(buf + len, PAGE_SIZE - len,
+			"file_offset:\t%zu\n", test_fw_config->file_offset);
+	len += scnprintf(buf + len, PAGE_SIZE - len,
+			"partial:\t\t%s\n",
+			test_fw_config->partial ? "true" : "false");
+	len += scnprintf(buf + len, PAGE_SIZE - len,
 			"sync_direct:\t\t%s\n",
 			test_fw_config->sync_direct ? "true" : "false");
-	len += scnprintf(buf+len, PAGE_SIZE - len,
+	len += scnprintf(buf + len, PAGE_SIZE - len,
 			"read_fw_idx:\t%u\n", test_fw_config->read_fw_idx);
 
 	mutex_unlock(&test_fw_mutex);
@@ -315,6 +331,30 @@ static ssize_t test_dev_config_show_bool(char *buf, bool val)
 	return snprintf(buf, PAGE_SIZE, "%d\n", val);
 }
 
+static int test_dev_config_update_size_t(const char *buf,
+					 size_t size,
+					 size_t *cfg)
+{
+	int ret;
+	long new;
+
+	ret = kstrtol(buf, 10, &new);
+	if (ret)
+		return ret;
+
+	mutex_lock(&test_fw_mutex);
+	*(size_t *)cfg = new;
+	mutex_unlock(&test_fw_mutex);
+
+	/* Always return full write size even if we didn't consume all */
+	return size;
+}
+
+static ssize_t test_dev_config_show_size_t(char *buf, size_t val)
+{
+	return snprintf(buf, PAGE_SIZE, "%zu\n", val);
+}
+
 static ssize_t test_dev_config_show_int(char *buf, int val)
 {
 	return snprintf(buf, PAGE_SIZE, "%d\n", val);
@@ -400,6 +440,83 @@ static ssize_t config_into_buf_show(struct device *dev,
 }
 static DEVICE_ATTR_RW(config_into_buf);
 
+static ssize_t config_buf_size_store(struct device *dev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	int rc;
+
+	mutex_lock(&test_fw_mutex);
+	if (test_fw_config->reqs) {
+		pr_err("Must call release_all_firmware prior to changing config\n");
+		rc = -EINVAL;
+		mutex_unlock(&test_fw_mutex);
+		goto out;
+	}
+	mutex_unlock(&test_fw_mutex);
+
+	rc = test_dev_config_update_size_t(buf, count,
+					   &test_fw_config->buf_size);
+
+out:
+	return rc;
+}
+
+static ssize_t config_buf_size_show(struct device *dev,
+				    struct device_attribute *attr,
+				    char *buf)
+{
+	return test_dev_config_show_size_t(buf, test_fw_config->buf_size);
+}
+static DEVICE_ATTR_RW(config_buf_size);
+
+static ssize_t config_file_offset_store(struct device *dev,
+					struct device_attribute *attr,
+					const char *buf, size_t count)
+{
+	int rc;
+
+	mutex_lock(&test_fw_mutex);
+	if (test_fw_config->reqs) {
+		pr_err("Must call release_all_firmware prior to changing config\n");
+		rc = -EINVAL;
+		mutex_unlock(&test_fw_mutex);
+		goto out;
+	}
+	mutex_unlock(&test_fw_mutex);
+
+	rc = test_dev_config_update_size_t(buf, count,
+					   &test_fw_config->file_offset);
+
+out:
+	return rc;
+}
+
+static ssize_t config_file_offset_show(struct device *dev,
+				       struct device_attribute *attr,
+				       char *buf)
+{
+	return test_dev_config_show_size_t(buf, test_fw_config->file_offset);
+}
+static DEVICE_ATTR_RW(config_file_offset);
+
+static ssize_t config_partial_store(struct device *dev,
+				    struct device_attribute *attr,
+				    const char *buf, size_t count)
+{
+	return test_dev_config_update_bool(buf,
+					   count,
+					   &test_fw_config->partial);
+}
+
+static ssize_t config_partial_show(struct device *dev,
+				   struct device_attribute *attr,
+				   char *buf)
+{
+	return test_dev_config_show_bool(buf, test_fw_config->partial);
+}
+static DEVICE_ATTR_RW(config_partial);
+
 static ssize_t config_sync_direct_store(struct device *dev,
 					struct device_attribute *attr,
 					const char *buf, size_t count)
@@ -655,11 +772,21 @@ static int test_fw_run_batch_request(void *data)
 		if (!test_buf)
 			return -ENOSPC;
 
-		req->rc = request_firmware_into_buf(&req->fw,
-						    req->name,
-						    req->dev,
-						    test_buf,
-						    TEST_FIRMWARE_BUF_SIZE);
+		if (test_fw_config->partial)
+			req->rc = request_partial_firmware_into_buf
+						(&req->fw,
+						 req->name,
+						 req->dev,
+						 test_buf,
+						 test_fw_config->buf_size,
+						 test_fw_config->file_offset);
+		else
+			req->rc = request_firmware_into_buf
+						(&req->fw,
+						 req->name,
+						 req->dev,
+						 test_buf,
+						 test_fw_config->buf_size);
 		if (!req->fw)
 			kfree(test_buf);
 	} else {
@@ -932,6 +1059,9 @@ static struct attribute *test_dev_attrs[] = {
 	TEST_FW_DEV_ATTR(config_name),
 	TEST_FW_DEV_ATTR(config_num_requests),
 	TEST_FW_DEV_ATTR(config_into_buf),
+	TEST_FW_DEV_ATTR(config_buf_size),
+	TEST_FW_DEV_ATTR(config_file_offset),
+	TEST_FW_DEV_ATTR(config_partial),
 	TEST_FW_DEV_ATTR(config_sync_direct),
 	TEST_FW_DEV_ATTR(config_send_uevent),
 	TEST_FW_DEV_ATTR(config_read_fw_idx),
diff --git a/tools/testing/selftests/firmware/fw_filesystem.sh b/tools/testing/selftests/firmware/fw_filesystem.sh
index fcc281373b4d..c2a2a100114b 100755
--- a/tools/testing/selftests/firmware/fw_filesystem.sh
+++ b/tools/testing/selftests/firmware/fw_filesystem.sh
@@ -149,6 +149,26 @@ config_unset_into_buf()
 	echo 0 >  $DIR/config_into_buf
 }
 
+config_set_buf_size()
+{
+	echo $1 >  $DIR/config_buf_size
+}
+
+config_set_file_offset()
+{
+	echo $1 >  $DIR/config_file_offset
+}
+
+config_set_partial()
+{
+	echo 1 >  $DIR/config_partial
+}
+
+config_unset_partial()
+{
+	echo 0 >  $DIR/config_partial
+}
+
 config_set_sync_direct()
 {
 	echo 1 >  $DIR/config_sync_direct
@@ -207,6 +227,35 @@ read_firmwares()
 	done
 }
 
+read_partial_firmwares()
+{
+	if [ "$(cat $DIR/config_into_buf)" == "1" ]; then
+		fwfile="${FW_INTO_BUF}"
+	else
+		fwfile="${FW}"
+	fi
+
+	if [ "$1" = "xzonly" ]; then
+		fwfile="${fwfile}-orig"
+	fi
+
+	# Strip fwfile down to match partial offset and length
+	partial_data="$(cat $fwfile)"
+	partial_data="${partial_data:$2:$3}"
+
+	for i in $(seq 0 3); do
+		config_set_read_fw_idx $i
+
+		read_firmware="$(cat $DIR/read_firmware)"
+
+		# Verify the contents are what we expect.
+		if [ $read_firmware != $partial_data ]; then
+			echo "request #$i: partial firmware was not loaded" >&2
+			exit 1
+		fi
+	done
+}
+
 read_firmwares_expect_nofile()
 {
 	for i in $(seq 0 3); do
@@ -242,6 +291,21 @@ test_batched_request_firmware_into_buf_nofile()
 	echo "OK"
 }
 
+test_request_partial_firmware_into_buf_nofile()
+{
+	echo -n "Test request_partial_firmware_into_buf() off=$1 size=$2 nofile: "
+	config_reset
+	config_set_name nope-test-firmware.bin
+	config_set_into_buf
+	config_set_partial
+	config_set_buf_size $2
+	config_set_file_offset $1
+	config_trigger_sync
+	read_firmwares_expect_nofile
+	release_all_firmware
+	echo "OK"
+}
+
 test_batched_request_firmware_direct_nofile()
 {
 	echo -n "Batched request_firmware_direct() nofile try #$1: "
@@ -356,6 +420,21 @@ test_request_firmware_nowait_custom()
 	echo "OK"
 }
 
+test_request_partial_firmware_into_buf()
+{
+	echo -n "Test request_partial_firmware_into_buf() off=$1 size=$2: "
+	config_reset
+	config_set_name $TEST_FIRMWARE_INTO_BUF_FILENAME
+	config_set_into_buf
+	config_set_partial
+	config_set_buf_size $2
+	config_set_file_offset $1
+	config_trigger_sync
+	read_partial_firmwares normal $1 $2
+	release_all_firmware
+	echo "OK"
+}
+
 # Only continue if batched request triggers are present on the
 # test-firmware driver
 test_config_present
@@ -383,6 +462,12 @@ for i in $(seq 1 5); do
 	test_request_firmware_nowait_custom $i normal
 done
 
+# Partial loads cannot use fallback, so do not repeat tests.
+test_request_partial_firmware_into_buf 0 10
+test_request_partial_firmware_into_buf 0 5
+test_request_partial_firmware_into_buf 1 6
+test_request_partial_firmware_into_buf 2 10
+
 # Test for file not found, errors are expected, the failure would be
 # a hung task, which would require a hard reset.
 echo
@@ -407,6 +492,12 @@ for i in $(seq 1 5); do
 	test_request_firmware_nowait_custom_nofile $i
 done
 
+# Partial loads cannot use fallback, so do not repeat tests.
+test_request_partial_firmware_into_buf_nofile 0 10
+test_request_partial_firmware_into_buf_nofile 0 5
+test_request_partial_firmware_into_buf_nofile 1 6
+test_request_partial_firmware_into_buf_nofile 2 10
+
 test "$HAS_FW_LOADER_COMPRESS" != "yes" && exit 0
 
 # test with both files present
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 01/17] test_firmware: Test platform fw loading on non-EFI systems
From: Kees Cook @ 2020-07-29 17:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Kees Cook, stable, Scott Branden, Luis Chamberlain, Mimi Zohar,
	Takashi Iwai, Jessica Yu, SeongJae Park, KP Singh, linux-efi,
	linux-security-module, linux-integrity, selinux, linux-kselftest,
	linux-kernel
In-Reply-To: <20200729175845.1745471-1-keescook@chromium.org>

On non-EFI systems, it wasn't possible to test the platform firmware
loader because it will have never set "checked_fw" during __init.
Instead, allow the test code to override this check. Additionally split
the declarations into a private header file so it there is greater
enforcement of the symbol visibility.

Fixes: 548193cba2a7 ("test_firmware: add support for firmware_request_platform")
Cc: stable@vger.kernel.org
Acked-by: Scott Branden <scott.branden@broadcom.com>
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 drivers/firmware/efi/embedded-firmware.c | 21 ++++++++++++++++-----
 drivers/firmware/efi/embedded-firmware.h | 21 +++++++++++++++++++++
 include/linux/efi_embedded_fw.h          | 13 -------------
 lib/test_firmware.c                      |  5 +++++
 4 files changed, 42 insertions(+), 18 deletions(-)
 create mode 100644 drivers/firmware/efi/embedded-firmware.h

diff --git a/drivers/firmware/efi/embedded-firmware.c b/drivers/firmware/efi/embedded-firmware.c
index a1b199de9006..0fb03cd0a5a2 100644
--- a/drivers/firmware/efi/embedded-firmware.c
+++ b/drivers/firmware/efi/embedded-firmware.c
@@ -14,11 +14,22 @@
 #include <linux/vmalloc.h>
 #include <crypto/sha.h>
 
+#include "embedded-firmware.h"
+
+#ifdef CONFIG_TEST_FIRMWARE
+# define EFI_EMBEDDED_FW_VISIBILITY
+#else
+# define EFI_EMBEDDED_FW_VISIBILITY static
+#endif
+
+EFI_EMBEDDED_FW_VISIBILITY LIST_HEAD(efi_embedded_fw_list);
+EFI_EMBEDDED_FW_VISIBILITY bool efi_embedded_fw_checked;
+
 /* Exported for use by lib/test_firmware.c only */
-LIST_HEAD(efi_embedded_fw_list);
+#ifdef CONFIG_TEST_FIRMWARE
 EXPORT_SYMBOL_GPL(efi_embedded_fw_list);
-
-static bool checked_for_fw;
+EXPORT_SYMBOL_GPL(efi_embedded_fw_checked);
+#endif
 
 static const struct dmi_system_id * const embedded_fw_table[] = {
 #ifdef CONFIG_TOUCHSCREEN_DMI
@@ -119,14 +130,14 @@ void __init efi_check_for_embedded_firmwares(void)
 		}
 	}
 
-	checked_for_fw = true;
+	efi_embedded_fw_checked = true;
 }
 
 int efi_get_embedded_fw(const char *name, const u8 **data, size_t *size)
 {
 	struct efi_embedded_fw *iter, *fw = NULL;
 
-	if (!checked_for_fw) {
+	if (!efi_embedded_fw_checked) {
 		pr_warn("Warning %s called while we did not check for embedded fw\n",
 			__func__);
 		return -ENOENT;
diff --git a/drivers/firmware/efi/embedded-firmware.h b/drivers/firmware/efi/embedded-firmware.h
new file mode 100644
index 000000000000..bb894eae0906
--- /dev/null
+++ b/drivers/firmware/efi/embedded-firmware.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _EFI_EMBEDDED_FW_INTERNAL_H_
+#define _EFI_EMBEDDED_FW_INTERNAL_H_
+
+/*
+ * This struct and efi_embedded_fw_list are private to the efi-embedded fw
+ * implementation they only in separate header for use by lib/test_firmware.c.
+ */
+struct efi_embedded_fw {
+	struct list_head list;
+	const char *name;
+	const u8 *data;
+	size_t length;
+};
+
+#ifdef CONFIG_TEST_FIRMWARE
+extern struct list_head efi_embedded_fw_list;
+extern bool efi_embedded_fw_checked;
+#endif
+
+#endif /* _EFI_EMBEDDED_FW_INTERNAL_H_ */
diff --git a/include/linux/efi_embedded_fw.h b/include/linux/efi_embedded_fw.h
index 57eac5241303..4ad5db9f5312 100644
--- a/include/linux/efi_embedded_fw.h
+++ b/include/linux/efi_embedded_fw.h
@@ -7,19 +7,6 @@
 
 #define EFI_EMBEDDED_FW_PREFIX_LEN		8
 
-/*
- * This struct and efi_embedded_fw_list are private to the efi-embedded fw
- * implementation they are in this header for use by lib/test_firmware.c only!
- */
-struct efi_embedded_fw {
-	struct list_head list;
-	const char *name;
-	const u8 *data;
-	size_t length;
-};
-
-extern struct list_head efi_embedded_fw_list;
-
 /**
  * struct efi_embedded_fw_desc - This struct is used by the EFI embedded-fw
  *                               code to search for embedded firmwares.
diff --git a/lib/test_firmware.c b/lib/test_firmware.c
index 9fee2b93a8d1..62af792e151c 100644
--- a/lib/test_firmware.c
+++ b/lib/test_firmware.c
@@ -489,6 +489,7 @@ static ssize_t trigger_request_store(struct device *dev,
 static DEVICE_ATTR_WO(trigger_request);
 
 #ifdef CONFIG_EFI_EMBEDDED_FIRMWARE
+#include "../drivers/firmware/efi/embedded-firmware.h"
 static ssize_t trigger_request_platform_store(struct device *dev,
 					      struct device_attribute *attr,
 					      const char *buf, size_t count)
@@ -501,6 +502,7 @@ static ssize_t trigger_request_platform_store(struct device *dev,
 	};
 	struct efi_embedded_fw efi_embedded_fw;
 	const struct firmware *firmware = NULL;
+	bool saved_efi_embedded_fw_checked;
 	char *name;
 	int rc;
 
@@ -513,6 +515,8 @@ static ssize_t trigger_request_platform_store(struct device *dev,
 	efi_embedded_fw.data = (void *)test_data;
 	efi_embedded_fw.length = sizeof(test_data);
 	list_add(&efi_embedded_fw.list, &efi_embedded_fw_list);
+	saved_efi_embedded_fw_checked = efi_embedded_fw_checked;
+	efi_embedded_fw_checked = true;
 
 	pr_info("loading '%s'\n", name);
 	rc = firmware_request_platform(&firmware, name, dev);
@@ -530,6 +534,7 @@ static ssize_t trigger_request_platform_store(struct device *dev,
 	rc = count;
 
 out:
+	efi_embedded_fw_checked = saved_efi_embedded_fw_checked;
 	release_firmware(firmware);
 	list_del(&efi_embedded_fw.list);
 	kfree(name);
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 02/17] fs/kernel_read_file: Remove FIRMWARE_PREALLOC_BUFFER enum
From: Kees Cook @ 2020-07-29 17:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Kees Cook, stable, Scott Branden, Mimi Zohar, Luis Chamberlain,
	Takashi Iwai, Jessica Yu, SeongJae Park, KP Singh, linux-efi,
	linux-security-module, linux-integrity, selinux, linux-kselftest,
	linux-kernel
In-Reply-To: <20200729175845.1745471-1-keescook@chromium.org>

FIRMWARE_PREALLOC_BUFFER is a "how", not a "what", and confuses the LSMs
that are interested in filtering between types of things. The "how"
should be an internal detail made uninteresting to the LSMs.

Fixes: a098ecd2fa7d ("firmware: support loading into a pre-allocated buffer")
Fixes: fd90bc559bfb ("ima: based on policy verify firmware signatures (pre-allocated buffer)")
Fixes: 4f0496d8ffa3 ("ima: based on policy warn about loading firmware (pre-allocated buffer)")
Cc: stable@vger.kernel.org
Acked-by: Scott Branden <scott.branden@broadcom.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
To aid in backporting, this change is made before moving
kernel_read_file() to separate header/source files.
---
 drivers/base/firmware_loader/main.c | 5 ++---
 fs/exec.c                           | 7 ++++---
 include/linux/fs.h                  | 2 +-
 kernel/module.c                     | 2 +-
 security/integrity/digsig.c         | 2 +-
 security/integrity/ima/ima_fs.c     | 2 +-
 security/integrity/ima/ima_main.c   | 6 ++----
 7 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 9da0c9d5f538..fe68ae278201 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -465,14 +465,12 @@ fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv,
 	int i, len;
 	int rc = -ENOENT;
 	char *path;
-	enum kernel_read_file_id id = READING_FIRMWARE;
 	size_t msize = INT_MAX;
 	void *buffer = NULL;
 
 	/* Already populated data member means we're loading into a buffer */
 	if (!decompress && fw_priv->data) {
 		buffer = fw_priv->data;
-		id = READING_FIRMWARE_PREALLOC_BUFFER;
 		msize = fw_priv->allocated_size;
 	}
 
@@ -496,7 +494,8 @@ fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv,
 
 		/* load firmware files from the mount namespace of init */
 		rc = kernel_read_file_from_path_initns(path, &buffer,
-						       &size, msize, id);
+						       &size, msize,
+						       READING_FIRMWARE);
 		if (rc) {
 			if (rc != -ENOENT)
 				dev_warn(device, "loading %s failed with error %d\n",
diff --git a/fs/exec.c b/fs/exec.c
index e6e8a9a70327..2bf549757ce7 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -927,6 +927,7 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size,
 {
 	loff_t i_size, pos;
 	ssize_t bytes = 0;
+	void *allocated = NULL;
 	int ret;
 
 	if (!S_ISREG(file_inode(file)->i_mode) || max_size < 0)
@@ -950,8 +951,8 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size,
 		goto out;
 	}
 
-	if (id != READING_FIRMWARE_PREALLOC_BUFFER)
-		*buf = vmalloc(i_size);
+	if (!*buf)
+		*buf = allocated = vmalloc(i_size);
 	if (!*buf) {
 		ret = -ENOMEM;
 		goto out;
@@ -980,7 +981,7 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size,
 
 out_free:
 	if (ret < 0) {
-		if (id != READING_FIRMWARE_PREALLOC_BUFFER) {
+		if (allocated) {
 			vfree(*buf);
 			*buf = NULL;
 		}
diff --git a/include/linux/fs.h b/include/linux/fs.h
index f5abba86107d..f34d47ba49de 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2993,10 +2993,10 @@ static inline void i_readcount_inc(struct inode *inode)
 #endif
 extern int do_pipe_flags(int *, int);
 
+/* This is a list of *what* is being read, not *how*. */
 #define __kernel_read_file_id(id) \
 	id(UNKNOWN, unknown)		\
 	id(FIRMWARE, firmware)		\
-	id(FIRMWARE_PREALLOC_BUFFER, firmware)	\
 	id(FIRMWARE_EFI_EMBEDDED, firmware)	\
 	id(MODULE, kernel-module)		\
 	id(KEXEC_IMAGE, kexec-image)		\
diff --git a/kernel/module.c b/kernel/module.c
index aa183c9ac0a2..d2d12c299dd8 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3991,7 +3991,7 @@ SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
 {
 	struct load_info info = { };
 	loff_t size;
-	void *hdr;
+	void *hdr = NULL;
 	int err;
 
 	err = may_init_module();
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index e9cbadade74b..ac02b7632353 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -169,7 +169,7 @@ int __init integrity_add_key(const unsigned int id, const void *data,
 
 int __init integrity_load_x509(const unsigned int id, const char *path)
 {
-	void *data;
+	void *data = NULL;
 	loff_t size;
 	int rc;
 	key_perm_t perm;
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index e3fcad871861..15a44c5022f7 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -272,7 +272,7 @@ static const struct file_operations ima_ascii_measurements_ops = {
 
 static ssize_t ima_read_policy(char *path)
 {
-	void *data;
+	void *data = NULL;
 	char *datap;
 	loff_t size;
 	int rc, pathlen = strlen(path);
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index c1583d98c5e5..f80ee4ce4669 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -611,19 +611,17 @@ void ima_post_path_mknod(struct dentry *dentry)
 int ima_read_file(struct file *file, enum kernel_read_file_id read_id)
 {
 	/*
-	 * READING_FIRMWARE_PREALLOC_BUFFER
-	 *
 	 * Do devices using pre-allocated memory run the risk of the
 	 * firmware being accessible to the device prior to the completion
 	 * of IMA's signature verification any more than when using two
-	 * buffers?
+	 * buffers? It may be desirable to include the buffer address
+	 * in this API and walk all the dma_map_single() mappings to check.
 	 */
 	return 0;
 }
 
 const int read_idmap[READING_MAX_ID] = {
 	[READING_FIRMWARE] = FIRMWARE_CHECK,
-	[READING_FIRMWARE_PREALLOC_BUFFER] = FIRMWARE_CHECK,
 	[READING_MODULE] = MODULE_CHECK,
 	[READING_KEXEC_IMAGE] = KEXEC_KERNEL_CHECK,
 	[READING_KEXEC_INITRAMFS] = KEXEC_INITRAMFS_CHECK,
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 03/17] fs/kernel_read_file: Remove FIRMWARE_EFI_EMBEDDED enum
From: Kees Cook @ 2020-07-29 17:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Kees Cook, stable, Scott Branden, Luis Chamberlain, Mimi Zohar,
	Takashi Iwai, Jessica Yu, SeongJae Park, KP Singh, linux-efi,
	linux-security-module, linux-integrity, selinux, linux-kselftest,
	linux-kernel
In-Reply-To: <20200729175845.1745471-1-keescook@chromium.org>

The "FIRMWARE_EFI_EMBEDDED" enum is a "where", not a "what". It
should not be distinguished separately from just "FIRMWARE", as this
confuses the LSMs about what is being loaded. Additionally, there was
no actual validation of the firmware contents happening.

Fixes: e4c2c0ff00ec ("firmware: Add new platform fallback mechanism and firmware_request_platform()")
Cc: stable@vger.kernel.org
Acked-by: Scott Branden <scott.branden@broadcom.com>
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
To aid in backporting, this change is made before moving
kernel_read_file() to separate header/source files.
---
 drivers/base/firmware_loader/fallback_platform.c | 2 +-
 include/linux/fs.h                               | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/base/firmware_loader/fallback_platform.c b/drivers/base/firmware_loader/fallback_platform.c
index 685edb7dd05a..6958ab1a8059 100644
--- a/drivers/base/firmware_loader/fallback_platform.c
+++ b/drivers/base/firmware_loader/fallback_platform.c
@@ -17,7 +17,7 @@ int firmware_fallback_platform(struct fw_priv *fw_priv, u32 opt_flags)
 	if (!(opt_flags & FW_OPT_FALLBACK_PLATFORM))
 		return -ENOENT;
 
-	rc = security_kernel_load_data(LOADING_FIRMWARE_EFI_EMBEDDED);
+	rc = security_kernel_load_data(LOADING_FIRMWARE);
 	if (rc)
 		return rc;
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index f34d47ba49de..0d4f7aacf286 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2993,11 +2993,10 @@ static inline void i_readcount_inc(struct inode *inode)
 #endif
 extern int do_pipe_flags(int *, int);
 
-/* This is a list of *what* is being read, not *how*. */
+/* This is a list of *what* is being read, not *how* nor *where*. */
 #define __kernel_read_file_id(id) \
 	id(UNKNOWN, unknown)		\
 	id(FIRMWARE, firmware)		\
-	id(FIRMWARE_EFI_EMBEDDED, firmware)	\
 	id(MODULE, kernel-module)		\
 	id(KEXEC_IMAGE, kexec-image)		\
 	id(KEXEC_INITRAMFS, kexec-initramfs)	\
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 00/17] Introduce partial kernel_read_file() support
From: Kees Cook @ 2020-07-29 17:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Kees Cook, Scott Branden, Mimi Zohar, Luis Chamberlain,
	Takashi Iwai, Jessica Yu, SeongJae Park, KP Singh, linux-efi,
	linux-security-module, linux-integrity, selinux, linux-kselftest,
	linux-kernel

v4:
- add more reviews (mimi, luis)
- adjusted comment (mimi)
- fixed build error when not building firmware tests (0day, sfr)
- fixed needless .xz read (tiwai)
- rebased to driver-core-next
v3: https://lore.kernel.org/lkml/20200724213640.389191-1-keescook@chromium.org/
v2: lost to the ether
v1: https://lore.kernel.org/lkml/20200717174309.1164575-1-keescook@chromium.org/

Hi,

Here's my tree for adding partial read support in kernel_read_file(),
which fixes a number of issues along the way. It's got Scott's firmware
and IMA patches ported and everything tests cleanly for me (even with
CONFIG_IMA_APPRAISE=y), and now appears to pass 0day. :)

The intention is for this to go via Greg's tree since Scott's driver
code will depend on it.

Thanks,

-Kees

Kees Cook (13):
  test_firmware: Test platform fw loading on non-EFI systems
  fs/kernel_read_file: Remove FIRMWARE_PREALLOC_BUFFER enum
  fs/kernel_read_file: Remove FIRMWARE_EFI_EMBEDDED enum
  fs/kernel_read_file: Split into separate source file
  fs/kernel_read_file: Remove redundant size argument
  fs/kernel_read_file: Switch buffer size arg to size_t
  fs/kernel_read_file: Add file_size output argument
  LSM: Introduce kernel_post_load_data() hook
  firmware_loader: Use security_post_load_data()
  module: Call security_kernel_post_load_data()
  LSM: Add "contents" flag to kernel_read_file hook
  fs/kernel_file_read: Add "offset" arg for partial reads
  firmware: Store opt_flags in fw_priv

Scott Branden (4):
  fs/kernel_read_file: Split into separate include file
  IMA: Add support for file reads without contents
  firmware: Add request_partial_firmware_into_buf()
  test_firmware: Test partial read support

 drivers/base/firmware_loader/fallback.c       |  19 +-
 drivers/base/firmware_loader/fallback.h       |   5 +-
 .../base/firmware_loader/fallback_platform.c  |  11 +-
 drivers/base/firmware_loader/firmware.h       |   7 +-
 drivers/base/firmware_loader/main.c           | 135 ++++++++++---
 drivers/firmware/efi/embedded-firmware.c      |  21 +-
 drivers/firmware/efi/embedded-firmware.h      |  21 ++
 fs/Makefile                                   |   3 +-
 fs/exec.c                                     | 132 +-----------
 fs/kernel_read_file.c                         | 189 ++++++++++++++++++
 include/linux/efi_embedded_fw.h               |  13 --
 include/linux/firmware.h                      |  12 ++
 include/linux/fs.h                            |  39 ----
 include/linux/ima.h                           |  19 +-
 include/linux/kernel_read_file.h              |  55 +++++
 include/linux/lsm_hook_defs.h                 |   6 +-
 include/linux/lsm_hooks.h                     |  12 ++
 include/linux/security.h                      |  19 +-
 kernel/kexec.c                                |   2 +-
 kernel/kexec_file.c                           |  19 +-
 kernel/module.c                               |  24 ++-
 lib/test_firmware.c                           | 159 +++++++++++++--
 security/integrity/digsig.c                   |   8 +-
 security/integrity/ima/ima_fs.c               |  10 +-
 security/integrity/ima/ima_main.c             |  70 +++++--
 security/integrity/ima/ima_policy.c           |   1 +
 security/loadpin/loadpin.c                    |  17 +-
 security/security.c                           |  26 ++-
 security/selinux/hooks.c                      |   8 +-
 .../selftests/firmware/fw_filesystem.sh       |  91 +++++++++
 30 files changed, 839 insertions(+), 314 deletions(-)
 create mode 100644 drivers/firmware/efi/embedded-firmware.h
 create mode 100644 fs/kernel_read_file.c
 create mode 100644 include/linux/kernel_read_file.h

-- 
2.25.1


^ permalink raw reply

* [PATCH v4 04/17] fs/kernel_read_file: Split into separate include file
From: Kees Cook @ 2020-07-29 17:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Kees Cook, Scott Branden, Christoph Hellwig, Mimi Zohar,
	Luis Chamberlain, Takashi Iwai, Jessica Yu, SeongJae Park,
	KP Singh, linux-efi, linux-security-module, linux-integrity,
	selinux, linux-kselftest, linux-kernel
In-Reply-To: <20200729175845.1745471-1-keescook@chromium.org>

From: Scott Branden <scott.branden@broadcom.com>

Move kernel_read_file* out of linux/fs.h to its own linux/kernel_read_file.h
include file. That header gets pulled in just about everywhere
and doesn't really need functions not related to the general fs interface.

Suggested-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Scott Branden <scott.branden@broadcom.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Link: https://lore.kernel.org/r/20200706232309.12010-2-scott.branden@broadcom.com
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 drivers/base/firmware_loader/main.c |  1 +
 fs/exec.c                           |  1 +
 include/linux/fs.h                  | 38 ---------------------
 include/linux/ima.h                 |  1 +
 include/linux/kernel_read_file.h    | 51 +++++++++++++++++++++++++++++
 include/linux/security.h            |  1 +
 kernel/kexec_file.c                 |  1 +
 kernel/module.c                     |  1 +
 security/integrity/digsig.c         |  1 +
 security/integrity/ima/ima_fs.c     |  1 +
 security/integrity/ima/ima_main.c   |  1 +
 security/integrity/ima/ima_policy.c |  1 +
 security/loadpin/loadpin.c          |  1 +
 security/security.c                 |  1 +
 security/selinux/hooks.c            |  1 +
 15 files changed, 64 insertions(+), 38 deletions(-)
 create mode 100644 include/linux/kernel_read_file.h

diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index fe68ae278201..7fd677281806 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -12,6 +12,7 @@
 
 #include <linux/capability.h>
 #include <linux/device.h>
+#include <linux/kernel_read_file.h>
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/timer.h>
diff --git a/fs/exec.c b/fs/exec.c
index 2bf549757ce7..07a7fe9ac5be 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -23,6 +23,7 @@
  * formats.
  */
 
+#include <linux/kernel_read_file.h>
 #include <linux/slab.h>
 #include <linux/file.h>
 #include <linux/fdtable.h>
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 0d4f7aacf286..76283ff04d37 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2993,44 +2993,6 @@ static inline void i_readcount_inc(struct inode *inode)
 #endif
 extern int do_pipe_flags(int *, int);
 
-/* This is a list of *what* is being read, not *how* nor *where*. */
-#define __kernel_read_file_id(id) \
-	id(UNKNOWN, unknown)		\
-	id(FIRMWARE, firmware)		\
-	id(MODULE, kernel-module)		\
-	id(KEXEC_IMAGE, kexec-image)		\
-	id(KEXEC_INITRAMFS, kexec-initramfs)	\
-	id(POLICY, security-policy)		\
-	id(X509_CERTIFICATE, x509-certificate)	\
-	id(MAX_ID, )
-
-#define __fid_enumify(ENUM, dummy) READING_ ## ENUM,
-#define __fid_stringify(dummy, str) #str,
-
-enum kernel_read_file_id {
-	__kernel_read_file_id(__fid_enumify)
-};
-
-static const char * const kernel_read_file_str[] = {
-	__kernel_read_file_id(__fid_stringify)
-};
-
-static inline const char *kernel_read_file_id_str(enum kernel_read_file_id id)
-{
-	if ((unsigned)id >= READING_MAX_ID)
-		return kernel_read_file_str[READING_UNKNOWN];
-
-	return kernel_read_file_str[id];
-}
-
-extern int kernel_read_file(struct file *, void **, loff_t *, loff_t,
-			    enum kernel_read_file_id);
-extern int kernel_read_file_from_path(const char *, void **, loff_t *, loff_t,
-				      enum kernel_read_file_id);
-extern int kernel_read_file_from_path_initns(const char *, void **, loff_t *, loff_t,
-					     enum kernel_read_file_id);
-extern int kernel_read_file_from_fd(int, void **, loff_t *, loff_t,
-				    enum kernel_read_file_id);
 extern ssize_t kernel_read(struct file *, void *, size_t, loff_t *);
 ssize_t __kernel_read(struct file *file, void *buf, size_t count, loff_t *pos);
 extern ssize_t kernel_write(struct file *, const void *, size_t, loff_t *);
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 9164e1534ec9..148636bfcc8f 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -7,6 +7,7 @@
 #ifndef _LINUX_IMA_H
 #define _LINUX_IMA_H
 
+#include <linux/kernel_read_file.h>
 #include <linux/fs.h>
 #include <linux/security.h>
 #include <linux/kexec.h>
diff --git a/include/linux/kernel_read_file.h b/include/linux/kernel_read_file.h
new file mode 100644
index 000000000000..78cf3d7dc835
--- /dev/null
+++ b/include/linux/kernel_read_file.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_KERNEL_READ_FILE_H
+#define _LINUX_KERNEL_READ_FILE_H
+
+#include <linux/file.h>
+#include <linux/types.h>
+
+/* This is a list of *what* is being read, not *how* nor *where*. */
+#define __kernel_read_file_id(id) \
+	id(UNKNOWN, unknown)		\
+	id(FIRMWARE, firmware)		\
+	id(MODULE, kernel-module)		\
+	id(KEXEC_IMAGE, kexec-image)		\
+	id(KEXEC_INITRAMFS, kexec-initramfs)	\
+	id(POLICY, security-policy)		\
+	id(X509_CERTIFICATE, x509-certificate)	\
+	id(MAX_ID, )
+
+#define __fid_enumify(ENUM, dummy) READING_ ## ENUM,
+#define __fid_stringify(dummy, str) #str,
+
+enum kernel_read_file_id {
+	__kernel_read_file_id(__fid_enumify)
+};
+
+static const char * const kernel_read_file_str[] = {
+	__kernel_read_file_id(__fid_stringify)
+};
+
+static inline const char *kernel_read_file_id_str(enum kernel_read_file_id id)
+{
+	if ((unsigned int)id >= READING_MAX_ID)
+		return kernel_read_file_str[READING_UNKNOWN];
+
+	return kernel_read_file_str[id];
+}
+
+int kernel_read_file(struct file *file,
+		     void **buf, loff_t *size, loff_t max_size,
+		     enum kernel_read_file_id id);
+int kernel_read_file_from_path(const char *path,
+			       void **buf, loff_t *size, loff_t max_size,
+			       enum kernel_read_file_id id);
+int kernel_read_file_from_path_initns(const char *path,
+				      void **buf, loff_t *size, loff_t max_size,
+				      enum kernel_read_file_id id);
+int kernel_read_file_from_fd(int fd,
+			     void **buf, loff_t *size, loff_t max_size,
+			     enum kernel_read_file_id id);
+
+#endif /* _LINUX_KERNEL_READ_FILE_H */
diff --git a/include/linux/security.h b/include/linux/security.h
index 0a0a03b36a3b..42df0d9b4c37 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -23,6 +23,7 @@
 #ifndef __LINUX_SECURITY_H
 #define __LINUX_SECURITY_H
 
+#include <linux/kernel_read_file.h>
 #include <linux/key.h>
 #include <linux/capability.h>
 #include <linux/fs.h>
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 09cc78df53c6..1358069ce9e9 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -24,6 +24,7 @@
 #include <linux/elf.h>
 #include <linux/elfcore.h>
 #include <linux/kernel.h>
+#include <linux/kernel_read_file.h>
 #include <linux/syscalls.h>
 #include <linux/vmalloc.h>
 #include "kexec_internal.h"
diff --git a/kernel/module.c b/kernel/module.c
index d2d12c299dd8..cc8d83841568 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -18,6 +18,7 @@
 #include <linux/fs.h>
 #include <linux/sysfs.h>
 #include <linux/kernel.h>
+#include <linux/kernel_read_file.h>
 #include <linux/slab.h>
 #include <linux/vmalloc.h>
 #include <linux/elf.h>
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index ac02b7632353..f8869be45d8f 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -10,6 +10,7 @@
 #include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/cred.h>
+#include <linux/kernel_read_file.h>
 #include <linux/key-type.h>
 #include <linux/digsig.h>
 #include <linux/vmalloc.h>
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 15a44c5022f7..e13ffece3726 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -13,6 +13,7 @@
  */
 
 #include <linux/fcntl.h>
+#include <linux/kernel_read_file.h>
 #include <linux/slab.h>
 #include <linux/init.h>
 #include <linux/seq_file.h>
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index f80ee4ce4669..dab4a13221cf 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -18,6 +18,7 @@
 #include <linux/module.h>
 #include <linux/file.h>
 #include <linux/binfmts.h>
+#include <linux/kernel_read_file.h>
 #include <linux/mount.h>
 #include <linux/mman.h>
 #include <linux/slab.h>
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index e493063a3c34..f8390f6081f0 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -9,6 +9,7 @@
 
 #include <linux/init.h>
 #include <linux/list.h>
+#include <linux/kernel_read_file.h>
 #include <linux/fs.h>
 #include <linux/security.h>
 #include <linux/magic.h>
diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
index ee5cb944f4ad..81bc95127f92 100644
--- a/security/loadpin/loadpin.c
+++ b/security/loadpin/loadpin.c
@@ -11,6 +11,7 @@
 
 #include <linux/module.h>
 #include <linux/fs.h>
+#include <linux/kernel_read_file.h>
 #include <linux/lsm_hooks.h>
 #include <linux/mount.h>
 #include <linux/path.h>
diff --git a/security/security.c b/security/security.c
index 70a7ad357bc6..19d3150f68f4 100644
--- a/security/security.c
+++ b/security/security.c
@@ -16,6 +16,7 @@
 #include <linux/export.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
+#include <linux/kernel_read_file.h>
 #include <linux/lsm_hooks.h>
 #include <linux/integrity.h>
 #include <linux/ima.h>
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index efa6108b1ce9..5de45010fb1a 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -24,6 +24,7 @@
 #include <linux/init.h>
 #include <linux/kd.h>
 #include <linux/kernel.h>
+#include <linux/kernel_read_file.h>
 #include <linux/tracehook.h>
 #include <linux/errno.h>
 #include <linux/sched/signal.h>
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 05/17] fs/kernel_read_file: Split into separate source file
From: Kees Cook @ 2020-07-29 17:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Kees Cook, Scott Branden, Mimi Zohar, Luis Chamberlain,
	Takashi Iwai, Jessica Yu, SeongJae Park, KP Singh, linux-efi,
	linux-security-module, linux-integrity, selinux, linux-kselftest,
	linux-kernel
In-Reply-To: <20200729175845.1745471-1-keescook@chromium.org>

These routines are used in places outside of exec(2), so in preparation
for refactoring them, move them into a separate source file,
fs/kernel_read_file.c.

Acked-by: Scott Branden <scott.branden@broadcom.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 fs/Makefile           |   3 +-
 fs/exec.c             | 132 ----------------------------------------
 fs/kernel_read_file.c | 138 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 140 insertions(+), 133 deletions(-)
 create mode 100644 fs/kernel_read_file.c

diff --git a/fs/Makefile b/fs/Makefile
index 2ce5112b02c8..a05fc247b2a7 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -13,7 +13,8 @@ obj-y :=	open.o read_write.o file_table.o super.o \
 		seq_file.o xattr.o libfs.o fs-writeback.o \
 		pnode.o splice.o sync.o utimes.o d_path.o \
 		stack.o fs_struct.o statfs.o fs_pin.o nsfs.o \
-		fs_types.o fs_context.o fs_parser.o fsopen.o
+		fs_types.o fs_context.o fs_parser.o fsopen.o \
+		kernel_read_file.o
 
 ifeq ($(CONFIG_BLOCK),y)
 obj-y +=	buffer.o block_dev.o direct-io.o mpage.o
diff --git a/fs/exec.c b/fs/exec.c
index 07a7fe9ac5be..d619b79aab30 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -923,138 +923,6 @@ struct file *open_exec(const char *name)
 }
 EXPORT_SYMBOL(open_exec);
 
-int kernel_read_file(struct file *file, void **buf, loff_t *size,
-		     loff_t max_size, enum kernel_read_file_id id)
-{
-	loff_t i_size, pos;
-	ssize_t bytes = 0;
-	void *allocated = NULL;
-	int ret;
-
-	if (!S_ISREG(file_inode(file)->i_mode) || max_size < 0)
-		return -EINVAL;
-
-	ret = deny_write_access(file);
-	if (ret)
-		return ret;
-
-	ret = security_kernel_read_file(file, id);
-	if (ret)
-		goto out;
-
-	i_size = i_size_read(file_inode(file));
-	if (i_size <= 0) {
-		ret = -EINVAL;
-		goto out;
-	}
-	if (i_size > SIZE_MAX || (max_size > 0 && i_size > max_size)) {
-		ret = -EFBIG;
-		goto out;
-	}
-
-	if (!*buf)
-		*buf = allocated = vmalloc(i_size);
-	if (!*buf) {
-		ret = -ENOMEM;
-		goto out;
-	}
-
-	pos = 0;
-	while (pos < i_size) {
-		bytes = kernel_read(file, *buf + pos, i_size - pos, &pos);
-		if (bytes < 0) {
-			ret = bytes;
-			goto out_free;
-		}
-
-		if (bytes == 0)
-			break;
-	}
-
-	if (pos != i_size) {
-		ret = -EIO;
-		goto out_free;
-	}
-
-	ret = security_kernel_post_read_file(file, *buf, i_size, id);
-	if (!ret)
-		*size = pos;
-
-out_free:
-	if (ret < 0) {
-		if (allocated) {
-			vfree(*buf);
-			*buf = NULL;
-		}
-	}
-
-out:
-	allow_write_access(file);
-	return ret;
-}
-EXPORT_SYMBOL_GPL(kernel_read_file);
-
-int kernel_read_file_from_path(const char *path, void **buf, loff_t *size,
-			       loff_t max_size, enum kernel_read_file_id id)
-{
-	struct file *file;
-	int ret;
-
-	if (!path || !*path)
-		return -EINVAL;
-
-	file = filp_open(path, O_RDONLY, 0);
-	if (IS_ERR(file))
-		return PTR_ERR(file);
-
-	ret = kernel_read_file(file, buf, size, max_size, id);
-	fput(file);
-	return ret;
-}
-EXPORT_SYMBOL_GPL(kernel_read_file_from_path);
-
-int kernel_read_file_from_path_initns(const char *path, void **buf,
-				      loff_t *size, loff_t max_size,
-				      enum kernel_read_file_id id)
-{
-	struct file *file;
-	struct path root;
-	int ret;
-
-	if (!path || !*path)
-		return -EINVAL;
-
-	task_lock(&init_task);
-	get_fs_root(init_task.fs, &root);
-	task_unlock(&init_task);
-
-	file = file_open_root(root.dentry, root.mnt, path, O_RDONLY, 0);
-	path_put(&root);
-	if (IS_ERR(file))
-		return PTR_ERR(file);
-
-	ret = kernel_read_file(file, buf, size, max_size, id);
-	fput(file);
-	return ret;
-}
-EXPORT_SYMBOL_GPL(kernel_read_file_from_path_initns);
-
-int kernel_read_file_from_fd(int fd, void **buf, loff_t *size, loff_t max_size,
-			     enum kernel_read_file_id id)
-{
-	struct fd f = fdget(fd);
-	int ret = -EBADF;
-
-	if (!f.file)
-		goto out;
-
-	ret = kernel_read_file(f.file, buf, size, max_size, id);
-out:
-	fdput(f);
-	return ret;
-}
-EXPORT_SYMBOL_GPL(kernel_read_file_from_fd);
-
 #if defined(CONFIG_HAVE_AOUT) || defined(CONFIG_BINFMT_FLAT) || \
     defined(CONFIG_BINFMT_ELF_FDPIC)
 ssize_t read_code(struct file *file, unsigned long addr, loff_t pos, size_t len)
diff --git a/fs/kernel_read_file.c b/fs/kernel_read_file.c
new file mode 100644
index 000000000000..54d972d4befc
--- /dev/null
+++ b/fs/kernel_read_file.c
@@ -0,0 +1,138 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/fs.h>
+#include <linux/fs_struct.h>
+#include <linux/kernel_read_file.h>
+#include <linux/security.h>
+#include <linux/vmalloc.h>
+
+int kernel_read_file(struct file *file, void **buf, loff_t *size,
+		     loff_t max_size, enum kernel_read_file_id id)
+{
+	loff_t i_size, pos;
+	ssize_t bytes = 0;
+	void *allocated = NULL;
+	int ret;
+
+	if (!S_ISREG(file_inode(file)->i_mode) || max_size < 0)
+		return -EINVAL;
+
+	ret = deny_write_access(file);
+	if (ret)
+		return ret;
+
+	ret = security_kernel_read_file(file, id);
+	if (ret)
+		goto out;
+
+	i_size = i_size_read(file_inode(file));
+	if (i_size <= 0) {
+		ret = -EINVAL;
+		goto out;
+	}
+	if (i_size > SIZE_MAX || (max_size > 0 && i_size > max_size)) {
+		ret = -EFBIG;
+		goto out;
+	}
+
+	if (!*buf)
+		*buf = allocated = vmalloc(i_size);
+	if (!*buf) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	pos = 0;
+	while (pos < i_size) {
+		bytes = kernel_read(file, *buf + pos, i_size - pos, &pos);
+		if (bytes < 0) {
+			ret = bytes;
+			goto out_free;
+		}
+
+		if (bytes == 0)
+			break;
+	}
+
+	if (pos != i_size) {
+		ret = -EIO;
+		goto out_free;
+	}
+
+	ret = security_kernel_post_read_file(file, *buf, i_size, id);
+	if (!ret)
+		*size = pos;
+
+out_free:
+	if (ret < 0) {
+		if (allocated) {
+			vfree(*buf);
+			*buf = NULL;
+		}
+	}
+
+out:
+	allow_write_access(file);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(kernel_read_file);
+
+int kernel_read_file_from_path(const char *path, void **buf, loff_t *size,
+			       loff_t max_size, enum kernel_read_file_id id)
+{
+	struct file *file;
+	int ret;
+
+	if (!path || !*path)
+		return -EINVAL;
+
+	file = filp_open(path, O_RDONLY, 0);
+	if (IS_ERR(file))
+		return PTR_ERR(file);
+
+	ret = kernel_read_file(file, buf, size, max_size, id);
+	fput(file);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(kernel_read_file_from_path);
+
+int kernel_read_file_from_path_initns(const char *path, void **buf,
+				      loff_t *size, loff_t max_size,
+				      enum kernel_read_file_id id)
+{
+	struct file *file;
+	struct path root;
+	int ret;
+
+	if (!path || !*path)
+		return -EINVAL;
+
+	task_lock(&init_task);
+	get_fs_root(init_task.fs, &root);
+	task_unlock(&init_task);
+
+	file = file_open_root(root.dentry, root.mnt, path, O_RDONLY, 0);
+	path_put(&root);
+	if (IS_ERR(file))
+		return PTR_ERR(file);
+
+	ret = kernel_read_file(file, buf, size, max_size, id);
+	fput(file);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(kernel_read_file_from_path_initns);
+
+int kernel_read_file_from_fd(int fd, void **buf, loff_t *size, loff_t max_size,
+			     enum kernel_read_file_id id)
+{
+	struct fd f = fdget(fd);
+	int ret = -EBADF;
+
+	if (!f.file)
+		goto out;
+
+	ret = kernel_read_file(f.file, buf, size, max_size, id);
+out:
+	fdput(f);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(kernel_read_file_from_fd);
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 08/17] fs/kernel_read_file: Add file_size output argument
From: Kees Cook @ 2020-07-29 17:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Kees Cook, Scott Branden, Mimi Zohar, Luis Chamberlain,
	Takashi Iwai, Jessica Yu, SeongJae Park, KP Singh, linux-efi,
	linux-security-module, linux-integrity, selinux, linux-kselftest,
	linux-kernel
In-Reply-To: <20200729175845.1745471-1-keescook@chromium.org>

In preparation for adding partial read support, add an optional output
argument to kernel_read_file*() that reports the file size so callers
can reason more easily about their reading progress.

Acked-by: Scott Branden <scott.branden@broadcom.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 drivers/base/firmware_loader/main.c |  1 +
 fs/kernel_read_file.c               | 19 +++++++++++++------
 include/linux/kernel_read_file.h    |  4 ++++
 kernel/kexec_file.c                 |  4 ++--
 kernel/module.c                     |  2 +-
 security/integrity/digsig.c         |  2 +-
 security/integrity/ima/ima_fs.c     |  2 +-
 7 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 9b425437afe6..6a5d407279e3 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -495,6 +495,7 @@ fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv,
 
 		/* load firmware files from the mount namespace of init */
 		rc = kernel_read_file_from_path_initns(path, &buffer, msize,
+						       NULL,
 						       READING_FIRMWARE);
 		if (rc < 0) {
 			if (rc != -ENOENT)
diff --git a/fs/kernel_read_file.c b/fs/kernel_read_file.c
index e21a76001fff..2e29c38eb4df 100644
--- a/fs/kernel_read_file.c
+++ b/fs/kernel_read_file.c
@@ -14,6 +14,8 @@
  *		@buf_size will be ignored)
  * @buf_size	size of buf, if already allocated. If @buf not
  *		allocated, this is the largest size to allocate.
+ * @file_size	if non-NULL, the full size of @file will be
+ *		written here.
  * @id		the kernel_read_file_id identifying the type of
  *		file contents being read (for LSMs to examine)
  *
@@ -22,7 +24,8 @@
  *
  */
 int kernel_read_file(struct file *file, void **buf,
-		     size_t buf_size, enum kernel_read_file_id id)
+		     size_t buf_size, size_t *file_size,
+		     enum kernel_read_file_id id)
 {
 	loff_t i_size, pos;
 	ssize_t bytes = 0;
@@ -49,6 +52,8 @@ int kernel_read_file(struct file *file, void **buf,
 		ret = -EFBIG;
 		goto out;
 	}
+	if (file_size)
+		*file_size = i_size;
 
 	if (!*buf)
 		*buf = allocated = vmalloc(i_size);
@@ -91,7 +96,8 @@ int kernel_read_file(struct file *file, void **buf,
 EXPORT_SYMBOL_GPL(kernel_read_file);
 
 int kernel_read_file_from_path(const char *path, void **buf,
-			       size_t buf_size, enum kernel_read_file_id id)
+			       size_t buf_size, size_t *file_size,
+			       enum kernel_read_file_id id)
 {
 	struct file *file;
 	int ret;
@@ -103,14 +109,14 @@ int kernel_read_file_from_path(const char *path, void **buf,
 	if (IS_ERR(file))
 		return PTR_ERR(file);
 
-	ret = kernel_read_file(file, buf, buf_size, id);
+	ret = kernel_read_file(file, buf, buf_size, file_size, id);
 	fput(file);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(kernel_read_file_from_path);
 
 int kernel_read_file_from_path_initns(const char *path, void **buf,
-				      size_t buf_size,
+				      size_t buf_size, size_t *file_size,
 				      enum kernel_read_file_id id)
 {
 	struct file *file;
@@ -129,13 +135,14 @@ int kernel_read_file_from_path_initns(const char *path, void **buf,
 	if (IS_ERR(file))
 		return PTR_ERR(file);
 
-	ret = kernel_read_file(file, buf, buf_size, id);
+	ret = kernel_read_file(file, buf, buf_size, file_size, id);
 	fput(file);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(kernel_read_file_from_path_initns);
 
 int kernel_read_file_from_fd(int fd, void **buf, size_t buf_size,
+			     size_t *file_size,
 			     enum kernel_read_file_id id)
 {
 	struct fd f = fdget(fd);
@@ -144,7 +151,7 @@ int kernel_read_file_from_fd(int fd, void **buf, size_t buf_size,
 	if (!f.file)
 		goto out;
 
-	ret = kernel_read_file(f.file, buf, buf_size, id);
+	ret = kernel_read_file(f.file, buf, buf_size, file_size, id);
 out:
 	fdput(f);
 	return ret;
diff --git a/include/linux/kernel_read_file.h b/include/linux/kernel_read_file.h
index 910039e7593e..023293eaf948 100644
--- a/include/linux/kernel_read_file.h
+++ b/include/linux/kernel_read_file.h
@@ -37,15 +37,19 @@ static inline const char *kernel_read_file_id_str(enum kernel_read_file_id id)
 
 int kernel_read_file(struct file *file,
 		     void **buf, size_t buf_size,
+		     size_t *file_size,
 		     enum kernel_read_file_id id);
 int kernel_read_file_from_path(const char *path,
 			       void **buf, size_t buf_size,
+			       size_t *file_size,
 			       enum kernel_read_file_id id);
 int kernel_read_file_from_path_initns(const char *path,
 				      void **buf, size_t buf_size,
+				      size_t *file_size,
 				      enum kernel_read_file_id id);
 int kernel_read_file_from_fd(int fd,
 			     void **buf, size_t buf_size,
+			     size_t *file_size,
 			     enum kernel_read_file_id id);
 
 #endif /* _LINUX_KERNEL_READ_FILE_H */
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index eda19ca256a3..878ca684a3a1 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -222,7 +222,7 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
 	void *ldata;
 
 	ret = kernel_read_file_from_fd(kernel_fd, &image->kernel_buf,
-				       INT_MAX, READING_KEXEC_IMAGE);
+				       INT_MAX, NULL, READING_KEXEC_IMAGE);
 	if (ret < 0)
 		return ret;
 	image->kernel_buf_len = ret;
@@ -242,7 +242,7 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
 	/* It is possible that there no initramfs is being loaded */
 	if (!(flags & KEXEC_FILE_NO_INITRAMFS)) {
 		ret = kernel_read_file_from_fd(initrd_fd, &image->initrd_buf,
-					       INT_MAX,
+					       INT_MAX, NULL,
 					       READING_KEXEC_INITRAMFS);
 		if (ret < 0)
 			goto out;
diff --git a/kernel/module.c b/kernel/module.c
index 0792ce3bf643..16558bc842de 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -4004,7 +4004,7 @@ SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
 		      |MODULE_INIT_IGNORE_VERMAGIC))
 		return -EINVAL;
 
-	err = kernel_read_file_from_fd(fd, &hdr, INT_MAX,
+	err = kernel_read_file_from_fd(fd, &hdr, INT_MAX, NULL,
 				       READING_MODULE);
 	if (err < 0)
 		return err;
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 04f779c4f5ed..8a523dfd7fd7 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -175,7 +175,7 @@ int __init integrity_load_x509(const unsigned int id, const char *path)
 	int rc;
 	key_perm_t perm;
 
-	rc = kernel_read_file_from_path(path, &data, INT_MAX,
+	rc = kernel_read_file_from_path(path, &data, INT_MAX, NULL,
 					READING_X509_CERTIFICATE);
 	if (rc < 0) {
 		pr_err("Unable to open file: %s (%d)", path, rc);
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 692b83e82edf..5fc56ccb6678 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -284,7 +284,7 @@ static ssize_t ima_read_policy(char *path)
 	datap = path;
 	strsep(&datap, "\n");
 
-	rc = kernel_read_file_from_path(path, &data, INT_MAX, READING_POLICY);
+	rc = kernel_read_file_from_path(path, &data, INT_MAX, NULL, READING_POLICY);
 	if (rc < 0) {
 		pr_err("Unable to open file: %s (%d)", path, rc);
 		return rc;
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 06/17] fs/kernel_read_file: Remove redundant size argument
From: Kees Cook @ 2020-07-29 17:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Kees Cook, Scott Branden, Mimi Zohar, Luis Chamberlain,
	Takashi Iwai, Jessica Yu, SeongJae Park, KP Singh, linux-efi,
	linux-security-module, linux-integrity, selinux, linux-kselftest,
	linux-kernel
In-Reply-To: <20200729175845.1745471-1-keescook@chromium.org>

In preparation for refactoring kernel_read_file*(), remove the redundant
"size" argument which is not needed: it can be included in the return
code, with callers adjusted. (VFS reads already cannot be larger than
INT_MAX.)

Acked-by: Scott Branden <scott.branden@broadcom.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 drivers/base/firmware_loader/main.c | 10 ++++++----
 fs/kernel_read_file.c               | 20 +++++++++-----------
 include/linux/kernel_read_file.h    |  8 ++++----
 kernel/kexec_file.c                 | 14 +++++++-------
 kernel/module.c                     |  7 +++----
 security/integrity/digsig.c         |  5 +++--
 security/integrity/ima/ima_fs.c     |  6 ++++--
 7 files changed, 36 insertions(+), 34 deletions(-)

diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 7fd677281806..9b425437afe6 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -462,7 +462,7 @@ fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv,
 					     size_t in_size,
 					     const void *in_buffer))
 {
-	loff_t size;
+	size_t size;
 	int i, len;
 	int rc = -ENOENT;
 	char *path;
@@ -494,10 +494,9 @@ fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv,
 		fw_priv->size = 0;
 
 		/* load firmware files from the mount namespace of init */
-		rc = kernel_read_file_from_path_initns(path, &buffer,
-						       &size, msize,
+		rc = kernel_read_file_from_path_initns(path, &buffer, msize,
 						       READING_FIRMWARE);
-		if (rc) {
+		if (rc < 0) {
 			if (rc != -ENOENT)
 				dev_warn(device, "loading %s failed with error %d\n",
 					 path, rc);
@@ -506,6 +505,9 @@ fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv,
 					 path);
 			continue;
 		}
+		size = rc;
+		rc = 0;
+
 		dev_dbg(device, "Loading firmware from %s\n", path);
 		if (decompress) {
 			dev_dbg(device, "f/w decompressing %s\n",
diff --git a/fs/kernel_read_file.c b/fs/kernel_read_file.c
index 54d972d4befc..dc28a8def597 100644
--- a/fs/kernel_read_file.c
+++ b/fs/kernel_read_file.c
@@ -5,7 +5,7 @@
 #include <linux/security.h>
 #include <linux/vmalloc.h>
 
-int kernel_read_file(struct file *file, void **buf, loff_t *size,
+int kernel_read_file(struct file *file, void **buf,
 		     loff_t max_size, enum kernel_read_file_id id)
 {
 	loff_t i_size, pos;
@@ -29,7 +29,7 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size,
 		ret = -EINVAL;
 		goto out;
 	}
-	if (i_size > SIZE_MAX || (max_size > 0 && i_size > max_size)) {
+	if (i_size > INT_MAX || (max_size > 0 && i_size > max_size)) {
 		ret = -EFBIG;
 		goto out;
 	}
@@ -59,8 +59,6 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size,
 	}
 
 	ret = security_kernel_post_read_file(file, *buf, i_size, id);
-	if (!ret)
-		*size = pos;
 
 out_free:
 	if (ret < 0) {
@@ -72,11 +70,11 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size,
 
 out:
 	allow_write_access(file);
-	return ret;
+	return ret == 0 ? pos : ret;
 }
 EXPORT_SYMBOL_GPL(kernel_read_file);
 
-int kernel_read_file_from_path(const char *path, void **buf, loff_t *size,
+int kernel_read_file_from_path(const char *path, void **buf,
 			       loff_t max_size, enum kernel_read_file_id id)
 {
 	struct file *file;
@@ -89,14 +87,14 @@ int kernel_read_file_from_path(const char *path, void **buf, loff_t *size,
 	if (IS_ERR(file))
 		return PTR_ERR(file);
 
-	ret = kernel_read_file(file, buf, size, max_size, id);
+	ret = kernel_read_file(file, buf, max_size, id);
 	fput(file);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(kernel_read_file_from_path);
 
 int kernel_read_file_from_path_initns(const char *path, void **buf,
-				      loff_t *size, loff_t max_size,
+				      loff_t max_size,
 				      enum kernel_read_file_id id)
 {
 	struct file *file;
@@ -115,13 +113,13 @@ int kernel_read_file_from_path_initns(const char *path, void **buf,
 	if (IS_ERR(file))
 		return PTR_ERR(file);
 
-	ret = kernel_read_file(file, buf, size, max_size, id);
+	ret = kernel_read_file(file, buf, max_size, id);
 	fput(file);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(kernel_read_file_from_path_initns);
 
-int kernel_read_file_from_fd(int fd, void **buf, loff_t *size, loff_t max_size,
+int kernel_read_file_from_fd(int fd, void **buf, loff_t max_size,
 			     enum kernel_read_file_id id)
 {
 	struct fd f = fdget(fd);
@@ -130,7 +128,7 @@ int kernel_read_file_from_fd(int fd, void **buf, loff_t *size, loff_t max_size,
 	if (!f.file)
 		goto out;
 
-	ret = kernel_read_file(f.file, buf, size, max_size, id);
+	ret = kernel_read_file(f.file, buf, max_size, id);
 out:
 	fdput(f);
 	return ret;
diff --git a/include/linux/kernel_read_file.h b/include/linux/kernel_read_file.h
index 78cf3d7dc835..0ca0bdbed1bd 100644
--- a/include/linux/kernel_read_file.h
+++ b/include/linux/kernel_read_file.h
@@ -36,16 +36,16 @@ static inline const char *kernel_read_file_id_str(enum kernel_read_file_id id)
 }
 
 int kernel_read_file(struct file *file,
-		     void **buf, loff_t *size, loff_t max_size,
+		     void **buf, loff_t max_size,
 		     enum kernel_read_file_id id);
 int kernel_read_file_from_path(const char *path,
-			       void **buf, loff_t *size, loff_t max_size,
+			       void **buf, loff_t max_size,
 			       enum kernel_read_file_id id);
 int kernel_read_file_from_path_initns(const char *path,
-				      void **buf, loff_t *size, loff_t max_size,
+				      void **buf, loff_t max_size,
 				      enum kernel_read_file_id id);
 int kernel_read_file_from_fd(int fd,
-			     void **buf, loff_t *size, loff_t max_size,
+			     void **buf, loff_t max_size,
 			     enum kernel_read_file_id id);
 
 #endif /* _LINUX_KERNEL_READ_FILE_H */
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 1358069ce9e9..eda19ca256a3 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -220,13 +220,12 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
 {
 	int ret;
 	void *ldata;
-	loff_t size;
 
 	ret = kernel_read_file_from_fd(kernel_fd, &image->kernel_buf,
-				       &size, INT_MAX, READING_KEXEC_IMAGE);
-	if (ret)
+				       INT_MAX, READING_KEXEC_IMAGE);
+	if (ret < 0)
 		return ret;
-	image->kernel_buf_len = size;
+	image->kernel_buf_len = ret;
 
 	/* Call arch image probe handlers */
 	ret = arch_kexec_kernel_image_probe(image, image->kernel_buf,
@@ -243,11 +242,12 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
 	/* It is possible that there no initramfs is being loaded */
 	if (!(flags & KEXEC_FILE_NO_INITRAMFS)) {
 		ret = kernel_read_file_from_fd(initrd_fd, &image->initrd_buf,
-					       &size, INT_MAX,
+					       INT_MAX,
 					       READING_KEXEC_INITRAMFS);
-		if (ret)
+		if (ret < 0)
 			goto out;
-		image->initrd_buf_len = size;
+		image->initrd_buf_len = ret;
+		ret = 0;
 	}
 
 	if (cmdline_len) {
diff --git a/kernel/module.c b/kernel/module.c
index cc8d83841568..0792ce3bf643 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3991,7 +3991,6 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
 SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
 {
 	struct load_info info = { };
-	loff_t size;
 	void *hdr = NULL;
 	int err;
 
@@ -4005,12 +4004,12 @@ SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
 		      |MODULE_INIT_IGNORE_VERMAGIC))
 		return -EINVAL;
 
-	err = kernel_read_file_from_fd(fd, &hdr, &size, INT_MAX,
+	err = kernel_read_file_from_fd(fd, &hdr, INT_MAX,
 				       READING_MODULE);
-	if (err)
+	if (err < 0)
 		return err;
 	info.hdr = hdr;
-	info.len = size;
+	info.len = err;
 
 	return load_module(&info, uargs, flags);
 }
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index f8869be45d8f..97661ffabc4e 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -171,16 +171,17 @@ int __init integrity_add_key(const unsigned int id, const void *data,
 int __init integrity_load_x509(const unsigned int id, const char *path)
 {
 	void *data = NULL;
-	loff_t size;
+	size_t size;
 	int rc;
 	key_perm_t perm;
 
-	rc = kernel_read_file_from_path(path, &data, &size, 0,
+	rc = kernel_read_file_from_path(path, &data, 0,
 					READING_X509_CERTIFICATE);
 	if (rc < 0) {
 		pr_err("Unable to open file: %s (%d)", path, rc);
 		return rc;
 	}
+	size = rc;
 
 	perm = (KEY_POS_ALL & ~KEY_POS_SETATTR) | KEY_USR_VIEW | KEY_USR_READ;
 
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index e13ffece3726..602f52717757 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -275,7 +275,7 @@ static ssize_t ima_read_policy(char *path)
 {
 	void *data = NULL;
 	char *datap;
-	loff_t size;
+	size_t size;
 	int rc, pathlen = strlen(path);
 
 	char *p;
@@ -284,11 +284,13 @@ static ssize_t ima_read_policy(char *path)
 	datap = path;
 	strsep(&datap, "\n");
 
-	rc = kernel_read_file_from_path(path, &data, &size, 0, READING_POLICY);
+	rc = kernel_read_file_from_path(path, &data, 0, READING_POLICY);
 	if (rc < 0) {
 		pr_err("Unable to open file: %s (%d)", path, rc);
 		return rc;
 	}
+	size = rc;
+	rc = 0;
 
 	datap = data;
 	while (size > 0 && (p = strsep(&datap, "\n"))) {
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 07/17] fs/kernel_read_file: Switch buffer size arg to size_t
From: Kees Cook @ 2020-07-29 17:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Kees Cook, Scott Branden, Mimi Zohar, Luis Chamberlain,
	Takashi Iwai, Jessica Yu, SeongJae Park, KP Singh, linux-efi,
	linux-security-module, linux-integrity, selinux, linux-kselftest,
	linux-kernel
In-Reply-To: <20200729175845.1745471-1-keescook@chromium.org>

In preparation for further refactoring of kernel_read_file*(), rename
the "max_size" argument to the more accurate "buf_size", and correct
its type to size_t. Add kerndoc to explain the specifics of how the
arguments will be used. Note that with buf_size now size_t, it can no
longer be negative (and was never called with a negative value). Adjust
callers to use it as a "maximum size" when *buf is NULL.

Acked-by: Scott Branden <scott.branden@broadcom.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 fs/kernel_read_file.c            | 34 +++++++++++++++++++++++---------
 include/linux/kernel_read_file.h |  8 ++++----
 security/integrity/digsig.c      |  2 +-
 security/integrity/ima/ima_fs.c  |  2 +-
 4 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/fs/kernel_read_file.c b/fs/kernel_read_file.c
index dc28a8def597..e21a76001fff 100644
--- a/fs/kernel_read_file.c
+++ b/fs/kernel_read_file.c
@@ -5,15 +5,31 @@
 #include <linux/security.h>
 #include <linux/vmalloc.h>
 
+/**
+ * kernel_read_file() - read file contents into a kernel buffer
+ *
+ * @file	file to read from
+ * @buf		pointer to a "void *" buffer for reading into (if
+ *		*@buf is NULL, a buffer will be allocated, and
+ *		@buf_size will be ignored)
+ * @buf_size	size of buf, if already allocated. If @buf not
+ *		allocated, this is the largest size to allocate.
+ * @id		the kernel_read_file_id identifying the type of
+ *		file contents being read (for LSMs to examine)
+ *
+ * Returns number of bytes read (no single read will be bigger
+ * than INT_MAX), or negative on error.
+ *
+ */
 int kernel_read_file(struct file *file, void **buf,
-		     loff_t max_size, enum kernel_read_file_id id)
+		     size_t buf_size, enum kernel_read_file_id id)
 {
 	loff_t i_size, pos;
 	ssize_t bytes = 0;
 	void *allocated = NULL;
 	int ret;
 
-	if (!S_ISREG(file_inode(file)->i_mode) || max_size < 0)
+	if (!S_ISREG(file_inode(file)->i_mode))
 		return -EINVAL;
 
 	ret = deny_write_access(file);
@@ -29,7 +45,7 @@ int kernel_read_file(struct file *file, void **buf,
 		ret = -EINVAL;
 		goto out;
 	}
-	if (i_size > INT_MAX || (max_size > 0 && i_size > max_size)) {
+	if (i_size > INT_MAX || i_size > buf_size) {
 		ret = -EFBIG;
 		goto out;
 	}
@@ -75,7 +91,7 @@ int kernel_read_file(struct file *file, void **buf,
 EXPORT_SYMBOL_GPL(kernel_read_file);
 
 int kernel_read_file_from_path(const char *path, void **buf,
-			       loff_t max_size, enum kernel_read_file_id id)
+			       size_t buf_size, enum kernel_read_file_id id)
 {
 	struct file *file;
 	int ret;
@@ -87,14 +103,14 @@ int kernel_read_file_from_path(const char *path, void **buf,
 	if (IS_ERR(file))
 		return PTR_ERR(file);
 
-	ret = kernel_read_file(file, buf, max_size, id);
+	ret = kernel_read_file(file, buf, buf_size, id);
 	fput(file);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(kernel_read_file_from_path);
 
 int kernel_read_file_from_path_initns(const char *path, void **buf,
-				      loff_t max_size,
+				      size_t buf_size,
 				      enum kernel_read_file_id id)
 {
 	struct file *file;
@@ -113,13 +129,13 @@ int kernel_read_file_from_path_initns(const char *path, void **buf,
 	if (IS_ERR(file))
 		return PTR_ERR(file);
 
-	ret = kernel_read_file(file, buf, max_size, id);
+	ret = kernel_read_file(file, buf, buf_size, id);
 	fput(file);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(kernel_read_file_from_path_initns);
 
-int kernel_read_file_from_fd(int fd, void **buf, loff_t max_size,
+int kernel_read_file_from_fd(int fd, void **buf, size_t buf_size,
 			     enum kernel_read_file_id id)
 {
 	struct fd f = fdget(fd);
@@ -128,7 +144,7 @@ int kernel_read_file_from_fd(int fd, void **buf, loff_t max_size,
 	if (!f.file)
 		goto out;
 
-	ret = kernel_read_file(f.file, buf, max_size, id);
+	ret = kernel_read_file(f.file, buf, buf_size, id);
 out:
 	fdput(f);
 	return ret;
diff --git a/include/linux/kernel_read_file.h b/include/linux/kernel_read_file.h
index 0ca0bdbed1bd..910039e7593e 100644
--- a/include/linux/kernel_read_file.h
+++ b/include/linux/kernel_read_file.h
@@ -36,16 +36,16 @@ static inline const char *kernel_read_file_id_str(enum kernel_read_file_id id)
 }
 
 int kernel_read_file(struct file *file,
-		     void **buf, loff_t max_size,
+		     void **buf, size_t buf_size,
 		     enum kernel_read_file_id id);
 int kernel_read_file_from_path(const char *path,
-			       void **buf, loff_t max_size,
+			       void **buf, size_t buf_size,
 			       enum kernel_read_file_id id);
 int kernel_read_file_from_path_initns(const char *path,
-				      void **buf, loff_t max_size,
+				      void **buf, size_t buf_size,
 				      enum kernel_read_file_id id);
 int kernel_read_file_from_fd(int fd,
-			     void **buf, loff_t max_size,
+			     void **buf, size_t buf_size,
 			     enum kernel_read_file_id id);
 
 #endif /* _LINUX_KERNEL_READ_FILE_H */
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 97661ffabc4e..04f779c4f5ed 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -175,7 +175,7 @@ int __init integrity_load_x509(const unsigned int id, const char *path)
 	int rc;
 	key_perm_t perm;
 
-	rc = kernel_read_file_from_path(path, &data, 0,
+	rc = kernel_read_file_from_path(path, &data, INT_MAX,
 					READING_X509_CERTIFICATE);
 	if (rc < 0) {
 		pr_err("Unable to open file: %s (%d)", path, rc);
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 602f52717757..692b83e82edf 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -284,7 +284,7 @@ static ssize_t ima_read_policy(char *path)
 	datap = path;
 	strsep(&datap, "\n");
 
-	rc = kernel_read_file_from_path(path, &data, 0, READING_POLICY);
+	rc = kernel_read_file_from_path(path, &data, INT_MAX, READING_POLICY);
 	if (rc < 0) {
 		pr_err("Unable to open file: %s (%d)", path, rc);
 		return rc;
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 12/17] LSM: Add "contents" flag to kernel_read_file hook
From: Kees Cook @ 2020-07-29 17:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Kees Cook, Scott Branden, Mimi Zohar, Luis Chamberlain,
	Takashi Iwai, Jessica Yu, SeongJae Park, KP Singh, linux-efi,
	linux-security-module, linux-integrity, selinux, linux-kselftest,
	linux-kernel
In-Reply-To: <20200729175845.1745471-1-keescook@chromium.org>

As with the kernel_load_data LSM hook, add a "contents" flag to the
kernel_read_file LSM hook that indicates whether the LSM can expect
a matching call to the kernel_post_read_file LSM hook with the full
contents of the file. With the coming addition of partial file read
support for kernel_read_file*() API, the LSM will no longer be able
to always see the entire contents of a file during the read calls.

For cases where the LSM must read examine the complete file contents,
it will need to do so on its own every time the kernel_read_file
hook is called with contents=false (or reject such cases). Adjust all
existing LSMs to retain existing behavior.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 fs/kernel_read_file.c             |  2 +-
 include/linux/ima.h               |  6 ++++--
 include/linux/lsm_hook_defs.h     |  2 +-
 include/linux/lsm_hooks.h         |  3 +++
 include/linux/security.h          |  6 ++++--
 security/integrity/ima/ima_main.c | 10 +++++++++-
 security/loadpin/loadpin.c        | 14 ++++++++++++--
 security/security.c               |  7 ++++---
 security/selinux/hooks.c          |  5 +++--
 9 files changed, 41 insertions(+), 14 deletions(-)

diff --git a/fs/kernel_read_file.c b/fs/kernel_read_file.c
index 2e29c38eb4df..d73bc3fa710a 100644
--- a/fs/kernel_read_file.c
+++ b/fs/kernel_read_file.c
@@ -39,7 +39,7 @@ int kernel_read_file(struct file *file, void **buf,
 	if (ret)
 		return ret;
 
-	ret = security_kernel_read_file(file, id);
+	ret = security_kernel_read_file(file, id, true);
 	if (ret)
 		goto out;
 
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 502e36ad7804..259023039dc9 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -23,7 +23,8 @@ extern int ima_file_mprotect(struct vm_area_struct *vma, unsigned long prot);
 extern int ima_load_data(enum kernel_load_data_id id, bool contents);
 extern int ima_post_load_data(char *buf, loff_t size,
 			      enum kernel_load_data_id id);
-extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
+extern int ima_read_file(struct file *file, enum kernel_read_file_id id,
+			 bool contents);
 extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
 			      enum kernel_read_file_id id);
 extern void ima_post_path_mknod(struct dentry *dentry);
@@ -91,7 +92,8 @@ static inline int ima_post_load_data(char *buf, loff_t size,
 	return 0;
 }
 
-static inline int ima_read_file(struct file *file, enum kernel_read_file_id id)
+static inline int ima_read_file(struct file *file, enum kernel_read_file_id id,
+				bool contents)
 {
 	return 0;
 }
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 7ed5d31ac9cc..f953aa938eaf 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -188,7 +188,7 @@ LSM_HOOK(int, 0, kernel_load_data, enum kernel_load_data_id id, bool contents)
 LSM_HOOK(int, 0, kernel_post_load_data, char *buf, loff_t size,
 	 enum kernel_read_file_id id)
 LSM_HOOK(int, 0, kernel_read_file, struct file *file,
-	 enum kernel_read_file_id id)
+	 enum kernel_read_file_id id, bool contents)
 LSM_HOOK(int, 0, kernel_post_read_file, struct file *file, char *buf,
 	 loff_t size, enum kernel_read_file_id id)
 LSM_HOOK(int, 0, task_fix_setuid, struct cred *new, const struct cred *old,
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 812d626195fc..b66433b5aa15 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -650,6 +650,7 @@
  *	@file contains the file structure pointing to the file being read
  *	by the kernel.
  *	@id kernel read file identifier
+ *	@contents if a subsequent @kernel_post_read_file will be called.
  *	Return 0 if permission is granted.
  * @kernel_post_read_file:
  *	Read a file specified by userspace.
@@ -658,6 +659,8 @@
  *	@buf pointer to buffer containing the file contents.
  *	@size length of the file contents.
  *	@id kernel read file identifier
+ *	This must be paired with a prior @kernel_read_file call that had
+ *	@contents set to true.
  *	Return 0 if permission is granted.
  * @task_fix_setuid:
  *	Update the module's state after setting one or more of the user
diff --git a/include/linux/security.h b/include/linux/security.h
index e748974c707b..a5d66b89cd6c 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -390,7 +390,8 @@ int security_kernel_module_request(char *kmod_name);
 int security_kernel_load_data(enum kernel_load_data_id id, bool contents);
 int security_kernel_post_load_data(char *buf, loff_t size,
 				   enum kernel_load_data_id id);
-int security_kernel_read_file(struct file *file, enum kernel_read_file_id id);
+int security_kernel_read_file(struct file *file, enum kernel_read_file_id id,
+			      bool contents);
 int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
 				   enum kernel_read_file_id id);
 int security_task_fix_setuid(struct cred *new, const struct cred *old,
@@ -1028,7 +1029,8 @@ static inline int security_kernel_post_load_data(char *buf, loff_t size,
 }
 
 static inline int security_kernel_read_file(struct file *file,
-					    enum kernel_read_file_id id)
+					    enum kernel_read_file_id id,
+					    bool contents)
 {
 	return 0;
 }
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 1a7bc4c7437d..dc4f90660aa6 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -602,6 +602,7 @@ void ima_post_path_mknod(struct dentry *dentry)
  * ima_read_file - pre-measure/appraise hook decision based on policy
  * @file: pointer to the file to be measured/appraised/audit
  * @read_id: caller identifier
+ * @contents: whether a subsequent call will be made to ima_post_read_file()
  *
  * Permit reading a file based on policy. The policy rules are written
  * in terms of the policy identifier.  Appraising the integrity of
@@ -609,8 +610,15 @@ void ima_post_path_mknod(struct dentry *dentry)
  *
  * For permission return 0, otherwise return -EACCES.
  */
-int ima_read_file(struct file *file, enum kernel_read_file_id read_id)
+int ima_read_file(struct file *file, enum kernel_read_file_id read_id,
+		  bool contents)
 {
+	/* Reject all partial reads during appraisal. */
+	if (!contents) {
+		if (ima_appraise & IMA_APPRAISE_ENFORCE)
+			return -EACCES;
+	}
+
 	/*
 	 * Do devices using pre-allocated memory run the risk of the
 	 * firmware being accessible to the device prior to the completion
diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
index db320a43f42e..a1778ebef137 100644
--- a/security/loadpin/loadpin.c
+++ b/security/loadpin/loadpin.c
@@ -117,11 +117,21 @@ static void loadpin_sb_free_security(struct super_block *mnt_sb)
 	}
 }
 
-static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
+static int loadpin_read_file(struct file *file, enum kernel_read_file_id id,
+			     bool contents)
 {
 	struct super_block *load_root;
 	const char *origin = kernel_read_file_id_str(id);
 
+	/*
+	 * If we will not know that we'll be seeing the full contents
+	 * then we cannot trust a load will be complete and unchanged
+	 * off disk. Treat all contents=false hooks as if there were
+	 * no associated file struct.
+	 */
+	if (!contents)
+		file = NULL;
+
 	/* If the file id is excluded, ignore the pinning. */
 	if ((unsigned int)id < ARRAY_SIZE(ignore_read_file_id) &&
 	    ignore_read_file_id[id]) {
@@ -178,7 +188,7 @@ static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
 
 static int loadpin_load_data(enum kernel_load_data_id id, bool contents)
 {
-	return loadpin_read_file(NULL, (enum kernel_read_file_id) id);
+	return loadpin_read_file(NULL, (enum kernel_read_file_id) id, contents);
 }
 
 static struct security_hook_list loadpin_hooks[] __lsm_ro_after_init = {
diff --git a/security/security.c b/security/security.c
index 568bb77e84f4..6a38fc533a5a 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1672,14 +1672,15 @@ int security_kernel_module_request(char *kmod_name)
 	return integrity_kernel_module_request(kmod_name);
 }
 
-int security_kernel_read_file(struct file *file, enum kernel_read_file_id id)
+int security_kernel_read_file(struct file *file, enum kernel_read_file_id id,
+			      bool contents)
 {
 	int ret;
 
-	ret = call_int_hook(kernel_read_file, 0, file, id);
+	ret = call_int_hook(kernel_read_file, 0, file, id, contents);
 	if (ret)
 		return ret;
-	return ima_read_file(file, id);
+	return ima_read_file(file, id, contents);
 }
 EXPORT_SYMBOL_GPL(security_kernel_read_file);
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 1a5c68196faf..6d183bbc12a6 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4004,13 +4004,14 @@ static int selinux_kernel_module_from_file(struct file *file)
 }
 
 static int selinux_kernel_read_file(struct file *file,
-				    enum kernel_read_file_id id)
+				    enum kernel_read_file_id id,
+				    bool contents)
 {
 	int rc = 0;
 
 	switch (id) {
 	case READING_MODULE:
-		rc = selinux_kernel_module_from_file(file);
+		rc = selinux_kernel_module_from_file(contents ? file : NULL);
 		break;
 	default:
 		break;
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 10/17] firmware_loader: Use security_post_load_data()
From: Kees Cook @ 2020-07-29 17:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Kees Cook, Scott Branden, Mimi Zohar, Luis Chamberlain,
	Takashi Iwai, Jessica Yu, SeongJae Park, KP Singh, linux-efi,
	linux-security-module, linux-integrity, selinux, linux-kselftest,
	linux-kernel
In-Reply-To: <20200729175845.1745471-1-keescook@chromium.org>

Now that security_post_load_data() is wired up, use it instead
of the NULL file argument style of security_post_read_file(),
and update the security_kernel_load_data() call to indicate that a
security_kernel_post_load_data() call is expected.

Wire up the IMA check to match earlier logic. Perhaps a generalized
change to ima_post_load_data() might look something like this:

    return process_buffer_measurement(buf, size,
                                      kernel_load_data_id_str(load_id),
                                      read_idmap[load_id] ?: FILE_CHECK,
                                      0, NULL);

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 drivers/base/firmware_loader/fallback.c       |  8 ++++----
 .../base/firmware_loader/fallback_platform.c  |  7 ++++++-
 security/integrity/ima/ima_main.c             | 20 +++++++++----------
 3 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index a196aacce22c..7cfdfdcb819c 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -272,9 +272,9 @@ static ssize_t firmware_loading_store(struct device *dev,
 				dev_err(dev, "%s: map pages failed\n",
 					__func__);
 			else
-				rc = security_kernel_post_read_file(NULL,
-						fw_priv->data, fw_priv->size,
-						READING_FIRMWARE);
+				rc = security_kernel_post_load_data(fw_priv->data,
+						fw_priv->size,
+						LOADING_FIRMWARE);
 
 			/*
 			 * Same logic as fw_load_abort, only the DONE bit
@@ -613,7 +613,7 @@ static bool fw_run_sysfs_fallback(u32 opt_flags)
 		return false;
 
 	/* Also permit LSMs and IMA to fail firmware sysfs fallback */
-	ret = security_kernel_load_data(LOADING_FIRMWARE, false);
+	ret = security_kernel_load_data(LOADING_FIRMWARE, true);
 	if (ret < 0)
 		return false;
 
diff --git a/drivers/base/firmware_loader/fallback_platform.c b/drivers/base/firmware_loader/fallback_platform.c
index a12c79d47efc..4d1157af0e86 100644
--- a/drivers/base/firmware_loader/fallback_platform.c
+++ b/drivers/base/firmware_loader/fallback_platform.c
@@ -17,7 +17,7 @@ int firmware_fallback_platform(struct fw_priv *fw_priv, u32 opt_flags)
 	if (!(opt_flags & FW_OPT_FALLBACK_PLATFORM))
 		return -ENOENT;
 
-	rc = security_kernel_load_data(LOADING_FIRMWARE, false);
+	rc = security_kernel_load_data(LOADING_FIRMWARE, true);
 	if (rc)
 		return rc;
 
@@ -27,6 +27,11 @@ int firmware_fallback_platform(struct fw_priv *fw_priv, u32 opt_flags)
 
 	if (fw_priv->data && size > fw_priv->allocated_size)
 		return -ENOMEM;
+
+	rc = security_kernel_post_load_data((u8 *)data, size, LOADING_FIRMWARE);
+	if (rc)
+		return rc;
+
 	if (!fw_priv->data)
 		fw_priv->data = vmalloc(size);
 	if (!fw_priv->data)
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 85000dc8595c..1a7bc4c7437d 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -648,15 +648,6 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
 	enum ima_hooks func;
 	u32 secid;
 
-	if (!file && read_id == READING_FIRMWARE) {
-		if ((ima_appraise & IMA_APPRAISE_FIRMWARE) &&
-		    (ima_appraise & IMA_APPRAISE_ENFORCE)) {
-			pr_err("Prevent firmware loading_store.\n");
-			return -EACCES;	/* INTEGRITY_UNKNOWN */
-		}
-		return 0;
-	}
-
 	/* permit signed certs */
 	if (!file && read_id == READING_X509_CERTIFICATE)
 		return 0;
@@ -706,7 +697,7 @@ int ima_load_data(enum kernel_load_data_id id, bool contents)
 		}
 		break;
 	case LOADING_FIRMWARE:
-		if (ima_enforce && (ima_appraise & IMA_APPRAISE_FIRMWARE)) {
+		if (ima_enforce && (ima_appraise & IMA_APPRAISE_FIRMWARE) && !contents) {
 			pr_err("Prevent firmware sysfs fallback loading.\n");
 			return -EACCES;	/* INTEGRITY_UNKNOWN */
 		}
@@ -739,6 +730,15 @@ int ima_load_data(enum kernel_load_data_id id, bool contents)
  */
 int ima_post_load_data(char *buf, loff_t size, enum kernel_load_data_id load_id)
 {
+	if (load_id == LOADING_FIRMWARE) {
+		if ((ima_appraise & IMA_APPRAISE_FIRMWARE) &&
+		    (ima_appraise & IMA_APPRAISE_ENFORCE)) {
+			pr_err("Prevent firmware loading_store.\n");
+			return -EACCES; /* INTEGRITY_UNKNOWN */
+		}
+		return 0;
+	}
+
 	return 0;
 }
 
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 11/17] module: Call security_kernel_post_load_data()
From: Kees Cook @ 2020-07-29 17:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Kees Cook, Jessica Yu, Scott Branden, Mimi Zohar,
	Luis Chamberlain, Takashi Iwai, SeongJae Park, KP Singh,
	linux-efi, linux-security-module, linux-integrity, selinux,
	linux-kselftest, linux-kernel
In-Reply-To: <20200729175845.1745471-1-keescook@chromium.org>

Now that there is an API for checking loaded contents for modules
loaded without a file, call into the LSM hooks.

Cc: Jessica Yu <jeyu@kernel.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 kernel/module.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/kernel/module.c b/kernel/module.c
index d773f32f8dfd..72e33e25d7b9 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2970,7 +2970,7 @@ static int copy_module_from_user(const void __user *umod, unsigned long len,
 	if (info->len < sizeof(*(info->hdr)))
 		return -ENOEXEC;
 
-	err = security_kernel_load_data(LOADING_MODULE, false);
+	err = security_kernel_load_data(LOADING_MODULE, true);
 	if (err)
 		return err;
 
@@ -2980,11 +2980,17 @@ static int copy_module_from_user(const void __user *umod, unsigned long len,
 		return -ENOMEM;
 
 	if (copy_chunked_from_user(info->hdr, umod, info->len) != 0) {
-		vfree(info->hdr);
-		return -EFAULT;
+		err = -EFAULT;
+		goto out;
 	}
 
-	return 0;
+	err = security_kernel_post_load_data((char *)info->hdr, info->len,
+					     LOADING_MODULE);
+out:
+	if (err)
+		vfree(info->hdr);
+
+	return err;
 }
 
 static void free_copy(struct load_info *info)
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 09/17] LSM: Introduce kernel_post_load_data() hook
From: Kees Cook @ 2020-07-29 17:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Kees Cook, Scott Branden, Mimi Zohar, Luis Chamberlain,
	Takashi Iwai, Jessica Yu, SeongJae Park, KP Singh, linux-efi,
	linux-security-module, linux-integrity, selinux, linux-kselftest,
	linux-kernel
In-Reply-To: <20200729175845.1745471-1-keescook@chromium.org>

There are a few places in the kernel where LSMs would like to have
visibility into the contents of a kernel buffer that has been loaded or
read. While security_kernel_post_read_file() (which includes the
buffer) exists as a pairing for security_kernel_read_file(), no such
hook exists to pair with security_kernel_load_data().

Earlier proposals for just using security_kernel_post_read_file() with a
NULL file argument were rejected (i.e. "file" should always be valid for
the security_..._file hooks, but it appears at least one case was
left in the kernel during earlier refactoring. (This will be fixed in
a subsequent patch.)

Since not all cases of security_kernel_load_data() can have a single
contiguous buffer made available to the LSM hook (e.g. kexec image
segments are separately loaded), there needs to be a way for the LSM to
reason about its expectations of the hook coverage. In order to handle
this, add a "contents" argument to the "kernel_load_data" hook that
indicates if the newly added "kernel_post_load_data" hook will be called
with the full contents once loaded. That way, LSMs requiring full contents
can choose to unilaterally reject "kernel_load_data" with contents=false
(which is effectively the existing hook coverage), but when contents=true
they can allow it and later evaluate the "kernel_post_load_data" hook
once the buffer is loaded.

With this change, LSMs can gain coverage over non-file-backed data loads
(e.g. init_module(2) and firmware userspace helper), which will happen
in subsequent patches.

Additionally prepare IMA to start processing these cases.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 drivers/base/firmware_loader/fallback.c       |  2 +-
 .../base/firmware_loader/fallback_platform.c  |  2 +-
 include/linux/ima.h                           | 12 +++++++++--
 include/linux/lsm_hook_defs.h                 |  4 +++-
 include/linux/lsm_hooks.h                     |  9 ++++++++
 include/linux/security.h                      | 12 +++++++++--
 kernel/kexec.c                                |  2 +-
 kernel/module.c                               |  2 +-
 security/integrity/ima/ima_main.c             | 21 ++++++++++++++++++-
 security/loadpin/loadpin.c                    |  2 +-
 security/security.c                           | 18 +++++++++++++---
 security/selinux/hooks.c                      |  2 +-
 12 files changed, 73 insertions(+), 15 deletions(-)

diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index 5327bfc6ba71..a196aacce22c 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -613,7 +613,7 @@ static bool fw_run_sysfs_fallback(u32 opt_flags)
 		return false;
 
 	/* Also permit LSMs and IMA to fail firmware sysfs fallback */
-	ret = security_kernel_load_data(LOADING_FIRMWARE);
+	ret = security_kernel_load_data(LOADING_FIRMWARE, false);
 	if (ret < 0)
 		return false;
 
diff --git a/drivers/base/firmware_loader/fallback_platform.c b/drivers/base/firmware_loader/fallback_platform.c
index 6958ab1a8059..a12c79d47efc 100644
--- a/drivers/base/firmware_loader/fallback_platform.c
+++ b/drivers/base/firmware_loader/fallback_platform.c
@@ -17,7 +17,7 @@ int firmware_fallback_platform(struct fw_priv *fw_priv, u32 opt_flags)
 	if (!(opt_flags & FW_OPT_FALLBACK_PLATFORM))
 		return -ENOENT;
 
-	rc = security_kernel_load_data(LOADING_FIRMWARE);
+	rc = security_kernel_load_data(LOADING_FIRMWARE, false);
 	if (rc)
 		return rc;
 
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 148636bfcc8f..502e36ad7804 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -20,7 +20,9 @@ extern void ima_post_create_tmpfile(struct inode *inode);
 extern void ima_file_free(struct file *file);
 extern int ima_file_mmap(struct file *file, unsigned long prot);
 extern int ima_file_mprotect(struct vm_area_struct *vma, unsigned long prot);
-extern int ima_load_data(enum kernel_load_data_id id);
+extern int ima_load_data(enum kernel_load_data_id id, bool contents);
+extern int ima_post_load_data(char *buf, loff_t size,
+			      enum kernel_load_data_id id);
 extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
 extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
 			      enum kernel_read_file_id id);
@@ -78,7 +80,13 @@ static inline int ima_file_mprotect(struct vm_area_struct *vma,
 	return 0;
 }
 
-static inline int ima_load_data(enum kernel_load_data_id id)
+static inline int ima_load_data(enum kernel_load_data_id id, bool contents)
+{
+	return 0;
+}
+
+static inline int ima_post_load_data(char *buf, loff_t size,
+				     enum kernel_load_data_id id)
 {
 	return 0;
 }
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index af998f93d256..7ed5d31ac9cc 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -184,7 +184,9 @@ LSM_HOOK(void, LSM_RET_VOID, cred_getsecid, const struct cred *c, u32 *secid)
 LSM_HOOK(int, 0, kernel_act_as, struct cred *new, u32 secid)
 LSM_HOOK(int, 0, kernel_create_files_as, struct cred *new, struct inode *inode)
 LSM_HOOK(int, 0, kernel_module_request, char *kmod_name)
-LSM_HOOK(int, 0, kernel_load_data, enum kernel_load_data_id id)
+LSM_HOOK(int, 0, kernel_load_data, enum kernel_load_data_id id, bool contents)
+LSM_HOOK(int, 0, kernel_post_load_data, char *buf, loff_t size,
+	 enum kernel_read_file_id id)
 LSM_HOOK(int, 0, kernel_read_file, struct file *file,
 	 enum kernel_read_file_id id)
 LSM_HOOK(int, 0, kernel_post_read_file, struct file *file, char *buf,
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 95b7c1d32062..812d626195fc 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -635,7 +635,16 @@
  * @kernel_load_data:
  *	Load data provided by userspace.
  *	@id kernel load data identifier
+ *	@contents if a subsequent @kernel_post_load_data will be called.
  *	Return 0 if permission is granted.
+ * @kernel_post_load_data:
+ *	Load data provided by a non-file source (usually userspace buffer).
+ *	@buf pointer to buffer containing the data contents.
+ *	@size length of the data contents.
+ *	@id kernel load data identifier
+ *	Return 0 if permission is granted.
+ *	This must be paired with a prior @kernel_load_data call that had
+ *	@contents set to true.
  * @kernel_read_file:
  *	Read a file specified by userspace.
  *	@file contains the file structure pointing to the file being read
diff --git a/include/linux/security.h b/include/linux/security.h
index 42df0d9b4c37..e748974c707b 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -387,7 +387,9 @@ void security_cred_getsecid(const struct cred *c, u32 *secid);
 int security_kernel_act_as(struct cred *new, u32 secid);
 int security_kernel_create_files_as(struct cred *new, struct inode *inode);
 int security_kernel_module_request(char *kmod_name);
-int security_kernel_load_data(enum kernel_load_data_id id);
+int security_kernel_load_data(enum kernel_load_data_id id, bool contents);
+int security_kernel_post_load_data(char *buf, loff_t size,
+				   enum kernel_load_data_id id);
 int security_kernel_read_file(struct file *file, enum kernel_read_file_id id);
 int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
 				   enum kernel_read_file_id id);
@@ -1014,7 +1016,13 @@ static inline int security_kernel_module_request(char *kmod_name)
 	return 0;
 }
 
-static inline int security_kernel_load_data(enum kernel_load_data_id id)
+static inline int security_kernel_load_data(enum kernel_load_data_id id, bool contents)
+{
+	return 0;
+}
+
+static inline int security_kernel_post_load_data(char *buf, loff_t size,
+						 enum kernel_load_data_id id)
 {
 	return 0;
 }
diff --git a/kernel/kexec.c b/kernel/kexec.c
index f977786fe498..c82c6c06f051 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -205,7 +205,7 @@ static inline int kexec_load_check(unsigned long nr_segments,
 		return -EPERM;
 
 	/* Permit LSMs and IMA to fail the kexec */
-	result = security_kernel_load_data(LOADING_KEXEC_IMAGE);
+	result = security_kernel_load_data(LOADING_KEXEC_IMAGE, false);
 	if (result < 0)
 		return result;
 
diff --git a/kernel/module.c b/kernel/module.c
index 16558bc842de..d773f32f8dfd 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2970,7 +2970,7 @@ static int copy_module_from_user(const void __user *umod, unsigned long len,
 	if (info->len < sizeof(*(info->hdr)))
 		return -ENOEXEC;
 
-	err = security_kernel_load_data(LOADING_MODULE);
+	err = security_kernel_load_data(LOADING_MODULE, false);
 	if (err)
 		return err;
 
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index dab4a13221cf..85000dc8595c 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -676,6 +676,8 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
 /**
  * ima_load_data - appraise decision based on policy
  * @id: kernel load data caller identifier
+ * @contents: whether the full contents will be available in a later
+ *	      call to ima_post_load_data().
  *
  * Callers of this LSM hook can not measure, appraise, or audit the
  * data provided by userspace.  Enforce policy rules requring a file
@@ -683,7 +685,7 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
  *
  * For permission return 0, otherwise return -EACCES.
  */
-int ima_load_data(enum kernel_load_data_id id)
+int ima_load_data(enum kernel_load_data_id id, bool contents)
 {
 	bool ima_enforce, sig_enforce;
 
@@ -723,6 +725,23 @@ int ima_load_data(enum kernel_load_data_id id)
 	return 0;
 }
 
+/**
+ * ima_post_load_data - appraise decision based on policy
+ * @buf: pointer to in memory file contents
+ * @size: size of in memory file contents
+ * @id: kernel load data caller identifier
+ *
+ * Measure/appraise/audit in memory buffer based on policy.  Policy rules
+ * are written in terms of a policy identifier.
+ *
+ * On success return 0.  On integrity appraisal error, assuming the file
+ * is in policy and IMA-appraisal is in enforcing mode, return -EACCES.
+ */
+int ima_post_load_data(char *buf, loff_t size, enum kernel_load_data_id load_id)
+{
+	return 0;
+}
+
 /*
  * process_buffer_measurement - Measure the buffer to ima log.
  * @buf: pointer to the buffer that needs to be added to the log.
diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
index 81bc95127f92..db320a43f42e 100644
--- a/security/loadpin/loadpin.c
+++ b/security/loadpin/loadpin.c
@@ -176,7 +176,7 @@ static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
 	return 0;
 }
 
-static int loadpin_load_data(enum kernel_load_data_id id)
+static int loadpin_load_data(enum kernel_load_data_id id, bool contents)
 {
 	return loadpin_read_file(NULL, (enum kernel_read_file_id) id);
 }
diff --git a/security/security.c b/security/security.c
index 19d3150f68f4..568bb77e84f4 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1695,17 +1695,29 @@ int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
 }
 EXPORT_SYMBOL_GPL(security_kernel_post_read_file);
 
-int security_kernel_load_data(enum kernel_load_data_id id)
+int security_kernel_load_data(enum kernel_load_data_id id, bool contents)
 {
 	int ret;
 
-	ret = call_int_hook(kernel_load_data, 0, id);
+	ret = call_int_hook(kernel_load_data, 0, id, contents);
 	if (ret)
 		return ret;
-	return ima_load_data(id);
+	return ima_load_data(id, contents);
 }
 EXPORT_SYMBOL_GPL(security_kernel_load_data);
 
+int security_kernel_post_load_data(char *buf, loff_t size,
+				   enum kernel_load_data_id id)
+{
+	int ret;
+
+	ret = call_int_hook(kernel_post_load_data, 0, buf, size, id);
+	if (ret)
+		return ret;
+	return ima_post_load_data(buf, size, id);
+}
+EXPORT_SYMBOL_GPL(security_kernel_post_load_data);
+
 int security_task_fix_setuid(struct cred *new, const struct cred *old,
 			     int flags)
 {
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 5de45010fb1a..1a5c68196faf 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4019,7 +4019,7 @@ static int selinux_kernel_read_file(struct file *file,
 	return rc;
 }
 
-static int selinux_kernel_load_data(enum kernel_load_data_id id)
+static int selinux_kernel_load_data(enum kernel_load_data_id id, bool contents)
 {
 	int rc = 0;
 
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 16/17] firmware: Add request_partial_firmware_into_buf()
From: Kees Cook @ 2020-07-29 17:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Kees Cook, Scott Branden, Mimi Zohar, Luis Chamberlain,
	Takashi Iwai, Jessica Yu, SeongJae Park, KP Singh, linux-efi,
	linux-security-module, linux-integrity, selinux, linux-kselftest,
	linux-kernel
In-Reply-To: <20200729175845.1745471-1-keescook@chromium.org>

From: Scott Branden <scott.branden@broadcom.com>

Add request_partial_firmware_into_buf() to allow for portions of a
firmware file to be read into a buffer. This is needed when large firmware
must be loaded in portions from a file on memory constrained systems.

Signed-off-by: Scott Branden <scott.branden@broadcom.com>
Co-developed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 drivers/base/firmware_loader/firmware.h |   4 +
 drivers/base/firmware_loader/main.c     | 101 +++++++++++++++++++-----
 include/linux/firmware.h                |  12 +++
 3 files changed, 99 insertions(+), 18 deletions(-)

diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h
index 7ad5fe52bc72..3f6eda46b3a2 100644
--- a/drivers/base/firmware_loader/firmware.h
+++ b/drivers/base/firmware_loader/firmware.h
@@ -32,6 +32,8 @@
  * @FW_OPT_FALLBACK_PLATFORM: Enable fallback to device fw copy embedded in
  *	the platform's main firmware. If both this fallback and the sysfs
  *      fallback are enabled, then this fallback will be tried first.
+ * @FW_OPT_PARTIAL: Allow partial read of firmware instead of needing to read
+ *	entire file.
  */
 enum fw_opt {
 	FW_OPT_UEVENT			= BIT(0),
@@ -41,6 +43,7 @@ enum fw_opt {
 	FW_OPT_NOCACHE			= BIT(4),
 	FW_OPT_NOFALLBACK_SYSFS		= BIT(5),
 	FW_OPT_FALLBACK_PLATFORM	= BIT(6),
+	FW_OPT_PARTIAL			= BIT(7),
 };
 
 enum fw_status {
@@ -68,6 +71,7 @@ struct fw_priv {
 	void *data;
 	size_t size;
 	size_t allocated_size;
+	size_t offset;
 	u32 opt_flags;
 #ifdef CONFIG_FW_LOADER_PAGED_BUF
 	bool is_paged_buf;
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 67d8afa91ae0..54c5d57b6327 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -170,10 +170,19 @@ static struct fw_priv *__allocate_fw_priv(const char *fw_name,
 					  struct firmware_cache *fwc,
 					  void *dbuf,
 					  size_t size,
+					  size_t offset,
 					  u32 opt_flags)
 {
 	struct fw_priv *fw_priv;
 
+	/* For a partial read, the buffer must be preallocated. */
+	if ((opt_flags & FW_OPT_PARTIAL) && !dbuf)
+		return NULL;
+
+	/* Only partial reads are allowed to use an offset. */
+	if (offset != 0 && !(opt_flags & FW_OPT_PARTIAL))
+		return NULL;
+
 	fw_priv = kzalloc(sizeof(*fw_priv), GFP_ATOMIC);
 	if (!fw_priv)
 		return NULL;
@@ -188,6 +197,7 @@ static struct fw_priv *__allocate_fw_priv(const char *fw_name,
 	fw_priv->fwc = fwc;
 	fw_priv->data = dbuf;
 	fw_priv->allocated_size = size;
+	fw_priv->offset = offset;
 	fw_priv->opt_flags = opt_flags;
 	fw_state_init(fw_priv);
 #ifdef CONFIG_FW_LOADER_USER_HELPER
@@ -216,12 +226,17 @@ static int alloc_lookup_fw_priv(const char *fw_name,
 				struct fw_priv **fw_priv,
 				void *dbuf,
 				size_t size,
+				size_t offset,
 				u32 opt_flags)
 {
 	struct fw_priv *tmp;
 
 	spin_lock(&fwc->lock);
-	if (!(opt_flags & FW_OPT_NOCACHE)) {
+	/*
+	 * Do not merge requests that are marked to be non-cached or
+	 * are performing partial reads.
+	 */
+	if (!(opt_flags & (FW_OPT_NOCACHE | FW_OPT_PARTIAL))) {
 		tmp = __lookup_fw_priv(fw_name);
 		if (tmp) {
 			kref_get(&tmp->ref);
@@ -232,7 +247,7 @@ static int alloc_lookup_fw_priv(const char *fw_name,
 		}
 	}
 
-	tmp = __allocate_fw_priv(fw_name, fwc, dbuf, size, opt_flags);
+	tmp = __allocate_fw_priv(fw_name, fwc, dbuf, size, offset, opt_flags);
 	if (tmp) {
 		INIT_LIST_HEAD(&tmp->list);
 		if (!(opt_flags & FW_OPT_NOCACHE))
@@ -485,6 +500,9 @@ fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv,
 		return -ENOMEM;
 
 	for (i = 0; i < ARRAY_SIZE(fw_path); i++) {
+		size_t file_size = 0;
+		size_t *file_size_ptr = NULL;
+
 		/* skip the unset customized path */
 		if (!fw_path[i][0])
 			continue;
@@ -498,9 +516,18 @@ fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv,
 
 		fw_priv->size = 0;
 
+		/*
+		 * The total file size is only examined when doing a partial
+		 * read; the "full read" case needs to fail if the whole
+		 * firmware was not completely loaded.
+		 */
+		if ((fw_priv->opt_flags & FW_OPT_PARTIAL) && buffer)
+			file_size_ptr = &file_size;
+
 		/* load firmware files from the mount namespace of init */
-		rc = kernel_read_file_from_path_initns(path, 0, &buffer, msize,
-						       NULL,
+		rc = kernel_read_file_from_path_initns(path, fw_priv->offset,
+						       &buffer, msize,
+						       file_size_ptr,
 						       READING_FIRMWARE);
 		if (rc < 0) {
 			if (rc != -ENOENT)
@@ -691,7 +718,7 @@ int assign_fw(struct firmware *fw, struct device *device)
 static int
 _request_firmware_prepare(struct firmware **firmware_p, const char *name,
 			  struct device *device, void *dbuf, size_t size,
-			  u32 opt_flags)
+			  size_t offset, u32 opt_flags)
 {
 	struct firmware *firmware;
 	struct fw_priv *fw_priv;
@@ -710,7 +737,7 @@ _request_firmware_prepare(struct firmware **firmware_p, const char *name,
 	}
 
 	ret = alloc_lookup_fw_priv(name, &fw_cache, &fw_priv, dbuf, size,
-				  opt_flags);
+				   offset, opt_flags);
 
 	/*
 	 * bind with 'priv' now to avoid warning in failure path
@@ -757,9 +784,10 @@ static void fw_abort_batch_reqs(struct firmware *fw)
 static int
 _request_firmware(const struct firmware **firmware_p, const char *name,
 		  struct device *device, void *buf, size_t size,
-		  u32 opt_flags)
+		  size_t offset, u32 opt_flags)
 {
 	struct firmware *fw = NULL;
+	bool nondirect = false;
 	int ret;
 
 	if (!firmware_p)
@@ -771,18 +799,22 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
 	}
 
 	ret = _request_firmware_prepare(&fw, name, device, buf, size,
-					opt_flags);
+					offset, opt_flags);
 	if (ret <= 0) /* error or already assigned */
 		goto out;
 
 	ret = fw_get_filesystem_firmware(device, fw->priv, "", NULL);
+
+	/* Only full reads can support decompression, platform, and sysfs. */
+	if (!(opt_flags & FW_OPT_PARTIAL))
+		nondirect = true;
+
 #ifdef CONFIG_FW_LOADER_COMPRESS
-	if (ret == -ENOENT)
+	if (ret == -ENOENT && nondirect)
 		ret = fw_get_filesystem_firmware(device, fw->priv, ".xz",
 						 fw_decompress_xz);
 #endif
-
-	if (ret == -ENOENT)
+	if (ret == -ENOENT && nondirect)
 		ret = firmware_fallback_platform(fw->priv);
 
 	if (ret) {
@@ -790,7 +822,9 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
 			dev_warn(device,
 				 "Direct firmware load for %s failed with error %d\n",
 				 name, ret);
-		ret = firmware_fallback_sysfs(fw, name, device, opt_flags, ret);
+		if (nondirect)
+			ret = firmware_fallback_sysfs(fw, name, device,
+						      opt_flags, ret);
 	} else
 		ret = assign_fw(fw, device);
 
@@ -833,7 +867,7 @@ request_firmware(const struct firmware **firmware_p, const char *name,
 
 	/* Need to pin this module until return */
 	__module_get(THIS_MODULE);
-	ret = _request_firmware(firmware_p, name, device, NULL, 0,
+	ret = _request_firmware(firmware_p, name, device, NULL, 0, 0,
 				FW_OPT_UEVENT);
 	module_put(THIS_MODULE);
 	return ret;
@@ -860,7 +894,7 @@ int firmware_request_nowarn(const struct firmware **firmware, const char *name,
 
 	/* Need to pin this module until return */
 	__module_get(THIS_MODULE);
-	ret = _request_firmware(firmware, name, device, NULL, 0,
+	ret = _request_firmware(firmware, name, device, NULL, 0, 0,
 				FW_OPT_UEVENT | FW_OPT_NO_WARN);
 	module_put(THIS_MODULE);
 	return ret;
@@ -884,7 +918,7 @@ int request_firmware_direct(const struct firmware **firmware_p,
 	int ret;
 
 	__module_get(THIS_MODULE);
-	ret = _request_firmware(firmware_p, name, device, NULL, 0,
+	ret = _request_firmware(firmware_p, name, device, NULL, 0, 0,
 				FW_OPT_UEVENT | FW_OPT_NO_WARN |
 				FW_OPT_NOFALLBACK_SYSFS);
 	module_put(THIS_MODULE);
@@ -909,7 +943,7 @@ int firmware_request_platform(const struct firmware **firmware,
 
 	/* Need to pin this module until return */
 	__module_get(THIS_MODULE);
-	ret = _request_firmware(firmware, name, device, NULL, 0,
+	ret = _request_firmware(firmware, name, device, NULL, 0, 0,
 				FW_OPT_UEVENT | FW_OPT_FALLBACK_PLATFORM);
 	module_put(THIS_MODULE);
 	return ret;
@@ -965,13 +999,44 @@ request_firmware_into_buf(const struct firmware **firmware_p, const char *name,
 		return -EOPNOTSUPP;
 
 	__module_get(THIS_MODULE);
-	ret = _request_firmware(firmware_p, name, device, buf, size,
+	ret = _request_firmware(firmware_p, name, device, buf, size, 0,
 				FW_OPT_UEVENT | FW_OPT_NOCACHE);
 	module_put(THIS_MODULE);
 	return ret;
 }
 EXPORT_SYMBOL(request_firmware_into_buf);
 
+/**
+ * request_partial_firmware_into_buf() - load partial firmware into a previously allocated buffer
+ * @firmware_p: pointer to firmware image
+ * @name: name of firmware file
+ * @device: device for which firmware is being loaded and DMA region allocated
+ * @buf: address of buffer to load firmware into
+ * @size: size of buffer
+ * @offset: offset into file to read
+ *
+ * This function works pretty much like request_firmware_into_buf except
+ * it allows a partial read of the file.
+ */
+int
+request_partial_firmware_into_buf(const struct firmware **firmware_p,
+				  const char *name, struct device *device,
+				  void *buf, size_t size, size_t offset)
+{
+	int ret;
+
+	if (fw_cache_is_setup(device, name))
+		return -EOPNOTSUPP;
+
+	__module_get(THIS_MODULE);
+	ret = _request_firmware(firmware_p, name, device, buf, size, offset,
+				FW_OPT_UEVENT | FW_OPT_NOCACHE |
+				FW_OPT_PARTIAL);
+	module_put(THIS_MODULE);
+	return ret;
+}
+EXPORT_SYMBOL(request_partial_firmware_into_buf);
+
 /**
  * release_firmware() - release the resource associated with a firmware image
  * @fw: firmware resource to release
@@ -1004,7 +1069,7 @@ static void request_firmware_work_func(struct work_struct *work)
 
 	fw_work = container_of(work, struct firmware_work, work);
 
-	_request_firmware(&fw, fw_work->name, fw_work->device, NULL, 0,
+	_request_firmware(&fw, fw_work->name, fw_work->device, NULL, 0, 0,
 			  fw_work->opt_flags);
 	fw_work->cont(fw, fw_work->context);
 	put_device(fw_work->device); /* taken in request_firmware_nowait() */
diff --git a/include/linux/firmware.h b/include/linux/firmware.h
index cb3e2c06ed8a..c15acadc6cf4 100644
--- a/include/linux/firmware.h
+++ b/include/linux/firmware.h
@@ -53,6 +53,9 @@ int request_firmware_direct(const struct firmware **fw, const char *name,
 			    struct device *device);
 int request_firmware_into_buf(const struct firmware **firmware_p,
 	const char *name, struct device *device, void *buf, size_t size);
+int request_partial_firmware_into_buf(const struct firmware **firmware_p,
+				      const char *name, struct device *device,
+				      void *buf, size_t size, size_t offset);
 
 void release_firmware(const struct firmware *fw);
 #else
@@ -102,6 +105,15 @@ static inline int request_firmware_into_buf(const struct firmware **firmware_p,
 	return -EINVAL;
 }
 
+static inline int request_partial_firmware_into_buf
+					(const struct firmware **firmware_p,
+					 const char *name,
+					 struct device *device,
+					 void *buf, size_t size, size_t offset)
+{
+	return -EINVAL;
+}
+
 #endif
 
 int firmware_request_cache(struct device *device, const char *name);
-- 
2.25.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox