All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] SELinux: seperate range transition rules to a seperate function
@ 2010-06-11 16:37 Eric Paris
  2010-06-11 16:37 ` [PATCH 2/4] SELinux: move genfs read to a separate function Eric Paris
                   ` (4 more replies)
  0 siblings, 5 replies; 26+ messages in thread
From: Eric Paris @ 2010-06-11 16:37 UTC (permalink / raw)
  To: selinux; +Cc: sds, jmorris

Move the range transition rule to a separate function, range_read(), rather
than doing it all in policydb_read()

Signed-off-by: Eric Paris <eparis@redhat.com>
---

 security/selinux/ss/policydb.c |  139 ++++++++++++++++++++++------------------
 1 files changed, 75 insertions(+), 64 deletions(-)

diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index c57802a..a39d38a 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -1701,6 +1701,78 @@ u32 string_to_av_perm(struct policydb *p, u16 tclass, const char *name)
 	return 1U << (perdatum->value-1);
 }
 
+static int range_read(struct policydb *p, void *fp)
+{
+	struct range_trans *rt = NULL;
+	struct mls_range *r = NULL;
+	int i, rc;
+	__le32 buf[2];
+	u32 nel;
+
+	if (p->policyvers < POLICYDB_VERSION_MLS)
+		return 0;
+
+	rc = next_entry(buf, fp, sizeof(u32));
+	if (rc)
+		goto out;
+
+	nel = le32_to_cpu(buf[0]);
+	for (i = 0; i < nel; i++) {
+		rc = -ENOMEM;
+		rt = kzalloc(sizeof(*rt), GFP_KERNEL);
+		if (!rt)
+			goto out;
+
+		rc = next_entry(buf, fp, (sizeof(u32) * 2));
+		if (rc)
+			goto out;
+
+		rt->source_type = le32_to_cpu(buf[0]);
+		rt->target_type = le32_to_cpu(buf[1]);
+		if (p->policyvers >= POLICYDB_VERSION_RANGETRANS) {
+			rc = next_entry(buf, fp, sizeof(u32));
+			if (rc)
+				goto out;
+			rt->target_class = le32_to_cpu(buf[0]);
+		} else
+			rt->target_class = p->process_class;
+
+		rc = -EINVAL;
+		if (!policydb_type_isvalid(p, rt->source_type) ||
+		    !policydb_type_isvalid(p, rt->target_type) ||
+		    !policydb_class_isvalid(p, rt->target_class))
+			goto out;
+
+		rc = -ENOMEM;
+		r = kzalloc(sizeof(*r), GFP_KERNEL);
+		if (!r)
+			goto out;
+
+		rc = mls_read_range_helper(r, fp);
+		if (rc)
+			goto out;
+
+		rc = -EINVAL;
+		if (!mls_range_isvalid(p, r)) {
+			printk(KERN_WARNING "SELinux:  rangetrans:  invalid range\n");
+			goto out;
+		}
+
+		rc = hashtab_insert(p->range_tr, rt, r);
+		if (rc)
+			goto out;
+
+		rt = NULL;
+		r = NULL;
+	}
+	rangetr_hash_eval(p->range_tr);
+	rc = 0;
+out:
+	kfree(rt);
+	kfree(r);
+	return rc;
+}
+
 /*
  * Read the configuration data from a policy database binary
  * representation file into a policy database structure.
@@ -1717,8 +1789,6 @@ int policydb_read(struct policydb *p, void *fp)
 	u32 len, len2, nprim, nel, nel2;
 	char *policydb_str;
 	struct policydb_compat_info *info;
-	struct range_trans *rt;
-	struct mls_range *r;
 
 	rc = policydb_init(p);
 	if (rc)
@@ -2131,68 +2201,9 @@ int policydb_read(struct policydb *p, void *fp)
 		}
 	}
 
-	if (p->policyvers >= POLICYDB_VERSION_MLS) {
-		int new_rangetr = p->policyvers >= POLICYDB_VERSION_RANGETRANS;
-		rc = next_entry(buf, fp, sizeof(u32));
-		if (rc < 0)
-			goto bad;
-		nel = le32_to_cpu(buf[0]);
-		for (i = 0; i < nel; i++) {
-			rt = kzalloc(sizeof(*rt), GFP_KERNEL);
-			if (!rt) {
-				rc = -ENOMEM;
-				goto bad;
-			}
-			rc = next_entry(buf, fp, (sizeof(u32) * 2));
-			if (rc < 0) {
-				kfree(rt);
-				goto bad;
-			}
-			rt->source_type = le32_to_cpu(buf[0]);
-			rt->target_type = le32_to_cpu(buf[1]);
-			if (new_rangetr) {
-				rc = next_entry(buf, fp, sizeof(u32));
-				if (rc < 0) {
-					kfree(rt);
-					goto bad;
-				}
-				rt->target_class = le32_to_cpu(buf[0]);
-			} else
-				rt->target_class = p->process_class;
-			if (!policydb_type_isvalid(p, rt->source_type) ||
-			    !policydb_type_isvalid(p, rt->target_type) ||
-			    !policydb_class_isvalid(p, rt->target_class)) {
-				kfree(rt);
-				rc = -EINVAL;
-				goto bad;
-			}
-			r = kzalloc(sizeof(*r), GFP_KERNEL);
-			if (!r) {
-				kfree(rt);
-				rc = -ENOMEM;
-				goto bad;
-			}
-			rc = mls_read_range_helper(r, fp);
-			if (rc) {
-				kfree(rt);
-				kfree(r);
-				goto bad;
-			}
-			if (!mls_range_isvalid(p, r)) {
-				printk(KERN_WARNING "SELinux:  rangetrans:  invalid range\n");
-				kfree(rt);
-				kfree(r);
-				goto bad;
-			}
-			rc = hashtab_insert(p->range_tr, rt, r);
-			if (rc) {
-				kfree(rt);
-				kfree(r);
-				goto bad;
-			}
-		}
-		rangetr_hash_eval(p->range_tr);
-	}
+	rc = range_read(p, fp);
+	if (rc)
+		goto bad;
 
 	p->type_attr_map = kmalloc(p->p_types.nprim * sizeof(struct ebitmap), GFP_KERNEL);
 	if (!p->type_attr_map)


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 2/4] SELinux: move genfs read to a separate function
  2010-06-11 16:37 [PATCH 1/4] SELinux: seperate range transition rules to a seperate function Eric Paris
@ 2010-06-11 16:37 ` Eric Paris
  2010-06-16 14:18   ` Stephen Smalley
  2010-06-11 16:37 ` [PATCH 3/4] SELinux: break ocontext reading into " Eric Paris
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 26+ messages in thread
From: Eric Paris @ 2010-06-11 16:37 UTC (permalink / raw)
  To: selinux; +Cc: sds, jmorris

move genfs read functionality out of policydb_read() and into a new
function called genfs_read()

Signed-off-by: Eric Paris <eparis@redhat.com>
---

 security/selinux/ss/policydb.c |  236 ++++++++++++++++++++++------------------
 1 files changed, 131 insertions(+), 105 deletions(-)

diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index a39d38a..66e217b 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -655,6 +655,9 @@ static int range_tr_destroy(void *key, void *datum, void *p)
 
 static void ocontext_destroy(struct ocontext *c, int i)
 {
+	if (!c)
+		return;
+
 	context_destroy(&c->context[0]);
 	context_destroy(&c->context[1]);
 	if (i == OCON_ISID || i == OCON_FS ||
@@ -1773,6 +1776,129 @@ out:
 	return rc;
 }
 
+static int genfs_read(struct policydb *p, void *fp)
+{
+	int i, j, rc;
+	u32 nel, nel2, len, len2;
+	__le32 buf[1];
+	struct ocontext *l, *c;
+	struct ocontext *newc = NULL;
+	struct genfs *genfs_p, *genfs;
+	struct genfs *newgenfs = NULL;
+
+	rc = next_entry(buf, fp, sizeof(u32));
+	if (rc < 0)
+		goto out;
+	nel = le32_to_cpu(buf[0]);
+
+	for (i = 0; i < nel; i++) {
+		rc = next_entry(buf, fp, sizeof(u32));
+		if (rc)
+			goto out;
+		len = le32_to_cpu(buf[0]);
+
+		rc = -ENOMEM;
+		newgenfs = kzalloc(sizeof(*newgenfs), GFP_KERNEL);
+		if (!newgenfs)
+			goto out;
+
+		rc = -ENOMEM;
+		newgenfs->fstype = kmalloc(len + 1, GFP_KERNEL);
+		if (!newgenfs->fstype)
+			goto out;
+
+		rc = next_entry(newgenfs->fstype, fp, len);
+		if (rc < 0)
+			goto out;
+
+		newgenfs->fstype[len] = 0;
+
+		for (genfs_p = NULL, genfs = p->genfs; genfs;
+		     genfs_p = genfs, genfs = genfs->next) {
+			rc = -EEXIST;
+			if (strcmp(newgenfs->fstype, genfs->fstype) == 0) {
+				printk(KERN_ERR "SELinux:  dup genfs fstype %s\n",
+				       newgenfs->fstype);
+				goto out;
+			}
+			if (strcmp(newgenfs->fstype, genfs->fstype) < 0)
+				break;
+		}
+		newgenfs->next = genfs;
+		if (genfs_p)
+			genfs_p->next = newgenfs;
+		else
+			p->genfs = newgenfs;
+		genfs = newgenfs;
+		newgenfs = NULL;
+
+		rc = next_entry(buf, fp, sizeof(u32));
+		if (rc)
+			goto out;
+
+		nel2 = le32_to_cpu(buf[0]);
+		for (j = 0; j < nel2; j++) {
+			rc = next_entry(buf, fp, sizeof(u32));
+			if (rc)
+				goto out;
+			len = le32_to_cpu(buf[0]);
+
+			rc = -ENOMEM;
+			newc = kzalloc(sizeof(*newc), GFP_KERNEL);
+			if (!newc)
+				goto out;
+
+			rc = -ENOMEM;
+			newc->u.name = kmalloc(len + 1, GFP_KERNEL);
+			if (!newc->u.name)
+				goto out;
+
+			rc = next_entry(newc->u.name, fp, len);
+			if (rc)
+				goto out;
+			newc->u.name[len] = 0;
+
+			rc = next_entry(buf, fp, sizeof(u32));
+			if (rc)
+				goto out;
+
+			newc->v.sclass = le32_to_cpu(buf[0]);
+			rc = context_read_and_validate(&newc->context[0], p, fp);
+			if (rc)
+				goto out;
+
+			for (l = NULL, c = genfs->head; c;
+			     l = c, c = c->next) {
+				if (!strcmp(newc->u.name, c->u.name) &&
+				    (!c->v.sclass || !newc->v.sclass ||
+				     newc->v.sclass == c->v.sclass)) {
+					printk(KERN_ERR "SELinux:  dup genfs entry (%s,%s)\n",
+					       genfs->fstype, c->u.name);
+					goto out;
+				}
+				len = strlen(newc->u.name);
+				len2 = strlen(c->u.name);
+				if (len > len2)
+					break;
+			}
+
+			newc->next = c;
+			if (l)
+				l->next = newc;
+			else
+				genfs->head = newc;
+			newc = NULL;
+		}
+	}
+out:
+	if (newgenfs)
+		kfree(newgenfs->fstype);
+	kfree(newgenfs);
+	ocontext_destroy(newc, OCON_FSUSE);
+
+	return rc;
+}
+
 /*
  * Read the configuration data from a policy database binary
  * representation file into a policy database structure.
@@ -1781,12 +1907,12 @@ int policydb_read(struct policydb *p, void *fp)
 {
 	struct role_allow *ra, *lra;
 	struct role_trans *tr, *ltr;
-	struct ocontext *l, *c, *newc;
-	struct genfs *genfs_p, *genfs, *newgenfs;
+	struct ocontext *l, *c;
 	int i, j, rc;
 	__le32 buf[4];
 	u32 nodebuf[8];
-	u32 len, len2, nprim, nel, nel2;
+	u32 len, nprim, nel;
+
 	char *policydb_str;
 	struct policydb_compat_info *info;
 
@@ -2099,107 +2225,9 @@ int policydb_read(struct policydb *p, void *fp)
 		}
 	}
 
-	rc = next_entry(buf, fp, sizeof(u32));
-	if (rc < 0)
+	rc = genfs_read(p, fp);
+	if (rc)
 		goto bad;
-	nel = le32_to_cpu(buf[0]);
-	genfs_p = NULL;
-	rc = -EINVAL;
-	for (i = 0; i < nel; i++) {
-		rc = next_entry(buf, fp, sizeof(u32));
-		if (rc < 0)
-			goto bad;
-		len = le32_to_cpu(buf[0]);
-		newgenfs = kzalloc(sizeof(*newgenfs), GFP_KERNEL);
-		if (!newgenfs) {
-			rc = -ENOMEM;
-			goto bad;
-		}
-
-		newgenfs->fstype = kmalloc(len + 1, GFP_KERNEL);
-		if (!newgenfs->fstype) {
-			rc = -ENOMEM;
-			kfree(newgenfs);
-			goto bad;
-		}
-		rc = next_entry(newgenfs->fstype, fp, len);
-		if (rc < 0) {
-			kfree(newgenfs->fstype);
-			kfree(newgenfs);
-			goto bad;
-		}
-		newgenfs->fstype[len] = 0;
-		for (genfs_p = NULL, genfs = p->genfs; genfs;
-		     genfs_p = genfs, genfs = genfs->next) {
-			if (strcmp(newgenfs->fstype, genfs->fstype) == 0) {
-				printk(KERN_ERR "SELinux:  dup genfs "
-				       "fstype %s\n", newgenfs->fstype);
-				kfree(newgenfs->fstype);
-				kfree(newgenfs);
-				goto bad;
-			}
-			if (strcmp(newgenfs->fstype, genfs->fstype) < 0)
-				break;
-		}
-		newgenfs->next = genfs;
-		if (genfs_p)
-			genfs_p->next = newgenfs;
-		else
-			p->genfs = newgenfs;
-		rc = next_entry(buf, fp, sizeof(u32));
-		if (rc < 0)
-			goto bad;
-		nel2 = le32_to_cpu(buf[0]);
-		for (j = 0; j < nel2; j++) {
-			rc = next_entry(buf, fp, sizeof(u32));
-			if (rc < 0)
-				goto bad;
-			len = le32_to_cpu(buf[0]);
-
-			newc = kzalloc(sizeof(*newc), GFP_KERNEL);
-			if (!newc) {
-				rc = -ENOMEM;
-				goto bad;
-			}
-
-			newc->u.name = kmalloc(len + 1, GFP_KERNEL);
-			if (!newc->u.name) {
-				rc = -ENOMEM;
-				goto bad_newc;
-			}
-			rc = next_entry(newc->u.name, fp, len);
-			if (rc < 0)
-				goto bad_newc;
-			newc->u.name[len] = 0;
-			rc = next_entry(buf, fp, sizeof(u32));
-			if (rc < 0)
-				goto bad_newc;
-			newc->v.sclass = le32_to_cpu(buf[0]);
-			if (context_read_and_validate(&newc->context[0], p, fp))
-				goto bad_newc;
-			for (l = NULL, c = newgenfs->head; c;
-			     l = c, c = c->next) {
-				if (!strcmp(newc->u.name, c->u.name) &&
-				    (!c->v.sclass || !newc->v.sclass ||
-				     newc->v.sclass == c->v.sclass)) {
-					printk(KERN_ERR "SELinux:  dup genfs "
-					       "entry (%s,%s)\n",
-					       newgenfs->fstype, c->u.name);
-					goto bad_newc;
-				}
-				len = strlen(newc->u.name);
-				len2 = strlen(c->u.name);
-				if (len > len2)
-					break;
-			}
-
-			newc->next = c;
-			if (l)
-				l->next = newc;
-			else
-				newgenfs->head = newc;
-		}
-	}
 
 	rc = range_read(p, fp);
 	if (rc)
@@ -2227,8 +2255,6 @@ int policydb_read(struct policydb *p, void *fp)
 	rc = 0;
 out:
 	return rc;
-bad_newc:
-	ocontext_destroy(newc, OCON_FSUSE);
 bad:
 	if (!rc)
 		rc = -EINVAL;


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 3/4] SELinux: break ocontext reading into a separate function
  2010-06-11 16:37 [PATCH 1/4] SELinux: seperate range transition rules to a seperate function Eric Paris
  2010-06-11 16:37 ` [PATCH 2/4] SELinux: move genfs read to a separate function Eric Paris
@ 2010-06-11 16:37 ` Eric Paris
  2010-06-16 14:39   ` Stephen Smalley
  2010-06-11 16:37 ` [PATCH 4/4] SELinux: allow userspace to read policy back out of the kernel Eric Paris
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 26+ messages in thread
From: Eric Paris @ 2010-06-11 16:37 UTC (permalink / raw)
  To: selinux; +Cc: sds, jmorris

Move the reading of ocontext type data out of policydb_read() in a separate
function ocontext_read()

Signed-off-by: Eric Paris <eparis@redhat.com>
---

 security/selinux/ss/policydb.c |  242 ++++++++++++++++++++++------------------
 1 files changed, 131 insertions(+), 111 deletions(-)

diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index 66e217b..99fff0c 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -1899,6 +1899,134 @@ out:
 	return rc;
 }
 
+static int ocontext_read(struct policydb *p, struct policydb_compat_info *info,
+			 void *fp)
+{
+	int i, j, rc;
+	u32 nel, len;
+	__le32 buf[3];
+	struct ocontext *l, *c;
+	u32 nodebuf[8];
+
+	for (i = 0; i < info->ocon_num; i++) {
+		rc = next_entry(buf, fp, sizeof(u32));
+		if (rc)
+			goto out;
+		nel = le32_to_cpu(buf[0]);
+
+		l = NULL;
+		for (j = 0; j < nel; j++) {
+			rc = -ENOMEM;
+			c = kzalloc(sizeof(*c), GFP_KERNEL);
+			if (!c)
+				goto out;
+			if (l)
+				l->next = c;
+			else
+				p->ocontexts[i] = c;
+			l = c;
+
+			switch (i) {
+			case OCON_ISID:
+				rc = next_entry(buf, fp, sizeof(u32));
+				if (rc)
+					goto out;
+
+				c->sid[0] = le32_to_cpu(buf[0]);
+				rc = context_read_and_validate(&c->context[0], p, fp);
+				if (rc)
+					goto out;
+				break;
+			case OCON_FS:
+			case OCON_NETIF:
+				rc = next_entry(buf, fp, sizeof(u32));
+				if (rc)
+					goto out;
+				len = le32_to_cpu(buf[0]);
+
+				rc = -ENOMEM;
+				c->u.name = kmalloc(len + 1, GFP_KERNEL);
+				if (!c->u.name)
+					goto out;
+
+				rc = next_entry(c->u.name, fp, len);
+				if (rc)
+					goto out;
+
+				c->u.name[len] = 0;
+				rc = context_read_and_validate(&c->context[0], p, fp);
+				if (rc)
+					goto out;
+				rc = context_read_and_validate(&c->context[1], p, fp);
+				if (rc)
+					goto out;
+				break;
+			case OCON_PORT:
+				rc = next_entry(buf, fp, sizeof(u32)*3);
+				if (rc)
+					goto out;
+				c->u.port.protocol = le32_to_cpu(buf[0]);
+				c->u.port.low_port = le32_to_cpu(buf[1]);
+				c->u.port.high_port = le32_to_cpu(buf[2]);
+				rc = context_read_and_validate(&c->context[0], p, fp);
+				if (rc)
+					goto out;
+				break;
+			case OCON_NODE:
+				rc = next_entry(nodebuf, fp, sizeof(u32) * 2);
+				if (rc)
+					goto out;
+				c->u.node.addr = nodebuf[0]; /* network order */
+				c->u.node.mask = nodebuf[1]; /* network order */
+				rc = context_read_and_validate(&c->context[0], p, fp);
+				if (rc)
+					goto out;
+				break;
+			case OCON_FSUSE:
+				rc = next_entry(buf, fp, sizeof(u32)*2);
+				if (rc)
+					goto out;
+				c->v.behavior = le32_to_cpu(buf[0]);
+				if (c->v.behavior > SECURITY_FS_USE_NONE)
+					goto out;
+
+				rc = -ENOMEM;
+				len = le32_to_cpu(buf[1]);
+				c->u.name = kmalloc(len + 1, GFP_KERNEL);
+				if (!c->u.name)
+					goto out;
+
+				rc = next_entry(c->u.name, fp, len);
+				if (rc)
+					goto out;
+				c->u.name[len] = 0;
+				rc = context_read_and_validate(&c->context[0], p, fp);
+				if (rc)
+					goto out;
+				break;
+			case OCON_NODE6: {
+				int k;
+
+				rc = next_entry(nodebuf, fp, sizeof(u32) * 8);
+				if (rc)
+					goto out;
+				for (k = 0; k < 4; k++)
+					c->u.node6.addr[k] = nodebuf[k];
+				for (k = 0; k < 4; k++)
+					c->u.node6.mask[k] = nodebuf[k+4];
+				rc = context_read_and_validate(&c->context[0], p, fp);
+				if (rc)
+					goto out;
+				break;
+			}
+			}
+		}
+	}
+	rc = 0;
+out:
+	return rc;
+}
+
 /*
  * Read the configuration data from a policy database binary
  * representation file into a policy database structure.
@@ -1907,10 +2035,8 @@ int policydb_read(struct policydb *p, void *fp)
 {
 	struct role_allow *ra, *lra;
 	struct role_trans *tr, *ltr;
-	struct ocontext *l, *c;
 	int i, j, rc;
 	__le32 buf[4];
-	u32 nodebuf[8];
 	u32 len, nprim, nel;
 
 	char *policydb_str;
@@ -2115,115 +2241,9 @@ int policydb_read(struct policydb *p, void *fp)
 	if (!p->process_trans_perms)
 		goto bad;
 
-	for (i = 0; i < info->ocon_num; i++) {
-		rc = next_entry(buf, fp, sizeof(u32));
-		if (rc < 0)
-			goto bad;
-		nel = le32_to_cpu(buf[0]);
-		l = NULL;
-		for (j = 0; j < nel; j++) {
-			c = kzalloc(sizeof(*c), GFP_KERNEL);
-			if (!c) {
-				rc = -ENOMEM;
-				goto bad;
-			}
-			if (l)
-				l->next = c;
-			else
-				p->ocontexts[i] = c;
-			l = c;
-			rc = -EINVAL;
-			switch (i) {
-			case OCON_ISID:
-				rc = next_entry(buf, fp, sizeof(u32));
-				if (rc < 0)
-					goto bad;
-				c->sid[0] = le32_to_cpu(buf[0]);
-				rc = context_read_and_validate(&c->context[0], p, fp);
-				if (rc)
-					goto bad;
-				break;
-			case OCON_FS:
-			case OCON_NETIF:
-				rc = next_entry(buf, fp, sizeof(u32));
-				if (rc < 0)
-					goto bad;
-				len = le32_to_cpu(buf[0]);
-				c->u.name = kmalloc(len + 1, GFP_KERNEL);
-				if (!c->u.name) {
-					rc = -ENOMEM;
-					goto bad;
-				}
-				rc = next_entry(c->u.name, fp, len);
-				if (rc < 0)
-					goto bad;
-				c->u.name[len] = 0;
-				rc = context_read_and_validate(&c->context[0], p, fp);
-				if (rc)
-					goto bad;
-				rc = context_read_and_validate(&c->context[1], p, fp);
-				if (rc)
-					goto bad;
-				break;
-			case OCON_PORT:
-				rc = next_entry(buf, fp, sizeof(u32)*3);
-				if (rc < 0)
-					goto bad;
-				c->u.port.protocol = le32_to_cpu(buf[0]);
-				c->u.port.low_port = le32_to_cpu(buf[1]);
-				c->u.port.high_port = le32_to_cpu(buf[2]);
-				rc = context_read_and_validate(&c->context[0], p, fp);
-				if (rc)
-					goto bad;
-				break;
-			case OCON_NODE:
-				rc = next_entry(nodebuf, fp, sizeof(u32) * 2);
-				if (rc < 0)
-					goto bad;
-				c->u.node.addr = nodebuf[0]; /* network order */
-				c->u.node.mask = nodebuf[1]; /* network order */
-				rc = context_read_and_validate(&c->context[0], p, fp);
-				if (rc)
-					goto bad;
-				break;
-			case OCON_FSUSE:
-				rc = next_entry(buf, fp, sizeof(u32)*2);
-				if (rc < 0)
-					goto bad;
-				c->v.behavior = le32_to_cpu(buf[0]);
-				if (c->v.behavior > SECURITY_FS_USE_NONE)
-					goto bad;
-				len = le32_to_cpu(buf[1]);
-				c->u.name = kmalloc(len + 1, GFP_KERNEL);
-				if (!c->u.name) {
-					rc = -ENOMEM;
-					goto bad;
-				}
-				rc = next_entry(c->u.name, fp, len);
-				if (rc < 0)
-					goto bad;
-				c->u.name[len] = 0;
-				rc = context_read_and_validate(&c->context[0], p, fp);
-				if (rc)
-					goto bad;
-				break;
-			case OCON_NODE6: {
-				int k;
-
-				rc = next_entry(nodebuf, fp, sizeof(u32) * 8);
-				if (rc < 0)
-					goto bad;
-				for (k = 0; k < 4; k++)
-					c->u.node6.addr[k] = nodebuf[k];
-				for (k = 0; k < 4; k++)
-					c->u.node6.mask[k] = nodebuf[k+4];
-				if (context_read_and_validate(&c->context[0], p, fp))
-					goto bad;
-				break;
-			}
-			}
-		}
-	}
+	rc = ocontext_read(p, info, fp);
+	if (rc)
+		goto bad;
 
 	rc = genfs_read(p, fp);
 	if (rc)


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 4/4] SELinux: allow userspace to read policy back out of the kernel
  2010-06-11 16:37 [PATCH 1/4] SELinux: seperate range transition rules to a seperate function Eric Paris
  2010-06-11 16:37 ` [PATCH 2/4] SELinux: move genfs read to a separate function Eric Paris
  2010-06-11 16:37 ` [PATCH 3/4] SELinux: break ocontext reading into " Eric Paris
