SELinux: Update policy version to support constraints info

SELinux: Update policy version to support constraints info

[Eric Paris]

From: Richard Haines <richard_c_haines@btinternet.com>
Date: Tue, 18 Dec 2012 19:37:46 +0000

Update the policy version (POLICYDB_VERSION_CONSTRAINT_NAMES) to allow
holding of policy source info for constraints.

Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
---
 security/selinux/include/security.h |    3 +-
 security/selinux/ss/constraint.h    |    1 +
 security/selinux/ss/policydb.c      |  100 ++++++++++++++++++++++++++++++++---
 security/selinux/ss/policydb.h      |   11 ++++
 4 files changed, 107 insertions(+), 8 deletions(-)

diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index 927fc14..bdf0a56 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -33,13 +33,14 @@
 #define POLICYDB_VERSION_ROLETRANS	26
 #define POLICYDB_VERSION_NEW_OBJECT_DEFAULTS	27
 #define POLICYDB_VERSION_DEFAULT_TYPE	28
+#define POLICYDB_VERSION_CONSTRAINT_NAMES	29
 
 /* Range of policy versions we understand*/
 #define POLICYDB_VERSION_MIN   POLICYDB_VERSION_BASE
 #ifdef CONFIG_SECURITY_SELINUX_POLICYDB_VERSION_MAX
 #define POLICYDB_VERSION_MAX	CONFIG_SECURITY_SELINUX_POLICYDB_VERSION_MAX_VALUE
 #else
-#define POLICYDB_VERSION_MAX	POLICYDB_VERSION_DEFAULT_TYPE
+#define POLICYDB_VERSION_MAX	POLICYDB_VERSION_CONSTRAINT_NAMES
 #endif
 
 /* Mask for just the mount related flags */
diff --git a/security/selinux/ss/constraint.h b/security/selinux/ss/constraint.h
index 149dda7..3855ea8 100644
--- a/security/selinux/ss/constraint.h
+++ b/security/selinux/ss/constraint.h
@@ -48,6 +48,7 @@ struct constraint_expr {
 	u32 op;			/* operator */
 
 	struct ebitmap names;	/* names */
+	struct type_set_datum *type_names;
 
 	struct constraint_expr *next;   /* next expression */
 };
diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index 9cd9b7c..f1b23df 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -143,6 +143,11 @@ static struct policydb_compat_info policydb_compat[] = {
 		.sym_num	= SYM_NUM,
 		.ocon_num	= OCON_NUM,
 	},
+	{
+		.version	= POLICYDB_VERSION_CONSTRAINT_NAMES,
+		.sym_num	= SYM_NUM,
+		.ocon_num	= OCON_NUM,
+	},
 };
 
 static struct policydb_compat_info *policydb_lookup_compat(int version)
@@ -613,11 +618,20 @@ static int common_destroy(void *key, void *datum, void *p)
 	return 0;
 }
 
