diff mbox

[v2,14/25] SELinux: Maintain secid to secctx mapping

Message ID 8f91e9c4-8edc-a09a-7a83-c903c3a22d53@schaufler-ca.com (mailing list archive)
State New, archived
Headers show

Commit Message

Casey Schaufler Aug. 19, 2016, 10:48 p.m. UTC
Subject: [PATCH v2 14/25] SELinux: Maintain secid to secctx mapping

Maintain the mapping from secid to secctx in context structures
that are on the sidtab. This has two important impacts:

	- Because the secctx is kept with the secid it is
	  not necessary to regenerate the context when looking
	  for the secctx by secid or the secid by secctx.
	- With the secctx being kept around the interfaces
	  that use it don't have to make copies.

The minor increase in memory usage (a secctx string with each
secid) is a small price to pay for the elimination of countless
alloc/free pairs in the networking and audit code.

A new boolean has been added to the context structure to flag
"forced" use of the "str" field. Minor changes were required to
identify the difference between real and forced context strings.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 security/selinux/avc.c         |  8 ++-----
 security/selinux/hooks.c       | 49 +++++++++++++++++++-----------------------
 security/selinux/selinuxfs.c   |  7 ------
 security/selinux/ss/context.h  | 11 +++++++---
 security/selinux/ss/services.c | 19 ++++++----------
 security/selinux/ss/services.h |  4 ++++
 security/selinux/ss/sidtab.c   | 19 +++++++++++++---
 security/selinux/xfrm.c        |  8 ++-----
 8 files changed, 61 insertions(+), 64 deletions(-)
diff mbox

Patch

diff --git a/security/selinux/avc.c b/security/selinux/avc.c
index e60c79d..09977a9 100644
--- a/security/selinux/avc.c
+++ b/security/selinux/avc.c
@@ -152,18 +152,14 @@  static void avc_dump_query(struct audit_buffer *ab, u32 ssid, u32 tsid, u16 tcla
 	rc = security_sid_to_context(ssid, &scontext, &scontext_len);
 	if (rc)
 		audit_log_format(ab, "ssid=%d", ssid);
-	else {
+	else
 		audit_log_format(ab, "scontext=%s", scontext);
-		kfree(scontext);
-	}
 
 	rc = security_sid_to_context(tsid, &scontext, &scontext_len);
 	if (rc)
 		audit_log_format(ab, " tsid=%d", tsid);
-	else {
+	else
 		audit_log_format(ab, " tcontext=%s", scontext);
-		kfree(scontext);
-	}
 
 	BUG_ON(!tclass || tclass >= ARRAY_SIZE(secclass_map));
 	audit_log_format(ab, " tclass=%s", secclass_map[tclass-1].name);
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 03717c2..e8c6c5d 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2872,7 +2872,9 @@  static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
 		rc = security_sid_to_context_force(newsid, &context, &clen);
 		if (rc)
 			return rc;
-		*value = context;
+		*value = kstrdup(context, GFP_KERNEL);
+		if (*value == NULL)
+			return -ENOMEM;
 		*len = clen;
 	}
 
@@ -3231,12 +3233,13 @@  static int selinux_inode_getsecurity(struct inode *inode, const char *name, void
 	if (error)
 		return error;
 	error = size;
-	if (alloc) {
+
+	/*
+	 * context will not be freed because selinux_release_context
+	 * does not free anything.
+	 */
+	if (alloc)
 		*buffer = context;
-		goto out_nofree;
-	}
-	kfree(context);
-out_nofree:
 	return error;
 }
 
@@ -4621,7 +4624,6 @@  static int selinux_socket_getpeersec_stream(struct socket *sock, char __user *op
 out_len:
 	if (put_user(scontext_len, optlen))
 		err = -EFAULT;
-	kfree(scontext);
 	return err;
 }
 