@ 2010-06-11 16:37 ` Eric Paris
  2010-06-14 14:48   ` Stephen Smalley
  2010-06-14 14:57   ` Stephen Smalley
  2010-06-16 13:02 ` [PATCH 1/4] SELinux: seperate range transition rules to a seperate function Stephen Smalley
  2010-06-17  5:02 ` James Morris
  4 siblings, 2 replies; 26+ messages in thread
From: Eric Paris @ 2010-06-11 16:37 UTC (permalink / raw)
  To: selinux; +Cc: sds, jmorris

There is interest in being able to see what the actual policy is that was
loaded into the kernel.  The patch creates a new selinuxfs file
/selinux/policy which can be read by userspace.  The actual policy that is
loaded into the kernel will be written back out to userspace.

Signed-off-by: Eric Paris <eparis@redhat.com>
---

 include/linux/kernel.h              |    1 
 security/selinux/include/security.h |    1 
 security/selinux/selinuxfs.c        |   46 ++
 security/selinux/ss/avtab.c         |   42 ++
 security/selinux/ss/avtab.h         |    2 
 security/selinux/ss/conditional.c   |  123 +++++
 security/selinux/ss/conditional.h   |    2 
 security/selinux/ss/ebitmap.c       |   83 +++
 security/selinux/ss/ebitmap.h       |    1 
 security/selinux/ss/policydb.c      |  838 +++++++++++++++++++++++++++++++++++
 security/selinux/ss/policydb.h      |   22 +
 security/selinux/ss/services.c      |   25 +
 12 files changed, 1183 insertions(+), 3 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 8317ec4..235f39f 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -59,6 +59,7 @@ extern const char linux_proc_banner[];
 #define FIELD_SIZEOF(t, f) (sizeof(((t*)0)->f))
 #define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
 #define roundup(x, y) ((((x) + ((y) - 1)) / (y)) * (y))
+#define rounddown(x, y) ((x) - ((x) % (y)))
 #define DIV_ROUND_CLOSEST(x, divisor)(			\
 {							\
 	typeof(divisor) __divisor = divisor;		\
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index 1f7c249..9ff9a39 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -82,6 +82,7 @@ extern int selinux_policycap_openperm;
 int security_mls_enabled(void);
 
 int security_load_policy(void *data, size_t len);
+int security_read_policy(void *data, ssize_t *len);
 
 int security_policycap_supported(unsigned int req_cap);
 
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index 0293843..5706246 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -110,6 +110,7 @@ enum sel_inos {
 	SEL_COMPAT_NET,	/* whether to use old compat network packet controls */
 	SEL_REJECT_UNKNOWN, /* export unknown reject handling to userspace */
 	SEL_DENY_UNKNOWN, /* export unknown deny handling to userspace */
+	SEL_READ_POLICY, /* read the policy back out of the kernel */
 	SEL_INO_NEXT,	/* The next inode number to use */
 };
 
@@ -358,6 +359,50 @@ static const struct file_operations sel_load_ops = {
 	.write		= sel_write_load,
 };
 
+static ssize_t sel_read_policy(struct file *filp, char __user *buf,
+			       size_t count, loff_t *ppos)
+{
+	int ret;
+	ssize_t length;
+	void *data = NULL;
+
+	mutex_lock(&sel_mutex);
+
+	ret = task_has_security(current, SECURITY__LOAD_POLICY);
+	if (ret)
+		goto out;
+
+	/* No partial reads. */
+	ret = -EINVAL;
+	if (*ppos != 0)
+		goto out;
+
+	length = min(count, (size_t)64 * 1024 * 1024);
+
+	ret = -ENOMEM;
+	data = vmalloc(length);
+	if (!data)
+		goto out;
+
+	ret = security_read_policy(data, &length);
+	if (ret)
+		goto out;
+
+	ret = -EFAULT;
+	if (copy_to_user(buf, data, length))
+		goto out;
+
+	ret = length;
+out:
+	mutex_unlock(&sel_mutex);
+	vfree(data);
+	return ret;
+}
+
+static const struct file_operations sel_policy_ops = {
+	.read		= sel_read_policy,
+};
+
 static ssize_t sel_write_context(struct file *file, char *buf, size_t size)
 {
 	char *canon;
@@ -1596,6 +1641,7 @@ static int sel_fill_super(struct super_block *sb, void *data, int silent)
 		[SEL_CHECKREQPROT] = {"checkreqprot", &sel_checkreqprot_ops, S_IRUGO|S_IWUSR},
 		[SEL_REJECT_UNKNOWN] = {"reject_unknown", &sel_handle_unknown_ops, S_IRUGO},
 		[SEL_DENY_UNKNOWN] = {"deny_unknown", &sel_handle_unknown_ops, S_IRUGO},
+		[SEL_READ_POLICY] = {"policy", &sel_policy_ops, S_IRUSR},
 		/* last one */ {""}
 	};
 	ret = simple_fill_super(sb, SELINUX_MAGIC, selinux_files);
diff --git a/security/selinux/ss/avtab.c b/security/selinux/ss/avtab.c
index 1215b8e..9d9680a 100644
--- a/security/selinux/ss/avtab.c
+++ b/security/selinux/ss/avtab.c
@@ -502,6 +502,48 @@ bad:
 	goto out;
 }
 
+int avtab_write_item(struct policydb *p, struct avtab_node *cur, void *fp)
+{
+	__le16 buf16[4];
+	__le32 buf32[1];
+	int rc;
+
+	buf16[0] = cpu_to_le16(cur->key.source_type);
+	buf16[1] = cpu_to_le16(cur->key.target_type);
+	buf16[2] = cpu_to_le16(cur->key.target_class);
+	buf16[3] = cpu_to_le16(cur->key.specified);
+	rc = put_entry(buf16, sizeof(u16), 4, fp);
+	if (rc)
+		return rc;
+	buf32[0] = cpu_to_le32(cur->datum.data);
+	rc = put_entry(buf32, sizeof(u32), 1, fp);
+	if (rc)
+		return rc;
+	return 0;
+}
+
+int avtab_write(struct policydb *p, struct avtab *a, void *fp)
+{
+	unsigned int i;
+	int rc = 0;
+	struct avtab_node *cur;
+	__le32 buf[1];
+
+	buf[0] = cpu_to_le32(a->nel);
+	rc = put_entry(buf, sizeof(u32), 1, fp);
+	if (rc)
+		return rc;
+
+	for (i = 0; i < a->nslot; i++) {
+		for (cur = a->htable[i]; cur; cur = cur->next) {
+			rc = avtab_write_item(p, cur, fp);
+			if (rc)
+				return rc;
+		}
+	}
+
+	return rc;
+}
 void avtab_cache_init(void)
 {
 	avtab_node_cachep = kmem_cache_create("avtab_node",
diff --git a/security/selinux/ss/avtab.h b/security/selinux/ss/avtab.h
index cd4f734..de1f8b6 100644
--- a/security/selinux/ss/avtab.h
+++ b/security/selinux/ss/avtab.h
@@ -71,6 +71,8 @@ int avtab_read_item(struct avtab *a, void *fp, struct policydb *pol,
 		    void *p);
 
 int avtab_read(struct avtab *a, void *fp, struct policydb *pol);
+int avtab_write_item(struct policydb *p, struct avtab_node *cur, void *fp);
+int avtab_write(struct policydb *p, struct avtab *a, void *fp);
 
 struct avtab_node *avtab_insert_nonunique(struct avtab *h, struct avtab_key *key,
 					  struct avtab_datum *datum);
diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c
index 4a4e35c..b88f295 100644
--- a/security/selinux/ss/conditional.c
+++ b/security/selinux/ss/conditional.c
@@ -475,6 +475,129 @@ err:
 	return -1;
 }
 
+int cond_write_bool(void *vkey, void *datum, void *ptr)
+{
+	char *key = vkey;
+	struct cond_bool_datum *booldatum = datum;
+	struct policy_data *pd = ptr;
+	void *fp = pd->fp;
+	__le32 buf[3];
+	u32 len;
+	int rc;
+
+	len = strlen(key);
+	buf[0] = cpu_to_le32(booldatum->value);
+	buf[1] = cpu_to_le32(booldatum->state);
+	buf[2] = cpu_to_le32(len);
+	rc = put_entry(buf, sizeof(u32), 3, fp);
+	if (rc)
+		return rc;
+	rc = put_entry(key, 1, len, fp);
+	if (rc)
+		return rc;
+	return 0;
+}
+
+/*
+ * cond_write_cond_av_list doesn't write out the av_list nodes.
+ * Instead it writes out the key/value pairs from the avtab. This
+ * is necessary because there is no way to uniquely identifying rules
+ * in the avtab so it is not possible to associate individual rules
+ * in the avtab with a conditional without saving them as part of
+ * the conditional. This means that the avtab with the conditional
+ * rules will not be saved but will be rebuilt on policy load.
+ */
+static int cond_write_av_list(struct policydb *p,
+			      struct cond_av_list *list, struct policy_file *fp)
+{
+	__le32 buf[1];
+	struct cond_av_list *cur_list;
+	u32 len;
+	int rc;
+
+	len = 0;
+	for (cur_list = list; cur_list != NULL; cur_list = cur_list->next)
+		len++;
+
+	buf[0] = cpu_to_le32(len);
+	rc = put_entry(buf, sizeof(u32), 1, fp);
+	if (rc)
+		return rc;
+
+	if (len == 0)
+		return 0;
+
+	for (cur_list = list; cur_list != NULL; cur_list = cur_list->next) {
+		rc = avtab_write_item(p, cur_list->node, fp);
+		if (rc)
+			return rc;
+	}
+
+	return 0;
+}
+
+int cond_write_node(struct policydb *p, struct cond_node *node,
+		    struct policy_file *fp)
+{
+	struct cond_expr *cur_expr;
+	__le32 buf[2];
+	int rc;
+	u32 len = 0;
+
+	buf[0] = cpu_to_le32(node->cur_state);
+	rc = put_entry(buf, sizeof(u32), 1, fp);
+	if (rc)
+		return rc;
+
+	for (cur_expr = node->expr; cur_expr != NULL; cur_expr = cur_expr->next)
+		len++;
+
+	buf[0] = cpu_to_le32(len);
+	rc = put_entry(buf, sizeof(u32), 1, fp);
+	if (rc)
+		return rc;
+
+	for (cur_expr = node->expr; cur_expr != NULL; cur_expr = cur_expr->next) {
+		buf[0] = cpu_to_le32(cur_expr->expr_type);
+		buf[1] = cpu_to_le32(cur_expr->bool);
+		rc = put_entry(buf, sizeof(u32), 2, fp);
+		if (rc)
+			return rc;
+	}
+
+	rc = cond_write_av_list(p, node->true_list, fp);
+	if (rc)
+		return rc;
+	rc = cond_write_av_list(p, node->false_list, fp);
+	if (rc)
+		return rc;
+
+	return 0;
+}
+
+int cond_write_list(struct policydb *p, struct cond_node *list, void *fp)
+{
+	struct cond_node *cur;
+	u32 len;
+	__le32 buf[1];
+	int rc;
+
+	len = 0;
+	for (cur = list; cur != NULL; cur = cur->next)
+		len++;
+	buf[0] = cpu_to_le32(len);
+	rc = put_entry(buf, sizeof(u32), 1, fp);
+	if (rc)
+		return rc;
+
+	for (cur = list; cur != NULL; cur = cur->next) {
+		rc = cond_write_node(p, cur, fp);
+		if (rc)
+			return rc;
+	}
+
+	return 0;
+}
 /* Determine whether additional permissions are granted by the conditional
  * av table, and if so, add them to the result
  */
diff --git a/security/selinux/ss/conditional.h b/security/selinux/ss/conditional.h
index 53ddb01..3f209c6 100644
--- a/security/selinux/ss/conditional.h
+++ b/security/selinux/ss/conditional.h
@@ -69,6 +69,8 @@ int cond_index_bool(void *key, void *datum, void *datap);
 
 int cond_read_bool(struct policydb *p, struct hashtab *h, void *fp);
 int cond_read_list(struct policydb *p, void *fp);
+int cond_write_bool(void *key, void *datum, void *ptr);
+int cond_write_list(struct policydb *p, struct cond_node *list, void *fp);
 
 void cond_compute_av(struct avtab *ctab, struct avtab_key *key, struct av_decision *avd);
 
diff --git a/security/selinux/ss/ebitmap.c b/security/selinux/ss/ebitmap.c
index 04b6145..01ff02a 100644
--- a/security/selinux/ss/ebitmap.c
+++ b/security/selinux/ss/ebitmap.c
@@ -22,6 +22,8 @@
 #include "ebitmap.h"
 #include "policydb.h"
 
+#define BITS_PER_U64	(sizeof(u64) * 8)
+
 int ebitmap_cmp(struct ebitmap *e1, struct ebitmap *e2)
 {
 	struct ebitmap_node *n1, *n2;
@@ -363,10 +365,11 @@ int ebitmap_read(struct ebitmap *e, void *fp)
 	e->highbit = le32_to_cpu(buf[1]);
 	count = le32_to_cpu(buf[2]);
 
-	if (mapunit != sizeof(u64) * 8) {
+	if (mapunit != BITS_PER_U64) {
 		printk(KERN_ERR "SELinux: ebitmap: map size %u does not "
 		       "match my size %Zd (high bit was %d)\n",
-		       mapunit, sizeof(u64) * 8, e->highbit);
+		       mapunit, BITS_PER_U64, e->highbit);
+		dump_stack();
 		goto bad;
 	}
 
@@ -446,3 +449,79 @@ bad:
 	ebitmap_destroy(e);
 	goto out;
 }
+
+int ebitmap_write(struct ebitmap *e, void *fp)
+{
+
+	struct ebitmap_node *n;
+	u32 count;
+	__le32 buf[3];
+	u64 map;
+	int bit, last_bit, last_startbit, rc;
+
+	buf[0] = cpu_to_le32(BITS_PER_U64);
+
+	count = 0;
+	last_bit = 0;
+	last_startbit = -1;
+	ebitmap_for_each_positive_bit(e, n, bit) {
+		if (rounddown(bit, (int)BITS_PER_U64) > last_startbit) {
+			count++;
+			last_startbit = rounddown(bit, BITS_PER_U64);
+		}
+		last_bit = roundup(bit + 1, BITS_PER_U64);
+	}
+	buf[1] = cpu_to_le32(last_bit);
+	buf[2] = cpu_to_le32(count);
+
+	rc = put_entry(buf, sizeof(u32), 3, fp);
+	if (rc)
+		return rc;
+
+	map = 0;
+	last_startbit = INT_MIN;
+	ebitmap_for_each_positive_bit(e, n, bit) {
+		if (rounddown(bit, (int)BITS_PER_U64) > last_startbit) {
+			__le64 buf64[1];
+
+			/* this is the very first bit */
+			if (!map) {
+				last_startbit = rounddown(bit, BITS_PER_U64);
+				map = (u64)1 << (bit - last_startbit);
+				continue;
+			}
+
+			/* write the last node */
+			buf[0] = cpu_to_le32(last_startbit);
+			rc = put_entry(buf, sizeof(u32), 1, fp);
+			if (rc)
+				return rc;
+
+			buf64[0] = cpu_to_le64(map);
+			rc = put_entry(buf64, sizeof(u64), 1, fp);
+			if (rc)
+				return rc;
+
+			/* set up for the next node */
+			map = 0;
+			last_startbit = rounddown(bit, BITS_PER_U64);
+		}
+		map |= (u64)1 << (bit - last_startbit);
+	}
+	/* write the last node */
+	if (map) {
+		__le64 buf64[1];
+
+		/* write the last node */
+		buf[0] = cpu_to_le32(last_startbit);
+		rc = put_entry(buf, sizeof(u32), 1, fp);
+		if (rc)
+			return rc;
+
+		buf64[0] = cpu_to_le64(map);
+		rc = put_entry(buf64, sizeof(u64), 1, fp);
+		if (rc)
+			return rc;
+	}
+	return 0;
+}
diff --git a/security/selinux/ss/ebitmap.h b/security/selinux/ss/ebitmap.h
index f283b43..1f4e93c 100644
--- a/security/selinux/ss/ebitmap.h
+++ b/security/selinux/ss/ebitmap.h
@@ -123,6 +123,7 @@ int ebitmap_get_bit(struct ebitmap *e, unsigned long bit);
 int ebitmap_set_bit(struct ebitmap *e, unsigned long bit, int value);
 void ebitmap_destroy(struct ebitmap *e);
 int ebitmap_read(struct ebitmap *e, void *fp);
+int ebitmap_write(struct ebitmap *e, void *fp);
 
 #ifdef CONFIG_NETLABEL
 int ebitmap_netlbl_export(struct ebitmap *ebmap,
diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index 99fff0c..0f28819 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -36,6 +36,7 @@
 #include "policydb.h"
 #include "conditional.h"
 #include "mls.h"
+#include "services.h"
 
 #define _DEBUG_HASHES
 
@@ -2281,3 +2282,840 @@ bad:
 	policydb_destroy(p);
 	goto out;
 }
+
+/*
+ * Write a MLS level structure to a policydb binary
+ * representation file.
+ */
+static int mls_write_level(struct mls_level *l, void *fp)
+{
+	__le32 buf[1];
+	int rc;
+
+	buf[1] = cpu_to_le32(l->sens);
+	rc = put_entry(buf, sizeof(u32), 1, fp);
+	if (rc)
+		return rc;
+
+	rc = ebitmap_write(&l->cat, fp);
+	if (rc)
+		return rc;
+
+	return 0;
+}
+
+/*
+ * Write a MLS range structure to a policydb binary
+ * representation file.
+ */
+static int mls_write_range_helper(struct mls_range *r, void *fp)
+{
+	__le32 buf[3];
+	size_t items;
+	int rc, eq;
+
+	eq = mls_level_eq(&r->level[1], &r->level[0]);
+
+	if (eq)
+		items = 2;
+	else
+		items = 3;
+	buf[0] = cpu_to_le32(items-1);
+	buf[1] = cpu_to_le32(r->level[0].sens);
+	if (!eq)
+		buf[2] = cpu_to_le32(r->level[1].sens);
+
+	BUG_ON(items > (sizeof(buf)/sizeof(buf[0])));
+
+	rc = put_entry(buf, sizeof(u32), items, fp);
+	if (rc)
+		return rc;
+
+	rc = ebitmap_write(&r->level[0].cat, fp);
+	if (rc)
+		return rc;
+	if (!eq) {
+		rc = ebitmap_write(&r->level[1].cat, fp);
+		if (rc)
+			return rc;
+	}
+
+	return 0;
+}
+
+static int sens_write(void *vkey, void *datum, void *ptr)
+{
+	char *key = vkey;
+	struct level_datum *levdatum = datum;
+	struct policy_data *pd = ptr;
+	void *fp = pd->fp;
+	__le32 buf[2];
+	size_t len;
+	int rc;
+
+	len = strlen(key);
+	buf[0] = cpu_to_le32(len);
+	buf[1] = cpu_to_le32(levdatum->isalias);
+	rc = put_entry(buf, sizeof(u32), 2, fp);
+	if (rc)
+		return rc;
+
+	rc = put_entry(key, 1, len, fp);
+	if (rc)
+		return rc;
+
+	rc = mls_write_level(levdatum->level, fp);
+	if (rc)
+		return rc;
+
+	return 0;
+}
+
+static int cat_write(void *vkey, void *datum, void *ptr)
+{
+	char *key = vkey;
+	struct cat_datum *catdatum = datum;
+	struct policy_data *pd = ptr;
+	void *fp = pd->fp;
+	__le32 buf[3];
+	size_t len;
+	int rc;
+
+	len = strlen(key);
+	buf[0] = cpu_to_le32(len);
+	buf[1] = cpu_to_le32(catdatum->value);
+	buf[2] = cpu_to_le32(catdatum->isalias);
+	rc = put_entry(buf, sizeof(u32), 3, fp);
+	if (rc)
+		return rc;
+
+	rc = put_entry(key, 1, len, fp);
+	if (rc)
+		return rc;
+
+	return 0;
+}
+
+static int role_trans_write(struct role_trans *r, void *fp)
+{
+	struct role_trans *tr;
+	u32 buf[3];
+	size_t nel;
+	int rc;
+
+	nel = 0;
+	for (tr = r; tr; tr = tr->next)
+		nel++;
+	buf[0] = cpu_to_le32(nel);
+	rc = put_entry(buf, sizeof(u32), 1, fp);
+	if (rc)
+		return rc;
+	for (tr = r; tr; tr = tr->next) {
+		buf[0] = cpu_to_le32(tr->role);
+		buf[1] = cpu_to_le32(tr->type);
+		buf[2] = cpu_to_le32(tr->new_role);
+		rc = put_entry(buf, sizeof(u32), 3, fp);
+		if (rc)
+			return rc;
+	}
+
+	return 0;
+}
+
+static int role_allow_write(struct role_allow *r, void *fp)
+{
+	struct role_allow *ra;
+	u32 buf[2];
+	size_t nel;
+	int rc;
+
+	nel = 0;
+	for (ra = r; ra; ra = ra->next)
+		nel++;
+	buf[0] = cpu_to_le32(nel);
+	rc = put_entry(buf, sizeof(u32), 1, fp);
+	if (rc)
+		return rc;
+	for (ra = r; ra; ra = ra->next) {
+		buf[0] = cpu_to_le32(ra->role);
+		buf[1] = cpu_to_le32(ra->new_role);
+		rc = put_entry(buf, sizeof(u32), 2, fp);
+		if (rc)
+			return rc;
+	}
+	return 0;
+}
+
+/*
+ * Write a security context structure
+ * to a policydb binary representation file.
+ */
+static int context_write(struct policydb *p, struct context *c,
+			 void *fp)
+{
+	int rc;
+	__le32 buf[3];
+
+	buf[0] = cpu_to_le32(c->user);
+	buf[1] = cpu_to_le32(c->role);
+	buf[2] = cpu_to_le32(c->type);
+
+	rc = put_entry(buf, sizeof(u32), 3, fp);
+	if (rc)
+		return rc;
+
+	rc = mls_write_range_helper(&c->range, fp);
+	if (rc)
+		return rc;
+
+	return 0;
+}
+
+/*
+ * The following *_write functions are used to
+ * write the symbol data to a policy database
+ * binary representation file.
+ */
+
+static int perm_write(void *vkey, void *datum, void *fp)
+{
+	char *key = vkey;
+	struct perm_datum *perdatum = datum;
+	__le32 buf[2];
+	size_t len;
+	int rc;
+
+	len = strlen(key);
+	buf[0] = cpu_to_le32(len);
+	buf[1] = cpu_to_le32(perdatum->value);
+	rc = put_entry(buf, sizeof(u32), 2, fp);
+	if (rc)
+		return rc;
+
+	rc = put_entry(key, 1, len, fp);
+	if (rc)
+		return rc;
+
+	return 0;
+}
+
+static int common_write(void *vkey, void *datum, void *ptr)
+{
+	char *key = vkey;
+	struct common_datum *comdatum = datum;
+	struct policy_data *pd = ptr;
+	void *fp = pd->fp;
+	__le32 buf[4];
+	size_t len;
+	int rc;
+
+	len = strlen(key);
+	buf[0] = cpu_to_le32(len);
+	buf[1] = cpu_to_le32(comdatum->value);
+	buf[2] = cpu_to_le32(comdatum->permissions.nprim);
+	buf[3] = cpu_to_le32(comdatum->permissions.table->nel);
+	rc = put_entry(buf, sizeof(u32), 4, fp);
+	if (rc)
+		return rc;
+
+	rc = put_entry(key, 1, len, fp);
+	if (rc)
+		return rc;
+
+	rc = hashtab_map(comdatum->permissions.table, perm_write, fp);
+	if (rc)
+		return rc;
+
+	return 0;
+}
+
+static int write_cons_helper(struct policydb *p, struct constraint_node *node,
+			     void *fp)
+{
+	struct constraint_node *c;
+	struct constraint_expr *e;
+	__le32 buf[3];
+	u32 nel;
+	int rc;
+
+	for (c = node; c; c = c->next) {
+		nel = 0;
+		for (e = c->expr; e; e = e->next)
+			nel++;
+		buf[0] = cpu_to_le32(c->permissions);
+		buf[1] = cpu_to_le32(nel);
+		rc = put_entry(buf, sizeof(u32), 2, fp);
+		if (rc)
+			return rc;
+		for (e = c->expr; e; e = e->next) {
+			buf[0] = cpu_to_le32(e->expr_type);
+			buf[1] = cpu_to_le32(e->attr);
+			buf[2] = cpu_to_le32(e->op);
+			rc = put_entry(buf, sizeof(u32), 3, fp);
+			if (rc)
+				return rc;
+
+			switch (e->expr_type) {
+			case CEXPR_NAMES:
+				rc = ebitmap_write(&e->names, fp);
+				if (rc)
+					return rc;
+				break;
+			default:
+				break;
+			}
+		}
+	}
+
+	return 0;
+}
+
+static int class_write(void *vkey, void *datum, void *ptr)
+{
+	char *key = vkey;
+	struct class_datum *cladatum = datum;
+	struct policy_data *pd = ptr;
+	void *fp = pd->fp;
+	struct policydb *p = pd->p;
+	struct constraint_node *c;
+	__le32 buf[6];
+	u32 ncons;
+	size_t len, len2;
+	int rc;
+
+	len = strlen(key);
+	if (cladatum->comkey)
+		len2 = strlen(cladatum->comkey);
+	else
+		len2 = 0;
+
+	ncons = 0;
+	for (c = cladatum->constraints; c; c = c->next)
+		ncons++;
+
+	buf[0] = cpu_to_le32(len);
+	buf[1] = cpu_to_le32(len2);
+	buf[2] = cpu_to_le32(cladatum->value);
+	buf[3] = cpu_to_le32(cladatum->permissions.nprim);
+	if (cladatum->permissions.table)
+		buf[4] = cpu_to_le32(cladatum->permissions.table->nel);
+	else
+		buf[4] = 0;
+	buf[5] = cpu_to_le32(ncons);
+	rc = put_entry(buf, sizeof(u32), 6, fp);
+	if (rc)
+		return rc;
+
+	rc = put_entry(key, 1, len, fp);
+	if (rc)
+		return rc;
+
+	if (cladatum->comkey) {
+		rc = put_entry(cladatum->comkey, 1, len2, fp);
+		if (rc)
+			return rc;
+	}
+
+	rc = hashtab_map(cladatum->permissions.table, perm_write, fp);
+	if (rc)
+		return rc;
+
+	rc = write_cons_helper(p, cladatum->constraints, fp);
+	if (rc)
+		return rc;
+
+	/* write out the validatetrans rule */
+	ncons = 0;
+	for (c = cladatum->validatetrans; c; c = c->next)
+		ncons++;
+
+	buf[0] = cpu_to_le32(ncons);
+	rc = put_entry(buf, sizeof(u32), 1, fp);
+	if (rc)
+		return rc;
+
+	rc = write_cons_helper(p, cladatum->validatetrans, fp);
+	if (rc)
+		return rc;
+
+	return 0;
+}
+
+static int role_write(void *vkey, void *datum, void *ptr)
+{
+	char *key = vkey;
+	struct role_datum *role = datum;
+	struct policy_data *pd = ptr;
+	void *fp = pd->fp;
+	struct policydb *p = pd->p;
+	__le32 buf[3];
+	size_t items, len;
+	int rc;
+
+	len = strlen(key);
+	items = 0;
+	buf[items++] = cpu_to_le32(len);
+	buf[items++] = cpu_to_le32(role->value);
+	if (p->policyvers >= POLICYDB_VERSION_BOUNDARY)
+		buf[items++] = cpu_to_le32(role->bounds);
+
+	BUG_ON(items > (sizeof(buf)/sizeof(buf[0])));
+
+	rc = put_entry(buf, sizeof(u32), items, fp);
+	if (rc)
+		return rc;
+
+	rc = put_entry(key, 1, len, fp);
+	if (rc)
+		return rc;
+
+	rc = ebitmap_write(&role->dominates, fp);
+	if (rc)
+		return rc;
+
+	rc = ebitmap_write(&role->types, fp);
+	if (rc)
+		return rc;
+
+	return 0;
+}
+
+static int type_write(void *vkey, void *datum, void *ptr)
+{
+	char *key = vkey;
+	struct type_datum *typdatum = datum;
+	struct policy_data *pd = ptr;
+	struct policydb *p = pd->p;
+	void *fp = pd->fp;
+	__le32 buf[4];
+	int rc;
+	size_t items, len;
+
+	len = strlen(key);
+	items = 0;
+	buf[items++] = cpu_to_le32(len);
+	buf[items++] = cpu_to_le32(typdatum->value);
+	if (p->policyvers >= POLICYDB_VERSION_BOUNDARY) {
+		u32 properties = 0;
+
+		if (typdatum->primary)
+			properties |= TYPEDATUM_PROPERTY_PRIMARY;
+
+		if (typdatum->attribute)
+			properties |= TYPEDATUM_PROPERTY_ATTRIBUTE;
+
+		buf[items++] = cpu_to_le32(properties);
+		buf[items++] = cpu_to_le32(typdatum->bounds);
+	} else {
+		buf[items++] = cpu_to_le32(typdatum->primary);
+	}
+	BUG_ON(items > (sizeof(buf) / sizeof(buf[0])));
+	rc = put_entry(buf, sizeof(u32), items, fp);
+	if (rc)
+		return rc;
+
+	rc = put_entry(key, 1, len, fp);
+	if (rc)
+		return rc;
+
+	return 0;
+}
+
+static int user_write(void *vkey, void *datum, void *ptr)
+{
+	char *key = vkey;
+	struct user_datum *usrdatum = datum;
+	struct policy_data *pd = ptr;
+	struct policydb *p = pd->p;
+	void *fp = pd->fp;
+	__le32 buf[3];
+	size_t items, len;
+	int rc;
+
+	len = strlen(key);
+	items = 0;
+	buf[items++] = cpu_to_le32(len);
+	buf[items++] = cpu_to_le32(usrdatum->value);
+	if (p->policyvers >= POLICYDB_VERSION_BOUNDARY)
+		buf[items++] = cpu_to_le32(usrdatum->bounds);
+	BUG_ON(items > (sizeof(buf) / sizeof(buf[0])));
+	rc = put_entry(buf, sizeof(u32), items, fp);
+	if (rc)
+		return rc;
+
+	rc = put_entry(key, 1, len, fp);
+	if (rc)
+		return rc;
+
+	rc = ebitmap_write(&usrdatum->roles, fp);
+	if (rc)
+		return rc;
+
+	rc = mls_write_range_helper(&usrdatum->range, fp);
+	if (rc)
+		return rc;
+
+	rc = mls_write_level(&usrdatum->dfltlevel, fp);
+	if (rc)
+		return rc;
+
+	return 0;
+}
+
+static int (*write_f[SYM_NUM]) (void *key, void *datum,
+				void *datap) =
+{
+	common_write,
+	class_write,
+	role_write,
+	type_write,
+	user_write,
+	cond_write_bool,
+	sens_write,
+	cat_write,
+};
+
+static int ocontext_write(struct policydb *p, struct policydb_compat_info *info,
+			  void *fp)
+{
+	unsigned int i, j, rc;
+	size_t nel, len;
+	__le32 buf[3];
+	u32 nodebuf[8];
+	struct ocontext *c;
+	for (i = 0; i < info->ocon_num; i++) {
+		nel = 0;
+		for (c = p->ocontexts[i]; c; c = c->next)
+			nel++;
+		buf[0] = cpu_to_le32(nel);
+		rc = put_entry(buf, sizeof(u32), 1, fp);
+		if (rc)
+			return rc;
+		for (c = p->ocontexts[i]; c; c = c->next) {
+			switch (i) {
+			case OCON_ISID:
+				buf[0] = cpu_to_le32(c->sid[0]);
+				rc = put_entry(buf, sizeof(u32), 1, fp);
+				if (rc)
+					return rc;
+				rc = context_write(p, &c->context[0], fp);
+				if (rc)
+					return rc;
+				break;
+			case OCON_FS:
+			case OCON_NETIF:
+				len = strlen(c->u.name);
+				buf[0] = cpu_to_le32(len);
+				rc = put_entry(buf, sizeof(u32), 1, fp);
+				if (rc)
+					return rc;
+				rc = put_entry(c->u.name, 1, len, fp);
+				if (rc)
+					return rc;
+				rc = context_write(p, &c->context[0], fp);
+				if (rc)
+					return rc;
+				rc = context_write(p, &c->context[1], fp);
+				if (rc)
+					return rc;
+				break;
+			case OCON_PORT:
+				buf[0] = cpu_to_le32(c->u.port.protocol);
+				buf[1] = cpu_to_le32(c->u.port.low_port);
+				buf[2] = cpu_to_le32(c->u.port.high_port);
+				rc = put_entry(buf, sizeof(u32), 3, fp);
+				if (rc)
+					return rc;
+				rc = context_write(p, &c->context[0], fp);
+				if (rc)
+					return rc;
+				break;
+			case OCON_NODE:
+				nodebuf[0] = c->u.node.addr; /* network order */
+				nodebuf[1] = c->u.node.mask; /* network order */
+				rc = put_entry(nodebuf, sizeof(u32), 2, fp);
+				if (rc)
+					return rc;
+				rc = context_write(p, &c->context[0], fp);
+				if (rc)
+					return rc;
+				break;
+			case OCON_FSUSE:
+				buf[0] = cpu_to_le32(c->v.behavior);
+				len = strlen(c->u.name);
+				buf[1] = cpu_to_le32(len);
+				rc = put_entry(buf, sizeof(u32), 2, fp);
+				if (rc)
+					return rc;
+				rc = put_entry(c->u.name, 1, len, fp);
+				if (rc)
+					return rc;
+				rc = context_write(p, &c->context[0], fp);
+				if (rc)
+					return rc;
+				break;
+			case OCON_NODE6:
+				for (j = 0; j < 4; j++)
+					nodebuf[j] = c->u.node6.addr[j]; /* network order */
+				for (j = 0; j < 4; j++)
+					nodebuf[j + 4] = c->u.node6.mask[j]; /* network order */
+				rc = put_entry(nodebuf, sizeof(u32), 8, fp);
+				if (rc)
+					return rc;
+				rc = context_write(p, &c->context[0], fp);
+				if (rc)
+					return rc;
+				break;
+			}
+		}
+	}
+	return 0;
+}
+
+static int genfs_write(struct policydb *p, void *fp)
+{
+	struct genfs *genfs;
+	struct ocontext *c;
+	size_t len;
+	__le32 buf[1];
+	int rc;
+
+	len = 0;
+	for (genfs = p->genfs; genfs; genfs = genfs->next)
+		len++;
+	buf[0] = cpu_to_le32(len);
+	rc = put_entry(buf, sizeof(u32), 1, fp);
+	if (rc)
+		return rc;
+	for (genfs = p->genfs; genfs; genfs = genfs->next) {
+		len = strlen(genfs->fstype);
+		buf[0] = cpu_to_le32(len);
+		rc = put_entry(buf, sizeof(u32), 1, fp);
+		if (rc)
+			return rc;
+		rc = put_entry(genfs->fstype, 1, len, fp);
+		if (rc)
+			return rc;
+		len = 0;
+		for (c = genfs->head; c; c = c->next)
+			len++;
+		buf[0] = cpu_to_le32(len);
+		rc = put_entry(buf, sizeof(u32), 1, fp);
+		if (rc)
+			return rc;
+		for (c = genfs->head; c; c = c->next) {
+			len = strlen(c->u.name);
+			buf[0] = cpu_to_le32(len);
+			rc = put_entry(buf, sizeof(u32), 1, fp);
+			if (rc)
+				return rc;
+			rc = put_entry(c->u.name, 1, len, fp);
+			if (rc)
+				return rc;
+			buf[0] = cpu_to_le32(c->v.sclass);
+			rc = put_entry(buf, sizeof(u32), 1, fp);
+			if (rc)
+				return rc;
+			rc = context_write(p, &c->context[0], fp);
+			if (rc)
+				return rc;
+		}
+	}
+	return 0;
+}
+
+static int range_count(void *key, void *data, void *ptr)
+{
+	int *cnt = ptr;
+	*cnt = *cnt + 1;
+
+	return 0;
+}
+
+static int range_write_helper(void *key, void *data, void *ptr)
+{
+	__le32 buf[2];
+	struct range_trans *rt = key;
+	struct mls_range *r = data;
+	struct policy_data *pd = ptr;
+	void *fp = pd->fp;
+	struct policydb *p = pd->p;
+	int rc;
+
+	buf[0] = cpu_to_le32(rt->source_type);
+	buf[1] = cpu_to_le32(rt->target_type);
+	rc = put_entry(buf, sizeof(u32), 2, fp);
+	if (rc)
+		return rc;
+	if (p->policyvers >= POLICYDB_VERSION_RANGETRANS) {
+		buf[0] = cpu_to_le32(rt->target_class);
+		rc = put_entry(buf, sizeof(u32), 1, fp);
+		if (rc)
+			return rc;
+	}
+	rc = mls_write_range_helper(r, fp);
+	if (rc)
+		return rc;
+
+	return 0;
+}
+
+static int range_write(struct policydb *p, void *fp)
+{
+	size_t nel;
+	__le32 buf[1];
+	int rc;
+	struct policy_data pd;
+
+	pd.p = p;
+	pd.fp = fp;
+
+	/* count the number of entries in the hashtab */
+	nel = 0;
+	rc = hashtab_map(p->range_tr, range_count, &nel);
+	if (rc)
+		return rc;
+
+	buf[0] = cpu_to_le32(nel);
+	rc = put_entry(buf, sizeof(u32), 1, fp);
+	if (rc)
+		return rc;
+
+	/* actually write all of the entries */
+	rc = hashtab_map(p->range_tr, range_write_helper, &pd);
+	if (rc)
+		return rc;
+
+	return 0;
+}
+
+/*
+ * Write the configuration data in a policy database
+ * structure to a policy database binary representation
+ * file.
+ */
+int policydb_write(struct policydb *p, void *fp)
+{
+	unsigned int i, num_syms;
+	int rc;
+	__le32 buf[4];
+	u32 config;
+	size_t len;
+	struct policydb_compat_info *info;
+
+	/*
+	 * refuse to write policy older than compressed avtab
+	 * to simplify the writer.  There are other tests dropped
+	 * since we assume this throughout the writer code.  Be
+	 * careful if you ever try to remove this restriction
+	 */
+	if (p->policyvers < POLICYDB_VERSION_AVTAB) {
+		printk(KERN_ERR "SELinux: refusing to write policy version %d."
+		       "  Because it is less than version %d\n", p->policyvers,
+		       POLICYDB_VERSION_AVTAB);
+		return -EINVAL;
+	}
+
+	config = 0;
+	if (p->mls_enabled)
+		config |= POLICYDB_CONFIG_MLS;
+
+	if (p->reject_unknown)
+		config |= REJECT_UNKNOWN;
+	if (p->allow_unknown)
+		config |= ALLOW_UNKNOWN;
+
+	/* Write the magic number and string identifiers. */
+	buf[0] = cpu_to_le32(POLICYDB_MAGIC);
+	len = strlen(POLICYDB_STRING);
+	buf[1] = cpu_to_le32(len);
+	rc = put_entry(buf, sizeof(u32), 2, fp);
+	if (rc)
+		return rc;
+	rc = put_entry(POLICYDB_STRING, 1, len, fp);
+	if (rc)
+		return rc;
+
+	/* Write the version, config, and table sizes. */
+	info = policydb_lookup_compat(p->policyvers);
+	if (!info) {
+		printk(KERN_ERR "SELinux: compatibility lookup failed for policy "
+		    "version %d", p->policyvers);
+		return rc;
+	}
+
+	buf[0] = cpu_to_le32(p->policyvers);
+	buf[1] = cpu_to_le32(config);
+	buf[2] = cpu_to_le32(info->sym_num);
+	buf[3] = cpu_to_le32(info->ocon_num);
+
+	rc = put_entry(buf, sizeof(u32), 4, fp);
+	if (rc)
+		return rc;
+
+	if (p->policyvers >= POLICYDB_VERSION_POLCAP) {
+		rc = ebitmap_write(&p->policycaps, fp);
+		if (rc)
+			return rc;
+	}
+
+	if (p->policyvers >= POLICYDB_VERSION_PERMISSIVE) {
+		rc = ebitmap_write(&p->permissive_map, fp);
+		if (rc)
+			return rc;
+	}
+
+	num_syms = info->sym_num;
+	for (i = 0; i < num_syms; i++) {
+		struct policy_data pd;
+
+		pd.fp = fp;
+		pd.p = p;
+
+		buf[0] = cpu_to_le32(p->symtab[i].nprim);
+		buf[1] = cpu_to_le32(p->symtab[i].table->nel);
+
+		rc = put_entry(buf, sizeof(u32), 2, fp);
+		if (rc)
+			return rc;
+		rc = hashtab_map(p->symtab[i].table, write_f[i], &pd);
+		if (rc)
+			return rc;
+	}
+
+	rc = avtab_write(p, &p->te_avtab, fp);
+	if (rc)
+		return rc;
+
+	rc = cond_write_list(p, p->cond_list, fp);
+	if (rc)
+		return rc;
+
+	rc = role_trans_write(p->role_tr, fp);
+	if (rc)
+		return rc;
+
+	rc = role_allow_write(p->role_allow, fp);
+	if (rc)
+		return rc;
+
+	rc = ocontext_write(p, info, fp);
+	if (rc)
+		return rc;
+
+	rc = genfs_write(p, fp);
+	if (rc)
+		return rc;
+
+	rc = range_write(p, fp);
+	if (rc)
+		return rc;
+
+	for (i = 0; i < p->p_types.nprim; i++) {
+		rc = ebitmap_write(&p->type_attr_map[i], fp);
+		if (rc)
+			return rc;
+	}
+
+	return 0;
+}
diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
index 26d9adf..9911768 100644
--- a/security/selinux/ss/policydb.h
+++ b/security/selinux/ss/policydb.h
@@ -268,6 +268,7 @@ extern int policydb_class_isvalid(struct policydb *p, unsigned int class);
 extern int policydb_type_isvalid(struct policydb *p, unsigned int type);
 extern int policydb_role_isvalid(struct policydb *p, unsigned int role);
 extern int policydb_read(struct policydb *p, void *fp);
+extern int policydb_write(struct policydb *p, void *fp);
 
 #define PERM_SYMTAB_SIZE 32
 
@@ -288,6 +289,11 @@ struct policy_file {
 	size_t len;
 };
 
+struct policy_data {
+	struct policydb *p;
+	void *fp;
+};
+
 static inline int next_entry(void *buf, struct policy_file *fp, size_t bytes)
 {
 	if (bytes > fp->len)
@@ -299,6 +305,22 @@ static inline int next_entry(void *buf, struct policy_file *fp, size_t bytes)
 	return 0;
 }
 
+static inline int put_entry(void *buf, size_t bytes, int num, struct policy_file *fp)
+{
+	size_t len = bytes * num;
+
+	if (len > fp->len) {
+		printk(KERN_DEBUG "%s: policy was larger than allowed in single read\n",
+		       __func__);
+		return -EINVAL;
+	}
+
+	memcpy(fp->data, buf, len);
+	fp->data += len;
+	fp->len -= len;
+	return 0;
+}
+
 extern u16 string_to_security_class(struct policydb *p, const char *name);
 extern u32 string_to_av_perm(struct policydb *p, u16 tclass, const char *name);
 
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 1de60ce..2e022db 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -87,7 +87,6 @@ int ss_initialized;
  */
 static u32 latest_granting;
 
-/* Forward declaration. */
 static int context_struct_to_string(struct context *context, char **scontext,
 				    u32 *scontext_len);
 
@@ -3126,3 +3125,27 @@ netlbl_sid_to_secattr_failure:
 	return rc;
 }
 #endif /* CONFIG_NETLABEL */
+
+/**
+ * security_read_policy - read the policy.
+ * @data: binary policy data
+ * @len: length of data in bytes
+ *
+ */
+int security_read_policy(void *data, ssize_t *len)
+{
+	int rc = 0;
+	struct policy_file file = { data, *len }, *fp = &file;
+
+	if (!ss_initialized)
+		return -EINVAL;
+
+	read_lock_irq(&policy_rwlock);
+	rc = policydb_write(&policydb, fp);
+	read_unlock_irq(&policy_rwlock);
+
+	*len = (unsigned long)fp->data - (unsigned long)data;
+
+	return rc;
+
+}


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH 4/4] SELinux: allow userspace to read policy back out of the kernel
  2010-06-11 16:37 ` [PATCH 4/4] SELinux: allow userspace to read policy back out of the kernel Eric Paris