-static int cls_destroy(void *key, void *datum, void *p)
+static void type_set_destroy(struct type_set_datum * t)
+{
+	if (t != NULL) {
+		ebitmap_destroy(&t->types);
+		ebitmap_destroy(&t->negset);
+	}
+}
+
+static int cls_destroy(void *key, void *datum, void *datap)
 {
 	struct class_datum *cladatum;
 	struct constraint_node *constraint, *ctemp;
 	struct constraint_expr *e, *etmp;
+	struct policydb *p = datap;
 
 	kfree(key);
 	if (datum) {
@@ -629,6 +643,10 @@ static int cls_destroy(void *key, void *datum, void *p)
 			e = constraint->expr;
 			while (e) {
 				ebitmap_destroy(&e->names);
+				if (p->policyvers >= POLICYDB_VERSION_CONSTRAINT_NAMES) {
+					type_set_destroy(e->type_names);
+					kfree(e->type_names);
+				}
 				etmp = e;
 				e = e->next;
 				kfree(etmp);
@@ -643,6 +661,10 @@ static int cls_destroy(void *key, void *datum, void *p)
 			e = constraint->expr;
 			while (e) {
 				ebitmap_destroy(&e->names);
+				if (p->policyvers >= POLICYDB_VERSION_CONSTRAINT_NAMES) {
+					type_set_destroy(e->type_names);
+					kfree(e->type_names);
+				}
 				etmp = e;
 				e = e->next;
 				kfree(etmp);
@@ -775,7 +797,7 @@ void policydb_destroy(struct policydb *p)
 
 	for (i = 0; i < SYM_NUM; i++) {
 		cond_resched();
-		hashtab_map(p->symtab[i].table, destroy_f[i], NULL);
+		hashtab_map(p->symtab[i].table, destroy_f[i], p);
 		hashtab_destroy(p->symtab[i].table);
 	}
 
@@ -1156,8 +1178,34 @@ bad:
 	return rc;
 }
 
-static int read_cons_helper(struct constraint_node **nodep, int ncons,
-			    int allowxtarget, void *fp)
+static void type_set_init(struct type_set_datum * t)
+{
+	ebitmap_init(&t->types);
+	ebitmap_init(&t->negset);
+}
+
+static int type_set_read(struct type_set_datum * t, void *fp)
+{
+	__le32 buf[1];
+	int rc;
+
+	if (ebitmap_read(&t->types, fp))
+		return -EINVAL;
+	if (ebitmap_read(&t->negset, fp))
+		return -EINVAL;
+
+	rc = next_entry(buf, fp, sizeof(u32));
+	if (rc < 0)
+		return -EINVAL;
+	t->flags = le32_to_cpu(buf[0]);
+
+	return 0;
+}
+
+
+static int read_cons_helper(struct policydb *p,
+				struct constraint_node **nodep,
+				int ncons, int allowxtarget, void *fp)
 {
 	struct constraint_node *c, *lc;
 	struct constraint_expr *e, *le;
@@ -1196,6 +1244,7 @@ static int read_cons_helper(struct constraint_node **nodep, int ncons,
 			rc = next_entry(buf, fp, (sizeof(u32) * 3));
 			if (rc)
 				return rc;
+
 			e->expr_type = le32_to_cpu(buf[0]);
 			e->attr = le32_to_cpu(buf[1]);
 			e->op = le32_to_cpu(buf[2]);
@@ -1225,6 +1274,17 @@ static int read_cons_helper(struct constraint_node **nodep, int ncons,
 				rc = ebitmap_read(&e->names, fp);
 				if (rc)
 					return rc;
+
+				if (p->policyvers >= POLICYDB_VERSION_CONSTRAINT_NAMES) {
+					e->type_names = kzalloc(sizeof(*e->type_names), GFP_KERNEL);
+					if (!e->type_names)
+						return -ENOMEM;
+
+					type_set_init(e->type_names);
+					rc = type_set_read(e->type_names, fp);
+					if (rc)
+						return rc;
+				}
 				break;
 			default:
 				return -EINVAL;
@@ -1233,6 +1293,7 @@ static int read_cons_helper(struct constraint_node **nodep, int ncons,
 		}
 		if (depth != 0)
 			return -EINVAL;
+
 		lc = c;
 	}
 
@@ -1301,7 +1362,7 @@ static int class_read(struct policydb *p, struct hashtab *h, void *fp)
 			goto bad;
 	}
 
-	rc = read_cons_helper(&cladatum->constraints, ncons, 0, fp);
+	rc = read_cons_helper(p, &cladatum->constraints, ncons, 0, fp);
 	if (rc)
 		goto bad;
 
@@ -1311,7 +1372,7 @@ static int class_read(struct policydb *p, struct hashtab *h, void *fp)
 		if (rc)
 			goto bad;
 		ncons = le32_to_cpu(buf[0]);
-		rc = read_cons_helper(&cladatum->validatetrans, ncons, 1, fp);
+		rc = read_cons_helper(p, &cladatum->validatetrans, ncons, 1, fp);
 		if (rc)
 			goto bad;
 	}
@@ -1339,7 +1400,7 @@ static int class_read(struct policydb *p, struct hashtab *h, void *fp)
 
 	return 0;
 bad:
-	cls_destroy(key, cladatum, NULL);
+	cls_destroy(key, cladatum, p);
 	return rc;
 }
 
@@ -2750,6 +2811,24 @@ static int common_write(void *vkey, void *datum, void *ptr)
 	return 0;
 }
 
+static int type_set_write(struct type_set_datum * t, void *fp)
+{
+	int rc;
+	__le32 buf[1];
+
+	if (ebitmap_write(&t->types, fp))
+		return -EINVAL;
+	if (ebitmap_write(&t->negset, fp))
+		return -EINVAL;
+
+	buf[0] = cpu_to_le32(t->flags);
+	rc = put_entry(buf, sizeof(u32), 1, fp);
+	if (rc)
+		return -EINVAL;
+
+	return 0;
+}
+
 static int write_cons_helper(struct policydb *p, struct constraint_node *node,
 			     void *fp)
 {
@@ -2781,6 +2860,13 @@ static int write_cons_helper(struct policydb *p, struct constraint_node *node,
 				rc = ebitmap_write(&e->names, fp);
 				if (rc)
 					return rc;
+
+				if (p->policyvers >= POLICYDB_VERSION_CONSTRAINT_NAMES) {
+					rc = type_set_write(e->type_names, fp);
+					if (rc)
+						return rc;
+				}
+
 				break;
 			default:
 				break;
diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
index da63747..c0bb6cb 100644
--- a/security/selinux/ss/policydb.h
+++ b/security/selinux/ss/policydb.h
@@ -154,6 +154,17 @@ struct cond_bool_datum {
 struct cond_node;
 
 /*
+ * type set preserves data needed to determine constraint info from
+ * policy source. This is not used by the kernel policy but allows
+ * utilities such as audit2allow to determine constraint denials.
+ */
+struct type_set_datum {
+	struct ebitmap types;
+	struct ebitmap negset;
+	u32 flags;
+};
+
+/*
  * The configuration data includes security contexts for
  * initial SIDs, unlabeled file systems, TCP and UDP port numbers,
  * network interfaces, and nodes.  This structure stores the
-- 
1.7.7.6




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

Re: SELinux: Update policy version to support constraints info

[Stephen Smalley]

On 10/21/2013 07:32 PM, Eric Paris wrote:
> From: Richard Haines <richard_c_haines@btinternet.com>
> Date: Tue, 18 Dec 2012 19:37:46 +0000
> 
> Update the policy version (POLICYDB_VERSION_CONSTRAINT_NAMES) to allow
> holding of policy source info for constraints.

Before we get into code review, let me ask the following:
Why do we need a new kernel binary policy version for information that
is only used by userspace?  Why can't this information be extracted from
the policy module format or stored in an auxiliary file used only by
userspace?

And then on a side note, why wasn't this patch posted to selinux list
for review before being included in Fedora?  Or was it and I just missed
it?  Introducing a new policy version is a big deal, and should never
happen without upstream review.  Fedora gets to eat the pain if we chose
not to support this and reuse policy version 29 for something else entirely.

> 
> Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
> ---
>  security/selinux/include/security.h |    3 +-
>  security/selinux/ss/constraint.h    |    1 +
>  security/selinux/ss/policydb.c      |  100 ++++++++++++++++++++++++++++++++---
>  security/selinux/ss/policydb.h      |   11 ++++
>  4 files changed, 107 insertions(+), 8 deletions(-)
> 
> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> index 927fc14..bdf0a56 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -33,13 +33,14 @@
>  #define POLICYDB_VERSION_ROLETRANS	26
>  #define POLICYDB_VERSION_NEW_OBJECT_DEFAULTS	27
>  #define POLICYDB_VERSION_DEFAULT_TYPE	28
> +#define POLICYDB_VERSION_CONSTRAINT_NAMES	29
>  
>  /* Range of policy versions we understand*/
>  #define POLICYDB_VERSION_MIN   POLICYDB_VERSION_BASE
>  #ifdef CONFIG_SECURITY_SELINUX_POLICYDB_VERSION_MAX
>  #define POLICYDB_VERSION_MAX	CONFIG_SECURITY_SELINUX_POLICYDB_VERSION_MAX_VALUE
>  #else
> -#define POLICYDB_VERSION_MAX	POLICYDB_VERSION_DEFAULT_TYPE
> +#define POLICYDB_VERSION_MAX	POLICYDB_VERSION_CONSTRAINT_NAMES
>  #endif
>  
>  /* Mask for just the mount related flags */
> diff --git a/security/selinux/ss/constraint.h b/security/selinux/ss/constraint.h
> index 149dda7..3855ea8 100644
> --- a/security/selinux/ss/constraint.h
> +++ b/security/selinux/ss/constraint.h
> @@ -48,6 +48,7 @@ struct constraint_expr {
>  	u32 op;			/* operator */
>  
>  	struct ebitmap names;	/* names */
> +	struct type_set_datum *type_names;
>  
>  	struct constraint_expr *next;   /* next expression */
>  };
> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index 9cd9b7c..f1b23df 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -143,6 +143,11 @@ static struct policydb_compat_info policydb_compat[] = {
>  		.sym_num	= SYM_NUM,
>  		.ocon_num	= OCON_NUM,
>  	},
> +	{
> +		.version	= POLICYDB_VERSION_CONSTRAINT_NAMES,
> +		.sym_num	= SYM_NUM,
> +		.ocon_num	= OCON_NUM,
> +	},
>  };
>  
>  static struct policydb_compat_info *policydb_lookup_compat(int version)
> @@ -613,11 +618,20 @@ static int common_destroy(void *key, void *datum, void *p)
>  	return 0;
>  }
>  
> -static int cls_destroy(void *key, void *datum, void *p)
> +static void type_set_destroy(struct type_set_datum * t)
> +{
> +	if (t != NULL) {
> +		ebitmap_destroy(&t->types);
> +		ebitmap_destroy(&t->negset);
> +	}
> +}
> +
> +static int cls_destroy(void *key, void *datum, void *datap)
>  {
>  	struct class_datum *cladatum;
>  	struct constraint_node *constraint, *ctemp;
>  	struct constraint_expr *e, *etmp;
> +	struct policydb *p = datap;
>  
>  	kfree(key);
>  	if (datum) {
> @@ -629,6 +643,10 @@ static int cls_destroy(void *key, void *datum, void *p)
>  			e = constraint->expr;
>  			while (e) {
>  				ebitmap_destroy(&e->names);
> +				if (p->policyvers >= POLICYDB_VERSION_CONSTRAINT_NAMES) {
> +					type_set_destroy(e->type_names);
> +					kfree(e->type_names);
> +				}
>  				etmp = e;
>  				e = e->next;
>  				kfree(etmp);
> @@ -643,6 +661,10 @@ static int cls_destroy(void *key, void *datum, void *p)
>  			e = constraint->expr;
>  			while (e) {
>  				ebitmap_destroy(&e->names);
> +				if (p->policyvers >= POLICYDB_VERSION_CONSTRAINT_NAMES) {
> +					type_set_destroy(e->type_names);
> +					kfree(e->type_names);
> +				}
>  				etmp = e;
>  				e = e->next;
>  				kfree(etmp);
> @@ -775,7 +797,7 @@ void policydb_destroy(struct policydb *p)
>  
>  	for (i = 0; i < SYM_NUM; i++) {
>  		cond_resched();
> -		hashtab_map(p->symtab[i].table, destroy_f[i], NULL);
> +		hashtab_map(p->symtab[i].table, destroy_f[i], p);
>  		hashtab_destroy(p->symtab[i].table);
>  	}
>  
> @@ -1156,8 +1178,34 @@ bad:
>  	return rc;
>  }
>  
> -static int read_cons_helper(struct constraint_node **nodep, int ncons,
> -			    int allowxtarget, void *fp)
> +static void type_set_init(struct type_set_datum * t)
> +{
> +	ebitmap_init(&t->types);
> +	ebitmap_init(&t->negset);
> +}
> +
> +static int type_set_read(struct type_set_datum * t, void *fp)
> +{
> +	__le32 buf[1];
> +	int rc;
> +
> +	if (ebitmap_read(&t->types, fp))
> +		return -EINVAL;
> +	if (ebitmap_read(&t->negset, fp))
> +		return -EINVAL;
> +
> +	rc = next_entry(buf, fp, sizeof(u32));
> +	if (rc < 0)
> +		return -EINVAL;
> +	t->flags = le32_to_cpu(buf[0]);
> +
> +	return 0;
> +}
> +
> +
> +static int read_cons_helper(struct policydb *p,
> +				struct constraint_node **nodep,
> +				int ncons, int allowxtarget, void *fp)
>  {
>  	struct constraint_node *c, *lc;
>  	struct constraint_expr *e, *le;
> @@ -1196,6 +1244,7 @@ static int read_cons_helper(struct constraint_node **nodep, int ncons,
>  			rc = next_entry(buf, fp, (sizeof(u32) * 3));
>  			if (rc)
>  				return rc;
> +
>  			e->expr_type = le32_to_cpu(buf[0]);
>  			e->attr = le32_to_cpu(buf[1]);
>  			e->op = le32_to_cpu(buf[2]);
> @@ -1225,6 +1274,17 @@ static int read_cons_helper(struct constraint_node **nodep, int ncons,
>  				rc = ebitmap_read(&e->names, fp);
>  				if (rc)
>  					return rc;
> +
> +				if (p->policyvers >= POLICYDB_VERSION_CONSTRAINT_NAMES) {
> +					e->type_names = kzalloc(sizeof(*e->type_names), GFP_KERNEL);
> +					if (!e->type_names)
> +						return -ENOMEM;
> +
> +					type_set_init(e->type_names);
> +					rc = type_set_read(e->type_names, fp);
> +					if (rc)
> +						return rc;
> +				}
>  				break;
>  			default:
>  				return -EINVAL;
> @@ -1233,6 +1293,7 @@ static int read_cons_helper(struct constraint_node **nodep, int ncons,
>  		}
>  		if (depth != 0)
>  			return -EINVAL;
> +
>  		lc = c;
>  	}
>  
> @@ -1301,7 +1362,7 @@ static int class_read(struct policydb *p, struct hashtab *h, void *fp)
>  			goto bad;
>  	}
>  
> -	rc = read_cons_helper(&cladatum->constraints, ncons, 0, fp);
> +	rc = read_cons_helper(p, &cladatum->constraints, ncons, 0, fp);
>  	if (rc)
>  		goto bad;
>  
> @@ -1311,7 +1372,7 @@ static int class_read(struct policydb *p, struct hashtab *h, void *fp)
>  		if (rc)
>  			goto bad;
>  		ncons = le32_to_cpu(buf[0]);
> -		rc = read_cons_helper(&cladatum->validatetrans, ncons, 1, fp);
> +		rc = read_cons_helper(p, &cladatum->validatetrans, ncons, 1, fp);
>  		if (rc)
>  			goto bad;
>  	}
> @@ -1339,7 +1400,7 @@ static int class_read(struct policydb *p, struct hashtab *h, void *fp)
>  
>  	return 0;
>  bad:
> -	cls_destroy(key, cladatum, NULL);
> +	cls_destroy(key, cladatum, p);
>  	return rc;
>  }
>  
> @@ -2750,6 +2811,24 @@ static int common_write(void *vkey, void *datum, void *ptr)
>  	return 0;
>  }
>  
> +static int type_set_write(struct type_set_datum * t, void *fp)
> +{
> +	int rc;
> +	__le32 buf[1];
> +
> +	if (ebitmap_write(&t->types, fp))
> +		return -EINVAL;
> +	if (ebitmap_write(&t->negset, fp))
> +		return -EINVAL;
> +
> +	buf[0] = cpu_to_le32(t->flags);
> +	rc = put_entry(buf, sizeof(u32), 1, fp);
> +	if (rc)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
>  static int write_cons_helper(struct policydb *p, struct constraint_node *node,
>  			     void *fp)
>  {
> @@ -2781,6 +2860,13 @@ static int write_cons_helper(struct policydb *p, struct constraint_node *node,
>  				rc = ebitmap_write(&e->names, fp);
>  				if (rc)
>  					return rc;
> +
> +				if (p->policyvers >= POLICYDB_VERSION_CONSTRAINT_NAMES) {
> +					rc = type_set_write(e->type_names, fp);
> +					if (rc)
> +						return rc;
> +				}
> +
>  				break;
>  			default:
>  				break;
> diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
> index da63747..c0bb6cb 100644
> --- a/security/selinux/ss/policydb.h
> +++ b/security/selinux/ss/policydb.h
> @@ -154,6 +154,17 @@ struct cond_bool_datum {
>  struct cond_node;
>  
>  /*
> + * type set preserves data needed to determine constraint info from
> + * policy source. This is not used by the kernel policy but allows
> + * utilities such as audit2allow to determine constraint denials.
> + */
> +struct type_set_datum {
> +	struct ebitmap types;
> +	struct ebitmap negset;
> +	u32 flags;
> +};
> +
> +/*
>   * The configuration data includes security contexts for
>   * initial SIDs, unlabeled file systems, TCP and UDP port numbers,
>   * network interfaces, and nodes.  This structure stores the
> 


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

Re: SELinux: Update policy version to support constraints info

[Richard Haines]

----- Original Message -----
> From: Stephen Smalley <sds@tycho.nsa.gov>
> To: Eric Paris <eparis@redhat.com>
> Cc: selinux@tycho.nsa.gov
> Sent: Tuesday, 22 October 2013, 13:43
> Subject: Re: SELinux: Update policy version to support constraints info
> 
> On 10/21/2013 07:32 PM, Eric Paris wrote:
>>  From: Richard Haines <richard_c_haines@btinternet.com>
>>  Date: Tue, 18 Dec 2012 19:37:46 +0000
>> 
>>  Update the policy version (POLICYDB_VERSION_CONSTRAINT_NAMES) to allow
>>  holding of policy source info for constraints.
> 
> Before we get into code review, let me ask the following:
> Why do we need a new kernel binary policy version for information that
> is only used by userspace?  Why can't this information be extracted from
> the policy module format or stored in an auxiliary file used only by
> userspace?

True it could be done the ways you suggest, however the module format would
only work for modular policies and the auxiliary file (I felt) would be
difficult to track (could be another config type file maybe?).

Anyway the requirement (or challenge) was to obtain the source policy defined
constraint information on denials so that administrators could decide what
actions were needed to resolve any issues.
The kernel policy constraints only had the types available. I did try to tie
the types to attributes but could not get a true list. I then decided to add
the source policy constraint info to the binary policy. This of course
resulted in requiring a policy version jump. It also increased the policy
size (the targeted policy was about 4K larger and the mls 5.5K, so not much
of an increase on the 5MB binaries).

The patchset also included a large patch to libsepol so that the constraints
could be interrogated. This is an edited comment from that patch:

/* sepol_compute_av_reason_buffer - returns the constraint expression
 * calculations whether allowed or denied in a buffer. This buffer is allocated
 * by this call and must be free'd by the caller using free(3). The contraint
 * buffer will contain any constraints in infix notation.
 * If the SHOW_GRANTED flag is set it will show granted and denied
 * constraints. The default is to show only denied constraints.
 */

> 
> And then on a side note, why wasn't this patch posted to selinux list
> for review before being included in Fedora? 

I developed the series of patches (including this kernel patch) on a "this
is how it can be done" basis and therefore saw no reason to submit it to
the list. However it appears that over time (I did this Dec '12) it has been
taken as either 'reviewed by the list' or at least 'sent to the list'.

> Or was it and I just missed it?  Introducing a new policy version is a big deal, and should never
> happen without upstream review.  Fedora gets to eat the pain if we chose
> not to support this and reuse policy version 29 for something else entirely.
> 
>> 
>>  Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
>>  ---
>>   security/selinux/include/security.h |    3 +-
>>   security/selinux/ss/constraint.h    |    1 +
>>   security/selinux/ss/policydb.c      |  100 
> ++++++++++++++++++++++++++++++++---
>>   security/selinux/ss/policydb.h      |   11 ++++
>>   4 files changed, 107 insertions(+), 8 deletions(-)
>> 
>>  diff --git a/security/selinux/include/security.h 
> b/security/selinux/include/security.h
>>  index 927fc14..bdf0a56 100644
>>  --- a/security/selinux/include/security.h
>>  +++ b/security/selinux/include/security.h
>>  @@ -33,13 +33,14 @@
>>   #define POLICYDB_VERSION_ROLETRANS    26
>>   #define POLICYDB_VERSION_NEW_OBJECT_DEFAULTS    27
>>   #define POLICYDB_VERSION_DEFAULT_TYPE    28
>>  +#define POLICYDB_VERSION_CONSTRAINT_NAMES    29
>>   
>>   /* Range of policy versions we understand*/
>>   #define POLICYDB_VERSION_MIN   POLICYDB_VERSION_BASE
>>   #ifdef CONFIG_SECURITY_SELINUX_POLICYDB_VERSION_MAX
>>   #define POLICYDB_VERSION_MAX    
> CONFIG_SECURITY_SELINUX_POLICYDB_VERSION_MAX_VALUE
>>   #else
>>  -#define POLICYDB_VERSION_MAX    POLICYDB_VERSION_DEFAULT_TYPE
>>  +#define POLICYDB_VERSION_MAX    POLICYDB_VERSION_CONSTRAINT_NAMES
>>   #endif
>>   
>>   /* Mask for just the mount related flags */
>>  diff --git a/security/selinux/ss/constraint.h 
> b/security/selinux/ss/constraint.h
>>  index 149dda7..3855ea8 100644
>>  --- a/security/selinux/ss/constraint.h
>>  +++ b/security/selinux/ss/constraint.h
>>  @@ -48,6 +48,7 @@ struct constraint_expr {
>>       u32 op;            /* operator */
>>   
>>       struct ebitmap names;    /* names */
>>  +    struct type_set_datum *type_names;
>>   
>>       struct constraint_expr *next;   /* next expression */
>>   };
>>  diff --git a/security/selinux/ss/policydb.c 
> b/security/selinux/ss/policydb.c
>>  index 9cd9b7c..f1b23df 100644
>>  --- a/security/selinux/ss/policydb.c
>>  +++ b/security/selinux/ss/policydb.c
>>  @@ -143,6 +143,11 @@ static struct policydb_compat_info policydb_compat[] = 
> {
>>           .sym_num    = SYM_NUM,
>>           .ocon_num    = OCON_NUM,
>>       },
>>  +    {
>>  +        .version    = POLICYDB_VERSION_CONSTRAINT_NAMES,
>>  +        .sym_num    = SYM_NUM,
>>  +        .ocon_num    = OCON_NUM,
>>  +    },
>>   };
>>   
>>   static struct policydb_compat_info *policydb_lookup_compat(int version)
>>  @@ -613,11 +618,20 @@ static int common_destroy(void *key, void *datum, 
> void *p)
>>       return 0;
>>   }
>>   
>>  -static int cls_destroy(void *key, void *datum, void *p)
>>  +static void type_set_destroy(struct type_set_datum * t)
>>  +{
>>  +    if (t != NULL) {
>>  +        ebitmap_destroy(&t->types);
>>  +        ebitmap_destroy(&t->negset);
>>  +    }
>>  +}
>>  +
>>  +static int cls_destroy(void *key, void *datum, void *datap)
>>   {
>>       struct class_datum *cladatum;
>>       struct constraint_node *constraint, *ctemp;
>>       struct constraint_expr *e, *etmp;
>>  +    struct policydb *p = datap;
>>   
>>       kfree(key);
>>       if (datum) {
>>  @@ -629,6 +643,10 @@ static int cls_destroy(void *key, void *datum, void 
> *p)
>>               e = constraint->expr;
>>               while (e) {
>>                   ebitmap_destroy(&e->names);
>>  +                if (p->policyvers >= 
> POLICYDB_VERSION_CONSTRAINT_NAMES) {
>>  +                    type_set_destroy(e->type_names);
>>  +                    kfree(e->type_names);
>>  +                }
>>                   etmp = e;
>>                   e = e->next;
>>                   kfree(etmp);
>>  @@ -643,6 +661,10 @@ static int cls_destroy(void *key, void *datum, void 
> *p)
>>               e = constraint->expr;
>>               while (e) {
>>                   ebitmap_destroy(&e->names);
>>  +                if (p->policyvers >= 
> POLICYDB_VERSION_CONSTRAINT_NAMES) {
>>  +                    type_set_destroy(e->type_names);
>>  +                    kfree(e->type_names);
>>  +                }
>>                   etmp = e;
>>                   e = e->next;
>>                   kfree(etmp);
>>  @@ -775,7 +797,7 @@ void policydb_destroy(struct policydb *p)
>>   
>>       for (i = 0; i < SYM_NUM; i++) {
>>           cond_resched();
>>  -        hashtab_map(p->symtab[i].table, destroy_f[i], NULL);
>>  +        hashtab_map(p->symtab[i].table, destroy_f[i], p);
>>           hashtab_destroy(p->symtab[i].table);
>>       }
>>   
>>  @@ -1156,8 +1178,34 @@ bad:
>>       return rc;
>>   }
>>   
>>  -static int read_cons_helper(struct constraint_node **nodep, int ncons,
>>  -                int allowxtarget, void *fp)
>>  +static void type_set_init(struct type_set_datum * t)
>>  +{
>>  +    ebitmap_init(&t->types);
>>  +    ebitmap_init(&t->negset);
>>  +}
>>  +
>>  +static int type_set_read(struct type_set_datum * t, void *fp)
>>  +{
>>  +    __le32 buf[1];
>>  +    int rc;
>>  +
>>  +    if (ebitmap_read(&t->types, fp))
>>  +        return -EINVAL;
>>  +    if (ebitmap_read(&t->negset, fp))
>>  +        return -EINVAL;
>>  +
>>  +    rc = next_entry(buf, fp, sizeof(u32));
>>  +    if (rc < 0)
>>  +        return -EINVAL;
>>  +    t->flags = le32_to_cpu(buf[0]);
>>  +
>>  +    return 0;
>>  +}
>>  +
>>  +
>>  +static int read_cons_helper(struct policydb *p,
>>  +                struct constraint_node **nodep,
>>  +                int ncons, int allowxtarget, void *fp)
>>   {
>>       struct constraint_node *c, *lc;
>>       struct constraint_expr *e, *le;
>>  @@ -1196,6 +1244,7 @@ static int read_cons_helper(struct constraint_node 
> **nodep, int ncons,
>>               rc = next_entry(buf, fp, (sizeof(u32) * 3));
>>               if (rc)
>>                   return rc;
>>  +
>>               e->expr_type = le32_to_cpu(buf[0]);
>>               e->attr = le32_to_cpu(buf[1]);
>>               e->op = le32_to_cpu(buf[2]);
>>  @@ -1225,6 +1274,17 @@ static int read_cons_helper(struct constraint_node 
> **nodep, int ncons,
>>                   rc = ebitmap_read(&e->names, fp);
>>                   if (rc)
>>                       return rc;
>>  +
>>  +                if (p->policyvers >= 
> POLICYDB_VERSION_CONSTRAINT_NAMES) {
>>  +                    e->type_names = kzalloc(sizeof(*e->type_names), 
> GFP_KERNEL);
>>  +                    if (!e->type_names)
>>  +                        return -ENOMEM;
>>  +
>>  +                    type_set_init(e->type_names);
>>  +                    rc = type_set_read(e->type_names, fp);
>>  +                    if (rc)
>>  +                        return rc;
>>  +                }
>>                   break;
>>               default:
>>                   return -EINVAL;
>>  @@ -1233,6 +1293,7 @@ static int read_cons_helper(struct constraint_node 
> **nodep, int ncons,
>>           }
>>           if (depth != 0)
>>               return -EINVAL;
>>  +
>>           lc = c;
>>       }
>>   
>>  @@ -1301,7 +1362,7 @@ static int class_read(struct policydb *p, struct 
> hashtab *h, void *fp)
>>               goto bad;
>>       }
>>   
>>  -    rc = read_cons_helper(&cladatum->constraints, ncons, 0, fp);
>>  +    rc = read_cons_helper(p, &cladatum->constraints, ncons, 0, fp);
>>       if (rc)
>>           goto bad;
>>   
>>  @@ -1311,7 +1372,7 @@ static int class_read(struct policydb *p, struct 
> hashtab *h, void *fp)
>>           if (rc)
>>               goto bad;
>>           ncons = le32_to_cpu(buf[0]);
>>  -        rc = read_cons_helper(&cladatum->validatetrans, ncons, 1, 
> fp);
>>  +        rc = read_cons_helper(p, &cladatum->validatetrans, ncons, 
> 1, fp);
>>           if (rc)
>>               goto bad;
>>       }
>>  @@ -1339,7 +1400,7 @@ static int class_read(struct policydb *p, struct 
> hashtab *h, void *fp)
>>   
>>       return 0;
>>   bad:
>>  -    cls_destroy(key, cladatum, NULL);
>>  +    cls_destroy(key, cladatum, p);
>>       return rc;
>>   }
>>   
>>  @@ -2750,6 +2811,24 @@ static int common_write(void *vkey, void *datum, 
> void *ptr)
>>       return 0;
>>   }
>>   
>>  +static int type_set_write(struct type_set_datum * t, void *fp)
>>  +{
>>  +    int rc;
>>  +    __le32 buf[1];
>>  +
>>  +    if (ebitmap_write(&t->types, fp))
>>  +        return -EINVAL;
>>  +    if (ebitmap_write(&t->negset, fp))
>>  +        return -EINVAL;
>>  +
>>  +    buf[0] = cpu_to_le32(t->flags);
>>  +    rc = put_entry(buf, sizeof(u32), 1, fp);
>>  +    if (rc)
>>  +        return -EINVAL;
>>  +
>>  +    return 0;
>>  +}
>>  +
>>   static int write_cons_helper(struct policydb *p, struct constraint_node 
> *node,
>>                    void *fp)
>>   {
>>  @@ -2781,6 +2860,13 @@ static int write_cons_helper(struct policydb *p, 
> struct constraint_node *node,
>>                   rc = ebitmap_write(&e->names, fp);
>>                   if (rc)
>>                       return rc;
>>  +
>>  +                if (p->policyvers >= 
> POLICYDB_VERSION_CONSTRAINT_NAMES) {
>>  +                    rc = type_set_write(e->type_names, fp);
>>  +                    if (rc)
>>  +                        return rc;
>>  +                }
>>  +
>>                   break;
>>               default:
>>                   break;
>>  diff --git a/security/selinux/ss/policydb.h 
> b/security/selinux/ss/policydb.h
>>  index da63747..c0bb6cb 100644
>>  --- a/security/selinux/ss/policydb.h
>>  +++ b/security/selinux/ss/policydb.h
>>  @@ -154,6 +154,17 @@ struct cond_bool_datum {
>>   struct cond_node;
>>   
>>   /*
>>  + * type set preserves data needed to determine constraint info from
>>  + * policy source. This is not used by the kernel policy but allows
>>  + * utilities such as audit2allow to determine constraint denials.
>>  + */
>>  +struct type_set_datum {
>>  +    struct ebitmap types;
>>  +    struct ebitmap negset;
>>  +    u32 flags;
>>  +};
>>  +
>>  +/*
>>    * The configuration data includes security contexts for
>>    * initial SIDs, unlabeled file systems, TCP and UDP port numbers,
>>    * network interfaces, and nodes.  This structure stores the
>> 
> 
> 
> --
> 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.

Re: SELinux: Update policy version to support constraints info

[Paul Moore]

On Thursday, October 24, 2013 11:22:44 AM Richard Haines wrote:
> ----- Original Message -----
> 
> > From: Stephen Smalley <sds@tycho.nsa.gov>
> > To: Eric Paris <eparis@redhat.com>
> > Cc: selinux@tycho.nsa.gov
> > Sent: Tuesday, 22 October 2013, 13:43
> > Subject: Re: SELinux: Update policy version to support constraints info
> > 
> > On 10/21/2013 07:32 PM, Eric Paris wrote:
> >>  From: Richard Haines <richard_c_haines@btinternet.com>
> >>  Date: Tue, 18 Dec 2012 19:37:46 +0000
> >>  
> >>  Update the policy version (POLICYDB_VERSION_CONSTRAINT_NAMES) to allow
> >>  holding of policy source info for constraints.
> > 
> > Before we get into code review, let me ask the following:
> > Why do we need a new kernel binary policy version for information that
> > is only used by userspace?  Why can't this information be extracted from
> > the policy module format or stored in an auxiliary file used only by
> > userspace?
> 
> True it could be done the ways you suggest, however the module format would
> only work for modular policies and the auxiliary file (I felt) would be
> difficult to track (could be another config type file maybe?).

My current two cents on the issue ...

Working under the assumption that the userspace tools would much prefer to 
read the policy directly from the kernel (for obvious reasons), it makes sense 
to me that we provide this additional constraint information along with the 
rest of the policy.  Having to pull policy information from both the kernel 
and a regular file seems like it could lead to all sorts of problems.

Further, I believe that for the majority of systems the extra ~5k of kernel 
memory is not a major deal breaker, although I will concede that for smaller 
systems (Android perhaps?) this might be more of a concern.  What if, along 
with this extra constraint information, we add a toggle (perhaps via a new 
policy capability) which would trigger the kernel to either preserve the 
constraint info or discard it when the policy is loaded? 

-- 
paul moore
www.paul-moore.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.

Re: SELinux: Update policy version to support constraints info

[Stephen Smalley]

On 10/24/2013 02:36 PM, Paul Moore wrote:
> On Thursday, October 24, 2013 11:22:44 AM Richard Haines wrote:
>> ----- Original Message -----
>>
>>> From: Stephen Smalley <sds@tycho.nsa.gov>
>>> To: Eric Paris <eparis@redhat.com>
>>> Cc: selinux@tycho.nsa.gov
>>> Sent: Tuesday, 22 October 2013, 13:43
>>> Subject: Re: SELinux: Update policy version to support constraints info
>>>
>>> On 10/21/2013 07:32 PM, Eric Paris wrote:
>>>>  From: Richard Haines <richard_c_haines@btinternet.com>
>>>>  Date: Tue, 18 Dec 2012 19:37:46 +0000
>>>>  
>>>>  Update the policy version (POLICYDB_VERSION_CONSTRAINT_NAMES) to allow
>>>>  holding of policy source info for constraints.
>>>
>>> Before we get into code review, let me ask the following:
>>> Why do we need a new kernel binary policy version for information that
>>> is only used by userspace?  Why can't this information be extracted from
>>> the policy module format or stored in an auxiliary file used only by
>>> userspace?
>>
>> True it could be done the ways you suggest, however the module format would
>> only work for modular policies and the auxiliary file (I felt) would be
>> difficult to track (could be another config type file maybe?).
> 
> My current two cents on the issue ...
> 
> Working under the assumption that the userspace tools would much prefer to 
> read the policy directly from the kernel (for obvious reasons), it makes sense 
> to me that we provide this additional constraint information along with the 
> rest of the policy.  Having to pull policy information from both the kernel 
> and a regular file seems like it could lead to all sorts of problems.
> 
> Further, I believe that for the majority of systems the extra ~5k of kernel 
> memory is not a major deal breaker, although I will concede that for smaller 
> systems (Android perhaps?) this might be more of a concern.  What if, along 
> with this extra constraint information, we add a toggle (perhaps via a new 
> policy capability) which would trigger the kernel to either preserve the 
> constraint info or discard it when the policy is loaded? 

No, if we take it, it should be unconditionally preserved IMHO.



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

Re: SELinux: Update policy version to support constraints info

[Stephen Smalley]

On 10/21/2013 07:32 PM, Eric Paris wrote:
> From: Richard Haines <richard_c_haines@btinternet.com>
> Date: Tue, 18 Dec 2012 19:37:46 +0000
> 
> Update the policy version (POLICYDB_VERSION_CONSTRAINT_NAMES) to allow
> holding of policy source info for constraints.
> 
> Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
> ---
>  security/selinux/include/security.h |    3 +-
>  security/selinux/ss/constraint.h    |    1 +
>  security/selinux/ss/policydb.c      |  100 ++++++++++++++++++++++++++++++++---
>  security/selinux/ss/policydb.h      |   11 ++++
>  4 files changed, 107 insertions(+), 8 deletions(-)
> 

> diff --git a/security/selinux/ss/constraint.h b/security/selinux/ss/constraint.h
> index 149dda7..3855ea8 100644
> --- a/security/selinux/ss/constraint.h
> +++ b/security/selinux/ss/constraint.h
> @@ -48,6 +48,7 @@ struct constraint_expr {
>  	u32 op;			/* operator */
>  
>  	struct ebitmap names;	/* names */
> +	struct type_set_datum *type_names;

Why did you change the name of the structure in the kernel from the one
used in libsepol (struct type_set)?

> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index 9cd9b7c..f1b23df 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -613,11 +618,20 @@ static int common_destroy(void *key, void *datum, void *p)
>  	return 0;
>  }
>  
> -static int cls_destroy(void *key, void *datum, void *p)
> +static void type_set_destroy(struct type_set_datum * t)
> +{
> +	if (t != NULL) {
> +		ebitmap_destroy(&t->types);
> +		ebitmap_destroy(&t->negset);
> +	}
> +}

So you don't free(t) here, yet every caller does so afterward.
Although I see it isn't that way in libsepol so maybe you are trying to
stay consistent in case you use these elsewhere in the kernel as is done
in libsepol?

> @@ -629,6 +643,10 @@ static int cls_destroy(void *key, void *datum, void *p)
>  			e = constraint->expr;
>  			while (e) {
>  				ebitmap_destroy(&e->names);
> +				if (p->policyvers >= POLICYDB_VERSION_CONSTRAINT_NAMES) {
> +					type_set_destroy(e->type_names);
> +					kfree(e->type_names);
> +				}

Should we have a constraint_expr_destroy() as we do in libsepol?

>  				etmp = e;
>  				e = e->next;
>  				kfree(etmp);
> @@ -643,6 +661,10 @@ static int cls_destroy(void *key, void *datum, void *p)
>  			e = constraint->expr;
>  			while (e) {
>  				ebitmap_destroy(&e->names);
> +				if (p->policyvers >= POLICYDB_VERSION_CONSTRAINT_NAMES) {
> +					type_set_destroy(e->type_names);
> +					kfree(e->type_names);
> +				}

So we don't have to repeat this?

>  				etmp = e;
>  				e = e->next;
>  				kfree(etmp);
> @@ -775,7 +797,7 @@ void policydb_destroy(struct policydb *p)
>  
>  	for (i = 0; i < SYM_NUM; i++) {
>  		cond_resched();
> -		hashtab_map(p->symtab[i].table, destroy_f[i], NULL);
> +		hashtab_map(p->symtab[i].table, destroy_f[i], p);

Couldn't we avoid the need for passing down the policydb and checking
the version just by ensuring e->type_names gets initialized to NULL and
then the type_set_destroy() and kfree() calls degenerate to no-ops?  I
suppose this works, but it complicates the code a bit.



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

Re: SELinux: Update policy version to support constraints info

[Stephen Smalley]

On 10/21/2013 07:32 PM, Eric Paris wrote:
> From: Richard Haines <richard_c_haines@btinternet.com>
> Date: Tue, 18 Dec 2012 19:37:46 +0000
> 
> Update the policy version (POLICYDB_VERSION_CONSTRAINT_NAMES) to allow
> holding of policy source info for constraints.
> 
> Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>

Can we get an updated version of this patch with my comments addressed
so we can get this queued for mainline?  The libsepol/checkpolicy I just
released includes the userspace support, so it would be nice if the
kernel support could become available in the not-too-distant future.


> ---
>  security/selinux/include/security.h |    3 +-
>  security/selinux/ss/constraint.h    |    1 +
>  security/selinux/ss/policydb.c      |  100 ++++++++++++++++++++++++++++++++---
>  security/selinux/ss/policydb.h      |   11 ++++
>  4 files changed, 107 insertions(+), 8 deletions(-)
> 
> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> index 927fc14..bdf0a56 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -33,13 +33,14 @@
>  #define POLICYDB_VERSION_ROLETRANS	26
>  #define POLICYDB_VERSION_NEW_OBJECT_DEFAULTS	27
>  #define POLICYDB_VERSION_DEFAULT_TYPE	28
> +#define POLICYDB_VERSION_CONSTRAINT_NAMES	29
>  
>  /* Range of policy versions we understand*/
>  #define POLICYDB_VERSION_MIN   POLICYDB_VERSION_BASE
>  #ifdef CONFIG_SECURITY_SELINUX_POLICYDB_VERSION_MAX
>  #define POLICYDB_VERSION_MAX	CONFIG_SECURITY_SELINUX_POLICYDB_VERSION_MAX_VALUE
>  #else
> -#define POLICYDB_VERSION_MAX	POLICYDB_VERSION_DEFAULT_TYPE
> +#define POLICYDB_VERSION_MAX	POLICYDB_VERSION_CONSTRAINT_NAMES
>  #endif
>  
>  /* Mask for just the mount related flags */
> diff --git a/security/selinux/ss/constraint.h b/security/selinux/ss/constraint.h
> index 149dda7..3855ea8 100644
> --- a/security/selinux/ss/constraint.h
> +++ b/security/selinux/ss/constraint.h
> @@ -48,6 +48,7 @@ struct constraint_expr {
>  	u32 op;			/* operator */
>  
>  	struct ebitmap names;	/* names */
> +	struct type_set_datum *type_names;
>  
>  	struct constraint_expr *next;   /* next expression */
>  };
> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index 9cd9b7c..f1b23df 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -143,6 +143,11 @@ static struct policydb_compat_info policydb_compat[] = {
>  		.sym_num	= SYM_NUM,
>  		.ocon_num	= OCON_NUM,
>  	},
> +	{
> +		.version	= POLICYDB_VERSION_CONSTRAINT_NAMES,
> +		.sym_num	= SYM_NUM,
> +		.ocon_num	= OCON_NUM,
> +	},
>  };
>  
>  static struct policydb_compat_info *policydb_lookup_compat(int version)
> @@ -613,11 +618,20 @@ static int common_destroy(void *key, void *datum, void *p)
>  	return 0;
>  }
>  
> -static int cls_destroy(void *key, void *datum, void *p)
> +static void type_set_destroy(struct type_set_datum * t)
> +{
> +	if (t != NULL) {
> +		ebitmap_destroy(&t->types);
> +		ebitmap_destroy(&t->negset);
> +	}
> +}
> +
> +static int cls_destroy(void *key, void *datum, void *datap)
>  {
>  	struct class_datum *cladatum;
>  	struct constraint_node *constraint, *ctemp;
>  	struct constraint_expr *e, *etmp;
> +	struct policydb *p = datap;
>  
>  	kfree(key);
>  	if (datum) {
> @@ -629,6 +643,10 @@ static int cls_destroy(void *key, void *datum, void *p)
>  			e = constraint->expr;
>  			while (e) {
>  				ebitmap_destroy(&e->names);
> +				if (p->policyvers >= POLICYDB_VERSION_CONSTRAINT_NAMES) {
> +					type_set_destroy(e->type_names);
> +					kfree(e->type_names);
> +				}
>  				etmp = e;
>  				e = e->next;
>  				kfree(etmp);
> @@ -643,6 +661,10 @@ static int cls_destroy(void *key, void *datum, void *p)
>  			e = constraint->expr;
>  			while (e) {
>  				ebitmap_destroy(&e->names);
> +				if (p->policyvers >= POLICYDB_VERSION_CONSTRAINT_NAMES) {
> +					type_set_destroy(e->type_names);
> +					kfree(e->type_names);
> +				}
>  				etmp = e;
>  				e = e->next;
>  				kfree(etmp);
> @@ -775,7 +797,7 @@ void policydb_destroy(struct policydb *p)
>  
>  	for (i = 0; i < SYM_NUM; i++) {
>  		cond_resched();
> -		hashtab_map(p->symtab[i].table, destroy_f[i], NULL);
> +		hashtab_map(p->symtab[i].table, destroy_f[i], p);
>  		hashtab_destroy(p->symtab[i].table);
>  	}
>  
> @@ -1156,8 +1178,34 @@ bad:
>  	return rc;
>  }
>  
> -static int read_cons_helper(struct constraint_node **nodep, int ncons,
> -			    int allowxtarget, void *fp)
> +static void type_set_init(struct type_set_datum * t)
> +{
> +	ebitmap_init(&t->types);
> +	ebitmap_init(&t->negset);
> +}
> +
> +static int type_set_read(struct type_set_datum * t, void *fp)
> +{
> +	__le32 buf[1];
> +	int rc;
> +
> +	if (ebitmap_read(&t->types, fp))
> +		return -EINVAL;
> +	if (ebitmap_read(&t->negset, fp))
> +		return -EINVAL;
> +
> +	rc = next_entry(buf, fp, sizeof(u32));
> +	if (rc < 0)
> +		return -EINVAL;
> +	t->flags = le32_to_cpu(buf[0]);
> +
> +	return 0;
> +}
> +
> +
> +static int read_cons_helper(struct policydb *p,
> +				struct constraint_node **nodep,
> +				int ncons, int allowxtarget, void *fp)
>  {
>  	struct constraint_node *c, *lc;
>  	struct constraint_expr *e, *le;
> @@ -1196,6 +1244,7 @@ static int read_cons_helper(struct constraint_node **nodep, int ncons,
>  			rc = next_entry(buf, fp, (sizeof(u32) * 3));
>  			if (rc)
>  				return rc;
> +
>  			e->expr_type = le32_to_cpu(buf[0]);
>  			e->attr = le32_to_cpu(buf[1]);
>  			e->op = le32_to_cpu(buf[2]);
> @@ -1225,6 +1274,17 @@ static int read_cons_helper(struct constraint_node **nodep, int ncons,
>  				rc = ebitmap_read(&e->names, fp);
>  				if (rc)
>  					return rc;
> +
> +				if (p->policyvers >= POLICYDB_VERSION_CONSTRAINT_NAMES) {
> +					e->type_names = kzalloc(sizeof(*e->type_names), GFP_KERNEL);
> +					if (!e->type_names)
> +						return -ENOMEM;
> +
> +					type_set_init(e->type_names);
> +					rc = type_set_read(e->type_names, fp);
> +					if (rc)
> +						return rc;
> +				}
>  				break;
>  			default:
>  				return -EINVAL;
> @@ -1233,6 +1293,7 @@ static int read_cons_helper(struct constraint_node **nodep, int ncons,
>  		}
>  		if (depth != 0)
>  			return -EINVAL;
> +
>  		lc = c;
>  	}
>  
> @@ -1301,7 +1362,7 @@ static int class_read(struct policydb *p, struct hashtab *h, void *fp)
>  			goto bad;
>  	}
>  
> -	rc = read_cons_helper(&cladatum->constraints, ncons, 0, fp);
> +	rc = read_cons_helper(p, &cladatum->constraints, ncons, 0, fp);
>  	if (rc)
>  		goto bad;
>  
> @@ -1311,7 +1372,7 @@ static int class_read(struct policydb *p, struct hashtab *h, void *fp)
>  		if (rc)
>  			goto bad;
>  		ncons = le32_to_cpu(buf[0]);
> -		rc = read_cons_helper(&cladatum->validatetrans, ncons, 1, fp);
> +		rc = read_cons_helper(p, &cladatum->validatetrans, ncons, 1, fp);
>  		if (rc)
>  			goto bad;
>  	}
> @@ -1339,7 +1400,7 @@ static int class_read(struct policydb *p, struct hashtab *h, void *fp)
>  
>  	return 0;
>  bad:
> -	cls_destroy(key, cladatum, NULL);
> +	cls_destroy(key, cladatum, p);
>  	return rc;
>  }
>  
> @@ -2750,6 +2811,24 @@ static int common_write(void *vkey, void *datum, void *ptr)
>  	return 0;
>  }
>  
> +static int type_set_write(struct type_set_datum * t, void *fp)
> +{
> +	int rc;
> +	__le32 buf[1];
> +
> +	if (ebitmap_write(&t->types, fp))
> +		return -EINVAL;
> +	if (ebitmap_write(&t->negset, fp))
> +		return -EINVAL;
> +
> +	buf[0] = cpu_to_le32(t->flags);
> +	rc = put_entry(buf, sizeof(u32), 1, fp);
> +	if (rc)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
>  static int write_cons_helper(struct policydb *p, struct constraint_node *node,
>  			     void *fp)
>  {
> @@ -2781,6 +2860,13 @@ static int write_cons_helper(struct policydb *p, struct constraint_node *node,
>  				rc = ebitmap_write(&e->names, fp);
>  				if (rc)
>  					return rc;
> +
> +				if (p->policyvers >= POLICYDB_VERSION_CONSTRAINT_NAMES) {
> +					rc = type_set_write(e->type_names, fp);
> +					if (rc)
> +						return rc;
> +				}
> +
>  				break;
>  			default:
>  				break;
> diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
> index da63747..c0bb6cb 100644
> --- a/security/selinux/ss/policydb.h
> +++ b/security/selinux/ss/policydb.h
> @@ -154,6 +154,17 @@ struct cond_bool_datum {
>  struct cond_node;
>  
>  /*
> + * type set preserves data needed to determine constraint info from
> + * policy source. This is not used by the kernel policy but allows
> + * utilities such as audit2allow to determine constraint denials.
> + */
> +struct type_set_datum {
> +	struct ebitmap types;
> +	struct ebitmap negset;
> +	u32 flags;
> +};
> +
> +/*
>   * The configuration data includes security contexts for
>   * initial SIDs, unlabeled file systems, TCP and UDP port numbers,
>   * network interfaces, and nodes.  This structure stores the
> 


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

Re: SELinux: Update policy version to support constraints info

[Richard Haines]

[-- Attachment #1: Type: text/plain, Size: 11992 bytes --]

Here is my go at another kernel patch. I've done most of what you asked
except I could not work out this  comment:

"Couldn't we avoid the need for passing down the policydb and checking
the version just by ensuring e->type_names gets initialized to NULL and
then the type_set_destroy() and kfree() calls degenerate to no-ops?  I
suppose this works, but it complicates the code a bit."

I've used the /sys/fs/selinux/policy facility to dump policy and loaded many
different policy types/versions and the kernel still lives.
Richard
 
----- Original Message -----
> From: Stephen Smalley <sds@tycho.nsa.gov>
> To: Eric Paris <eparis@redhat.com>
> Cc: selinux@tycho.nsa.gov; Richard Haines <richard_c_haines@btinternet.com>; Paul Moore <paul@paul-moore.com>
> Sent: Wednesday, 30 October 2013, 20:27
> Subject: Re: SELinux: Update policy version to support constraints info
> 
> On 10/21/2013 07:32 PM, Eric Paris wrote:
>>  From: Richard Haines <richard_c_haines@btinternet.com>
>>  Date: Tue, 18 Dec 2012 19:37:46 +0000
>> 
>>  Update the policy version (POLICYDB_VERSION_CONSTRAINT_NAMES) to allow
>>  holding of policy source info for constraints.
>> 
>>  Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
> 
> Can we get an updated version of this patch with my comments addressed
> so we can get this queued for mainline?  The libsepol/checkpolicy I just
> released includes the userspace support, so it would be nice if the
> kernel support could become available in the not-too-distant future.
> 
> 
>>  ---
>>   security/selinux/include/security.h |    3 +-
>>   security/selinux/ss/constraint.h    |    1 +
>>   security/selinux/ss/policydb.c      |  100 
> ++++++++++++++++++++++++++++++++---
>>   security/selinux/ss/policydb.h      |   11 ++++
>>   4 files changed, 107 insertions(+), 8 deletions(-)
>> 
>>  diff --git a/security/selinux/include/security.h 
> b/security/selinux/include/security.h
>>  index 927fc14..bdf0a56 100644
>>  --- a/security/selinux/include/security.h
>>  +++ b/security/selinux/include/security.h
>>  @@ -33,13 +33,14 @@
>>   #define POLICYDB_VERSION_ROLETRANS    26
>>   #define POLICYDB_VERSION_NEW_OBJECT_DEFAULTS    27
>>   #define POLICYDB_VERSION_DEFAULT_TYPE    28
>>  +#define POLICYDB_VERSION_CONSTRAINT_NAMES    29
>>   
>>   /* Range of policy versions we understand*/
>>   #define POLICYDB_VERSION_MIN   POLICYDB_VERSION_BASE
>>   #ifdef CONFIG_SECURITY_SELINUX_POLICYDB_VERSION_MAX
>>   #define POLICYDB_VERSION_MAX    
> CONFIG_SECURITY_SELINUX_POLICYDB_VERSION_MAX_VALUE
>>   #else
>>  -#define POLICYDB_VERSION_MAX    POLICYDB_VERSION_DEFAULT_TYPE
>>  +#define POLICYDB_VERSION_MAX    POLICYDB_VERSION_CONSTRAINT_NAMES
>>   #endif
>>   
>>   /* Mask for just the mount related flags */
>>  diff --git a/security/selinux/ss/constraint.h 
> b/security/selinux/ss/constraint.h
>>  index 149dda7..3855ea8 100644
>>  --- a/security/selinux/ss/constraint.h
>>  +++ b/security/selinux/ss/constraint.h
>>  @@ -48,6 +48,7 @@ struct constraint_expr {
>>       u32 op;            /* operator */
>>   
>>       struct ebitmap names;    /* names */
>>  +    struct type_set_datum *type_names;
>>   
>>       struct constraint_expr *next;   /* next expression */
>>   };
>>  diff --git a/security/selinux/ss/policydb.c 
> b/security/selinux/ss/policydb.c
>>  index 9cd9b7c..f1b23df 100644
>>  --- a/security/selinux/ss/policydb.c
>>  +++ b/security/selinux/ss/policydb.c
>>  @@ -143,6 +143,11 @@ static struct policydb_compat_info policydb_compat[] = 
> {
>>           .sym_num    = SYM_NUM,
>>           .ocon_num    = OCON_NUM,
>>       },
>>  +    {
>>  +        .version    = POLICYDB_VERSION_CONSTRAINT_NAMES,
>>  +        .sym_num    = SYM_NUM,
>>  +        .ocon_num    = OCON_NUM,
>>  +    },
>>   };
>>   
>>   static struct policydb_compat_info *policydb_lookup_compat(int version)
>>  @@ -613,11 +618,20 @@ static int common_destroy(void *key, void *datum, 
> void *p)
>>       return 0;
>>   }
>>   
>>  -static int cls_destroy(void *key, void *datum, void *p)
>>  +static void type_set_destroy(struct type_set_datum * t)
>>  +{
>>  +    if (t != NULL) {
>>  +        ebitmap_destroy(&t->types);
>>  +        ebitmap_destroy(&t->negset);
>>  +    }
>>  +}
>>  +
>>  +static int cls_destroy(void *key, void *datum, void *datap)
>>   {
>>       struct class_datum *cladatum;
>>       struct constraint_node *constraint, *ctemp;
>>       struct constraint_expr *e, *etmp;
>>  +    struct policydb *p = datap;
>>   
>>       kfree(key);
>>       if (datum) {
>>  @@ -629,6 +643,10 @@ static int cls_destroy(void *key, void *datum, void 
> *p)
>>               e = constraint->expr;
>>               while (e) {
>>                   ebitmap_destroy(&e->names);
>>  +                if (p->policyvers >= 
> POLICYDB_VERSION_CONSTRAINT_NAMES) {
>>  +                    type_set_destroy(e->type_names);
>>  +                    kfree(e->type_names);
>>  +                }
>>                   etmp = e;
>>                   e = e->next;
>>                   kfree(etmp);
>>  @@ -643,6 +661,10 @@ static int cls_destroy(void *key, void *datum, void 
> *p)
>>               e = constraint->expr;
>>               while (e) {
>>                   ebitmap_destroy(&e->names);
>>  +                if (p->policyvers >= 
> POLICYDB_VERSION_CONSTRAINT_NAMES) {
>>  +                    type_set_destroy(e->type_names);
>>  +                    kfree(e->type_names);
>>  +                }
>>                   etmp = e;
>>                   e = e->next;
>>                   kfree(etmp);
>>  @@ -775,7 +797,7 @@ void policydb_destroy(struct policydb *p)
>>   
>>       for (i = 0; i < SYM_NUM; i++) {
>>           cond_resched();
>>  -        hashtab_map(p->symtab[i].table, destroy_f[i], NULL);
>>  +        hashtab_map(p->symtab[i].table, destroy_f[i], p);
>>           hashtab_destroy(p->symtab[i].table);
>>       }
>>   
>>  @@ -1156,8 +1178,34 @@ bad:
>>       return rc;
>>   }
>>   
>>  -static int read_cons_helper(struct constraint_node **nodep, int ncons,
>>  -                int allowxtarget, void *fp)
>>  +static void type_set_init(struct type_set_datum * t)
>>  +{
>>  +    ebitmap_init(&t->types);
>>  +    ebitmap_init(&t->negset);
>>  +}
>>  +
>>  +static int type_set_read(struct type_set_datum * t, void *fp)
>>  +{
>>  +    __le32 buf[1];
>>  +    int rc;
>>  +
>>  +    if (ebitmap_read(&t->types, fp))
>>  +        return -EINVAL;
>>  +    if (ebitmap_read(&t->negset, fp))
>>  +        return -EINVAL;
>>  +
>>  +    rc = next_entry(buf, fp, sizeof(u32));
>>  +    if (rc < 0)
>>  +        return -EINVAL;
>>  +    t->flags = le32_to_cpu(buf[0]);
>>  +
>>  +    return 0;
>>  +}
>>  +
>>  +
>>  +static int read_cons_helper(struct policydb *p,
>>  +                struct constraint_node **nodep,
>>  +                int ncons, int allowxtarget, void *fp)
>>   {
>>       struct constraint_node *c, *lc;
>>       struct constraint_expr *e, *le;
>>  @@ -1196,6 +1244,7 @@ static int read_cons_helper(struct constraint_node 
> **nodep, int ncons,
>>               rc = next_entry(buf, fp, (sizeof(u32) * 3));
>>               if (rc)
>>                   return rc;
>>  +
>>               e->expr_type = le32_to_cpu(buf[0]);
>>               e->attr = le32_to_cpu(buf[1]);
>>               e->op = le32_to_cpu(buf[2]);
>>  @@ -1225,6 +1274,17 @@ static int read_cons_helper(struct constraint_node 
> **nodep, int ncons,
>>                   rc = ebitmap_read(&e->names, fp);
>>                   if (rc)
>>                       return rc;
>>  +
>>  +                if (p->policyvers >= 
> POLICYDB_VERSION_CONSTRAINT_NAMES) {
>>  +                    e->type_names = kzalloc(sizeof(*e->type_names), 
> GFP_KERNEL);
>>  +                    if (!e->type_names)
>>  +                        return -ENOMEM;
>>  +
>>  +                    type_set_init(e->type_names);
>>  +                    rc = type_set_read(e->type_names, fp);
>>  +                    if (rc)
>>  +                        return rc;
>>  +                }
>>                   break;
>>               default:
>>                   return -EINVAL;
>>  @@ -1233,6 +1293,7 @@ static int read_cons_helper(struct constraint_node 
> **nodep, int ncons,
>>           }
>>           if (depth != 0)
>>               return -EINVAL;
>>  +
>>           lc = c;
>>       }
>>   
>>  @@ -1301,7 +1362,7 @@ static int class_read(struct policydb *p, struct 
> hashtab *h, void *fp)
>>               goto bad;
>>       }
>>   
>>  -    rc = read_cons_helper(&cladatum->constraints, ncons, 0, fp);
>>  +    rc = read_cons_helper(p, &cladatum->constraints, ncons, 0, fp);
>>       if (rc)
>>           goto bad;
>>   
>>  @@ -1311,7 +1372,7 @@ static int class_read(struct policydb *p, struct 
> hashtab *h, void *fp)
>>           if (rc)
>>               goto bad;
>>           ncons = le32_to_cpu(buf[0]);
>>  -        rc = read_cons_helper(&cladatum->validatetrans, ncons, 1, 
> fp);
>>  +        rc = read_cons_helper(p, &cladatum->validatetrans, ncons, 
> 1, fp);
>>           if (rc)
>>               goto bad;
>>       }
>>  @@ -1339,7 +1400,7 @@ static int class_read(struct policydb *p, struct 
> hashtab *h, void *fp)
>>   
>>       return 0;
>>   bad:
>>  -    cls_destroy(key, cladatum, NULL);
>>  +    cls_destroy(key, cladatum, p);
>>       return rc;
>>   }
>>   
>>  @@ -2750,6 +2811,24 @@ static int common_write(void *vkey, void *datum, 
> void *ptr)
>>       return 0;
>>   }
>>   
>>  +static int type_set_write(struct type_set_datum * t, void *fp)
>>  +{
>>  +    int rc;
>>  +    __le32 buf[1];
>>  +
>>  +    if (ebitmap_write(&t->types, fp))
>>  +        return -EINVAL;
>>  +    if (ebitmap_write(&t->negset, fp))
>>  +        return -EINVAL;
>>  +
>>  +    buf[0] = cpu_to_le32(t->flags);
>>  +    rc = put_entry(buf, sizeof(u32), 1, fp);
>>  +    if (rc)
>>  +        return -EINVAL;
>>  +
>>  +    return 0;
>>  +}
>>  +
>>   static int write_cons_helper(struct policydb *p, struct constraint_node 
> *node,
>>                    void *fp)
>>   {
>>  @@ -2781,6 +2860,13 @@ static int write_cons_helper(struct policydb *p, 
> struct constraint_node *node,
>>                   rc = ebitmap_write(&e->names, fp);
>>                   if (rc)
>>                       return rc;
>>  +
>>  +                if (p->policyvers >= 
> POLICYDB_VERSION_CONSTRAINT_NAMES) {
>>  +                    rc = type_set_write(e->type_names, fp);
>>  +                    if (rc)
>>  +                        return rc;
>>  +                }
>>  +
>>                   break;
>>               default:
>>                   break;
>>  diff --git a/security/selinux/ss/policydb.h 
> b/security/selinux/ss/policydb.h
>>  index da63747..c0bb6cb 100644
>>  --- a/security/selinux/ss/policydb.h
>>  +++ b/security/selinux/ss/policydb.h
>>  @@ -154,6 +154,17 @@ struct cond_bool_datum {
>>   struct cond_node;
>>   
>>   /*
>>  + * type set preserves data needed to determine constraint info from
>>  + * policy source. This is not used by the kernel policy but allows
>>  + * utilities such as audit2allow to determine constraint denials.
>>  + */
>>  +struct type_set_datum {
>>  +    struct ebitmap types;
>>  +    struct ebitmap negset;
>>  +    u32 flags;
>>  +};
>>  +
>>  +/*
>>    * The configuration data includes security contexts for
>>    * initial SIDs, unlabeled file systems, TCP and UDP port numbers,
>>    * network interfaces, and nodes.  This structure stores the
>> 
> 
> 
> --
> 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.
>  

[-- Attachment #2: 0001-SELinux-Update-policy-version-to-support-constraints.patch --]
[-- Type: text/plain, Size: 7630 bytes --]

From bba97b7724ca62d8db95ae9b74193161c8385e4b Mon Sep 17 00:00:00 2001
From: Richard Haines <richard_c_haines@btinternet.com>
Date: Sat, 26 Oct 2013 15:39:10 +0100
Subject: [PATCH] SELinux: Update policy version to support constraints info

Update the policy version (POLICYDB_VERSION_CONSTRAINT_NAMES) to allow
holding of policy source info for constraints.

Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
---
 security/selinux/include/security.h |  3 +-
 security/selinux/ss/constraint.h    |  1 +
 security/selinux/ss/policydb.c      | 95 ++++++++++++++++++++++++++++++++-----
 security/selinux/ss/policydb.h      | 11 +++++
 4 files changed, 98 insertions(+), 12 deletions(-)

diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index 927fc14..bdf0a56 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -33,13 +33,14 @@
 #define POLICYDB_VERSION_ROLETRANS	26
 #define POLICYDB_VERSION_NEW_OBJECT_DEFAULTS	27
 #define POLICYDB_VERSION_DEFAULT_TYPE	28
+#define POLICYDB_VERSION_CONSTRAINT_NAMES	29
 
 /* Range of policy versions we understand*/
 #define POLICYDB_VERSION_MIN   POLICYDB_VERSION_BASE
 #ifdef CONFIG_SECURITY_SELINUX_POLICYDB_VERSION_MAX
 #define POLICYDB_VERSION_MAX	CONFIG_SECURITY_SELINUX_POLICYDB_VERSION_MAX_VALUE
 #else
-#define POLICYDB_VERSION_MAX	POLICYDB_VERSION_DEFAULT_TYPE
+#define POLICYDB_VERSION_MAX	POLICYDB_VERSION_CONSTRAINT_NAMES
 #endif
 
 /* Mask for just the mount related flags */
diff --git a/security/selinux/ss/constraint.h b/security/selinux/ss/constraint.h
index 149dda7..96fd947 100644
--- a/security/selinux/ss/constraint.h
+++ b/security/selinux/ss/constraint.h
@@ -48,6 +48,7 @@ struct constraint_expr {
 	u32 op;			/* operator */
 
 	struct ebitmap names;	/* names */
+	struct type_set *type_names;
 
 	struct constraint_expr *next;   /* next expression */
 };
diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index 9cd9b7c..3fcd75d 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -143,6 +143,11 @@ static struct policydb_compat_info policydb_compat[] = {
 		.sym_num	= SYM_NUM,
 		.ocon_num	= OCON_NUM,
 	},
+	{
+		.version	= POLICYDB_VERSION_CONSTRAINT_NAMES,
+		.sym_num	= SYM_NUM,
+		.ocon_num	= OCON_NUM,
+	},
 };
 
 static struct policydb_compat_info *policydb_lookup_compat(int version)
@@ -613,6 +618,19 @@ static int common_destroy(void *key, void *datum, void *p)
 	return 0;
 }
 
+static void constraint_expr_destroy(struct constraint_expr *expr)
+{
+	if (expr != NULL) {
+		ebitmap_destroy(&expr->names);
+		if (expr->type_names != NULL) {
+			ebitmap_destroy(&expr->type_names->types);
+			ebitmap_destroy(&expr->type_names->negset);
+			kfree(expr->type_names);
+		}
+		kfree(expr);
+	}
+}
+
 static int cls_destroy(void *key, void *datum, void *p)
 {
 	struct class_datum *cladatum;
@@ -628,10 +646,9 @@ static int cls_destroy(void *key, void *datum, void *p)
 		while (constraint) {
 			e = constraint->expr;
 			while (e) {
-				ebitmap_destroy(&e->names);
 				etmp = e;
 				e = e->next;
-				kfree(etmp);
+				constraint_expr_destroy(etmp);
 			}
 			ctemp = constraint;
 			constraint = constraint->next;
@@ -642,16 +659,14 @@ static int cls_destroy(void *key, void *datum, void *p)
 		while (constraint) {
 			e = constraint->expr;
 			while (e) {
-				ebitmap_destroy(&e->names);
 				etmp = e;
 				e = e->next;
-				kfree(etmp);
+				constraint_expr_destroy(etmp);
 			}
 			ctemp = constraint;
 			constraint = constraint->next;
 			kfree(ctemp);
 		}
-
 		kfree(cladatum->comkey);
 	}
 	kfree(datum);
@@ -775,7 +790,7 @@ void policydb_destroy(struct policydb *p)
 
 	for (i = 0; i < SYM_NUM; i++) {
 		cond_resched();
-		hashtab_map(p->symtab[i].table, destroy_f[i], NULL);
+		hashtab_map(p->symtab[i].table, destroy_f[i], p);
 		hashtab_destroy(p->symtab[i].table);
 	}
 
@@ -1156,8 +1171,34 @@ bad:
 	return rc;
 }
 
-static int read_cons_helper(struct constraint_node **nodep, int ncons,
-			    int allowxtarget, void *fp)
+static void type_set_init(struct type_set *t)
+{
+	ebitmap_init(&t->types);
+	ebitmap_init(&t->negset);
+}
+
+static int type_set_read(struct type_set *t, void *fp)
+{
+	__le32 buf[1];
+	int rc;
+
+	if (ebitmap_read(&t->types, fp))
+		return -EINVAL;
+	if (ebitmap_read(&t->negset, fp))
+		return -EINVAL;
+
+	rc = next_entry(buf, fp, sizeof(u32));
+	if (rc < 0)
+		return -EINVAL;
+	t->flags = le32_to_cpu(buf[0]);
+
+	return 0;
+}
+
+
+static int read_cons_helper(struct policydb *p,
+				struct constraint_node **nodep,
+				int ncons, int allowxtarget, void *fp)
 {
 	struct constraint_node *c, *lc;
 	struct constraint_expr *e, *le;
@@ -1225,6 +1266,15 @@ static int read_cons_helper(struct constraint_node **nodep, int ncons,
 				rc = ebitmap_read(&e->names, fp);
 				if (rc)
 					return rc;
+				if (p->policyvers >= POLICYDB_VERSION_CONSTRAINT_NAMES) {
+					e->type_names = kzalloc(sizeof(*e->type_names), GFP_KERNEL);
+					if (!e->type_names)
+						return -ENOMEM;
+					type_set_init(e->type_names);
+					rc = type_set_read(e->type_names, fp);
+					if (rc)
+						return rc;
+				}
 				break;
 			default:
 				return -EINVAL;
@@ -1301,7 +1351,7 @@ static int class_read(struct policydb *p, struct hashtab *h, void *fp)
 			goto bad;
 	}
 
-	rc = read_cons_helper(&cladatum->constraints, ncons, 0, fp);
+	rc = read_cons_helper(p, &cladatum->constraints, ncons, 0, fp);
 	if (rc)
 		goto bad;
 
@@ -1311,7 +1361,7 @@ static int class_read(struct policydb *p, struct hashtab *h, void *fp)
 		if (rc)
 			goto bad;
 		ncons = le32_to_cpu(buf[0]);
-		rc = read_cons_helper(&cladatum->validatetrans, ncons, 1, fp);
+		rc = read_cons_helper(p, &cladatum->validatetrans, ncons, 1, fp);
 		if (rc)
 			goto bad;
 	}
@@ -1339,7 +1389,7 @@ static int class_read(struct policydb *p, struct hashtab *h, void *fp)
 
 	return 0;
 bad:
-	cls_destroy(key, cladatum, NULL);
+	cls_destroy(key, cladatum, p);
 	return rc;
 }
 
@@ -2750,6 +2800,24 @@ static int common_write(void *vkey, void *datum, void *ptr)
 	return 0;
 }
 
+static int type_set_write(struct type_set *t, void *fp)
+{
+	int rc;
+	__le32 buf[1];
+
+	if (ebitmap_write(&t->types, fp))
+		return -EINVAL;
+	if (ebitmap_write(&t->negset, fp))
+		return -EINVAL;
+
+	buf[0] = cpu_to_le32(t->flags);
+	rc = put_entry(buf, sizeof(u32), 1, fp);
+	if (rc)
+		return -EINVAL;
+
+	return 0;
+}
+
 static int write_cons_helper(struct policydb *p, struct constraint_node *node,
 			     void *fp)
 {
@@ -2781,6 +2849,11 @@ static int write_cons_helper(struct policydb *p, struct constraint_node *node,
 				rc = ebitmap_write(&e->names, fp);
 				if (rc)
 					return rc;
+				if (p->policyvers >= POLICYDB_VERSION_CONSTRAINT_NAMES) {
+					rc = type_set_write(e->type_names, fp);
+					if (rc)
+						return rc;
+				}
 				break;
 			default:
 				break;
diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
index da63747..725d594 100644
--- a/security/selinux/ss/policydb.h
+++ b/security/selinux/ss/policydb.h
@@ -154,6 +154,17 @@ struct cond_bool_datum {
 struct cond_node;
 
 /*
+ * type set preserves data needed to determine constraint info from
+ * policy source. This is not used by the kernel policy but allows
+ * utilities such as audit2allow to determine constraint denials.
+ */
+struct type_set {
+	struct ebitmap types;
+	struct ebitmap negset;
+	u32 flags;
+};
+
+/*
  * The configuration data includes security contexts for
  * initial SIDs, unlabeled file systems, TCP and UDP port numbers,
  * network interfaces, and nodes.  This structure stores the
-- 
1.8.3.1

Re: SELinux: Update policy version to support constraints info

[Stephen Smalley]

On 10/31/2013 10:08 AM, Richard Haines wrote:
> Here is my go at another kernel patch. I've done most of what you asked
> except I could not work out this  comment:
> 
> "Couldn't we avoid the need for passing down the policydb and checking
> the version just by ensuring e->type_names gets initialized to NULL and
> then the type_set_destroy() and kfree() calls degenerate to no-ops?  I
> suppose this works, but it complicates the code a bit."

In the prior patch, you changed cls_destroy() to extract the policydb
from the void* argument and check the p->policyvers to decide whether or
not to free e->type_names.  I pointed out that if you always initialize
it to NULL (which I think you get for free by virtue of the kazalloc of
the constraint_expr), then you can unconditionally free it.  In this
patch, it appears you switched to unconditionally calling
constraint_expr_destroy() but you still have the change to pass p
(policydb) to the hashtab_map(p->symtab[i].table, destroy_f[i], p);
Which I don't believe you need anymore.  Right?  That was my point.

You can abbreviate if (a != NULL) as if (a).

./scripts/checkpatch.pl has some complaints about your line lengths.
Those aren't mandatory but consider whether you can fix them without
making the code ugly.

Try to post the patch in a fresh message with the patch inlined if
possible or send directly via git-send-email for easier commenting
inline and application.







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