@@ -5674,6 +5676,7 @@  static int selinux_getprocattr(struct task_struct *p,
 	u32 sid;
 	int error;
 	unsigned len;
+	char *vp;
 
 	if (current != p) {
 		error = current_has_perm(p, PROCESS__GETATTR);
@@ -5705,21 +5708,17 @@  static int selinux_getprocattr(struct task_struct *p,
 	if (!sid)
 		return 0;
 
-	if (strcmp(name, "context")) {
-		error = security_sid_to_context(sid, value, &len);
-	} else {
-		char *vp;
-
-		error = security_sid_to_context(sid, &vp, &len);
-		if (!error) {
-			*value = kasprintf(GFP_KERNEL, "selinux='%s'", vp);
-			if (*value == NULL)
-				error = -ENOMEM;
-		}
-	}
-
+	error = security_sid_to_context(sid, &vp, &len);
 	if (error)
 		return error;
+
+	if (strcmp(name, "context"))
+		*value = kstrdup(vp, GFP_KERNEL);
+	else
+		*value = kasprintf(GFP_KERNEL, "selinux='%s'", vp);
+
+	if (*value == NULL)
+		return -ENOMEM;
 	return len;
 
 invalid:
@@ -5876,11 +5875,6 @@  static int selinux_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid)
 	return security_context_to_sid(secdata, seclen, secid, GFP_KERNEL);
 }
 
-static void selinux_release_secctx(char *secdata, u32 seclen)
-{
-	kfree(secdata);
-}
-
 static void selinux_inode_invalidate_secctx(struct inode *inode)
 {
 	struct inode_security_struct *isec = inode->i_security;
@@ -5978,7 +5972,9 @@  static int selinux_key_getsecurity(struct key *key, char **_buffer)
 	rc = security_sid_to_context(ksec->sid, &context, &len);
 	if (!rc)
 		rc = len;
-	*_buffer = context;
+	*_buffer = kstrdup(context, GFP_KERNEL);
+	if (*_buffer == NULL)
+		rc = -ENOMEM;
 	return rc;
 }
 
@@ -6123,7 +6119,6 @@  static struct security_hook_list selinux_hooks[] = {
 	LSM_HOOK_INIT(ismaclabel, selinux_ismaclabel),
 	LSM_HOOK_INIT(secid_to_secctx, selinux_secid_to_secctx),
 	LSM_HOOK_INIT(secctx_to_secid, selinux_secctx_to_secid),
-	LSM_HOOK_INIT(release_secctx, selinux_release_secctx),
 	LSM_HOOK_INIT(inode_invalidate_secctx, selinux_inode_invalidate_secctx),
 	LSM_HOOK_INIT(inode_notifysecctx, selinux_inode_notifysecctx),
 	LSM_HOOK_INIT(inode_setsecctx, selinux_inode_setsecctx),
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index 2519e26..96674d8 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -582,7 +582,6 @@  static ssize_t sel_write_context(struct file *file, char *buf, size_t size)
 	memcpy(buf, canon, len);
 	length = len;
 out:
-	kfree(canon);
 	return length;
 }
 
@@ -902,7 +901,6 @@  static ssize_t sel_write_create(struct file *file, char *buf, size_t size)
 	memcpy(buf, newcon, len);
 	length = len;
 out:
-	kfree(newcon);
 	kfree(namebuf);
 	kfree(tcon);
 	kfree(scon);
@@ -959,7 +957,6 @@  static ssize_t sel_write_relabel(struct file *file, char *buf, size_t size)
 	memcpy(buf, newcon, len);
 	length = len;
 out:
-	kfree(newcon);
 	kfree(tcon);
 	kfree(scon);
 	return length;
@@ -1009,12 +1006,10 @@  static ssize_t sel_write_user(struct file *file, char *buf, size_t size)
 			goto out;
 		}
 		if ((length + len) >= SIMPLE_TRANSACTION_LIMIT) {
-			kfree(newcon);
 			length = -ERANGE;
 			goto out;
 		}
 		memcpy(ptr, newcon, len);
-		kfree(newcon);
 		ptr += len;
 		length += len;
 	}
@@ -1078,7 +1073,6 @@  static ssize_t sel_write_member(struct file *file, char *buf, size_t size)
 	memcpy(buf, newcon, len);
 	length = len;
 out:
-	kfree(newcon);
 	kfree(tcon);
 	kfree(scon);
 	return length;
@@ -1521,7 +1515,6 @@  static ssize_t sel_read_initcon(struct file *file, char __user *buf,
 		return ret;
 
 	ret = simple_read_from_buffer(buf, count, ppos, con, len);
-	kfree(con);
 	return ret;
 }
 