@ 2010-06-14 14:48   ` Stephen Smalley
  2010-06-14 15:12     ` Eric Paris
  2010-06-14 14:57   ` Stephen Smalley
  1 sibling, 1 reply; 26+ messages in thread
From: Stephen Smalley @ 2010-06-14 14:48 UTC (permalink / raw)
  To: Eric Paris; +Cc: selinux, jmorris

On Fri, 2010-06-11 at 12:37 -0400, Eric Paris wrote:
> There is interest in being able to see what the actual policy is that was
> loaded into the kernel.  The patch creates a new selinuxfs file
> /selinux/policy which can be read by userspace.  The actual policy that is
> loaded into the kernel will be written back out to userspace.

Why a new node vs a read op for /selinux/load?

> Signed-off-by: Eric Paris <eparis@redhat.com>
> ---
> 

> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index 1de60ce..2e022db 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -3126,3 +3125,27 @@ netlbl_sid_to_secattr_failure:
>  	return rc;
>  }
>  #endif /* CONFIG_NETLABEL */
> +
> +/**
> + * security_read_policy - read the policy.
> + * @data: binary policy data
> + * @len: length of data in bytes
> + *
> + */
> +int security_read_policy(void *data, ssize_t *len)
> +{
> +	int rc = 0;
> +	struct policy_file file = { data, *len }, *fp = &file;
> +
> +	if (!ss_initialized)
> +		return -EINVAL;
> +
> +	read_lock_irq(&policy_rwlock);
> +	rc = policydb_write(&policydb, fp);
> +	read_unlock_irq(&policy_rwlock);
> +
> +	*len = (unsigned long)fp->data - (unsigned long)data;
> +
> +	return rc;
> +
> +}

Why _irq?

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 4/4] SELinux: allow userspace to read policy back out of the kernel
  2010-06-11 16:37 ` [PATCH 4/4] SELinux: allow userspace to read policy back out of the kernel Eric Paris
  2010-06-14 14:48   ` Stephen Smalley
@ 2010-06-14 14:57   ` Stephen Smalley
  2010-06-14 14:59     ` Stephen Smalley
  2010-06-14 15:24     ` Eric Paris
  1 sibling, 2 replies; 26+ messages in thread
