SELinux: seperate range transition rules to a seperate function

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

Signed-off-by Eric Paris <[email protected]>

---

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

security/selinux/ss/policydb.c +89 -0

index a39d38a..66e217b 100644
@@ -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)

security/selinux/ss/policydb.c +29 -105

+				}
+				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;

Quoted 106 lines · Eric Paris

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

Signed-off-by Eric Paris <[email protected]>

---

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

security/selinux/ss/policydb.c +89 -0

index a39d38a..66e217b 100644
@@ -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)

security/selinux/ss/policydb.c out: · line 1877

Quoted 10 lines

security/selinux/ss/policydb.c +10 -0

+				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.

Quoted 5 lines

security/selinux/ss/policydb.c +5 -0

+				}
+				len = strlen(newc->u.name);
+				len2 = strlen(c->u.name);
+				if (len > len2)
+					break;

security/selinux/ss/policydb.c out: · line 1892

Quoted 10 lines

security/selinux/ss/policydb.c +10 -0

+			}
+
+			newc->next = c;
+			if (l)
+				l->next = newc;
+			else
+				genfs->head = newc;
+			newc = NULL;
+		}
+	}

Quoted 12 lines

security/selinux/ss/policydb.c +9 -0

+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.

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

Signed-off-by Eric Paris <[email protected]>

---

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

security/selinux/ss/policydb.c +80 -0

index 66e217b..99fff0c 100644
@@ -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);

security/selinux/ss/policydb.c +41 -111

+
+				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)

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 <[email protected]>

---

security/selinux/ss/services.c +14 -0

index 1de60ce..2e022db 100644
@@ -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;

Quoted 4 lines · Eric Paris

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?

Quoted 31 lines

Signed-off-by Eric Paris <[email protected]>

---

security/selinux/ss/services.c +18 -0

index 1de60ce..2e022db 100644
@@ -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);

security/selinux/ss/services.c netlbl_sid_to_secattr_failure: · line 3151

Quoted 6 lines

security/selinux/ss/services.c +6 -0

+
+	*len = (unsigned long)fp->data - (unsigned long)data;
+
+	return rc;
+
+}

Why _irq?

Quoted 7 lines · Stephen Smalley

Quoted 5 lines · Eric Paris

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.

Quoted 2 lines

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

Quoted 30 lines

Quoted 30 lines

Signed-off-by Eric Paris <[email protected]>

---

security/selinux/ss/services.c +18 -0

index 1de60ce..2e022db 100644
@@ -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);

Quoted 9 lines

security/selinux/ss/services.c netlbl_sid_to_secattr_failure: · line 3151

Quoted 7 lines

security/selinux/ss/services.c +6 -0

+
+	*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 [email protected] with the words "unsubscribe selinux" without quotes as the message.

Quoted 9 lines · Eric Paris

Quoted 7 lines · Stephen Smalley

Quoted 5 lines · Eric Paris

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.

Quoted 7 lines

Quoted 2 lines

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?

Quoted 18 lines · Casey Schaufler

Quoted 9 lines · Eric Paris

Quoted 7 lines · Stephen Smalley

Quoted 5 lines · Eric Paris

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.

Quoted 7 lines

Quoted 2 lines

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.

Quoted 5 lines

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....

Quoted 9 lines · Eric Paris

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.

Quoted 3 lines

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.

Quoted 12 lines · Stephen Smalley

Quoted 9 lines · Eric Paris

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.

Quoted 2 lines

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?

Quoted 10 lines

Quoted 3 lines

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....

Quoted 5 lines · Eric Paris

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.

Quoted 3 lines

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].

Quoted 25 lines · Eric Paris

Quoted 18 lines · Casey Schaufler

Quoted 9 lines · Eric Paris

Quoted 7 lines · Stephen Smalley

Quoted 5 lines · Eric Paris

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.

Quoted 7 lines

Quoted 2 lines

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.

Quoted 5 lines

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?

Quoted 9 lines

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.

Quoted 3 lines

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.

Quoted 43 lines · Stephen Smalley

Quoted 25 lines · Eric Paris

Quoted 18 lines · Casey Schaufler

Quoted 9 lines · Eric Paris

Quoted 7 lines · Stephen Smalley

Quoted 5 lines · Eric Paris

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.

Quoted 7 lines

Quoted 2 lines

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.

Quoted 5 lines

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?

Quoted 9 lines

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.

Quoted 3 lines

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....

Quoted 7 lines

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.

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.

Quoted 4 lines · Eric Paris

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.

Quoted 4 lines · Eric Paris

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.

Quoted 10 lines · Stephen Smalley

Quoted 4 lines · Eric Paris

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)

Quoted 12 lines · Eric Paris

Quoted 10 lines · Stephen Smalley

Quoted 4 lines · Eric Paris

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.

Quoted 4 lines

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.

Quoted 13 lines · Stephen Smalley

Quoted 12 lines · Eric Paris

Quoted 10 lines · Stephen Smalley

Quoted 4 lines · Eric Paris

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.

Quoted 7 lines

Quoted 4 lines

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....

Quoted 13 lines · Stephen Smalley

Quoted 12 lines · Eric Paris

Quoted 10 lines · Stephen Smalley

Quoted 4 lines · Eric Paris

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.

Quoted 7 lines

Quoted 4 lines

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....

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

Signed-off-by Eric Paris <[email protected]>

---

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

security/selinux/ss/policydb.c +75 -64

index c57802a..a39d38a 100644
@@ -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)

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

Signed-off-by Eric Paris <[email protected]>