diff --git a/security/selinux/ss/context.h b/security/selinux/ss/context.h
index 212e347..582edf0 100644
--- a/security/selinux/ss/context.h
+++ b/security/selinux/ss/context.h
@@ -29,7 +29,8 @@  struct context {
 	u32 type;
 	u32 len;        /* length of string in bytes */
 	struct mls_range range;
-	char *str;	/* string representation if context cannot be mapped. */
+	char *str;	/* string representation. */
+	bool forced;	/* not really a context */
 };
 
 static inline void mls_context_init(struct context *c)
@@ -121,6 +122,7 @@  static inline int context_cpy(struct context *dst, struct context *src)
 	dst->user = src->user;
 	dst->role = src->role;
 	dst->type = src->type;
+	dst->forced = src->forced;
 	if (src->str) {
 		dst->str = kstrdup(src->str, GFP_ATOMIC);
 		if (!dst->str)
@@ -144,15 +146,18 @@  static inline void context_destroy(struct context *c)
 	kfree(c->str);
 	c->str = NULL;
 	c->len = 0;
+	c->forced = false;
 	mls_context_destroy(c);
 }
 
 static inline int context_cmp(struct context *c1, struct context *c2)
 {
-	if (c1->len && c2->len)
+	if (c1->forced && c2->forced)
 		return (c1->len == c2->len && !strcmp(c1->str, c2->str));
-	if (c1->len || c2->len)
+	if (c1->forced || c2->forced)
 		return 0;
+	if (c1->str == c2->str && c1->str != NULL)
+		return 1;
 	return ((c1->user == c2->user) &&
 		(c1->role == c2->role) &&
 		(c1->type == c2->type) &&
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 082b20c..08551b2 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -89,8 +89,6 @@  int ss_initialized;
 static u32 latest_granting;
 
 /* Forward declaration. */
-static int context_struct_to_string(struct context *context, char **scontext,
-				    u32 *scontext_len);
 
 static void context_struct_compute_av(struct context *scontext,
 					struct context *tcontext,
@@ -1176,7 +1174,8 @@  allow:
  * to point to this string and set `*scontext_len' to
  * the length of the string.
  */
-static int context_struct_to_string(struct context *context, char **scontext, u32 *scontext_len)
+int context_struct_to_string(struct context *context, char **scontext,
+				u32 *scontext_len)
 {
 	char *scontextp;
 
@@ -1245,18 +1244,11 @@  static int security_sid_to_context_core(u32 sid, char **scontext,
 
 	if (!ss_initialized) {
 		if (sid <= SECINITSID_NUM) {
-			char *scontextp;
 
 			*scontext_len = strlen(initial_sid_to_string[sid]) + 1;
 			if (!scontext)
 				goto out;
-			scontextp = kmemdup(initial_sid_to_string[sid],
-					    *scontext_len, GFP_ATOMIC);
-			if (!scontextp) {
-				rc = -ENOMEM;
-				goto out;
-			}
-			*scontext = scontextp;
+			*scontext = (char *)initial_sid_to_string[sid];
 			goto out;
 		}
 		printk(KERN_ERR "SELinux: %s:  called before initial "
@@ -1435,6 +1427,7 @@  static int security_context_to_sid_core(const char *scontext, u32 scontext_len,
 	if (rc == -EINVAL && force) {
 		context.str = str;
 		context.len = scontext_len;
+		context.forced = true;
 		str = NULL;
 	} else if (rc)
 		goto out_unlock;
@@ -1862,7 +1855,7 @@  static int convert_context(u32 key,
 
 	args = p;
 
-	if (c->str) {
+	if (c->str && c->forced) {
 		struct context ctx;
 
 		rc = -ENOMEM;
@@ -1878,6 +1871,7 @@  static int convert_context(u32 key,
 			       c->str);
 			/* Replace string with mapped representation. */
 			kfree(c->str);
+			c->forced = false;
 			memcpy(c, &ctx, sizeof(*c));
 			goto out;
 		} else if (rc == -EINVAL) {
@@ -1976,6 +1970,7 @@  bad:
 	context_destroy(c);
 	c->str = s;
 	c->len = len;
+	c->forced = true;
 	printk(KERN_INFO "SELinux:  Context %s became invalid (unmapped).\n",
 	       c->str);
 	rc = 0;
diff --git a/security/selinux/ss/services.h b/security/selinux/ss/services.h
index 6abcd87..dd9dffd 100644
--- a/security/selinux/ss/services.h
+++ b/security/selinux/ss/services.h
@@ -17,5 +17,9 @@  void services_compute_xperms_drivers(struct extended_perms *xperms,
 void services_compute_xperms_decision(struct extended_perms_decision *xpermd,
 					struct avtab_node *node);
 
+int context_struct_to_string(struct context *context, char **scontext,
+				u32 *scontext_len);
+
+
 #endif	/* _SS_SERVICES_H_ */
 
diff --git a/security/selinux/ss/sidtab.c b/security/selinux/ss/sidtab.c
index 5840a35..3876f40 100644
--- a/security/selinux/ss/sidtab.c
+++ b/security/selinux/ss/sidtab.c
@@ -9,6 +9,7 @@ 
 #include <linux/errno.h>
 #include "flask.h"
 #include "security.h"
+#include "services.h"
 #include "sidtab.h"
 
 #define SIDTAB_HASH(sid) \
@@ -34,6 +35,8 @@  int sidtab_insert(struct sidtab *s, u32 sid, struct context *context)
 {
 	int hvalue, rc = 0;
 	struct sidtab_node *prev, *cur, *newnode;
+	char *newstr;
+	int newlen;
 
 	if (!s) {
 		rc = -ENOMEM;
@@ -64,6 +67,16 @@  int sidtab_insert(struct sidtab *s, u32 sid, struct context *context)
 		rc = -ENOMEM;
 		goto out;
 	}
+	if (!context->str) {
+		if (context_struct_to_string(context, &newstr, &newlen) < 0) {
+			context_destroy(&newnode->context);
+			kfree(newnode);
+			rc = -ENOMEM;
+			goto out;
+		}
+		newnode->context.str = newstr;
+		newnode->context.len = newlen;
+	}
 
 	if (prev) {
 		newnode->next = prev->next;
@@ -95,10 +108,10 @@  static struct context *sidtab_search_core(struct sidtab *s, u32 sid, int force)
 	while (cur && sid > cur->sid)
 		cur = cur->next;
 
-	if (force && cur && sid == cur->sid && cur->context.len)
+	if (force && cur && sid == cur->sid && cur->context.forced)
 		return &cur->context;
 
-	if (cur == NULL || sid != cur->sid || cur->context.len) {
+	if (cur == NULL || sid != cur->sid || cur->context.forced) {
 		/* Remap invalid SIDs to the unlabeled SID. */
 		sid = SECINITSID_UNLABELED;
 		hvalue = SIDTAB_HASH(sid);
@@ -219,7 +232,7 @@  int sidtab_context_to_sid(struct sidtab *s,
 			goto unlock_out;
 		}
 		sid = s->next_sid++;
-		if (context->len)
+		if (context->forced)
 			printk(KERN_INFO
 		       "SELinux:  Context %s is not valid (left unmapped).\n",
 			       context->str);
diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c
index 789d07b..ca165d3 100644
--- a/security/selinux/xfrm.c
+++ b/security/selinux/xfrm.c
@@ -357,10 +357,8 @@  int selinux_xfrm_state_alloc_acquire(struct xfrm_state *x,
 		return rc;
 
 	ctx = kmalloc(sizeof(*ctx) + str_len, GFP_ATOMIC);
-	if (!ctx) {
-		rc = -ENOMEM;
-		goto out;
-	}
+	if (!ctx)
+		return -ENOMEM;
 
 	ctx->ctx_doi = XFRM_SC_DOI_LSM;
 	ctx->ctx_alg = XFRM_SC_ALG_SELINUX;
@@ -370,8 +368,6 @@  int selinux_xfrm_state_alloc_acquire(struct xfrm_state *x,
 
 	x->security = ctx;
 	atomic_inc(&selinux_xfrm_refcount);
-out:
-	kfree(ctx_str);
 	return rc;
 }