From: Stephen Smalley @ 2010-06-14 14:57 UTC (permalink / raw)
  To: Eric Paris; +Cc: selinux, jmorris

On Fri, 2010-06-11 at 12:37 -0400, Eric Paris wrote:
> There is interest in being able to see what the actual policy is that was
> loaded into the kernel.  The patch creates a new selinuxfs file
> /selinux/policy which can be read by userspace.  The actual policy that is
> loaded into the kernel will be written back out to userspace.

How do you expect this to be used?  As with /selinux/load, we can't use
coreutils utilities to manipulate it unfortunately.  Nor can we do
things like checkpolicy -b /selinux/policy since it doesn't support
mmap.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 4/4] SELinux: allow userspace to read policy back out of the kernel
  2010-06-14 14:57   ` Stephen Smalley
@ 2010-06-14 14:59     ` Stephen Smalley
  2010-06-14 15:24     ` Eric Paris
  1 sibling, 0 replies; 26+ messages in thread
From: Stephen Smalley @ 2010-06-14 14:59 UTC (permalink / raw)
  To: Eric Paris; +Cc: selinux, jmorris

On Mon, 2010-06-14 at 10:57 -0400, Stephen Smalley wrote:
> On Fri, 2010-06-11 at 12:37 -0400, Eric Paris wrote:
> > There is interest in being able to see what the actual policy is that was
> > loaded into the kernel.  The patch creates a new selinuxfs file
> > /selinux/policy which can be read by userspace.  The actual policy that is
> > loaded into the kernel will be written back out to userspace.
> 
> How do you expect this to be used?  As with /selinux/load, we can't use
> coreutils utilities to manipulate it unfortunately.  Nor can we do
> things like checkpolicy -b /selinux/policy since it doesn't support
> mmap.

It doesn't seem useful if I can't do things like:
seinfo /selinux/policy
cmp /selinux/policy /etc/selinux/$SELINUXTYPE/policy/policy.24
sediff /selinux/policy \; /etc/selinux/$SELINUXTYPE/poilcy/policy.24

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 4/4] SELinux: allow userspace to read policy back out of the kernel
  2010-06-14 14:48   ` Stephen Smalley
@ 2010-06-14 15:12     ` Eric Paris
  2010-06-15  4:42       ` Casey Schaufler
  0 siblings, 1 reply; 26+ messages in thread
From: Eric Paris @ 2010-06-14 15:12 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: selinux, jmorris

On Mon, 2010-06-14 at 10:48 -0400, Stephen Smalley wrote:
> On Fri, 2010-06-11 at 12:37 -0400, Eric Paris wrote:
> > There is interest in being able to see what the actual policy is that was
> > loaded into the kernel.  The patch creates a new selinuxfs file
> > /selinux/policy which can be read by userspace.  The actual policy that is
> > loaded into the kernel will be written back out to userspace.
> 
> Why a new node vs a read op for /selinux/load?

No reason why I couldn't.  Just 'load' seemed to imply a connotation
which wasn't appropriate.  If you prefer I'll switch it when I do
another version.

-Eric
> 
> > Signed-off-by: Eric Paris <eparis@redhat.com>
> > ---
> > 
> 
> > diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> > index 1de60ce..2e022db 100644
> > --- a/security/selinux/ss/services.c
> > +++ b/security/selinux/ss/services.c
> > @@ -3126,3 +3125,27 @@ netlbl_sid_to_secattr_failure:
> >  	return rc;
> >  }
> >  #endif /* CONFIG_NETLABEL */
> > +
> > +/**
> > + * security_read_policy - read the policy.
> > + * @data: binary policy data
> > + * @len: length of data in bytes
> > + *
> > + */
> > +int security_read_policy(void *data, ssize_t *len)
> > +{
> > +	int rc = 0;
> > +	struct policy_file file = { data, *len }, *fp = &file;
> > +
> > +	if (!ss_initialized)
> > +		return -EINVAL;
> > +
> > +	read_lock_irq(&policy_rwlock);
> > +	rc = policydb_write(&policydb, fp);
> > +	read_unlock_irq(&policy_rwlock);
> > +
> > +	*len = (unsigned long)fp->data - (unsigned long)data;
> > +
> > +	return rc;
> > +
> > +}
> 
> Why _irq?

Stolen from security_load_policy() and shouldn't have been.  Will fix.


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 4/4] SELinux: allow userspace to read policy back out of the kernel
  2010-06-14 14:57   ` Stephen Smalley
  2010-06-14 14:59     ` Stephen Smalley
@ 2010-06-14 15:24     ` Eric Paris
  2010-06-14 16:14       ` Stephen Smalley
  1 sibling, 1 reply; 26+ messages in thread
From: Eric Paris @ 2010-06-14 15:24 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: selinux, jmorris

On Mon, 2010-06-14 at 10:57 -0400, Stephen Smalley wrote:
> On Fri, 2010-06-11 at 12:37 -0400, Eric Paris wrote:
> > There is interest in being able to see what the actual policy is that was
> > loaded into the kernel.  The patch creates a new selinuxfs file
> > /selinux/policy which can be read by userspace.  The actual policy that is
> > loaded into the kernel will be written back out to userspace.
> 
> How do you expect this to be used?  As with /selinux/load, we can't use
> coreutils utilities to manipulate it unfortunately.  Nor can we do
> things like checkpolicy -b /selinux/policy since it doesn't support
> mmap.

I used my own program to pull it out to a file and poke it after it was
out.  I can certainly take a look at generating the policy on open()
which would allow us to support ppos easily (and maybe mmap, but I've
never written an mmap handler)

#include <fcntl.h>
#include <stdlib.h>
#include <stdio.h>
#include <sys/mman.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <unistd.h>

#define BUFLEN  (10 * 1024 * 1024)

int main(int argc, char *argv[])
{
	char *buf;
	ssize_t len;
	int polfd;
	int outfd;
	int ret;

	polfd = open("/selinux/policy", O_RDONLY);
	if (polfd < 0)
		return 1;

	outfd = open("policy.from.kern", O_RDWR|O_CREAT, S_IRUSR|S_IWUSR);
	if (outfd < 0)
		return 2;

	ret = ftruncate(outfd, BUFLEN);
	if (ret) {
		perror("ftruncate");
		return 3;
	}

	buf =  mmap(NULL, BUFLEN, PROT_WRITE, MAP_SHARED, outfd, 0);
	if (buf == MAP_FAILED) {
		perror("mmap");
		return 4;
	}

	ret = read(polfd, buf, BUFLEN);
	if (ret < 0) {
		perror("write");
		return 5;
	}

	len = ret;

	msync(buf, len, MS_SYNC);

	ftruncate(outfd, len);

	return 0;
}



--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 4/4] SELinux: allow userspace to read policy back out of the kernel
  2010-06-14 15:24     ` Eric Paris
@ 2010-06-14 16:14       ` Stephen Smalley
  2010-06-14 17:55         ` Eric Paris
  0 siblings, 1 reply; 26+ messages in thread
From: Stephen Smalley @ 2010-06-14 16:14 UTC (permalink / raw)
  To: Eric Paris; +Cc: selinux, jmorris

On Mon, 2010-06-14 at 11:24 -0400, Eric Paris wrote:
> On Mon, 2010-06-14 at 10:57 -0400, Stephen Smalley wrote:
> > On Fri, 2010-06-11 at 12:37 -0400, Eric Paris wrote:
> > > There is interest in being able to see what the actual policy is that was
> > > loaded into the kernel.  The patch creates a new selinuxfs file
> > > /selinux/policy which can be read by userspace.  The actual policy that is
> > > loaded into the kernel will be written back out to userspace.
> > 
> > How do you expect this to be used?  As with /selinux/load, we can't use
> > coreutils utilities to manipulate it unfortunately.  Nor can we do
> > things like checkpolicy -b /selinux/policy since it doesn't support
> > mmap.
> 
> I used my own program to pull it out to a file and poke it after it was
> out.  I can certainly take a look at generating the policy on open()
> which would allow us to support ppos easily (and maybe mmap, but I've
> never written an mmap handler)

Hmm...the resulting policy.from.kern doesn't match the binary policy
file that was loaded, nor is it a well-formed policy.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 4/4] SELinux: allow userspace to read policy back out of the kernel
  2010-06-14 16:14       ` Stephen Smalley
@ 2010-06-14 17:55         ` Eric Paris
  2010-06-14 18:04           ` Stephen Smalley
  2010-06-18 12:01           ` Christopher J. PeBenito
  0 siblings, 2 replies; 26+ messages in thread
From: Eric Paris @ 2010-06-14 17:55 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: selinux, jmorris

On Mon, 2010-06-14 at 12:14 -0400, Stephen Smalley wrote:
> On Mon, 2010-06-14 at 11:24 -0400, Eric Paris wrote:
> > On Mon, 2010-06-14 at 10:57 -0400, Stephen Smalley wrote:
> > > On Fri, 2010-06-11 at 12:37 -0400, Eric Paris wrote:
> > > > There is interest in being able to see what the actual policy is that was
> > > > loaded into the kernel.  The patch creates a new selinuxfs file
> > > > /selinux/policy which can be read by userspace.  The actual policy that is
> > > > loaded into the kernel will be written back out to userspace.
> > > 
> > > How do you expect this to be used?  As with /selinux/load, we can't use
> > > coreutils utilities to manipulate it unfortunately.  Nor can we do
> > > things like checkpolicy -b /selinux/policy since it doesn't support
> > > mmap.
> > 
> > I used my own program to pull it out to a file and poke it after it was
> > out.  I can certainly take a look at generating the policy on open()
> > which would allow us to support ppos easily (and maybe mmap, but I've
> > never written an mmap handler)
> 
> Hmm...the resulting policy.from.kern doesn't match the binary policy
> file that was loaded, nor is it a well-formed policy.

It won't be a binary perfect match since we switched range transition
rules to a hashtab and we lose ordering in the kernel.  (although the
second load and resulting read should be the same binary policy)

What is not well-formed about your result?  I got back the same policy
(according to sediff) but I was using selinux-policy-minimum....

-Eric


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 4/4] SELinux: allow userspace to read policy back out of the kernel
  2010-06-14 17:55         ` Eric Paris
@ 2010-06-14 18:04           ` Stephen Smalley
  2010-06-18 12:01           ` Christopher J. PeBenito
  1 sibling, 0 replies; 26+ messages in thread
From: Stephen Smalley @ 2010-06-14 18:04 UTC (permalink / raw)
  To: Eric Paris; +Cc: selinux, jmorris

On Mon, 2010-06-14 at 13:55 -0400, Eric Paris wrote:
> On Mon, 2010-06-14 at 12:14 -0400, Stephen Smalley wrote:
> > On Mon, 2010-06-14 at 11:24 -0400, Eric Paris wrote:
> > > On Mon, 2010-06-14 at 10:57 -0400, Stephen Smalley wrote:
> > > > On Fri, 2010-06-11 at 12:37 -0400, Eric Paris wrote:
> > > > > There is interest in being able to see what the actual policy is that was
> > > > > loaded into the kernel.  The patch creates a new selinuxfs file
> > > > > /selinux/policy which can be read by userspace.  The actual policy that is
> > > > > loaded into the kernel will be written back out to userspace.
> > > > 
> > > > How do you expect this to be used?  As with /selinux/load, we can't use
> > > > coreutils utilities to manipulate it unfortunately.  Nor can we do
> > > > things like checkpolicy -b /selinux/policy since it doesn't support
> > > > mmap.
> > > 
> > > I used my own program to pull it out to a file and poke it after it was
> > > out.  I can certainly take a look at generating the policy on open()
> > > which would allow us to support ppos easily (and maybe mmap, but I've
> > > never written an mmap handler)
> > 
> > Hmm...the resulting policy.from.kern doesn't match the binary policy
> > file that was loaded, nor is it a well-formed policy.
> 
> It won't be a binary perfect match since we switched range transition
> rules to a hashtab and we lose ordering in the kernel.  (although the
> second load and resulting read should be the same binary policy)
> 
> What is not well-formed about your result?  I got back the same policy
> (according to sediff) but I was using selinux-policy-minimum....

Breakpoint 2, sens_index (key=0x6ac120 "s0", datum=0x6ac100, datap=0x658960)
    at policydb.c:753
753				return -EINVAL;
(gdb) print *levdatum->level
$1 = {sens = 1501285952, cat = {node = 0x6ac160, highbit = 1024}}
(gdb) where
#0  sens_index (key=0x6ac120 "s0", datum=0x6ac100, datap=0x658960)
    at policydb.c:753
#1  0x0000000000423915 in hashtab_map (h=0x65e7a0, 
    apply=0x42f5d3 <sens_index>, args=0x658960) at hashtab.c:235
#2  0x000000000042fdb3 in policydb_index_others (handle=0x0, p=0x658960, 
    verbose=1) at policydb.c:917
#3  0x0000000000436571 in policydb_read (p=0x658960, fp=0x7fffffffe0d0, 
    verbose=1) at policydb.c:3486
#4  0x0000000000401f85 in main (argc=3, argv=0x7fffffffe3e8)
    at checkpolicy.c:526


-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 4/4] SELinux: allow userspace to read policy back out of the kernel
  2010-06-14 15:12     ` Eric Paris
@ 2010-06-15  4:42       ` Casey Schaufler
  2010-06-15 14:33         ` Eric Paris
  0 siblings, 1 reply; 26+ messages in thread
From: Casey Schaufler @ 2010-06-15  4:42 UTC (permalink / raw)
  To: Eric Paris; +Cc: Stephen Smalley, selinux, jmorris, Casey Schaufler

Eric Paris wrote:
> On Mon, 2010-06-14 at 10:48 -0400, Stephen Smalley wrote:
>   
>> On Fri, 2010-06-11 at 12:37 -0400, Eric Paris wrote:
>>     
>>> There is interest in being able to see what the actual policy is that was
>>> loaded into the kernel.  The patch creates a new selinuxfs file
>>> /selinux/policy which can be read by userspace.  The actual policy that is
>>> loaded into the kernel will be written back out to userspace.
>>>       
>> Why a new node vs a read op for /selinux/load?
>>     
>
> No reason why I couldn't.  Just 'load' seemed to imply a connotation
> which wasn't appropriate.  If you prefer I'll switch it when I do
> another version.
>   

If it makes any difference Smack /smack/load does read as well as write.
You have the opportunity to make 2 or 3 users less confused if you do
things consistently. After all, Smack uses load because SELinux does,
the name is actually arbitrary, and why do something differently when
it doesn't really matter?

> -Eric
>   
>>> Signed-off-by: Eric Paris <eparis@redhat.com>
>>> ---
>>>
>>>       
>>> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
>>> index 1de60ce..2e022db 100644
>>> --- a/security/selinux/ss/services.c
>>> +++ b/security/selinux/ss/services.c
>>> @@ -3126,3 +3125,27 @@ netlbl_sid_to_secattr_failure:
>>>  	return rc;
>>>  }
>>>  #endif /* CONFIG_NETLABEL */
>>> +
>>> +/**
>>> + * security_read_policy - read the policy.
>>> + * @data: binary policy data
>>> + * @len: length of data in bytes
>>> + *
>>> + */
>>> +int security_read_policy(void *data, ssize_t *len)
>>> +{
>>> +	int rc = 0;
>>> +	struct policy_file file = { data, *len }, *fp = &file;
>>> +
>>> +	if (!ss_initialized)
>>> +		return -EINVAL;
>>> +
>>> +	read_lock_irq(&policy_rwlock);
>>> +	rc = policydb_write(&policydb, fp);
>>> +	read_unlock_irq(&policy_rwlock);
>>> +
>>> +	*len = (unsigned long)fp->data - (unsigned long)data;
>>> +
>>> +	return rc;
>>> +
>>> +}
>>>       
>> Why _irq?
>>     
>
> Stolen from security_load_policy() and shouldn't have been.  Will fix.
>
>
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
> the words "unsubscribe selinux" without quotes as the message.
>
>
>   


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 4/4] SELinux: allow userspace to read policy back out of the kernel
  2010-06-15  4:42       ` Casey Schaufler
@ 2010-06-15 14:33         ` Eric Paris
  2010-06-16 14:53           ` Stephen Smalley
  0 siblings, 1 reply; 26+ messages in thread
From: Eric Paris @ 2010-06-15 14:33 UTC (permalink / raw)
  To: Casey Schaufler; +Cc: Stephen Smalley, selinux, jmorris

On Mon, 2010-06-14 at 21:42 -0700, Casey Schaufler wrote:
> Eric Paris wrote:
> > On Mon, 2010-06-14 at 10:48 -0400, Stephen Smalley wrote:
> >   
> >> On Fri, 2010-06-11 at 12:37 -0400, Eric Paris wrote:
> >>     
> >>> There is interest in being able to see what the actual policy is that was
> >>> loaded into the kernel.  The patch creates a new selinuxfs file
> >>> /selinux/policy which can be read by userspace.  The actual policy that is
> >>> loaded into the kernel will be written back out to userspace.
> >>>       
> >> Why a new node vs a read op for /selinux/load?
> >>     
> >
> > No reason why I couldn't.  Just 'load' seemed to imply a connotation
> > which wasn't appropriate.  If you prefer I'll switch it when I do
> > another version.
> >   
> 
> If it makes any difference Smack /smack/load does read as well as write.
> You have the opportunity to make 2 or 3 users less confused if you do
> things consistently. After all, Smack uses load because SELinux does,
> the name is actually arbitrary, and why do something differently when
> it doesn't really matter?

I did two things yesterday.  First I switch the read
from /selinux/policy to /selinux/load.  Then I undid that change and
started generating the in kernel policy buffer on open() rather than on
read().  It allowed me to use cat /etc/policy > policy rather than using
my own half ass hacked utility.  The reason I undid the policy->load
change was because I didn't really want to store the old policy on open
if they were going to write() a new policy.  I can probably make the
determination based on the f_mode, but didn't really play with it yet.
I  try to do both in the next go-round.

I'm still trying to figure out what I did to make malformed policies.
Must have screwed something up ripping out my prink's and debug hooks,
because it isn't working for me now either....

Unrelated note, can we take patches 1,2,3 ?  They are just cleanups....

-Eric


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/4] SELinux: seperate range transition rules to a seperate function
  2010-06-11 16:37 [PATCH 1/4] SELinux: seperate range transition rules to a seperate function Eric Paris
                   ` (2 preceding siblings ...)
  2010-06-11 16:37 ` [PATCH 4/4] SELinux: allow userspace to read policy back out of the kernel Eric Paris
@ 2010-06-16 13:02 ` Stephen Smalley
  2010-06-17  5:02 ` James Morris
  4 siblings, 0 replies; 26+ messages in thread
From: Stephen Smalley @ 2010-06-16 13:02 UTC (permalink / raw)
  To: Eric Paris; +Cc: selinux, jmorris

On Fri, 2010-06-11 at 12:37 -0400, Eric Paris wrote:
> Move the range transition rule to a separate function, range_read(), rather
> than doing it all in policydb_read()
> 
> Signed-off-by: Eric Paris <eparis@redhat.com>

Acked-by:  Stephen Smalley <sds@tycho.nsa.gov>

> ---
> 
>  security/selinux/ss/policydb.c |  139 ++++++++++++++++++++++------------------
>  1 files changed, 75 insertions(+), 64 deletions(-)
> 
> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index c57802a..a39d38a 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -1701,6 +1701,78 @@ u32 string_to_av_perm(struct policydb *p, u16 tclass, const char *name)
>  	return 1U << (perdatum->value-1);
>  }
>  
> +static int range_read(struct policydb *p, void *fp)
> +{
> +	struct range_trans *rt = NULL;
> +	struct mls_range *r = NULL;
> +	int i, rc;
> +	__le32 buf[2];
> +	u32 nel;
> +
> +	if (p->policyvers < POLICYDB_VERSION_MLS)
> +		return 0;
> +
> +	rc = next_entry(buf, fp, sizeof(u32));
> +	if (rc)
> +		goto out;
> +
> +	nel = le32_to_cpu(buf[0]);
> +	for (i = 0; i < nel; i++) {
> +		rc = -ENOMEM;
> +		rt = kzalloc(sizeof(*rt), GFP_KERNEL);
> +		if (!rt)
> +			goto out;
> +
> +		rc = next_entry(buf, fp, (sizeof(u32) * 2));
> +		if (rc)
> +			goto out;
> +
> +		rt->source_type = le32_to_cpu(buf[0]);
> +		rt->target_type = le32_to_cpu(buf[1]);
> +		if (p->policyvers >= POLICYDB_VERSION_RANGETRANS) {
> +			rc = next_entry(buf, fp, sizeof(u32));
> +			if (rc)
> +				goto out;
> +			rt->target_class = le32_to_cpu(buf[0]);
> +		} else
> +			rt->target_class = p->process_class;
> +
> +		rc = -EINVAL;
> +		if (!policydb_type_isvalid(p, rt->source_type) ||
> +		    !policydb_type_isvalid(p, rt->target_type) ||
> +		    !policydb_class_isvalid(p, rt->target_class))
> +			goto out;
> +
> +		rc = -ENOMEM;
> +		r = kzalloc(sizeof(*r), GFP_KERNEL);
> +		if (!r)
> +			goto out;
> +
> +		rc = mls_read_range_helper(r, fp);
> +		if (rc)
> +			goto out;
> +
> +		rc = -EINVAL;
> +		if (!mls_range_isvalid(p, r)) {
> +			printk(KERN_WARNING "SELinux:  rangetrans:  invalid range\n");
> +			goto out;
> +		}
> +
> +		rc = hashtab_insert(p->range_tr, rt, r);
> +		if (rc)
> +			goto out;
> +
> +		rt = NULL;
> +		r = NULL;
> +	}
> +	rangetr_hash_eval(p->range_tr);
> +	rc = 0;
> +out:
> +	kfree(rt);
> +	kfree(r);
> +	return rc;
> +}
> +
>  /*
>   * Read the configuration data from a policy database binary
>   * representation file into a policy database structure.
> @@ -1717,8 +1789,6 @@ int policydb_read(struct policydb *p, void *fp)
>  	u32 len, len2, nprim, nel, nel2;
>  	char *policydb_str;
>  	struct policydb_compat_info *info;
> -	struct range_trans *rt;
> -	struct mls_range *r;
>  
>  	rc = policydb_init(p);
>  	if (rc)
> @@ -2131,68 +2201,9 @@ int policydb_read(struct policydb *p, void *fp)
>  		}
>  	}
>  
> -	if (p->policyvers >= POLICYDB_VERSION_MLS) {
> -		int new_rangetr = p->policyvers >= POLICYDB_VERSION_RANGETRANS;
> -		rc = next_entry(buf, fp, sizeof(u32));
> -		if (rc < 0)
> -			goto bad;
> -		nel = le32_to_cpu(buf[0]);
> -		for (i = 0; i < nel; i++) {
> -			rt = kzalloc(sizeof(*rt), GFP_KERNEL);
> -			if (!rt) {
> -				rc = -ENOMEM;
> -				goto bad;
> -			}
> -			rc = next_entry(buf, fp, (sizeof(u32) * 2));
> -			if (rc < 0) {
> -				kfree(rt);
> -				goto bad;
> -			}
> -			rt->source_type = le32_to_cpu(buf[0]);
> -			rt->target_type = le32_to_cpu(buf[1]);
> -			if (new_rangetr) {
> -				rc = next_entry(buf, fp, sizeof(u32));
> -				if (rc < 0) {
> -					kfree(rt);
> -					goto bad;
> -				}
> -				rt->target_class = le32_to_cpu(buf[0]);
> -			} else
> -				rt->target_class = p->process_class;
> -			if (!policydb_type_isvalid(p, rt->source_type) ||
> -			    !policydb_type_isvalid(p, rt->target_type) ||
> -			    !policydb_class_isvalid(p, rt->target_class)) {
> -				kfree(rt);
> -				rc = -EINVAL;
> -				goto bad;
> -			}
> -			r = kzalloc(sizeof(*r), GFP_KERNEL);
> -			if (!r) {
> -				kfree(rt);
> -				rc = -ENOMEM;
> -				goto bad;
> -			}
> -			rc = mls_read_range_helper(r, fp);
> -			if (rc) {
> -				kfree(rt);
> -				kfree(r);
> -				goto bad;
> -			}
> -			if (!mls_range_isvalid(p, r)) {
> -				printk(KERN_WARNING "SELinux:  rangetrans:  invalid range\n");
> -				kfree(rt);
> -				kfree(r);
> -				goto bad;
> -			}
> -			rc = hashtab_insert(p->range_tr, rt, r);
> -			if (rc) {
> -				kfree(rt);
> -				kfree(r);
> -				goto bad;
> -			}
> -		}
> -		rangetr_hash_eval(p->range_tr);
> -	}
> +	rc = range_read(p, fp);
> +	if (rc)
> +		goto bad;
>  
>  	p->type_attr_map = kmalloc(p->p_types.nprim * sizeof(struct ebitmap), GFP_KERNEL);
>  	if (!p->type_attr_map)

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/4] SELinux: move genfs read to a separate function
  2010-06-11 16:37 ` [PATCH 2/4] SELinux: move genfs read to a separate function Eric Paris
@ 2010-06-16 14:18   ` Stephen Smalley
  2010-06-16 14:24     ` Stephen Smalley
  0 siblings, 1 reply; 26+ messages in thread
From: Stephen Smalley @ 2010-06-16 14:18 UTC (permalink / raw)
  To: Eric Paris; +Cc: selinux, jmorris

On Fri, 2010-06-11 at 12:37 -0400, Eric Paris wrote:
> move genfs read functionality out of policydb_read() and into a new
> function called genfs_read()
> 
> Signed-off-by: Eric Paris <eparis@redhat.com>
> ---
> 
>  security/selinux/ss/policydb.c |  236 ++++++++++++++++++++++------------------
>  1 files changed, 131 insertions(+), 105 deletions(-)
> 
> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index a39d38a..66e217b 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -1773,6 +1776,129 @@ out:
>  	return rc;
>  }
>  
> +static int genfs_read(struct policydb *p, void *fp)
> +{
> +	int i, j, rc;
> +	u32 nel, nel2, len, len2;
> +	__le32 buf[1];
> +	struct ocontext *l, *c;
> +	struct ocontext *newc = NULL;
> +	struct genfs *genfs_p, *genfs;
> +	struct genfs *newgenfs = NULL;
> +
> +	rc = next_entry(buf, fp, sizeof(u32));
> +	if (rc < 0)
> +		goto out;
> +	nel = le32_to_cpu(buf[0]);
> +
> +	for (i = 0; i < nel; i++) {
> +		rc = next_entry(buf, fp, sizeof(u32));
> +		if (rc)
> +			goto out;
> +		len = le32_to_cpu(buf[0]);
> +
> +		rc = -ENOMEM;
> +		newgenfs = kzalloc(sizeof(*newgenfs), GFP_KERNEL);
> +		if (!newgenfs)
> +			goto out;
> +
> +		rc = -ENOMEM;
> +		newgenfs->fstype = kmalloc(len + 1, GFP_KERNEL);
> +		if (!newgenfs->fstype)
> +			goto out;
> +
> +		rc = next_entry(newgenfs->fstype, fp, len);
> +		if (rc < 0)
> +			goto out;
> +
> +		newgenfs->fstype[len] = 0;
> +
> +		for (genfs_p = NULL, genfs = p->genfs; genfs;
> +		     genfs_p = genfs, genfs = genfs->next) {
> +			rc = -EEXIST;
> +			if (strcmp(newgenfs->fstype, genfs->fstype) == 0) {
> +				printk(KERN_ERR "SELinux:  dup genfs fstype %s\n",
> +				       newgenfs->fstype);
> +				goto out;
> +			}
> +			if (strcmp(newgenfs->fstype, genfs->fstype) < 0)
> +				break;
> +		}
> +		newgenfs->next = genfs;
> +		if (genfs_p)
> +			genfs_p->next = newgenfs;
> +		else
> +			p->genfs = newgenfs;
> +		genfs = newgenfs;
> +		newgenfs = NULL;
> +
> +		rc = next_entry(buf, fp, sizeof(u32));
> +		if (rc)
> +			goto out;
> +
> +		nel2 = le32_to_cpu(buf[0]);
> +		for (j = 0; j < nel2; j++) {
> +			rc = next_entry(buf, fp, sizeof(u32));
> +			if (rc)
> +				goto out;
> +			len = le32_to_cpu(buf[0]);
> +
> +			rc = -ENOMEM;
> +			newc = kzalloc(sizeof(*newc), GFP_KERNEL);
> +			if (!newc)
> +				goto out;
> +
> +			rc = -ENOMEM;
> +			newc->u.name = kmalloc(len + 1, GFP_KERNEL);
> +			if (!newc->u.name)
> +				goto out;
> +
> +			rc = next_entry(newc->u.name, fp, len);
> +			if (rc)
> +				goto out;
> +			newc->u.name[len] = 0;
> +
> +			rc = next_entry(buf, fp, sizeof(u32));
> +			if (rc)
> +				goto out;
> +
> +			newc->v.sclass = le32_to_cpu(buf[0]);
> +			rc = context_read_and_validate(&newc->context[0], p, fp);
> +			if (rc)
> +				goto out;
> +
> +			for (l = NULL, c = genfs->head; c;
> +			     l = c, c = c->next) {
> +				if (!strcmp(newc->u.name, c->u.name) &&
> +				    (!c->v.sclass || !newc->v.sclass ||
> +				     newc->v.sclass == c->v.sclass)) {
> +					printk(KERN_ERR "SELinux:  dup genfs entry (%s,%s)\n",
> +					       genfs->fstype, c->u.name);
> +					goto out;

rc wasn't set.

> +				}
> +				len = strlen(newc->u.name);
> +				len2 = strlen(c->u.name);
> +				if (len > len2)
> +					break;
> +			}
> +
> +			newc->next = c;
> +			if (l)
> +				l->next = newc;
> +			else
> +				genfs->head = newc;
> +			newc = NULL;
> +		}
> +	}
> +out:
> +	if (newgenfs)
> +		kfree(newgenfs->fstype);
> +	kfree(newgenfs);
> +	ocontext_destroy(newc, OCON_FSUSE);
> +
> +	return rc;
> +}
> +
>  /*
>   * Read the configuration data from a policy database binary
>   * representation file into a policy database structure.
> @@ -1781,12 +1907,12 @@ int policydb_read(struct policydb *p, void *fp)
>  {
>  	struct role_allow *ra, *lra;
>  	struct role_trans *tr, *ltr;
> -	struct ocontext *l, *c, *newc;
> -	struct genfs *genfs_p, *genfs, *newgenfs;
> +	struct ocontext *l, *c;
>  	int i, j, rc;
>  	__le32 buf[4];
>  	u32 nodebuf[8];
> -	u32 len, len2, nprim, nel, nel2;
> +	u32 len, nprim, nel;
> +
>  	char *policydb_str;
>  	struct policydb_compat_info *info;
>  
> @@ -2099,107 +2225,9 @@ int policydb_read(struct policydb *p, void *fp)
>  		}
>  	}
>  
> -	rc = next_entry(buf, fp, sizeof(u32));
> -	if (rc < 0)
> +	rc = genfs_read(p, fp);
> +	if (rc)
>  		goto bad;
> -	nel = le32_to_cpu(buf[0]);
> -	genfs_p = NULL;
> -	rc = -EINVAL;
> -	for (i = 0; i < nel; i++) {
> -		rc = next_entry(buf, fp, sizeof(u32));
> -		if (rc < 0)
> -			goto bad;
> -		len = le32_to_cpu(buf[0]);
> -		newgenfs = kzalloc(sizeof(*newgenfs), GFP_KERNEL);
> -		if (!newgenfs) {
> -			rc = -ENOMEM;
> -			goto bad;
> -		}
> -
> -		newgenfs->fstype = kmalloc(len + 1, GFP_KERNEL);
> -		if (!newgenfs->fstype) {
> -			rc = -ENOMEM;
> -			kfree(newgenfs);
> -			goto bad;
> -		}
> -		rc = next_entry(newgenfs->fstype, fp, len);
> -		if (rc < 0) {
> -			kfree(newgenfs->fstype);
> -			kfree(newgenfs);
> -			goto bad;
> -		}
> -		newgenfs->fstype[len] = 0;
> -		for (genfs_p = NULL, genfs = p->genfs; genfs;
> -		     genfs_p = genfs, genfs = genfs->next) {
> -			if (strcmp(newgenfs->fstype, genfs->fstype) == 0) {
> -				printk(KERN_ERR "SELinux:  dup genfs "
> -				       "fstype %s\n", newgenfs->fstype);
> -				kfree(newgenfs->fstype);
> -				kfree(newgenfs);
> -				goto bad;
> -			}
> -			if (strcmp(newgenfs->fstype, genfs->fstype) < 0)
> -				break;
> -		}
> -		newgenfs->next = genfs;
> -		if (genfs_p)
> -			genfs_p->next = newgenfs;
> -		else
> -			p->genfs = newgenfs;
> -		rc = next_entry(buf, fp, sizeof(u32));
> -		if (rc < 0)
> -			goto bad;
> -		nel2 = le32_to_cpu(buf[0]);
> -		for (j = 0; j < nel2; j++) {
> -			rc = next_entry(buf, fp, sizeof(u32));
> -			if (rc < 0)
> -				goto bad;
> -			len = le32_to_cpu(buf[0]);
> -
> -			newc = kzalloc(sizeof(*newc), GFP_KERNEL);
> -			if (!newc) {
> -				rc = -ENOMEM;
> -				goto bad;
> -			}
> -
> -			newc->u.name = kmalloc(len + 1, GFP_KERNEL);
> -			if (!newc->u.name) {
> -				rc = -ENOMEM;
> -				goto bad_newc;
> -			}
> -			rc = next_entry(newc->u.name, fp, len);
> -			if (rc < 0)
> -				goto bad_newc;
> -			newc->u.name[len] = 0;
> -			rc = next_entry(buf, fp, sizeof(u32));
> -			if (rc < 0)
> -				goto bad_newc;
> -			newc->v.sclass = le32_to_cpu(buf[0]);
> -			if (context_read_and_validate(&newc->context[0], p, fp))
> -				goto bad_newc;
> -			for (l = NULL, c = newgenfs->head; c;
> -			     l = c, c = c->next) {
> -				if (!strcmp(newc->u.name, c->u.name) &&
> -				    (!c->v.sclass || !newc->v.sclass ||
> -				     newc->v.sclass == c->v.sclass)) {
> -					printk(KERN_ERR "SELinux:  dup genfs "
> -					       "entry (%s,%s)\n",
> -					       newgenfs->fstype, c->u.name);
> -					goto bad_newc;
> -				}
> -				len = strlen(newc->u.name);
> -				len2 = strlen(c->u.name);
> -				if (len > len2)
> -					break;
> -			}
> -
> -			newc->next = c;
> -			if (l)
> -				l->next = newc;
> -			else
> -				newgenfs->head = newc;
> -		}
> -	}
>  
>  	rc = range_read(p, fp);
>  	if (rc)
> @@ -2227,8 +2255,6 @@ int policydb_read(struct policydb *p, void *fp)
>  	rc = 0;
>  out:
>  	return rc;
> -bad_newc:
> -	ocontext_destroy(newc, OCON_FSUSE);
>  bad:
>  	if (!rc)
>  		rc = -EINVAL;

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/4] SELinux: move genfs read to a separate function
  2010-06-16 14:18   ` Stephen Smalley
@ 2010-06-16 14:24     ` Stephen Smalley
  0 siblings, 0 replies; 26+ messages in thread
From: Stephen Smalley @ 2010-06-16 14:24 UTC (permalink / raw)
  To: Eric Paris; +Cc: selinux, jmorris

On Wed, 2010-06-16 at 10:18 -0400, Stephen Smalley wrote:
> On Fri, 2010-06-11 at 12:37 -0400, Eric Paris wrote:
> > move genfs read functionality out of policydb_read() and into a new
> > function called genfs_read()
> > 
> > Signed-off-by: Eric Paris <eparis@redhat.com>
> > ---
> > 
> >  security/selinux/ss/policydb.c |  236 ++++++++++++++++++++++------------------
> >  1 files changed, 131 insertions(+), 105 deletions(-)
> > 
> > diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> > index a39d38a..66e217b 100644
> > --- a/security/selinux/ss/policydb.c
> > +++ b/security/selinux/ss/policydb.c
> > @@ -1773,6 +1776,129 @@ out:
> >  	return rc;
> >  }
> >  
> > +static int genfs_read(struct policydb *p, void *fp)
> > +{
> > +	int i, j, rc;
> > +	u32 nel, nel2, len, len2;
> > +	__le32 buf[1];
> > +	struct ocontext *l, *c;
> > +	struct ocontext *newc = NULL;
> > +	struct genfs *genfs_p, *genfs;
> > +	struct genfs *newgenfs = NULL;
> > +
> > +	rc = next_entry(buf, fp, sizeof(u32));
> > +	if (rc < 0)
> > +		goto out;
> > +	nel = le32_to_cpu(buf[0]);
> > +
> > +	for (i = 0; i < nel; i++) {
> > +		rc = next_entry(buf, fp, sizeof(u32));
> > +		if (rc)
> > +			goto out;
> > +		len = le32_to_cpu(buf[0]);
> > +
> > +		rc = -ENOMEM;
> > +		newgenfs = kzalloc(sizeof(*newgenfs), GFP_KERNEL);
> > +		if (!newgenfs)
> > +			goto out;
> > +
> > +		rc = -ENOMEM;
> > +		newgenfs->fstype = kmalloc(len + 1, GFP_KERNEL);
> > +		if (!newgenfs->fstype)
> > +			goto out;
> > +
> > +		rc = next_entry(newgenfs->fstype, fp, len);
> > +		if (rc < 0)
> > +			goto out;
> > +
> > +		newgenfs->fstype[len] = 0;
> > +
> > +		for (genfs_p = NULL, genfs = p->genfs; genfs;
> > +		     genfs_p = genfs, genfs = genfs->next) {
> > +			rc = -EEXIST;
> > +			if (strcmp(newgenfs->fstype, genfs->fstype) == 0) {
> > +				printk(KERN_ERR "SELinux:  dup genfs fstype %s\n",
> > +				       newgenfs->fstype);
> > +				goto out;
> > +			}
> > +			if (strcmp(newgenfs->fstype, genfs->fstype) < 0)
> > +				break;
> > +		}
> > +		newgenfs->next = genfs;
> > +		if (genfs_p)
> > +			genfs_p->next = newgenfs;
> > +		else
> > +			p->genfs = newgenfs;
> > +		genfs = newgenfs;
> > +		newgenfs = NULL;
> > +
> > +		rc = next_entry(buf, fp, sizeof(u32));
> > +		if (rc)
> > +			goto out;
> > +
> > +		nel2 = le32_to_cpu(buf[0]);
> > +		for (j = 0; j < nel2; j++) {
> > +			rc = next_entry(buf, fp, sizeof(u32));
> > +			if (rc)
> > +				goto out;
> > +			len = le32_to_cpu(buf[0]);
> > +
> > +			rc = -ENOMEM;
> > +			newc = kzalloc(sizeof(*newc), GFP_KERNEL);
> > +			if (!newc)
> > +				goto out;
> > +
> > +			rc = -ENOMEM;
> > +			newc->u.name = kmalloc(len + 1, GFP_KERNEL);
> > +			if (!newc->u.name)
> > +				goto out;
> > +
> > +			rc = next_entry(newc->u.name, fp, len);
> > +			if (rc)
> > +				goto out;
> > +			newc->u.name[len] = 0;
> > +
> > +			rc = next_entry(buf, fp, sizeof(u32));
> > +			if (rc)
> > +				goto out;
> > +
> > +			newc->v.sclass = le32_to_cpu(buf[0]);
> > +			rc = context_read_and_validate(&newc->context[0], p, fp);
> > +			if (rc)
> > +				goto out;
> > +
> > +			for (l = NULL, c = genfs->head; c;
> > +			     l = c, c = c->next) {
> > +				if (!strcmp(newc->u.name, c->u.name) &&
> > +				    (!c->v.sclass || !newc->v.sclass ||
> > +				     newc->v.sclass == c->v.sclass)) {
> > +					printk(KERN_ERR "SELinux:  dup genfs entry (%s,%s)\n",
> > +					       genfs->fstype, c->u.name);
> > +					goto out;
> 
> rc wasn't set.
> 
> > +				}
> > +				len = strlen(newc->u.name);
> > +				len2 = strlen(c->u.name);
> > +				if (len > len2)
> > +					break;
> > +			}
> > +
> > +			newc->next = c;
> > +			if (l)
> > +				l->next = newc;
> > +			else
> > +				genfs->head = newc;
> > +			newc = NULL;
> > +		}
> > +	}

Also, for safety, shouldn't we explicitly set rc = 0; here so that no
prior assignment of rc will fall through.  Regardless of whether that is
possible with the current code - just do it now for maintainability.

> > +out:
> > +	if (newgenfs)
> > +		kfree(newgenfs->fstype);
> > +	kfree(newgenfs);
> > +	ocontext_destroy(newc, OCON_FSUSE);
> > +
> > +	return rc;
> > +}
> > +
> >  /*
> >   * Read the configuration data from a policy database binary
> >   * representation file into a policy database structure.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 3/4] SELinux: break ocontext reading into a separate function
  2010-06-11 16:37 ` [PATCH 3/4] SELinux: break ocontext reading into " Eric Paris
@ 2010-06-16 14:39   ` Stephen Smalley
  0 siblings, 0 replies; 26+ messages in thread
From: Stephen Smalley @ 2010-06-16 14:39 UTC (permalink / raw)
  To: Eric Paris; +Cc: selinux, jmorris

On Fri, 2010-06-11 at 12:37 -0400, Eric Paris wrote:
> Move the reading of ocontext type data out of policydb_read() in a separate
> function ocontext_read()
> 
> Signed-off-by: Eric Paris <eparis@redhat.com>
> ---
> 
>  security/selinux/ss/policydb.c |  242 ++++++++++++++++++++++------------------
>  1 files changed, 131 insertions(+), 111 deletions(-)
> 
> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index 66e217b..99fff0c 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -1899,6 +1899,134 @@ out:
>  	return rc;
>  }
>  
> +static int ocontext_read(struct policydb *p, struct policydb_compat_info *info,
> +			 void *fp)
> +{
> +	int i, j, rc;
> +	u32 nel, len;
> +	__le32 buf[3];
> +	struct ocontext *l, *c;
> +	u32 nodebuf[8];
> +
> +	for (i = 0; i < info->ocon_num; i++) {
> +		rc = next_entry(buf, fp, sizeof(u32));
> +		if (rc)
> +			goto out;
> +		nel = le32_to_cpu(buf[0]);
> +
> +		l = NULL;
> +		for (j = 0; j < nel; j++) {
> +			rc = -ENOMEM;
> +			c = kzalloc(sizeof(*c), GFP_KERNEL);
> +			if (!c)
> +				goto out;
> +			if (l)
> +				l->next = c;
> +			else
> +				p->ocontexts[i] = c;
> +			l = c;
> +
> +			switch (i) {
> +			case OCON_ISID:
> +				rc = next_entry(buf, fp, sizeof(u32));
> +				if (rc)
> +					goto out;
> +
> +				c->sid[0] = le32_to_cpu(buf[0]);
> +				rc = context_read_and_validate(&c->context[0], p, fp);
> +				if (rc)
> +					goto out;
> +				break;
> +			case OCON_FS:
> +			case OCON_NETIF:
> +				rc = next_entry(buf, fp, sizeof(u32));
> +				if (rc)
> +					goto out;
> +				len = le32_to_cpu(buf[0]);
> +
> +				rc = -ENOMEM;
> +				c->u.name = kmalloc(len + 1, GFP_KERNEL);
> +				if (!c->u.name)
> +					goto out;
> +
> +				rc = next_entry(c->u.name, fp, len);
> +				if (rc)
> +					goto out;
> +
> +				c->u.name[len] = 0;
> +				rc = context_read_and_validate(&c->context[0], p, fp);
> +				if (rc)
> +					goto out;
> +				rc = context_read_and_validate(&c->context[1], p, fp);
> +				if (rc)
> +					goto out;
> +				break;
> +			case OCON_PORT:
> +				rc = next_entry(buf, fp, sizeof(u32)*3);
> +				if (rc)
> +					goto out;
> +				c->u.port.protocol = le32_to_cpu(buf[0]);
> +				c->u.port.low_port = le32_to_cpu(buf[1]);
> +				c->u.port.high_port = le32_to_cpu(buf[2]);
> +				rc = context_read_and_validate(&c->context[0], p, fp);
> +				if (rc)
> +					goto out;
> +				break;
> +			case OCON_NODE:
> +				rc = next_entry(nodebuf, fp, sizeof(u32) * 2);
> +				if (rc)
> +					goto out;
> +				c->u.node.addr = nodebuf[0]; /* network order */
> +				c->u.node.mask = nodebuf[1]; /* network order */
> +				rc = context_read_and_validate(&c->context[0], p, fp);
> +				if (rc)
> +					goto out;
> +				break;
> +			case OCON_FSUSE:
> +				rc = next_entry(buf, fp, sizeof(u32)*2);
> +				if (rc)
> +					goto out;
> +				c->v.behavior = le32_to_cpu(buf[0]);
> +				if (c->v.behavior > SECURITY_FS_USE_NONE)
> +					goto out;

rc is not set here.

> +
> +				rc = -ENOMEM;
> +				len = le32_to_cpu(buf[1]);
> +				c->u.name = kmalloc(len + 1, GFP_KERNEL);
> +				if (!c->u.name)
> +					goto out;
> +
> +				rc = next_entry(c->u.name, fp, len);
> +				if (rc)
> +					goto out;
> +				c->u.name[len] = 0;
> +				rc = context_read_and_validate(&c->context[0], p, fp);
> +				if (rc)
> +					goto out;
> +				break;
> +			case OCON_NODE6: {
> +				int k;
> +
> +				rc = next_entry(nodebuf, fp, sizeof(u32) * 8);
> +				if (rc)
> +					goto out;
> +				for (k = 0; k < 4; k++)
> +					c->u.node6.addr[k] = nodebuf[k];
> +				for (k = 0; k < 4; k++)
> +					c->u.node6.mask[k] = nodebuf[k+4];
> +				rc = context_read_and_validate(&c->context[0], p, fp);
> +				if (rc)
> +					goto out;
> +				break;
> +			}
> +			}
> +		}
> +	}
> +	rc = 0;
> +out:
> +	return rc;
> +}
> +
>  /*
>   * Read the configuration data from a policy database binary
>   * representation file into a policy database structure.
> @@ -1907,10 +2035,8 @@ int policydb_read(struct policydb *p, void *fp)
>  {
>  	struct role_allow *ra, *lra;
>  	struct role_trans *tr, *ltr;
> -	struct ocontext *l, *c;
>  	int i, j, rc;
>  	__le32 buf[4];
> -	u32 nodebuf[8];
>  	u32 len, nprim, nel;
>  
>  	char *policydb_str;
> @@ -2115,115 +2241,9 @@ int policydb_read(struct policydb *p, void *fp)
>  	if (!p->process_trans_perms)
>  		goto bad;
>  
> -	for (i = 0; i < info->ocon_num; i++) {
> -		rc = next_entry(buf, fp, sizeof(u32));
> -		if (rc < 0)
> -			goto bad;
> -		nel = le32_to_cpu(buf[0]);
> -		l = NULL;
> -		for (j = 0; j < nel; j++) {
> -			c = kzalloc(sizeof(*c), GFP_KERNEL);
> -			if (!c) {
> -				rc = -ENOMEM;
> -				goto bad;
> -			}
> -			if (l)
> -				l->next = c;
> -			else
> -				p->ocontexts[i] = c;
> -			l = c;
> -			rc = -EINVAL;
> -			switch (i) {
> -			case OCON_ISID:
> -				rc = next_entry(buf, fp, sizeof(u32));
> -				if (rc < 0)
> -					goto bad;
> -				c->sid[0] = le32_to_cpu(buf[0]);
> -				rc = context_read_and_validate(&c->context[0], p, fp);
> -				if (rc)
> -					goto bad;
> -				break;
> -			case OCON_FS:
> -			case OCON_NETIF:
> -				rc = next_entry(buf, fp, sizeof(u32));
> -				if (rc < 0)
> -					goto bad;
> -				len = le32_to_cpu(buf[0]);
> -				c->u.name = kmalloc(len + 1, GFP_KERNEL);
> -				if (!c->u.name) {
> -					rc = -ENOMEM;
> -					goto bad;
> -				}
> -				rc = next_entry(c->u.name, fp, len);
> -				if (rc < 0)
> -					goto bad;
> -				c->u.name[len] = 0;
> -				rc = context_read_and_validate(&c->context[0], p, fp);
> -				if (rc)
> -					goto bad;
> -				rc = context_read_and_validate(&c->context[1], p, fp);
> -				if (rc)
> -					goto bad;
> -				break;
> -			case OCON_PORT:
> -				rc = next_entry(buf, fp, sizeof(u32)*3);
> -				if (rc < 0)
> -					goto bad;
> -				c->u.port.protocol = le32_to_cpu(buf[0]);
> -				c->u.port.low_port = le32_to_cpu(buf[1]);
> -				c->u.port.high_port = le32_to_cpu(buf[2]);
> -				rc = context_read_and_validate(&c->context[0], p, fp);
> -				if (rc)
> -					goto bad;
> -				break;
> -			case OCON_NODE:
> -				rc = next_entry(nodebuf, fp, sizeof(u32) * 2);
> -				if (rc < 0)
> -					goto bad;
> -				c->u.node.addr = nodebuf[0]; /* network order */
> -				c->u.node.mask = nodebuf[1]; /* network order */
> -				rc = context_read_and_validate(&c->context[0], p, fp);
> -				if (rc)
> -					goto bad;
> -				break;
> -			case OCON_FSUSE:
> -				rc = next_entry(buf, fp, sizeof(u32)*2);
> -				if (rc < 0)
> -					goto bad;
> -				c->v.behavior = le32_to_cpu(buf[0]);
> -				if (c->v.behavior > SECURITY_FS_USE_NONE)
> -					goto bad;
> -				len = le32_to_cpu(buf[1]);
> -				c->u.name = kmalloc(len + 1, GFP_KERNEL);
> -				if (!c->u.name) {
> -					rc = -ENOMEM;
> -					goto bad;
> -				}
> -				rc = next_entry(c->u.name, fp, len);
> -				if (rc < 0)
> -					goto bad;
> -				c->u.name[len] = 0;
> -				rc = context_read_and_validate(&c->context[0], p, fp);
> -				if (rc)
> -					goto bad;
> -				break;
> -			case OCON_NODE6: {
> -				int k;
> -
> -				rc = next_entry(nodebuf, fp, sizeof(u32) * 8);
> -				if (rc < 0)
> -					goto bad;
> -				for (k = 0; k < 4; k++)
> -					c->u.node6.addr[k] = nodebuf[k];
> -				for (k = 0; k < 4; k++)
> -					c->u.node6.mask[k] = nodebuf[k+4];
> -				if (context_read_and_validate(&c->context[0], p, fp))
> -					goto bad;
> -				break;
> -			}
> -			}
> -		}
> -	}
> +	rc = ocontext_read(p, info, fp);
> +	if (rc)
> +		goto bad;
>  
>  	rc = genfs_read(p, fp);
>  	if (rc)

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 4/4] SELinux: allow userspace to read policy back out of the kernel
  2010-06-15 14:33         ` Eric Paris
@ 2010-06-16 14:53           ` Stephen Smalley
  2010-06-16 15:26             ` Eric Paris
  2010-06-17  7:26             ` KaiGai Kohei
  0 siblings, 2 replies; 26+ messages in thread
From: Stephen Smalley @ 2010-06-16 14:53 UTC (permalink / raw)
  To: Eric Paris, KaiGai Kohei; +Cc: Casey Schaufler, selinux, jmorris

On Tue, 2010-06-15 at 10:33 -0400, Eric Paris wrote:
> On Mon, 2010-06-14 at 21:42 -0700, Casey Schaufler wrote:
> > Eric Paris wrote:
> > > On Mon, 2010-06-14 at 10:48 -0400, Stephen Smalley wrote:
> > >   
> > >> On Fri, 2010-06-11 at 12:37 -0400, Eric Paris wrote:
> > >>     
> > >>> There is interest in being able to see what the actual policy is that was
> > >>> loaded into the kernel.  The patch creates a new selinuxfs file
> > >>> /selinux/policy which can be read by userspace.  The actual policy that is
> > >>> loaded into the kernel will be written back out to userspace.
> > >>>       
> > >> Why a new node vs a read op for /selinux/load?
> > >>     
> > >
> > > No reason why I couldn't.  Just 'load' seemed to imply a connotation
> > > which wasn't appropriate.  If you prefer I'll switch it when I do
> > > another version.
> > >   
> > 
> > If it makes any difference Smack /smack/load does read as well as write.
> > You have the opportunity to make 2 or 3 users less confused if you do
> > things consistently. After all, Smack uses load because SELinux does,
> > the name is actually arbitrary, and why do something differently when
> > it doesn't really matter?
> 
> I did two things yesterday.  First I switch the read
> from /selinux/policy to /selinux/load.  Then I undid that change and
> started generating the in kernel policy buffer on open() rather than on
> read().  It allowed me to use cat /etc/policy > policy rather than using
> my own half ass hacked utility.  The reason I undid the policy->load
> change was because I didn't really want to store the old policy on open
> if they were going to write() a new policy.  I can probably make the
> determination based on the f_mode, but didn't really play with it yet.
> I  try to do both in the next go-round.

Unfortunately it appears that libselinux security_load_policy() does
open("/selinux/load", O_RDWR).  Don't ask me why.

> I'm still trying to figure out what I did to make malformed policies.
> Must have screwed something up ripping out my prink's and debug hooks,
> because it isn't working for me now either....

Assuming you've just reused the userspace policydb_write() code with
minor cleanups for everything except the new ebitmap format, I'd look
more closely there. 
KaiGai - this is the first time where we need to convert the new kernel
ebitmap format back to the old one for generating a policy image from
the kernel policydb that can be compared to a policy file.
 
> Unrelated note, can we take patches 1,2,3 ?  They are just cleanups....

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 4/4] SELinux: allow userspace to read policy back out of the kernel
  2010-06-16 14:53           ` Stephen Smalley
@ 2010-06-16 15:26             ` Eric Paris
  2010-06-16 16:41               ` Stephen Smalley
  2010-06-17  7:26             ` KaiGai Kohei
  1 sibling, 1 reply; 26+ messages in thread
From: Eric Paris @ 2010-06-16 15:26 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: KaiGai Kohei, Casey Schaufler, selinux, jmorris

On Wed, 2010-06-16 at 10:53 -0400, Stephen Smalley wrote:
> On Tue, 2010-06-15 at 10:33 -0400, Eric Paris wrote:

> > I did two things yesterday.  First I switch the read
> > from /selinux/policy to /selinux/load.  Then I undid that change and
> > started generating the in kernel policy buffer on open() rather than on
> > read().  It allowed me to use cat /etc/policy > policy rather than using
> > my own half ass hacked utility.  The reason I undid the policy->load
> > change was because I didn't really want to store the old policy on open
> > if they were going to write() a new policy.  I can probably make the
> > determination based on the f_mode, but didn't really play with it yet.
> > I  try to do both in the next go-round.
> 
> Unfortunately it appears that libselinux security_load_policy() does
> open("/selinux/load", O_RDWR).  Don't ask me why.

I could still generate the policy on open() if it was opened O_RDONLY.
If it was opened O_RDWR read() I 'could' make read() work if the buf was
large enough in a single shot.  Is that quirk worth the trouble of not
creating a new node in /selinux?

> > I'm still trying to figure out what I did to make malformed policies.
> > Must have screwed something up ripping out my prink's and debug hooks,
> > because it isn't working for me now either....
> 
> Assuming you've just reused the userspace policydb_write() code with
> minor cleanups for everything except the new ebitmap format, I'd look
> more closely there. 
> KaiGai - this is the first time where we need to convert the new kernel
> ebitmap format back to the old one for generating a policy image from
> the kernel policydb that can be compared to a policy file.

No question wrapping my head around the new ebitmap format was the tough
part.  I added printk's to display every ebitmap and node as it was read
in and as I wrote them out.  Got the same thing for the couple thousand
lines I could show in dmesg, so I think I'm ok there.

I was trying to use gdb yesterday to figure out what was wrong, but
could get the darn thing to break where I wanted it to.  I'll debug like
I'm used to (in the kernel) and see what I did....

-Eric


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 4/4] SELinux: allow userspace to read policy back out of the kernel
  2010-06-16 15:26             ` Eric Paris
@ 2010-06-16 16:41               ` Stephen Smalley
  2010-06-16 16:58                 ` Eric Paris
  0 siblings, 1 reply; 26+ messages in thread
From: Stephen Smalley @ 2010-06-16 16:41 UTC (permalink / raw)
  To: Eric Paris; +Cc: KaiGai Kohei, Casey Schaufler, selinux, jmorris

On Wed, 2010-06-16 at 11:26 -0400, Eric Paris wrote:
> On Wed, 2010-06-16 at 10:53 -0400, Stephen Smalley wrote:
> > On Tue, 2010-06-15 at 10:33 -0400, Eric Paris wrote:
> 
> > > I did two things yesterday.  First I switch the read
> > > from /selinux/policy to /selinux/load.  Then I undid that change and
> > > started generating the in kernel policy buffer on open() rather than on
> > > read().  It allowed me to use cat /etc/policy > policy rather than using
> > > my own half ass hacked utility.  The reason I undid the policy->load
> > > change was because I didn't really want to store the old policy on open
> > > if they were going to write() a new policy.  I can probably make the
> > > determination based on the f_mode, but didn't really play with it yet.
> > > I  try to do both in the next go-round.
> > 
> > Unfortunately it appears that libselinux security_load_policy() does
> > open("/selinux/load", O_RDWR).  Don't ask me why.
> 
> I could still generate the policy on open() if it was opened O_RDONLY.
> If it was opened O_RDWR read() I 'could' make read() work if the buf was
> large enough in a single shot.  Is that quirk worth the trouble of not
> creating a new node in /selinux?

Probably not.

> > > I'm still trying to figure out what I did to make malformed policies.
> > > Must have screwed something up ripping out my prink's and debug hooks,
> > > because it isn't working for me now either....
> > 
> > Assuming you've just reused the userspace policydb_write() code with
> > minor cleanups for everything except the new ebitmap format, I'd look
> > more closely there. 
> > KaiGai - this is the first time where we need to convert the new kernel
> > ebitmap format back to the old one for generating a policy image from
> > the kernel policydb that can be compared to a policy file.
> 
> No question wrapping my head around the new ebitmap format was the tough
> part.  I added printk's to display every ebitmap and node as it was read
> in and as I wrote them out.  Got the same thing for the couple thousand
> lines I could show in dmesg, so I think I'm ok there.
> 
> I was trying to use gdb yesterday to figure out what was wrong, but
> could get the darn thing to break where I wanted it to.  I'll debug like
> I'm used to (in the kernel) and see what I did....

Oh, I found it.  mls_write_level() in your patch sets buf[1] rather than
buf[0] and then writes buf[0].


-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 4/4] SELinux: allow userspace to read policy back out of the kernel
  2010-06-16 16:41               ` Stephen Smalley
@ 2010-06-16 16:58                 ` Eric Paris
  0 siblings, 0 replies; 26+ messages in thread
From: Eric Paris @ 2010-06-16 16:58 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: KaiGai Kohei, Casey Schaufler, selinux, jmorris

On Wed, 2010-06-16 at 12:41 -0400, Stephen Smalley wrote:
> On Wed, 2010-06-16 at 11:26 -0400, Eric Paris wrote:

> > No question wrapping my head around the new ebitmap format was the tough
> > part.  I added printk's to display every ebitmap and node as it was read
> > in and as I wrote them out.  Got the same thing for the couple thousand
> > lines I could show in dmesg, so I think I'm ok there.
> > 
> > I was trying to use gdb yesterday to figure out what was wrong, but
> > could get the darn thing to break where I wanted it to.  I'll debug like
> > I'm used to (in the kernel) and see what I did....
> 
> Oh, I found it.  mls_write_level() in your patch sets buf[1] rather than
> buf[0] and then writes buf[0].

/me feels like the compiler should have been able to find that
particular one....


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/4] SELinux: seperate range transition rules to a seperate function
  2010-06-11 16:37 [PATCH 1/4] SELinux: seperate range transition rules to a seperate function Eric Paris
                   ` (3 preceding siblings ...)
  2010-06-16 13:02 ` [PATCH 1/4] SELinux: seperate range transition rules to a seperate function Stephen Smalley
@ 2010-06-17  5:02 ` James Morris
  4 siblings, 0 replies; 26+ messages in thread
From: James Morris @ 2010-06-17  5:02 UTC (permalink / raw)
  To: Eric Paris; +Cc: selinux, sds

On Fri, 11 Jun 2010, Eric Paris wrote:

> Move the range transition rule to a separate function, range_read(), rather
> than doing it all in policydb_read()
> 
> Signed-off-by: Eric Paris <eparis@redhat.com>

Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next

-- 
James Morris
<jmorris@namei.org>

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 4/4] SELinux: allow userspace to read policy back out of the kernel
  2010-06-16 14:53           ` Stephen Smalley
  2010-06-16 15:26             ` Eric Paris
@ 2010-06-17  7:26             ` KaiGai Kohei
  2010-06-17 14:51               ` Eric Paris
  1 sibling, 1 reply; 26+ messages in thread
From: KaiGai Kohei @ 2010-06-17  7:26 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: Eric Paris, KaiGai Kohei, Casey Schaufler, selinux, jmorris

(2010/06/16 23:53), Stephen Smalley wrote:
> On Tue, 2010-06-15 at 10:33 -0400, Eric Paris wrote:
>> On Mon, 2010-06-14 at 21:42 -0700, Casey Schaufler wrote:
>>> Eric Paris wrote:
>>>> On Mon, 2010-06-14 at 10:48 -0400, Stephen Smalley wrote:
>>>>
>>>>> On Fri, 2010-06-11 at 12:37 -0400, Eric Paris wrote:
>>>>>
>>>>>> There is interest in being able to see what the actual policy is that was
>>>>>> loaded into the kernel.  The patch creates a new selinuxfs file
>>>>>> /selinux/policy which can be read by userspace.  The actual policy that is
>>>>>> loaded into the kernel will be written back out to userspace.
>>>>>>
>>>>> Why a new node vs a read op for /selinux/load?
>>>>>
>>>>
>>>> No reason why I couldn't.  Just 'load' seemed to imply a connotation
>>>> which wasn't appropriate.  If you prefer I'll switch it when I do
>>>> another version.
>>>>
>>>
>>> If it makes any difference Smack /smack/load does read as well as write.
>>> You have the opportunity to make 2 or 3 users less confused if you do
>>> things consistently. After all, Smack uses load because SELinux does,
>>> the name is actually arbitrary, and why do something differently when
>>> it doesn't really matter?
>>
>> I did two things yesterday.  First I switch the read
>> from /selinux/policy to /selinux/load.  Then I undid that change and
>> started generating the in kernel policy buffer on open() rather than on
>> read().  It allowed me to use cat /etc/policy>  policy rather than using
>> my own half ass hacked utility.  The reason I undid the policy->load
>> change was because I didn't really want to store the old policy on open
>> if they were going to write() a new policy.  I can probably make the
>> determination based on the f_mode, but didn't really play with it yet.
>> I  try to do both in the next go-round.
> 
> Unfortunately it appears that libselinux security_load_policy() does
> open("/selinux/load", O_RDWR).  Don't ask me why.
> 
>> I'm still trying to figure out what I did to make malformed policies.
>> Must have screwed something up ripping out my prink's and debug hooks,
>> because it isn't working for me now either....
> 
> Assuming you've just reused the userspace policydb_write() code with
> minor cleanups for everything except the new ebitmap format, I'd look
> more closely there.
> KaiGai - this is the first time where we need to convert the new kernel
> ebitmap format back to the old one for generating a policy image from
> the kernel policydb that can be compared to a policy file.
> 
I have a question. Is it necessary the following command being succeeded?

  % diff /etc/selinux/targeted/policy/policy.24 /selinux/load

Although it was a few years ago I worked for the ebitmap improvement,
it seems to me the new ebitmap_write() correctly writes back entries
of the given ebitmap object.

However, it is unclear for me whether this routine can correctly repair
the original file image, or not.
For example, if startbit of each ebitmap_node was not aligned to u64
on the policy file, it will be fixed up on ebitmap_read(), so we lost
an information that what was the actual startbit of ebitmap_node in the
result.

Of course, it might be too much requirement, if we don't need the above
diff command returns success.

Thanks,
-- 
KaiGai Kohei <kaigai@ak.jp.nec.com>

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 4/4] SELinux: allow userspace to read policy back out of the kernel
  2010-06-17  7:26             ` KaiGai Kohei
@ 2010-06-17 14:51               ` Eric Paris
  0 siblings, 0 replies; 26+ messages in thread
From: Eric Paris @ 2010-06-17 14:51 UTC (permalink / raw)
  To: KaiGai Kohei
  Cc: Stephen Smalley, KaiGai Kohei, Casey Schaufler, selinux, jmorris

On Thu, 2010-06-17 at 16:26 +0900, KaiGai Kohei wrote:
> (2010/06/16 23:53), Stephen Smalley wrote:
> > On Tue, 2010-06-15 at 10:33 -0400, Eric Paris wrote:
> >> On Mon, 2010-06-14 at 21:42 -0700, Casey Schaufler wrote:
> >>> Eric Paris wrote:
> >>>> On Mon, 2010-06-14 at 10:48 -0400, Stephen Smalley wrote:
> >>>>
> >>>>> On Fri, 2010-06-11 at 12:37 -0400, Eric Paris wrote:
> >>>>>
> >>>>>> There is interest in being able to see what the actual policy is that was
> >>>>>> loaded into the kernel.  The patch creates a new selinuxfs file
> >>>>>> /selinux/policy which can be read by userspace.  The actual policy that is
> >>>>>> loaded into the kernel will be written back out to userspace.
> >>>>>>
> >>>>> Why a new node vs a read op for /selinux/load?
> >>>>>
> >>>>
> >>>> No reason why I couldn't.  Just 'load' seemed to imply a connotation
> >>>> which wasn't appropriate.  If you prefer I'll switch it when I do
> >>>> another version.
> >>>>
> >>>
> >>> If it makes any difference Smack /smack/load does read as well as write.
> >>> You have the opportunity to make 2 or 3 users less confused if you do
> >>> things consistently. After all, Smack uses load because SELinux does,
> >>> the name is actually arbitrary, and why do something differently when
> >>> it doesn't really matter?
> >>
> >> I did two things yesterday.  First I switch the read
> >> from /selinux/policy to /selinux/load.  Then I undid that change and
> >> started generating the in kernel policy buffer on open() rather than on
> >> read().  It allowed me to use cat /etc/policy>  policy rather than using
> >> my own half ass hacked utility.  The reason I undid the policy->load
> >> change was because I didn't really want to store the old policy on open
> >> if they were going to write() a new policy.  I can probably make the
> >> determination based on the f_mode, but didn't really play with it yet.
> >> I  try to do both in the next go-round.
> > 
> > Unfortunately it appears that libselinux security_load_policy() does
> > open("/selinux/load", O_RDWR).  Don't ask me why.
> > 
> >> I'm still trying to figure out what I did to make malformed policies.
> >> Must have screwed something up ripping out my prink's and debug hooks,
> >> because it isn't working for me now either....
> > 
> > Assuming you've just reused the userspace policydb_write() code with
> > minor cleanups for everything except the new ebitmap format, I'd look
> > more closely there.
> > KaiGai - this is the first time where we need to convert the new kernel
> > ebitmap format back to the old one for generating a policy image from
> > the kernel policydb that can be compared to a policy file.
> > 
> I have a question. Is it necessary the following command being succeeded?
> 
>   % diff /etc/selinux/targeted/policy/policy.24 /selinux/load
> 
> Although it was a few years ago I worked for the ebitmap improvement,
> it seems to me the new ebitmap_write() correctly writes back entries
> of the given ebitmap object.
> 
> However, it is unclear for me whether this routine can correctly repair
> the original file image, or not.
> For example, if startbit of each ebitmap_node was not aligned to u64
> on the policy file, it will be fixed up on ebitmap_read(), so we lost
> an information that what was the actual startbit of ebitmap_node in the
> result.
> 
> Of course, it might be too much requirement, if we don't need the above
> diff command returns success.

They do NOT need to be the same (because I deem it so).  We already know
that range transition rules will likely be reordered in /selinux/load vs
policy.24 so minor non-meaningful differences in ebitmaps should not be
a problem.

If at some point a person has a reason to require they be the same
binary I suggest we change userspace rather than try to maintain
needless quirks in the kernel.

-Eric


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 4/4] SELinux: allow userspace to read policy back out of the kernel
  2010-06-14 17:55         ` Eric Paris
  2010-06-14 18:04           ` Stephen Smalley
@ 2010-06-18 12:01           ` Christopher J. PeBenito
  1 sibling, 0 replies; 26+ messages in thread
From: Christopher J. PeBenito @ 2010-06-18 12:01 UTC (permalink / raw)
  To: Eric Paris; +Cc: Stephen Smalley, selinux, jmorris

On Mon, 2010-06-14 at 13:55 -0400, Eric Paris wrote:
> On Mon, 2010-06-14 at 12:14 -0400, Stephen Smalley wrote:
> > On Mon, 2010-06-14 at 11:24 -0400, Eric Paris wrote:
> > > On Mon, 2010-06-14 at 10:57 -0400, Stephen Smalley wrote:
> > > > On Fri, 2010-06-11 at 12:37 -0400, Eric Paris wrote:
> > > > > There is interest in being able to see what the actual policy is that was
> > > > > loaded into the kernel.  The patch creates a new selinuxfs file
> > > > > /selinux/policy which can be read by userspace.  The actual policy that is
> > > > > loaded into the kernel will be written back out to userspace.
> > > > 
> > > > How do you expect this to be used?  As with /selinux/load, we can't use
> > > > coreutils utilities to manipulate it unfortunately.  Nor can we do
> > > > things like checkpolicy -b /selinux/policy since it doesn't support
> > > > mmap.
> > > 
> > > I used my own program to pull it out to a file and poke it after it was
> > > out.  I can certainly take a look at generating the policy on open()
> > > which would allow us to support ppos easily (and maybe mmap, but I've
> > > never written an mmap handler)
> > 
> > Hmm...the resulting policy.from.kern doesn't match the binary policy
> > file that was loaded, nor is it a well-formed policy.
> 
> It won't be a binary perfect match since we switched range transition
> rules to a hashtab and we lose ordering in the kernel.  (although the
> second load and resulting read should be the same binary policy)
> 
> What is not well-formed about your result?  I got back the same policy
> (according to sediff) but I was using selinux-policy-minimum....

I'm just catching up, but a word of warning: sediff does not diff
constraints yet.

-- 
Chris PeBenito
Tresys Technology, LLC
www.tresys.com | oss.tresys.com


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2010-06-18 12:01 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-06-11 16:37 [PATCH 1/4] SELinux: seperate range transition rules to a seperate function Eric Paris
2010-06-11 16:37 ` [PATCH 2/4] SELinux: move genfs read to a separate function Eric Paris
2010-06-16 14:18   ` Stephen Smalley
2010-06-16 14:24     ` Stephen Smalley
2010-06-11 16:37 ` [PATCH 3/4] SELinux: break ocontext reading into " Eric Paris
2010-06-16 14:39   ` Stephen Smalley
2010-06-11 16:37 ` [PATCH 4/4] SELinux: allow userspace to read policy back out of the kernel Eric Paris
2010-06-14 14:48   ` Stephen Smalley
2010-06-14 15:12     ` Eric Paris
2010-06-15  4:42       ` Casey Schaufler
2010-06-15 14:33         ` Eric Paris
2010-06-16 14:53           ` Stephen Smalley
2010-06-16 15:26             ` Eric Paris
2010-06-16 16:41               ` Stephen Smalley
2010-06-16 16:58                 ` Eric Paris
2010-06-17  7:26             ` KaiGai Kohei
2010-06-17 14:51               ` Eric Paris
2010-06-14 14:57   ` Stephen Smalley
2010-06-14 14:59     ` Stephen Smalley
2010-06-14 15:24     ` Eric Paris
2010-06-14 16:14       ` Stephen Smalley
2010-06-14 17:55         ` Eric Paris
2010-06-14 18:04           ` Stephen Smalley
2010-06-18 12:01           ` Christopher J. PeBenito
2010-06-16 13:02 ` [PATCH 1/4] SELinux: seperate range transition rules to a seperate function Stephen Smalley
2010-06-17  5:02 ` James Morris

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.