[PATCH 0/3] Some SELinux patches for 2.6.26

[PATCH 0/3] Some SELinux patches for 2.6.26

[Paul Moore]

Here are three patches I'd like to throw out for 2.6.26.  The first you have
already seen, it's the secid/secctx cleanup patch with some feedback
incorporated (and a sign-off).  The second patch is a tweak of the network
node cache that was introduced in 2.6.25 which should make cache additions
faster and less kludgy.  The third, and final, patch is the network port cache
that was suggested several months ago; it is similar to the existing network
interface and port cacheseseses.

All have gone through various levels of testing on various states of Rawhide
but I just did another simple boot/ping/ssh test on today's Rawhide without
any drama.  If they look okay to everyone please consider these for 2.6.26.

Thanks.

-- 
paul moore
linux security @ hp

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

[PATCH 1/3] SELinux: Cleanup the secid/secctx conversion functions

[Paul Moore]

While looking at the SELinux secid/secctx conversion functions I realized they
could probably do with a little cleanup to reduce the amount of code and make
better use of existing string processing functions in the kernel.  Making use
of the kernel's existing string processing functions is a good idea as many
architectures have specialized/optimized routines which should be an
improvement over the generic code in the SELinux security server.

Signed-off-by: Paul Moore <paul.moore@hp.com>
---

 security/selinux/ss/mls.c      |   61 +++++--------
 security/selinux/ss/mls.h      |    3 -
 security/selinux/ss/services.c |  188 ++++++++++++++++------------------------
 3 files changed, 99 insertions(+), 153 deletions(-)

diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c
index feaf0a5..ba211b4 100644
--- a/security/selinux/ss/mls.c
+++ b/security/selinux/ss/mls.c
@@ -239,8 +239,7 @@ int mls_context_isvalid(struct policydb *p, struct context *c)
  * Policy read-lock must be held for sidtab lookup.
  *
  */
-int mls_context_to_sid(char oldc,
-		       char **scontext,
+int mls_context_to_sid(char **scontext,
 		       struct context *context,
 		       struct sidtab *s,
 		       u32 def_sid)
@@ -250,10 +249,10 @@ int mls_context_to_sid(char oldc,
 	char *scontextp, *p, *rngptr;
 	struct level_datum *levdatum;
 	struct cat_datum *catdatum, *rngdatum;
-	int l, rc = -EINVAL;
+	int l, rc;
 
 	if (!selinux_mls_enabled) {
-		if (def_sid != SECSID_NULL && oldc)
+		if (def_sid != SECSID_NULL && *scontext && **scontext)
 			*scontext += strlen(*scontext)+1;
 		return 0;
 	}
@@ -262,18 +261,17 @@ int mls_context_to_sid(char oldc,
 	 * No MLS component to the security context, try and map to
 	 * default if provided.
 	 */
-	if (!oldc) {
+	if (!(*scontext) || !(**scontext)) {
 		struct context *defcon;
 
 		if (def_sid == SECSID_NULL)
-			goto out;
+			return -EINVAL;
 
 		defcon = sidtab_search(s, def_sid);
 		if (!defcon)
-			goto out;
+			return -EINVAL;
 
-		rc = mls_context_cpy(context, defcon);
-		goto out;
+		return mls_context_cpy(context, defcon);
 	}
 
 	/* Extract low sensitivity. */
@@ -287,10 +285,8 @@ int mls_context_to_sid(char oldc,
 
 	for (l = 0; l < 2; l++) {
 		levdatum = hashtab_search(policydb.p_levels.table, scontextp);
-		if (!levdatum) {
-			rc = -EINVAL;
-			goto out;
-		}
+		if (!levdatum)
+			return -EINVAL;
 
 		context->range.level[l].sens = levdatum->level->sens;
 
@@ -312,35 +308,29 @@ int mls_context_to_sid(char oldc,
 
 				catdatum = hashtab_search(policydb.p_cats.table,
 				                          scontextp);
-				if (!catdatum) {
-					rc = -EINVAL;
-					goto out;
-				}
+				if (!catdatum)
+					return -EINVAL;
 
 				rc = ebitmap_set_bit(&context->range.level[l].cat,
 				                     catdatum->value - 1, 1);
 				if (rc)
-					goto out;
+					return rc;
 
 				/* If range, set all categories in range */
 				if (rngptr) {
 					int i;
 
 					rngdatum = hashtab_search(policydb.p_cats.table, rngptr);
-					if (!rngdatum) {
-						rc = -EINVAL;
-						goto out;
-					}
+					if (!rngdatum)
+						return -EINVAL;
 
-					if (catdatum->value >= rngdatum->value) {
-						rc = -EINVAL;
-						goto out;
-					}
+					if (catdatum->value >= rngdatum->value)
+						return -EINVAL;
 
 					for (i = catdatum->value; i < rngdatum->value; i++) {
 						rc = ebitmap_set_bit(&context->range.level[l].cat, i, 1);
 						if (rc)
-							goto out;
+							return rc;
 					}
 				}
 
@@ -366,12 +356,10 @@ int mls_context_to_sid(char oldc,
 		rc = ebitmap_cpy(&context->range.level[1].cat,
 				 &context->range.level[0].cat);
 		if (rc)
-			goto out;
+			return rc;
 	}
 	*scontext = ++p;
-	rc = 0;
-out:
-	return rc;
+	return 0;
 }
 
 /*
@@ -391,13 +379,10 @@ int mls_from_string(char *str, struct context *context, gfp_t gfp_mask)
 	/* we need freestr because mls_context_to_sid will change
 	   the value of tmpstr */
 	tmpstr = freestr = kstrdup(str, gfp_mask);
-	if (!tmpstr) {
-		rc = -ENOMEM;
-	} else {
-		rc = mls_context_to_sid(':', &tmpstr, context,
-		                        NULL, SECSID_NULL);
-		kfree(freestr);
-	}
+	if (!tmpstr)
+		return -ENOMEM;
+	rc = mls_context_to_sid(&tmpstr, context, NULL, SECSID_NULL);
+	kfree(freestr);
 
 	return rc;
 }
diff --git a/security/selinux/ss/mls.h b/security/selinux/ss/mls.h
index ab53663..b364f61 100644
--- a/security/selinux/ss/mls.h
+++ b/security/selinux/ss/mls.h
@@ -30,8 +30,7 @@ int mls_context_isvalid(struct policydb *p, struct context *c);
 int mls_range_isvalid(struct policydb *p, struct mls_range *r);
 int mls_level_isvalid(struct policydb *p, struct mls_level *l);
 
-int mls_context_to_sid(char oldc,
-	               char **scontext,
+int mls_context_to_sid(char **scontext,
 		       struct context *context,
 		       struct sidtab *s,
 		       u32 def_sid);
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index f374186..c9943ca 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -573,47 +573,42 @@ out:
 	return rc;
 }
 
-/*
- * Write the security context string representation of
- * the context structure `context' into a dynamically
- * allocated string of the correct size.  Set `*scontext'
- * to point to this string and set `*scontext_len' to
- * the length of the string.
+/**
+ * context_struct_to_string - Generate a string representation of a context
+ * @context: native security context
+ * @scontext: security context string
+ * @scontext_len: length of security context string
+ *
+ * Description:
+ * Write the security context string representation of @context into
+ * @scontext.  The caller must free @scontext when finished.  Returns zero on
+ * success, negative values on failure.
+ *
  */
 static int context_struct_to_string(struct context *context, char **scontext, u32 *scontext_len)
 {
-	char *scontextp;
+	char *ctx;
 
-	*scontext = NULL;
-	*scontext_len = 0;
-
-	/* Compute the size of the context. */
-	*scontext_len += strlen(policydb.p_user_val_to_name[context->user - 1]) + 1;
-	*scontext_len += strlen(policydb.p_role_val_to_name[context->role - 1]) + 1;
-	*scontext_len += strlen(policydb.p_type_val_to_name[context->type - 1]) + 1;
-	*scontext_len += mls_compute_context_len(context);
+	*scontext_len = strlen(policydb.p_user_val_to_name[context->user - 1]) +
+		strlen(policydb.p_role_val_to_name[context->role - 1]) +
+		strlen(policydb.p_type_val_to_name[context->type - 1]) +
+		mls_compute_context_len(context) + 3;
 
 	/* Allocate space for the context; caller must free this space. */
-	scontextp = kmalloc(*scontext_len, GFP_ATOMIC);
-	if (!scontextp) {
+	*scontext = kmalloc(*scontext_len, GFP_ATOMIC);
+	if (!*scontext) {
+		*scontext_len = 0;
 		return -ENOMEM;
 	}
-	*scontext = scontextp;
 
-	/*
-	 * Copy the user name, role name and type name into the context.
-	 */
-	sprintf(scontextp, "%s:%s:%s",
-		policydb.p_user_val_to_name[context->user - 1],
-		policydb.p_role_val_to_name[context->role - 1],
-		policydb.p_type_val_to_name[context->type - 1]);
-	scontextp += strlen(policydb.p_user_val_to_name[context->user - 1]) +
-	             1 + strlen(policydb.p_role_val_to_name[context->role - 1]) +
-	             1 + strlen(policydb.p_type_val_to_name[context->type - 1]);
-
-	mls_sid_to_context(context, &scontextp);
-
-	*scontextp = 0;
+	ctx = *scontext;
+	strcpy(ctx, policydb.p_user_val_to_name[context->user - 1]);
+	strcat(ctx, ":");
+	strcat(ctx, policydb.p_role_val_to_name[context->role - 1]);
+	strcat(ctx, ":");
+	strcat(ctx, policydb.p_type_val_to_name[context->type - 1]);
+	ctx += strlen(ctx);
+	mls_sid_to_context(context, &ctx);
 
 	return 0;
 }
@@ -640,68 +635,61 @@ const char *security_get_initial_sid_context(u32 sid)
 int security_sid_to_context(u32 sid, char **scontext, u32 *scontext_len)
 {
 	struct context *context;
-	int rc = 0;
-
-	*scontext = NULL;
-	*scontext_len  = 0;
+	int rc;
 
 	if (!ss_initialized) {
-		if (sid <= SECINITSID_NUM) {
-			char *scontextp;
-
-			*scontext_len = strlen(initial_sid_to_string[sid]) + 1;
-			scontextp = kmalloc(*scontext_len,GFP_ATOMIC);
-			if (!scontextp) {
-				rc = -ENOMEM;
-				goto out;
-			}
-			strcpy(scontextp, initial_sid_to_string[sid]);
-			*scontext = scontextp;
-			goto out;
+		if (sid > SECINITSID_NUM) {
+			printk(KERN_ERR
+			       "security_sid_to_context:  called before "
+			       "initial load_policy on unknown SID %d\n", sid);
+			goto err;
 		}
-		printk(KERN_ERR "security_sid_to_context:  called before initial "
-		       "load_policy on unknown SID %d\n", sid);
-		rc = -EINVAL;
-		goto out;
+		*scontext = kstrdup(initial_sid_to_string[sid], GFP_ATOMIC);
+		if (*scontext == NULL)
+			return -ENOMEM;
+		*scontext_len = strlen(*scontext) + 1;
+		return 0;
 	}
+
 	POLICY_RDLOCK;
 	context = sidtab_search(&sidtab, sid);
 	if (!context) {
-		printk(KERN_ERR "security_sid_to_context:  unrecognized SID "
-		       "%d\n", sid);
-		rc = -EINVAL;
-		goto out_unlock;
+		POLICY_RDUNLOCK;
+		printk(KERN_ERR
+		       "security_sid_to_context:  unrecognized SID %d\n", sid);
+		goto err;
 	}
 	rc = context_struct_to_string(context, scontext, scontext_len);
-out_unlock:
 	POLICY_RDUNLOCK;
-out:
 	return rc;
 
+err:
+	*scontext = NULL;
+	*scontext_len = 0;
+	return -EINVAL;
 }
 
 static int security_context_to_sid_core(char *scontext, u32 scontext_len, u32 *sid, u32 def_sid)
 {
-	char *scontext2;
+	char *scontext_dup, *scontext_iter, *scontext_next;
 	struct context context;
-	struct role_datum *role;
+	struct role_datum *roledatum;
 	struct type_datum *typdatum;
 	struct user_datum *usrdatum;
-	char *scontextp, *p, oldc;
-	int rc = 0;
+	int rc = -EINVAL;
 
 	if (!ss_initialized) {
 		int i;
 
-		for (i = 1; i < SECINITSID_NUM; i++) {
+		for (i = 1; i < SECINITSID_NUM; i++)
 			if (!strcmp(initial_sid_to_string[i], scontext)) {
 				*sid = i;
-				goto out;
+				return 0;
 			}
-		}
 		*sid = SECINITSID_KERNEL;
-		goto out;
+		return 0;
 	}
+
 	*sid = SECSID_NULL;
 
 	/* Copy the string so that we can modify the copy as we parse it.
@@ -709,73 +697,46 @@ static int security_context_to_sid_core(char *scontext, u32 scontext_len, u32 *s
 	   null suffix to the copy to avoid problems with the existing
 	   attr package, which doesn't view the null terminator as part
 	   of the attribute value. */
-	scontext2 = kmalloc(scontext_len+1,GFP_KERNEL);
-	if (!scontext2) {
-		rc = -ENOMEM;
-		goto out;
-	}
-	memcpy(scontext2, scontext, scontext_len);
-	scontext2[scontext_len] = 0;
+	scontext_dup = kmemdup(scontext, scontext_len + 1, GFP_KERNEL);
+	if (!scontext_dup)
+		return -ENOMEM;
+	scontext_dup[scontext_len] = 0;
 
 	context_init(&context);
-	*sid = SECSID_NULL;
+	scontext_next = scontext_dup;
 
 	POLICY_RDLOCK;
 
-	/* Parse the security context. */
-
-	rc = -EINVAL;
-	scontextp = (char *) scontext2;
-
 	/* Extract the user. */
-	p = scontextp;
-	while (*p && *p != ':')
-		p++;
-
-	if (*p == 0)
+	scontext_iter = strsep(&scontext_next, ":");
+	if (!scontext_next)
 		goto out_unlock;
-
-	*p++ = 0;
-
-	usrdatum = hashtab_search(policydb.p_users.table, scontextp);
+	usrdatum = hashtab_search(policydb.p_users.table, scontext_iter);
 	if (!usrdatum)
 		goto out_unlock;
-
 	context.user = usrdatum->value;
 
 	/* Extract role. */
-	scontextp = p;
-	while (*p && *p != ':')
-		p++;
-
-	if (*p == 0)
+	scontext_iter = strsep(&scontext_next, ":");
+	if (!scontext_next)
 		goto out_unlock;
-
-	*p++ = 0;
-
-	role = hashtab_search(policydb.p_roles.table, scontextp);
-	if (!role)
+	roledatum = hashtab_search(policydb.p_roles.table, scontext_iter);
+	if (!roledatum)
 		goto out_unlock;
-	context.role = role->value;
+	context.role = roledatum->value;
 
 	/* Extract type. */
-	scontextp = p;
-	while (*p && *p != ':')
-		p++;
-	oldc = *p;
-	*p++ = 0;
-
-	typdatum = hashtab_search(policydb.p_types.table, scontextp);
+	scontext_iter = strsep(&scontext_next, ":");
+	typdatum = hashtab_search(policydb.p_types.table, scontext_iter);
 	if (!typdatum)
 		goto out_unlock;
-
 	context.type = typdatum->value;
 
-	rc = mls_context_to_sid(oldc, &p, &context, &sidtab, def_sid);
+	/* Extract MLS range. */
+	rc = mls_context_to_sid(&scontext_next, &context, &sidtab, def_sid);
 	if (rc)
 		goto out_unlock;
-
-	if ((p - scontext2) < scontext_len) {
+	if (scontext_next - scontext_dup < scontext_len) {
 		rc = -EINVAL;
 		goto out_unlock;
 	}
@@ -785,13 +746,14 @@ static int security_context_to_sid_core(char *scontext, u32 scontext_len, u32 *s
 		rc = -EINVAL;
 		goto out_unlock;
 	}
+
 	/* Obtain the new sid. */
 	rc = sidtab_context_to_sid(&sidtab, &context, sid);
+
 out_unlock:
 	POLICY_RDUNLOCK;
 	context_destroy(&context);
-	kfree(scontext2);
-out:
+	kfree(scontext_dup);
 	return rc;
 }
 


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

Re: [PATCH 1/3] SELinux: Cleanup the secid/secctx conversion functions

[Stephen Smalley]

On Mon, 2008-04-07 at 19:11 -0400, Paul Moore wrote:
> While looking at the SELinux secid/secctx conversion functions I realized they
> could probably do with a little cleanup to reduce the amount of code and make
> better use of existing string processing functions in the kernel.  Making use
> of the kernel's existing string processing functions is a good idea as many
> architectures have specialized/optimized routines which should be an
> improvement over the generic code in the SELinux security server.

Needs to be re-based for the recent GFP_NOFS changes,
http://marc.info/?t=120716967100004&r=1&w=2
http://marc.info/?l=git-commits-head&m=120762715428197&w=2

> 
> Signed-off-by: Paul Moore <paul.moore@hp.com>
> ---
> 
>  security/selinux/ss/mls.c      |   61 +++++--------
>  security/selinux/ss/mls.h      |    3 -
>  security/selinux/ss/services.c |  188 ++++++++++++++++------------------------
>  3 files changed, 99 insertions(+), 153 deletions(-)
> 
> diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c
> index feaf0a5..ba211b4 100644
> --- a/security/selinux/ss/mls.c
> +++ b/security/selinux/ss/mls.c
> @@ -239,8 +239,7 @@ int mls_context_isvalid(struct policydb *p, struct context *c)
>   * Policy read-lock must be held for sidtab lookup.
>   *
>   */
> -int mls_context_to_sid(char oldc,
> -		       char **scontext,
> +int mls_context_to_sid(char **scontext,
>  		       struct context *context,
>  		       struct sidtab *s,
>  		       u32 def_sid)
> @@ -250,10 +249,10 @@ int mls_context_to_sid(char oldc,
>  	char *scontextp, *p, *rngptr;
>  	struct level_datum *levdatum;
>  	struct cat_datum *catdatum, *rngdatum;
> -	int l, rc = -EINVAL;
> +	int l, rc;
>  
>  	if (!selinux_mls_enabled) {
> -		if (def_sid != SECSID_NULL && oldc)
> +		if (def_sid != SECSID_NULL && *scontext && **scontext)
>  			*scontext += strlen(*scontext)+1;
>  		return 0;
>  	}
> @@ -262,18 +261,17 @@ int mls_context_to_sid(char oldc,
>  	 * No MLS component to the security context, try and map to
>  	 * default if provided.
>  	 */
> -	if (!oldc) {
> +	if (!(*scontext) || !(**scontext)) {
>  		struct context *defcon;
>  
>  		if (def_sid == SECSID_NULL)
> -			goto out;
> +			return -EINVAL;
>  
>  		defcon = sidtab_search(s, def_sid);
>  		if (!defcon)
> -			goto out;
> +			return -EINVAL;
>  
> -		rc = mls_context_cpy(context, defcon);
> -		goto out;
> +		return mls_context_cpy(context, defcon);
>  	}
>  
>  	/* Extract low sensitivity. */
> @@ -287,10 +285,8 @@ int mls_context_to_sid(char oldc,
>  
>  	for (l = 0; l < 2; l++) {
>  		levdatum = hashtab_search(policydb.p_levels.table, scontextp);
> -		if (!levdatum) {
> -			rc = -EINVAL;
> -			goto out;
> -		}
> +		if (!levdatum)
> +			return -EINVAL;
>  
>  		context->range.level[l].sens = levdatum->level->sens;
>  
> @@ -312,35 +308,29 @@ int mls_context_to_sid(char oldc,
>  
>  				catdatum = hashtab_search(policydb.p_cats.table,
>  				                          scontextp);
> -				if (!catdatum) {
> -					rc = -EINVAL;
> -					goto out;
> -				}
> +				if (!catdatum)
> +					return -EINVAL;
>  
>  				rc = ebitmap_set_bit(&context->range.level[l].cat,
>  				                     catdatum->value - 1, 1);
>  				if (rc)
> -					goto out;
> +					return rc;
>  
>  				/* If range, set all categories in range */
>  				if (rngptr) {
>  					int i;
>  
>  					rngdatum = hashtab_search(policydb.p_cats.table, rngptr);
> -					if (!rngdatum) {
> -						rc = -EINVAL;
> -						goto out;
> -					}
> +					if (!rngdatum)
> +						return -EINVAL;
>  
> -					if (catdatum->value >= rngdatum->value) {
> -						rc = -EINVAL;
> -						goto out;
> -					}
> +					if (catdatum->value >= rngdatum->value)
> +						return -EINVAL;
>  
>  					for (i = catdatum->value; i < rngdatum->value; i++) {
>  						rc = ebitmap_set_bit(&context->range.level[l].cat, i, 1);
>  						if (rc)
> -							goto out;
> +							return rc;
>  					}
>  				}
>  
> @@ -366,12 +356,10 @@ int mls_context_to_sid(char oldc,
>  		rc = ebitmap_cpy(&context->range.level[1].cat,
>  				 &context->range.level[0].cat);
>  		if (rc)
> -			goto out;
> +			return rc;
>  	}
>  	*scontext = ++p;
> -	rc = 0;
> -out:
> -	return rc;
> +	return 0;
>  }
>  
>  /*
> @@ -391,13 +379,10 @@ int mls_from_string(char *str, struct context *context, gfp_t gfp_mask)
>  	/* we need freestr because mls_context_to_sid will change
>  	   the value of tmpstr */
>  	tmpstr = freestr = kstrdup(str, gfp_mask);
> -	if (!tmpstr) {
> -		rc = -ENOMEM;
> -	} else {
> -		rc = mls_context_to_sid(':', &tmpstr, context,
> -		                        NULL, SECSID_NULL);
> -		kfree(freestr);
> -	}
> +	if (!tmpstr)
> +		return -ENOMEM;
> +	rc = mls_context_to_sid(&tmpstr, context, NULL, SECSID_NULL);
> +	kfree(freestr);
>  
>  	return rc;
>  }
> diff --git a/security/selinux/ss/mls.h b/security/selinux/ss/mls.h
> index ab53663..b364f61 100644
> --- a/security/selinux/ss/mls.h
> +++ b/security/selinux/ss/mls.h
> @@ -30,8 +30,7 @@ int mls_context_isvalid(struct policydb *p, struct context *c);
>  int mls_range_isvalid(struct policydb *p, struct mls_range *r);
>  int mls_level_isvalid(struct policydb *p, struct mls_level *l);
>  
> -int mls_context_to_sid(char oldc,
> -	               char **scontext,
> +int mls_context_to_sid(char **scontext,
>  		       struct context *context,
>  		       struct sidtab *s,
>  		       u32 def_sid);
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index f374186..c9943ca 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -573,47 +573,42 @@ out:
>  	return rc;
>  }
>  
> -/*
> - * Write the security context string representation of
> - * the context structure `context' into a dynamically
> - * allocated string of the correct size.  Set `*scontext'
> - * to point to this string and set `*scontext_len' to
> - * the length of the string.
> +/**
> + * context_struct_to_string - Generate a string representation of a context
> + * @context: native security context
> + * @scontext: security context string
> + * @scontext_len: length of security context string
> + *
> + * Description:
> + * Write the security context string representation of @context into
> + * @scontext.  The caller must free @scontext when finished.  Returns zero on
> + * success, negative values on failure.
> + *
>   */
>  static int context_struct_to_string(struct context *context, char **scontext, u32 *scontext_len)
>  {
> -	char *scontextp;
> +	char *ctx;
>  
> -	*scontext = NULL;
> -	*scontext_len = 0;
> -
> -	/* Compute the size of the context. */
> -	*scontext_len += strlen(policydb.p_user_val_to_name[context->user - 1]) + 1;
> -	*scontext_len += strlen(policydb.p_role_val_to_name[context->role - 1]) + 1;
> -	*scontext_len += strlen(policydb.p_type_val_to_name[context->type - 1]) + 1;
> -	*scontext_len += mls_compute_context_len(context);
> +	*scontext_len = strlen(policydb.p_user_val_to_name[context->user - 1]) +
> +		strlen(policydb.p_role_val_to_name[context->role - 1]) +
> +		strlen(policydb.p_type_val_to_name[context->type - 1]) +
> +		mls_compute_context_len(context) + 3;
>  
>  	/* Allocate space for the context; caller must free this space. */
> -	scontextp = kmalloc(*scontext_len, GFP_ATOMIC);
> -	if (!scontextp) {
> +	*scontext = kmalloc(*scontext_len, GFP_ATOMIC);
> +	if (!*scontext) {
> +		*scontext_len = 0;
>  		return -ENOMEM;
>  	}
> -	*scontext = scontextp;
>  
> -	/*
> -	 * Copy the user name, role name and type name into the context.
> -	 */
> -	sprintf(scontextp, "%s:%s:%s",
> -		policydb.p_user_val_to_name[context->user - 1],
> -		policydb.p_role_val_to_name[context->role - 1],
> -		policydb.p_type_val_to_name[context->type - 1]);
> -	scontextp += strlen(policydb.p_user_val_to_name[context->user - 1]) +
> -	             1 + strlen(policydb.p_role_val_to_name[context->role - 1]) +
> -	             1 + strlen(policydb.p_type_val_to_name[context->type - 1]);
> -
> -	mls_sid_to_context(context, &scontextp);
> -
> -	*scontextp = 0;
> +	ctx = *scontext;
> +	strcpy(ctx, policydb.p_user_val_to_name[context->user - 1]);
> +	strcat(ctx, ":");
> +	strcat(ctx, policydb.p_role_val_to_name[context->role - 1]);
> +	strcat(ctx, ":");
> +	strcat(ctx, policydb.p_type_val_to_name[context->type - 1]);
> +	ctx += strlen(ctx);
> +	mls_sid_to_context(context, &ctx);
>  
>  	return 0;
>  }
> @@ -640,68 +635,61 @@ const char *security_get_initial_sid_context(u32 sid)
>  int security_sid_to_context(u32 sid, char **scontext, u32 *scontext_len)
>  {
>  	struct context *context;
> -	int rc = 0;
> -
> -	*scontext = NULL;
> -	*scontext_len  = 0;
> +	int rc;
>  
>  	if (!ss_initialized) {
> -		if (sid <= SECINITSID_NUM) {
> -			char *scontextp;
> -
> -			*scontext_len = strlen(initial_sid_to_string[sid]) + 1;
> -			scontextp = kmalloc(*scontext_len,GFP_ATOMIC);
> -			if (!scontextp) {
> -				rc = -ENOMEM;
> -				goto out;
> -			}
> -			strcpy(scontextp, initial_sid_to_string[sid]);
> -			*scontext = scontextp;
> -			goto out;
> +		if (sid > SECINITSID_NUM) {
> +			printk(KERN_ERR
> +			       "security_sid_to_context:  called before "
> +			       "initial load_policy on unknown SID %d\n", sid);
> +			goto err;
>  		}
> -		printk(KERN_ERR "security_sid_to_context:  called before initial "
> -		       "load_policy on unknown SID %d\n", sid);
> -		rc = -EINVAL;
> -		goto out;
> +		*scontext = kstrdup(initial_sid_to_string[sid], GFP_ATOMIC);
> +		if (*scontext == NULL)
> +			return -ENOMEM;
> +		*scontext_len = strlen(*scontext) + 1;
> +		return 0;
>  	}
> +
>  	POLICY_RDLOCK;
>  	context = sidtab_search(&sidtab, sid);
>  	if (!context) {
> -		printk(KERN_ERR "security_sid_to_context:  unrecognized SID "
> -		       "%d\n", sid);
> -		rc = -EINVAL;
> -		goto out_unlock;
> +		POLICY_RDUNLOCK;
> +		printk(KERN_ERR
> +		       "security_sid_to_context:  unrecognized SID %d\n", sid);
> +		goto err;
>  	}
>  	rc = context_struct_to_string(context, scontext, scontext_len);
> -out_unlock:
>  	POLICY_RDUNLOCK;
> -out:
>  	return rc;
>  
> +err:
> +	*scontext = NULL;
> +	*scontext_len = 0;
> +	return -EINVAL;
>  }
>  
>  static int security_context_to_sid_core(char *scontext, u32 scontext_len, u32 *sid, u32 def_sid)
>  {
> -	char *scontext2;
> +	char *scontext_dup, *scontext_iter, *scontext_next;
>  	struct context context;
> -	struct role_datum *role;
> +	struct role_datum *roledatum;
>  	struct type_datum *typdatum;
>  	struct user_datum *usrdatum;
> -	char *scontextp, *p, oldc;
> -	int rc = 0;
> +	int rc = -EINVAL;
>  
>  	if (!ss_initialized) {
>  		int i;
>  
> -		for (i = 1; i < SECINITSID_NUM; i++) {
> +		for (i = 1; i < SECINITSID_NUM; i++)
>  			if (!strcmp(initial_sid_to_string[i], scontext)) {
>  				*sid = i;
> -				goto out;
> +				return 0;
>  			}
> -		}
>  		*sid = SECINITSID_KERNEL;
> -		goto out;
> +		return 0;
>  	}
> +
>  	*sid = SECSID_NULL;
>  
>  	/* Copy the string so that we can modify the copy as we parse it.
> @@ -709,73 +697,46 @@ static int security_context_to_sid_core(char *scontext, u32 scontext_len, u32 *s
>  	   null suffix to the copy to avoid problems with the existing
>  	   attr package, which doesn't view the null terminator as part
>  	   of the attribute value. */
> -	scontext2 = kmalloc(scontext_len+1,GFP_KERNEL);
> -	if (!scontext2) {
> -		rc = -ENOMEM;
> -		goto out;
> -	}
> -	memcpy(scontext2, scontext, scontext_len);
> -	scontext2[scontext_len] = 0;
> +	scontext_dup = kmemdup(scontext, scontext_len + 1, GFP_KERNEL);
> +	if (!scontext_dup)
> +		return -ENOMEM;
> +	scontext_dup[scontext_len] = 0;
>  
>  	context_init(&context);
> -	*sid = SECSID_NULL;
> +	scontext_next = scontext_dup;
>  
>  	POLICY_RDLOCK;
>  
> -	/* Parse the security context. */
> -
> -	rc = -EINVAL;
> -	scontextp = (char *) scontext2;
> -
>  	/* Extract the user. */
> -	p = scontextp;
> -	while (*p && *p != ':')
> -		p++;
> -
> -	if (*p == 0)
> +	scontext_iter = strsep(&scontext_next, ":");
> +	if (!scontext_next)
>  		goto out_unlock;
> -
> -	*p++ = 0;
> -
> -	usrdatum = hashtab_search(policydb.p_users.table, scontextp);
> +	usrdatum = hashtab_search(policydb.p_users.table, scontext_iter);
>  	if (!usrdatum)
>  		goto out_unlock;
> -
>  	context.user = usrdatum->value;
>  
>  	/* Extract role. */
> -	scontextp = p;
> -	while (*p && *p != ':')
> -		p++;
> -
> -	if (*p == 0)
> +	scontext_iter = strsep(&scontext_next, ":");
> +	if (!scontext_next)
>  		goto out_unlock;
> -
> -	*p++ = 0;
> -
> -	role = hashtab_search(policydb.p_roles.table, scontextp);
> -	if (!role)
> +	roledatum = hashtab_search(policydb.p_roles.table, scontext_iter);
> +	if (!roledatum)
>  		goto out_unlock;
> -	context.role = role->value;
> +	context.role = roledatum->value;
>  
>  	/* Extract type. */
> -	scontextp = p;
> -	while (*p && *p != ':')
> -		p++;
> -	oldc = *p;
> -	*p++ = 0;
> -
> -	typdatum = hashtab_search(policydb.p_types.table, scontextp);
> +	scontext_iter = strsep(&scontext_next, ":");
> +	typdatum = hashtab_search(policydb.p_types.table, scontext_iter);
>  	if (!typdatum)
>  		goto out_unlock;
> -
>  	context.type = typdatum->value;
>  
> -	rc = mls_context_to_sid(oldc, &p, &context, &sidtab, def_sid);
> +	/* Extract MLS range. */
> +	rc = mls_context_to_sid(&scontext_next, &context, &sidtab, def_sid);
>  	if (rc)
>  		goto out_unlock;
> -
> -	if ((p - scontext2) < scontext_len) {
> +	if (scontext_next - scontext_dup < scontext_len) {
>  		rc = -EINVAL;
>  		goto out_unlock;
>  	}
> @@ -785,13 +746,14 @@ static int security_context_to_sid_core(char *scontext, u32 scontext_len, u32 *s
>  		rc = -EINVAL;
>  		goto out_unlock;
>  	}
> +
>  	/* Obtain the new sid. */
>  	rc = sidtab_context_to_sid(&sidtab, &context, sid);
> +
>  out_unlock:
>  	POLICY_RDUNLOCK;
>  	context_destroy(&context);
> -	kfree(scontext2);
> -out:
> +	kfree(scontext_dup);
>  	return rc;
>  }
>  
> 
> 
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
> the words "unsubscribe selinux" without quotes as the message.
-- 
Stephen Smalley
National Security Agency


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

Re: [PATCH 1/3] SELinux: Cleanup the secid/secctx conversion functions

[Stephen Smalley]

On Mon, 2008-04-07 at 19:11 -0400, Paul Moore wrote:
> While looking at the SELinux secid/secctx conversion functions I realized they
> could probably do with a little cleanup to reduce the amount of code and make
> better use of existing string processing functions in the kernel.  Making use
> of the kernel's existing string processing functions is a good idea as many
> architectures have specialized/optimized routines which should be an
> improvement over the generic code in the SELinux security server.
> 
> Signed-off-by: Paul Moore <paul.moore@hp.com>
> ---
> 
>  security/selinux/ss/mls.c      |   61 +++++--------
>  security/selinux/ss/mls.h      |    3 -
>  security/selinux/ss/services.c |  188 ++++++++++++++++------------------------
>  3 files changed, 99 insertions(+), 153 deletions(-)

> @@ -709,73 +697,46 @@ static int security_context_to_sid_core(char *scontext, u32 scontext_len, u32 *s
>  	   null suffix to the copy to avoid problems with the existing
>  	   attr package, which doesn't view the null terminator as part
>  	   of the attribute value. */
> -	scontext2 = kmalloc(scontext_len+1,GFP_KERNEL);
> -	if (!scontext2) {
> -		rc = -ENOMEM;
> -		goto out;
> -	}
> -	memcpy(scontext2, scontext, scontext_len);
> -	scontext2[scontext_len] = 0;
> +	scontext_dup = kmemdup(scontext, scontext_len + 1, GFP_KERNEL);

Also, in addition to the gfp_flags change, I'm not clear that the above
change is correct.  We are taking a byte array "scontext" of length
"scontext_len" and copying it into a buffer of length "scontext_len+1"
so that we can ensure that it is NUL terminated prior to parsing.  Won't
kmemdup with scontext_len+1 ultimately run off the end of the original
string?

> +	if (!scontext_dup)
> +		return -ENOMEM;
> +	scontext_dup[scontext_len] = 0;

>  
>  	context_init(&context);
> -	*sid = SECSID_NULL;
> +	scontext_next = scontext_dup;
>  
>  	POLICY_RDLOCK;
>  
> -	/* Parse the security context. */
> -
> -	rc = -EINVAL;
> -	scontextp = (char *) scontext2;
> -
>  	/* Extract the user. */
> -	p = scontextp;
> -	while (*p && *p != ':')
> -		p++;
> -
> -	if (*p == 0)
> +	scontext_iter = strsep(&scontext_next, ":");
> +	if (!scontext_next)
>  		goto out_unlock;
> -
> -	*p++ = 0;
> -
> -	usrdatum = hashtab_search(policydb.p_users.table, scontextp);
> +	usrdatum = hashtab_search(policydb.p_users.table, scontext_iter);
>  	if (!usrdatum)
>  		goto out_unlock;
> -
>  	context.user = usrdatum->value;
>  
>  	/* Extract role. */
> -	scontextp = p;
> -	while (*p && *p != ':')
> -		p++;
> -
> -	if (*p == 0)
> +	scontext_iter = strsep(&scontext_next, ":");
> +	if (!scontext_next)
>  		goto out_unlock;
> -
> -	*p++ = 0;
> -
> -	role = hashtab_search(policydb.p_roles.table, scontextp);
> -	if (!role)
> +	roledatum = hashtab_search(policydb.p_roles.table, scontext_iter);
> +	if (!roledatum)
>  		goto out_unlock;
> -	context.role = role->value;
> +	context.role = roledatum->value;
>  
>  	/* Extract type. */
> -	scontextp = p;
> -	while (*p && *p != ':')
> -		p++;
> -	oldc = *p;
> -	*p++ = 0;
> -
> -	typdatum = hashtab_search(policydb.p_types.table, scontextp);
> +	scontext_iter = strsep(&scontext_next, ":");
> +	typdatum = hashtab_search(policydb.p_types.table, scontext_iter);
>  	if (!typdatum)
>  		goto out_unlock;
> -
>  	context.type = typdatum->value;
>  
> -	rc = mls_context_to_sid(oldc, &p, &context, &sidtab, def_sid);
> +	/* Extract MLS range. */
> +	rc = mls_context_to_sid(&scontext_next, &context, &sidtab, def_sid);
>  	if (rc)
>  		goto out_unlock;
> -
> -	if ((p - scontext2) < scontext_len) {
> +	if (scontext_next - scontext_dup < scontext_len) {
>  		rc = -EINVAL;
>  		goto out_unlock;
>  	}
> @@ -785,13 +746,14 @@ static int security_context_to_sid_core(char *scontext, u32 scontext_len, u32 *s
>  		rc = -EINVAL;
>  		goto out_unlock;
>  	}
> +
>  	/* Obtain the new sid. */
>  	rc = sidtab_context_to_sid(&sidtab, &context, sid);
> +
>  out_unlock:
>  	POLICY_RDUNLOCK;
>  	context_destroy(&context);
> -	kfree(scontext2);
> -out:
> +	kfree(scontext_dup);
>  	return rc;
>  }
>  
> 
> 
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
> the words "unsubscribe selinux" without quotes as the message.
-- 
Stephen Smalley
National Security Agency


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

Re: [PATCH 1/3] SELinux: Cleanup the secid/secctx conversion functions

[Paul Moore]

On Tuesday 08 April 2008 10:43:44 am Stephen Smalley wrote:
> On Mon, 2008-04-07 at 19:11 -0400, Paul Moore wrote:
> > @@ -709,73 +697,46 @@ static int security_context_to_sid_core(char
> > *scontext, u32 scontext_len, u32 *s null suffix to the copy to
> > avoid problems with the existing attr package, which doesn't view
> > the null terminator as part of the attribute value. */
> > -	scontext2 = kmalloc(scontext_len+1,GFP_KERNEL);
> > -	if (!scontext2) {
> > -		rc = -ENOMEM;
> > -		goto out;
> > -	}
> > -	memcpy(scontext2, scontext, scontext_len);
> > -	scontext2[scontext_len] = 0;
> > +	scontext_dup = kmemdup(scontext, scontext_len + 1, GFP_KERNEL);
>
> Also, in addition to the gfp_flags change, I'm not clear that the
> above change is correct.  We are taking a byte array "scontext" of
> length "scontext_len" and copying it into a buffer of length
> "scontext_len+1" so that we can ensure that it is NUL terminated
> prior to parsing.  Won't kmemdup with scontext_len+1 ultimately run
> off the end of the original string?

Good point, I believe you're right.  I'll add this and the gfp stuff to 
the list of needed changes.

I think I may also suggest shelving this patch for 2.6.26 as a little 
birdie mentioned it would be a good idea to give this a through testing 
on non-MLS/MCS systems which I haven't yet done and don't expect to be 
able to do so before the merge window opens.

I haven't seen any objections to the other two patches, so I'll 
re-submit those for 2.6.27 and leave the secid/secctx cleanup for the 
next time around.

Thanks for the review.

-- 
paul moore
linux @ hp

--
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: [PATCH 1/3] SELinux: Cleanup the secid/secctx conversion functions

[Stephen Smalley]

On Tue, 2008-04-08 at 17:13 -0400, Paul Moore wrote:
> On Tuesday 08 April 2008 10:43:44 am Stephen Smalley wrote:
> > On Mon, 2008-04-07 at 19:11 -0400, Paul Moore wrote:
> > > @@ -709,73 +697,46 @@ static int security_context_to_sid_core(char
> > > *scontext, u32 scontext_len, u32 *s null suffix to the copy to
> > > avoid problems with the existing attr package, which doesn't view
> > > the null terminator as part of the attribute value. */
> > > -	scontext2 = kmalloc(scontext_len+1,GFP_KERNEL);
> > > -	if (!scontext2) {
> > > -		rc = -ENOMEM;
> > > -		goto out;
> > > -	}
> > > -	memcpy(scontext2, scontext, scontext_len);
> > > -	scontext2[scontext_len] = 0;
> > > +	scontext_dup = kmemdup(scontext, scontext_len + 1, GFP_KERNEL);
> >
> > Also, in addition to the gfp_flags change, I'm not clear that the
> > above change is correct.  We are taking a byte array "scontext" of
> > length "scontext_len" and copying it into a buffer of length
> > "scontext_len+1" so that we can ensure that it is NUL terminated
> > prior to parsing.  Won't kmemdup with scontext_len+1 ultimately run
> > off the end of the original string?
> 
> Good point, I believe you're right.  I'll add this and the gfp stuff to 
> the list of needed changes.
> 
> I think I may also suggest shelving this patch for 2.6.26 as a little 
> birdie mentioned it would be a good idea to give this a through testing 
> on non-MLS/MCS systems which I haven't yet done and don't expect to be 
> able to do so before the merge window opens.

Just for future reference for others, since I didn't cc the list on that
reply:  When making a change that substantively affects the secid/secctx
conversion functions, it would be good idea (albeit painful) to test
each of the following cases:
1) kernel with MLS/MCS policy, filesystem without MLS/MCS labels (to
exercise the compatibility code to map to a default MLS/MCS label).
This is encountered e.g. when mounting a filesystem created on a
non-MCS/MLS distribution on a MCS/MLS distribution, as with an upgrade
from RHEL4 to RHEL5 or Fedora <= 4 to Fedora >= 5.
2) kernel without MLS/MCS policy, filesystem with MLS/MCS label (to
exercise the compatibility code to skip the MCS/MLS label).  This is
encountered in the reverse direction, particularly if sharing a
filesystem across multiple distributions on a multi-boot system.
3) kernel with MLS/MCS policy, filesystem with MLS/MCS labels.  This is
a clean install of RHEL5 or Fedora >= 5.
4) kernel without MLS/MCS policy, filesystem without MLS/MCS label.
This is a RHEL4 or Fedora <= 4 or Ubuntu Hardy Heron or Hardened Gentoo
install.

-- 
Stephen Smalley
National Security Agency


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

[PATCH 2/3] SELinux: Made netnode cache adds faster

[Paul Moore]

When adding new entries to the network node cache we would walk the entire
hash bucket to make sure we didn't cross a threshold (done to bound the cache
size).  This isn't a very quick or elegant solution for something which is
supposed to be quick-ish so add a counter to each hash bucket to track the
size of the bucket and eliminate the need to walk the entire bucket list on
each add.

Signed-off-by: Paul Moore <paul.moore@hp.com>
---

 security/selinux/netnode.c |   57 +++++++++++++++++++++-----------------------
 1 files changed, 27 insertions(+), 30 deletions(-)

diff --git a/security/selinux/netnode.c b/security/selinux/netnode.c
index f3c526f..b3a5a65 100644
--- a/security/selinux/netnode.c
+++ b/security/selinux/netnode.c
@@ -40,11 +40,17 @@
 #include <net/ipv6.h>
 #include <asm/bug.h>
 
+#include "netnode.h"
 #include "objsec.h"
 
 #define SEL_NETNODE_HASH_SIZE       256
 #define SEL_NETNODE_HASH_BKT_LIMIT   16
 
+struct sel_netnode_bkt {
+	u32 size;
+	struct list_head list;
+};
+
 struct sel_netnode {
 	struct netnode_security_struct nsec;
 
@@ -60,7 +66,7 @@ struct sel_netnode {
 
 static LIST_HEAD(sel_netnode_list);
 static DEFINE_SPINLOCK(sel_netnode_lock);
-static struct list_head sel_netnode_hash[SEL_NETNODE_HASH_SIZE];
+static struct sel_netnode_bkt sel_netnode_hash[SEL_NETNODE_HASH_SIZE];
 
 /**
  * sel_netnode_free - Frees a node entry
@@ -137,7 +143,7 @@ static struct sel_netnode *sel_netnode_find(const void *addr, u16 family)
 		BUG();
 	}
 
-	list_for_each_entry_rcu(node, &sel_netnode_hash[idx], list)
+	list_for_each_entry_rcu(node, &sel_netnode_hash[idx].list, list)
 		if (node->nsec.family == family)
 			switch (family) {
 			case PF_INET:
@@ -166,8 +172,6 @@ static struct sel_netnode *sel_netnode_find(const void *addr, u16 family)
 static int sel_netnode_insert(struct sel_netnode *node)
 {
 	u32 idx;
-	u32 count = 0;
-	struct sel_netnode *iter;
 
 	switch (node->nsec.family) {
 	case PF_INET:
@@ -179,35 +183,22 @@ static int sel_netnode_insert(struct sel_netnode *node)
 	default:
 		BUG();
 	}
-	list_add_rcu(&node->list, &sel_netnode_hash[idx]);
 
 	/* we need to impose a limit on the growth of the hash table so check
 	 * this bucket to make sure it is within the specified bounds */
-	list_for_each_entry(iter, &sel_netnode_hash[idx], list)
-		if (++count > SEL_NETNODE_HASH_BKT_LIMIT) {
-			list_del_rcu(&iter->list);
-			call_rcu(&iter->rcu, sel_netnode_free);
-			break;
-		}
+	list_add_rcu(&node->list, &sel_netnode_hash[idx].list);
+	if (sel_netnode_hash[idx].size == SEL_NETNODE_HASH_BKT_LIMIT) {
+		struct sel_netnode *tail;
+		tail = list_entry(node->list.prev, struct sel_netnode, list);
+		__list_del(node->list.prev, node->list.next);
+		call_rcu(&tail->rcu, sel_netnode_free);
+	} else
+		sel_netnode_hash[idx].size++;
 
 	return 0;
 }
 
 /**
- * sel_netnode_destroy - Remove a node record from the table
- * @node: the existing node record
- *
- * Description:
- * Remove an existing node record from the network address table.
- *
- */
-static void sel_netnode_destroy(struct sel_netnode *node)
-{
-	list_del_rcu(&node->list);
-	call_rcu(&node->rcu, sel_netnode_free);
-}
-
-/**
  * sel_netnode_sid_slow - Lookup the SID of a network address using the policy
  * @addr: the IP address
  * @family: the address family
@@ -316,9 +307,13 @@ static void sel_netnode_flush(void)
 	struct sel_netnode *node;
 
 	spin_lock_bh(&sel_netnode_lock);
-	for (idx = 0; idx < SEL_NETNODE_HASH_SIZE; idx++)
-		list_for_each_entry(node, &sel_netnode_hash[idx], list)
-			sel_netnode_destroy(node);
+	for (idx = 0; idx < SEL_NETNODE_HASH_SIZE; idx++) {
+		list_for_each_entry(node, &sel_netnode_hash[idx].list, list) {
+			list_del_rcu(&node->list);
+			call_rcu(&node->rcu, sel_netnode_free);
+		}
+		sel_netnode_hash[idx].size = 0;
+	}
 	spin_unlock_bh(&sel_netnode_lock);
 }
 
@@ -340,8 +335,10 @@ static __init int sel_netnode_init(void)
 	if (!selinux_enabled)
 		return 0;
 
-	for (iter = 0; iter < SEL_NETNODE_HASH_SIZE; iter++)
-		INIT_LIST_HEAD(&sel_netnode_hash[iter]);
+	for (iter = 0; iter < SEL_NETNODE_HASH_SIZE; iter++) {
+		INIT_LIST_HEAD(&sel_netnode_hash[iter].list);
+		sel_netnode_hash[iter].size = 0;
+	}
 
 	ret = avc_add_callback(sel_netnode_avc_callback, AVC_CALLBACK_RESET,
 	                       SECSID_NULL, SECSID_NULL, SECCLASS_NULL, 0);


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

Re: [PATCH 2/3] SELinux: Made netnode cache adds faster

[Stephen Smalley]

On Mon, 2008-04-07 at 19:11 -0400, Paul Moore wrote:
> When adding new entries to the network node cache we would walk the entire
> hash bucket to make sure we didn't cross a threshold (done to bound the cache
> size).  This isn't a very quick or elegant solution for something which is
> supposed to be quick-ish so add a counter to each hash bucket to track the
> size of the bucket and eliminate the need to walk the entire bucket list on
> each add.
> 
> Signed-off-by: Paul Moore <paul.moore@hp.com>
> ---
> 
>  security/selinux/netnode.c |   57 +++++++++++++++++++++-----------------------
>  1 files changed, 27 insertions(+), 30 deletions(-)
> 
> diff --git a/security/selinux/netnode.c b/security/selinux/netnode.c
> index f3c526f..b3a5a65 100644
> --- a/security/selinux/netnode.c
> +++ b/security/selinux/netnode.c
> @@ -40,11 +40,17 @@
>  #include <net/ipv6.h>
>  #include <asm/bug.h>
>  
> +#include "netnode.h"
>  #include "objsec.h"
>  
>  #define SEL_NETNODE_HASH_SIZE       256
>  #define SEL_NETNODE_HASH_BKT_LIMIT   16
>  
> +struct sel_netnode_bkt {
> +	u32 size;

Technically this could just be an ordinary unsigned int, right?
We tend to overuse fixed-size integers in our code at present.

> +	struct list_head list;
> +};
> +
>  struct sel_netnode {
>  	struct netnode_security_struct nsec;
>  

> @@ -179,35 +183,22 @@ static int sel_netnode_insert(struct sel_netnode *node)
>  	default:
>  		BUG();
>  	}
> -	list_add_rcu(&node->list, &sel_netnode_hash[idx]);
>  
>  	/* we need to impose a limit on the growth of the hash table so check
>  	 * this bucket to make sure it is within the specified bounds */
> -	list_for_each_entry(iter, &sel_netnode_hash[idx], list)
> -		if (++count > SEL_NETNODE_HASH_BKT_LIMIT) {
> -			list_del_rcu(&iter->list);
> -			call_rcu(&iter->rcu, sel_netnode_free);
> -			break;
> -		}
> +	list_add_rcu(&node->list, &sel_netnode_hash[idx].list);
> +	if (sel_netnode_hash[idx].size == SEL_NETNODE_HASH_BKT_LIMIT) {
> +		struct sel_netnode *tail;
> +		tail = list_entry(node->list.prev, struct sel_netnode, list);
> +		__list_del(node->list.prev, node->list.next);

Can you clarify the change from list_del_rcu() to __list_del() here?

> +		call_rcu(&tail->rcu, sel_netnode_free);
> +	} else
> +		sel_netnode_hash[idx].size++;
>  
>  	return 0;
>  }

-- 
Stephen Smalley
National Security Agency


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

Re: [PATCH 2/3] SELinux: Made netnode cache adds faster

[Paul Moore]

On Wednesday 09 April 2008 1:08:55 pm Stephen Smalley wrote:
> On Mon, 2008-04-07 at 19:11 -0400, Paul Moore wrote:
> > When adding new entries to the network node cache we would walk the
> > entire hash bucket to make sure we didn't cross a threshold (done
> > to bound the cache size).  This isn't a very quick or elegant
> > solution for something which is supposed to be quick-ish so add a
> > counter to each hash bucket to track the size of the bucket and
> > eliminate the need to walk the entire bucket list on each add.
> >
> > Signed-off-by: Paul Moore <paul.moore@hp.com>
> > ---
> >
> >  security/selinux/netnode.c |   57
> > +++++++++++++++++++++----------------------- 1 files changed, 27
> > insertions(+), 30 deletions(-)
> >
> > diff --git a/security/selinux/netnode.c
> > b/security/selinux/netnode.c index f3c526f..b3a5a65 100644
> > --- a/security/selinux/netnode.c
> > +++ b/security/selinux/netnode.c
> > @@ -40,11 +40,17 @@
> >  #include <net/ipv6.h>
> >  #include <asm/bug.h>
> >
> > +#include "netnode.h"
> >  #include "objsec.h"
> >
> >  #define SEL_NETNODE_HASH_SIZE       256
> >  #define SEL_NETNODE_HASH_BKT_LIMIT   16
> >
> > +struct sel_netnode_bkt {
> > +	u32 size;
>
> Technically this could just be an ordinary unsigned int, right?
> We tend to overuse fixed-size integers in our code at present.

[Thanks for taking a look]

Yep, we can make this anything we want.  I suppose if we are adverse to 
fixed-size scalars (not really an issue here, but I understand your 
point) a size_t variable might be a better pick since it is a "size" 
even though it isn't the traditional memory chunk size.  The only 
drawback is that it varies in size, but it shouldn't messup the packing 
of the struct.

I guess I still kinda like the limit imposed by the u32 (I suppose I 
could make it smaller, u8?, but the compiler is just going to waste the 
memory for alignment reasons anyway) but if you feel strongly about 
this I can change it to an int or size_t type.

> > +	struct list_head list;
> > +};

...

> > @@ -179,35 +183,22 @@ static int sel_netnode_insert(struct
> > sel_netnode *node) default:
> >  		BUG();
> >  	}
> > -	list_add_rcu(&node->list, &sel_netnode_hash[idx]);
> >
> >  	/* we need to impose a limit on the growth of the hash table so
> > check * this bucket to make sure it is within the specified bounds
> > */ -	list_for_each_entry(iter, &sel_netnode_hash[idx], list)
> > -		if (++count > SEL_NETNODE_HASH_BKT_LIMIT) {
> > -			list_del_rcu(&iter->list);
> > -			call_rcu(&iter->rcu, sel_netnode_free);
> > -			break;
> > -		}
> > +	list_add_rcu(&node->list, &sel_netnode_hash[idx].list);
> > +	if (sel_netnode_hash[idx].size == SEL_NETNODE_HASH_BKT_LIMIT) {
> > +		struct sel_netnode *tail;
> > +		tail = list_entry(node->list.prev, struct sel_netnode, list);
> > +		__list_del(node->list.prev, node->list.next);
>
> Can you clarify the change from list_del_rcu() to __list_del() here?

Well, the main reason for the change is I want to delete the tail of the 
list without having to traverse the entire list, there doesn't appear 
to be a list_*_rcu() that will allow me to do this so I simply used the 
__list_del() function to do what I needed.  The only danger here is 
maintaining all of the right RCUisms and looking at the different 
implementations the only real difference is that the RCU version sets 
the removed entry's prev pointer to a poison value.  Since 
I "immediately" remove the entry and don't make use of the prev pointer 
in any of the list functions I decided to just skip that step.

However, I am not an RCU expert (tricky thing to master) so if this 
sounds problematic to you let me know and I'll add the "poisoning" to 
the code.

> > +		call_rcu(&tail->rcu, sel_netnode_free);
> > +	} else
> > +		sel_netnode_hash[idx].size++;
> >
> >  	return 0;
> >  }

-- 
paul moore
linux @ hp

--
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: [PATCH 2/3] SELinux: Made netnode cache adds faster

[Stephen Smalley]

On Wed, 2008-04-09 at 13:41 -0400, Paul Moore wrote:
> On Wednesday 09 April 2008 1:08:55 pm Stephen Smalley wrote:
> > On Mon, 2008-04-07 at 19:11 -0400, Paul Moore wrote:
> > > When adding new entries to the network node cache we would walk the
> > > entire hash bucket to make sure we didn't cross a threshold (done
> > > to bound the cache size).  This isn't a very quick or elegant
> > > solution for something which is supposed to be quick-ish so add a
> > > counter to each hash bucket to track the size of the bucket and
> > > eliminate the need to walk the entire bucket list on each add.
> > >
> > > Signed-off-by: Paul Moore <paul.moore@hp.com>
> > > ---
> > >
> > >  security/selinux/netnode.c |   57
> > > +++++++++++++++++++++----------------------- 1 files changed, 27
> > > insertions(+), 30 deletions(-)
> > >
> > > diff --git a/security/selinux/netnode.c
> > > b/security/selinux/netnode.c index f3c526f..b3a5a65 100644
> > > --- a/security/selinux/netnode.c
> > > +++ b/security/selinux/netnode.c
> > > @@ -40,11 +40,17 @@
> > >  #include <net/ipv6.h>
> > >  #include <asm/bug.h>
> > >
> > > +#include "netnode.h"
> > >  #include "objsec.h"
> > >
> > >  #define SEL_NETNODE_HASH_SIZE       256
> > >  #define SEL_NETNODE_HASH_BKT_LIMIT   16
> > >
> > > +struct sel_netnode_bkt {
> > > +	u32 size;
> >
> > Technically this could just be an ordinary unsigned int, right?
> > We tend to overuse fixed-size integers in our code at present.
> 
> [Thanks for taking a look]
> 
> Yep, we can make this anything we want.  I suppose if we are adverse to 
> fixed-size scalars (not really an issue here, but I understand your 
> point) a size_t variable might be a better pick since it is a "size" 
> even though it isn't the traditional memory chunk size.  The only 
> drawback is that it varies in size, but it shouldn't messup the packing 
> of the struct.
> 
> I guess I still kinda like the limit imposed by the u32 (I suppose I 
> could make it smaller, u8?, but the compiler is just going to waste the 
> memory for alignment reasons anyway) but if you feel strongly about 
> this I can change it to an int or size_t type.

Just pointing out that we tend to use fixed-size integers too widely,
when an ordinary unsigned int or unsigned long would suffice.  We only
generally need the fixed size integers for file (e.g. policy image) and
network packet formats.  Not new to this code, but something to be aware
of.

> 
> > > +	struct list_head list;
> > > +};
> 
> ...
> 
> > > @@ -179,35 +183,22 @@ static int sel_netnode_insert(struct
> > > sel_netnode *node) default:
> > >  		BUG();
> > >  	}
> > > -	list_add_rcu(&node->list, &sel_netnode_hash[idx]);
> > >
> > >  	/* we need to impose a limit on the growth of the hash table so
> > > check * this bucket to make sure it is within the specified bounds
> > > */ -	list_for_each_entry(iter, &sel_netnode_hash[idx], list)
> > > -		if (++count > SEL_NETNODE_HASH_BKT_LIMIT) {
> > > -			list_del_rcu(&iter->list);
> > > -			call_rcu(&iter->rcu, sel_netnode_free);
> > > -			break;
> > > -		}
> > > +	list_add_rcu(&node->list, &sel_netnode_hash[idx].list);
> > > +	if (sel_netnode_hash[idx].size == SEL_NETNODE_HASH_BKT_LIMIT) {
> > > +		struct sel_netnode *tail;
> > > +		tail = list_entry(node->list.prev, struct sel_netnode, list);
> > > +		__list_del(node->list.prev, node->list.next);
> >
> > Can you clarify the change from list_del_rcu() to __list_del() here?
> 
> Well, the main reason for the change is I want to delete the tail of the 
> list without having to traverse the entire list, there doesn't appear 
> to be a list_*_rcu() that will allow me to do this so I simply used the 
> __list_del() function to do what I needed.  The only danger here is 
> maintaining all of the right RCUisms and looking at the different 
> implementations the only real difference is that the RCU version sets 
> the removed entry's prev pointer to a poison value.  Since 
> I "immediately" remove the entry and don't make use of the prev pointer 
> in any of the list functions I decided to just skip that step.
> 
> However, I am not an RCU expert (tricky thing to master) so if this 
> sounds problematic to you let me know and I'll add the "poisoning" to 
> the code.

I don't quite understand.  Prior to the patch, it was a list_del_rcu().
Which is just a __list_del() followed by poisoning of the prev pointer.
In the new code, you switched to a direct call to __list_del().  So the
only difference in the final code is losing the poisoning, which seems
bad for debugging and makes the code less self-documenting wrt RCU
issues.  And usually directly calling a __function should be avoided
outside of the implementations of the interfaces except where absolutely
needed.

-- 
Stephen Smalley
National Security Agency


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

Re: [PATCH 2/3] SELinux: Made netnode cache adds faster

[Paul Moore]

On Thursday 10 April 2008 8:29:19 am Stephen Smalley wrote:
> On Wed, 2008-04-09 at 13:41 -0400, Paul Moore wrote:
> > On Wednesday 09 April 2008 1:08:55 pm Stephen Smalley wrote:
> > > On Mon, 2008-04-07 at 19:11 -0400, Paul Moore wrote:
> > > > diff --git a/security/selinux/netnode.c
> > > > b/security/selinux/netnode.c index f3c526f..b3a5a65 100644
> > > > --- a/security/selinux/netnode.c
> > > > +++ b/security/selinux/netnode.c
> > > > @@ -40,11 +40,17 @@
> > > >  #include <net/ipv6.h>
> > > >  #include <asm/bug.h>
> > > >
> > > > +#include "netnode.h"
> > > >  #include "objsec.h"
> > > >
> > > >  #define SEL_NETNODE_HASH_SIZE       256
> > > >  #define SEL_NETNODE_HASH_BKT_LIMIT   16
> > > >
> > > > +struct sel_netnode_bkt {
> > > > +	u32 size;
> > >
> > > Technically this could just be an ordinary unsigned int, right?
> > > We tend to overuse fixed-size integers in our code at present.
> >
> > [Thanks for taking a look]
> >
> > Yep, we can make this anything we want.  I suppose if we are
> > adverse to fixed-size scalars (not really an issue here, but I
> > understand your point) a size_t variable might be a better pick
> > since it is a "size" even though it isn't the traditional memory
> > chunk size.  The only drawback is that it varies in size, but it
> > shouldn't messup the packing of the struct.
> >
> > I guess I still kinda like the limit imposed by the u32 (I suppose
> > I could make it smaller, u8?, but the compiler is just going to
> > waste the memory for alignment reasons anyway) but if you feel
> > strongly about this I can change it to an int or size_t type.
>
> Just pointing out that we tend to use fixed-size integers too widely,
> when an ordinary unsigned int or unsigned long would suffice.  We
> only generally need the fixed size integers for file (e.g. policy
> image) and network packet formats.  Not new to this code, but
> something to be aware of.

I switched it over to an unsigned int in the "v2" revision I posted last 
night.  You're right in that there is no _need_ for it to be a fixed 
signed integer so we might as well let it be a more "native" type.

> > > > @@ -179,35 +183,22 @@ static int sel_netnode_insert(struct
> > > > sel_netnode *node) default:
> > > >  		BUG();
> > > >  	}
> > > > -	list_add_rcu(&node->list, &sel_netnode_hash[idx]);
> > > >
> > > >  	/* we need to impose a limit on the growth of the hash table
> > > > so check * this bucket to make sure it is within the specified
> > > > bounds */ -	list_for_each_entry(iter, &sel_netnode_hash[idx],
> > > > list) -		if (++count > SEL_NETNODE_HASH_BKT_LIMIT) {
> > > > -			list_del_rcu(&iter->list);
> > > > -			call_rcu(&iter->rcu, sel_netnode_free);
> > > > -			break;
> > > > -		}
> > > > +	list_add_rcu(&node->list, &sel_netnode_hash[idx].list);
> > > > +	if (sel_netnode_hash[idx].size == SEL_NETNODE_HASH_BKT_LIMIT)
> > > > { +		struct sel_netnode *tail;
> > > > +		tail = list_entry(node->list.prev, struct sel_netnode,
> > > > list); +		__list_del(node->list.prev, node->list.next);
> > >
> > > Can you clarify the change from list_del_rcu() to __list_del()
> > > here?
> >
> > Well, the main reason for the change is I want to delete the tail
> > of the list without having to traverse the entire list, there
> > doesn't appear to be a list_*_rcu() that will allow me to do this
> > so I simply used the __list_del() function to do what I needed. 
> > The only danger here is maintaining all of the right RCUisms and
> > looking at the different implementations the only real difference
> > is that the RCU version sets the removed entry's prev pointer to a
> > poison value.  Since I "immediately" remove the entry and don't
> > make use of the prev pointer in any of the list functions I decided
> > to just skip that step.
> >
> > However, I am not an RCU expert (tricky thing to master) so if this
> > sounds problematic to you let me know and I'll add the "poisoning"
> > to the code.
>
> I don't quite understand. {snip}

Yeah, now I guess I don't either.  For some reason when I wrote this 
code I thought I needed to use a __list_del() call to remove the last 
entry in the list when in reality I can do the same thing with 
list_del_rcu().  No idea what I was thinking then ... give me a few 
hours and I'll resubmit.

Thanks.

-- 
paul moore
linux @ hp

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

[PATCH 3/3] SELinux: Add network port SID cache

[Paul Moore]

Much like we added a network node cache, this patch adds a network port cache.
The design is taken almost completely from the network node cache which in
turn was taken from the network interface cache.  The basic idea is to cache
entries in a hash table based on protocol/port information.  The hash
function only takes the port number into account since the number of different
protocols in use at any one time is expected to be relatively small.

Signed-off-by: Paul Moore <paul.moore@hp.com>
---

 security/selinux/Makefile           |    1 
 security/selinux/hooks.c            |   20 +-
 security/selinux/include/netport.h  |   31 ++++
 security/selinux/include/objsec.h   |    6 +
 security/selinux/include/security.h |    3 
 security/selinux/netport.c          |  286 +++++++++++++++++++++++++++++++++++
 security/selinux/ss/services.c      |    8 -
 7 files changed, 334 insertions(+), 21 deletions(-)

diff --git a/security/selinux/Makefile b/security/selinux/Makefile
index 00afd85..d47fc5e 100644
--- a/security/selinux/Makefile
+++ b/security/selinux/Makefile
@@ -11,6 +11,7 @@ selinux-y := avc.o \
 	     nlmsgtab.o \
 	     netif.o \
 	     netnode.o \
+	     netport.o \
 	     exports.o
 
 selinux-$(CONFIG_SECURITY_NETWORK_XFRM) += xfrm.o
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 820d07a..415269f 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -80,6 +80,7 @@
 #include "objsec.h"
 #include "netif.h"
 #include "netnode.h"
+#include "netport.h"
 #include "xfrm.h"
 #include "netlabel.h"
 
@@ -3626,10 +3627,8 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in
 			inet_get_local_port_range(&low, &high);
 
 			if (snum < max(PROT_SOCK, low) || snum > high) {
-				err = security_port_sid(sk->sk_family,
-							sk->sk_type,
-							sk->sk_protocol, snum,
-							&sid);
+				err = sel_netport_sid(sk->sk_protocol,
+						      snum, &sid);
 				if (err)
 					goto out;
 				AVC_AUDIT_DATA_INIT(&ad,NET);
@@ -3717,8 +3716,7 @@ static int selinux_socket_connect(struct socket *sock, struct sockaddr *address,
 			snum = ntohs(addr6->sin6_port);
 		}
 
-		err = security_port_sid(sk->sk_family, sk->sk_type,
-					sk->sk_protocol, snum, &sid);
+		err = sel_netport_sid(sk->sk_protocol, snum, &sid);
 		if (err)
 			goto out;
 
@@ -3949,9 +3947,8 @@ static int selinux_sock_rcv_skb_iptables_compat(struct sock *sk,
 
 	if (!recv_perm)
 		return 0;
-	err = security_port_sid(sk->sk_family, sk->sk_type,
-				sk->sk_protocol, ntohs(ad->u.net.sport),
-				&port_sid);
+	err = sel_netport_sid(sk->sk_protocol,
+			      ntohs(ad->u.net.sport), &port_sid);
 	if (unlikely(err)) {
 		printk(KERN_WARNING
 		       "SELinux: failure in"
@@ -4372,9 +4369,8 @@ static int selinux_ip_postroute_iptables_compat(struct sock *sk,
 	if (send_perm != 0)
 		return 0;
 
-	err = security_port_sid(sk->sk_family, sk->sk_type,
-				sk->sk_protocol, ntohs(ad->u.net.dport),
-				&port_sid);
+	err = sel_netport_sid(sk->sk_protocol,
+			      ntohs(ad->u.net.dport), &port_sid);
 	if (unlikely(err)) {
 		printk(KERN_WARNING
 		       "SELinux: failure in"
diff --git a/security/selinux/include/netport.h b/security/selinux/include/netport.h
new file mode 100644
index 0000000..8991752
--- /dev/null
+++ b/security/selinux/include/netport.h
@@ -0,0 +1,31 @@
+/*
+ * Network port table
+ *
+ * SELinux must keep a mapping of network ports to labels/SIDs.  This
+ * mapping is maintained as part of the normal policy but a fast cache is
+ * needed to reduce the lookup overhead.
+ *
+ * Author: Paul Moore <paul.moore@hp.com>
+ *
+ */
+
+/*
+ * (c) Copyright Hewlett-Packard Development Company, L.P., 2008
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef _SELINUX_NETPORT_H
+#define _SELINUX_NETPORT_H
+
+int sel_netport_sid(u8 protocol, u16 pnum, u32 *sid);
+
+#endif
diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
index c6c2bb4..04dfdfe 100644
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -109,6 +109,12 @@ struct netnode_security_struct {
 	u16 family;			/* address family */
 };
 
+struct netport_security_struct {
+	u32 sid;			/* SID for this node */
+	u16 port;			/* port number */
+	u8 protocol;			/* transport protocol */
+};
+
 struct sk_security_struct {
 	struct sock *sk;		/* back pointer to sk object */
 	u32 sid;			/* SID of this object */
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index f7d2f03..9901d9d 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -91,8 +91,7 @@ int security_context_to_sid_default(char *scontext, u32 scontext_len, u32 *out_s
 int security_get_user_sids(u32 callsid, char *username,
 			   u32 **sids, u32 *nel);
 
-int security_port_sid(u16 domain, u16 type, u8 protocol, u16 port,
-	u32 *out_sid);
+int security_port_sid(u8 protocol, u16 port, u32 *out_sid);
 
 int security_netif_sid(char *name, u32 *if_sid);
 
diff --git a/security/selinux/netport.c b/security/selinux/netport.c
new file mode 100644
index 0000000..eeff975
--- /dev/null
+++ b/security/selinux/netport.c
@@ -0,0 +1,286 @@
+/*
+ * Network port table
+ *
+ * SELinux must keep a mapping of network ports to labels/SIDs.  This
+ * mapping is maintained as part of the normal policy but a fast cache is
+ * needed to reduce the lookup overhead.
+ *
+ * Author: Paul Moore <paul.moore@hp.com>
+ *
+ * This code is heavily based on the "netif" concept originally developed by
+ * James Morris <jmorris@redhat.com>
+ *   (see security/selinux/netif.c for more information)
+ *
+ */
+
+/*
+ * (c) Copyright Hewlett-Packard Development Company, L.P., 2008
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/types.h>
+#include <linux/rcupdate.h>
+#include <linux/list.h>
+#include <linux/spinlock.h>
+#include <linux/in.h>
+#include <linux/in6.h>
+#include <linux/ip.h>
+#include <linux/ipv6.h>
+#include <net/ip.h>
+#include <net/ipv6.h>
+#include <asm/bug.h>
+
+#include "netport.h"
+#include "objsec.h"
+
+#define SEL_NETPORT_HASH_SIZE       256
+#define SEL_NETPORT_HASH_BKT_LIMIT   16
+
+struct sel_netport_bkt {
+	u32 size;
+	struct list_head list;
+};
+
+struct sel_netport {
+	struct netport_security_struct psec;
+
+	struct list_head list;
+	struct rcu_head rcu;
+};
+
+/* NOTE: we are using a combined hash table for both IPv4 and IPv6, the reason
+ * for this is that I suspect most users will not make heavy use of both
+ * address families at the same time so one table will usually end up wasted,
+ * if this becomes a problem we can always add a hash table for each address
+ * family later */
+
+static LIST_HEAD(sel_netport_list);
+static DEFINE_SPINLOCK(sel_netport_lock);
+static struct sel_netport_bkt sel_netport_hash[SEL_NETPORT_HASH_SIZE];
+
+/**
+ * sel_netport_free - Frees a port entry
+ * @p: the entry's RCU field
+ *
+ * Description:
+ * This function is designed to be used as a callback to the call_rcu()
+ * function so that memory allocated to a hash table port entry can be
+ * released safely.
+ *
+ */
+static void sel_netport_free(struct rcu_head *p)
+{
+	struct sel_netport *port = container_of(p, struct sel_netport, rcu);
+	kfree(port);
+}
+
+/**
+ * sel_netport_hashfn - Hashing function for the port table
+ * @pnum: port number
+ *
+ * Description:
+ * This is the hashing function for the port table, it returns the bucket
+ * number for the given port.
+ *
+ */
+static u32 sel_netport_hashfn(u16 pnum)
+{
+	return (pnum & (SEL_NETPORT_HASH_SIZE - 1));
+}
+
+/**
+ * sel_netport_find - Search for a port record
+ * @protocol: protocol
+ * @port: pnum
+ *
+ * Description:
+ * Search the network port table and return the matching record.  If an entry
+ * can not be found in the table return NULL.
+ *
+ */
+static struct sel_netport *sel_netport_find(u8 protocol, u16 pnum)
+{
+	u32 idx;
+	struct sel_netport *port;
+
+	idx = sel_netport_hashfn(pnum);
+	list_for_each_entry_rcu(port, &sel_netport_hash[idx].list, list)
+		if (port->psec.port == pnum &&
+		    port->psec.protocol == protocol)
+			return port;
+
+	return NULL;
+}
+
+/**
+ * sel_netport_insert - Insert a new port into the table
+ * @port: the new port record
+ *
+ * Description:
+ * Add a new port record to the network address hash table.  Returns zero on
+ * success, negative values on failure.
+ *
+ */
+static int sel_netport_insert(struct sel_netport *port)
+{
+	u32 idx;
+
+	/* we need to impose a limit on the growth of the hash table so check
+	 * this bucket to make sure it is within the specified bounds */
+	idx = sel_netport_hashfn(port->psec.port);
+	list_add_rcu(&port->list, &sel_netport_hash[idx].list);
+	if (sel_netport_hash[idx].size == SEL_NETPORT_HASH_BKT_LIMIT) {
+		struct sel_netport *tail;
+		tail = list_entry(port->list.prev, struct sel_netport, list);
+		__list_del(port->list.prev, port->list.next);
+		call_rcu(&tail->rcu, sel_netport_free);
+	} else
+		sel_netport_hash[idx].size++;
+
+	return 0;
+}
+
+/**
+ * sel_netport_sid_slow - Lookup the SID of a network address using the policy
+ * @protocol: protocol
+ * @pnum: port
+ * @sid: port SID
+ *
+ * Description:
+ * This function determines the SID of a network port by quering the security
+ * policy.  The result is added to the network port table to speedup future
+ * queries.  Returns zero on success, negative values on failure.
+ *
+ */
+static int sel_netport_sid_slow(u8 protocol, u16 pnum, u32 *sid)
+{
+	int ret;
+	struct sel_netport *port;
+	struct sel_netport *new = NULL;
+
+	spin_lock_bh(&sel_netport_lock);
+	port = sel_netport_find(protocol, pnum);
+	if (port != NULL) {
+		*sid = port->psec.sid;
+		ret = 0;
+		goto out;
+	}
+	new = kzalloc(sizeof(*new), GFP_ATOMIC);
+	if (new == NULL) {
+		ret = -ENOMEM;
+		goto out;
+	}
+	ret = security_port_sid(protocol, pnum, &new->psec.sid);
+	if (ret != 0)
+		goto out;
+	new->psec.port = pnum;
+	new->psec.protocol = protocol;
+	ret = sel_netport_insert(new);
+	if (ret != 0)
+		goto out;
+	*sid = new->psec.sid;
+
+out:
+	spin_unlock_bh(&sel_netport_lock);
+	if (unlikely(ret)) {
+		printk(KERN_WARNING
+		       "SELinux: failure in sel_netport_sid_slow(),"
+		       " unable to determine network port label\n");
+		kfree(new);
+	}
+	return ret;
+}
+
+/**
+ * sel_netport_sid - Lookup the SID of a network port
+ * @protocol: protocol
+ * @pnum: port
+ * @sid: port SID
+ *
+ * Description:
+ * This function determines the SID of a network port using the fastest method
+ * possible.  First the port table is queried, but if an entry can't be found
+ * then the policy is queried and the result is added to the table to speedup
+ * future queries.  Returns zero on success, negative values on failure.
+ *
+ */
+int sel_netport_sid(u8 protocol, u16 pnum, u32 *sid)
+{
+	struct sel_netport *port;
+
+	rcu_read_lock();
+	port = sel_netport_find(protocol, pnum);
+	if (port != NULL) {
+		*sid = port->psec.sid;
+		rcu_read_unlock();
+		return 0;
+	}
+	rcu_read_unlock();
+
+	return sel_netport_sid_slow(protocol, pnum, sid);
+}
+
+/**
+ * sel_netport_flush - Flush the entire network port table
+ *
+ * Description:
+ * Remove all entries from the network address table.
+ *
+ */
+static void sel_netport_flush(void)
+{
+	u32 idx;
+	struct sel_netport *port;
+
+	spin_lock_bh(&sel_netport_lock);
+	for (idx = 0; idx < SEL_NETPORT_HASH_SIZE; idx++) {
+		list_for_each_entry(port, &sel_netport_hash[idx].list, list) {
+			list_del_rcu(&port->list);
+			call_rcu(&port->rcu, sel_netport_free);
+		}
+		sel_netport_hash[idx].size = 0;
+	}
+	spin_unlock_bh(&sel_netport_lock);
+}
+
+static int sel_netport_avc_callback(u32 event, u32 ssid, u32 tsid,
+				    u16 class, u32 perms, u32 *retained)
+{
+	if (event == AVC_CALLBACK_RESET) {
+		sel_netport_flush();
+		synchronize_net();
+	}
+	return 0;
+}
+
+static __init int sel_netport_init(void)
+{
+	int iter;
+	int ret;
+
+	if (!selinux_enabled)
+		return 0;
+
+	for (iter = 0; iter < SEL_NETPORT_HASH_SIZE; iter++) {
+		INIT_LIST_HEAD(&sel_netport_hash[iter].list);
+		sel_netport_hash[iter].size = 0;
+	}
+
+	ret = avc_add_callback(sel_netport_avc_callback, AVC_CALLBACK_RESET,
+	                       SECSID_NULL, SECSID_NULL, SECCLASS_NULL, 0);
+	if (ret != 0)
+		panic("avc_add_callback() failed, error %d\n", ret);
+
+	return ret;
+}
+
+__initcall(sel_netport_init);
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index c9943ca..2dd6d11 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -1403,17 +1403,11 @@ err:
 
 /**
  * security_port_sid - Obtain the SID for a port.
- * @domain: communication domain aka address family
- * @type: socket type
  * @protocol: protocol number
  * @port: port number
  * @out_sid: security identifier
  */
-int security_port_sid(u16 domain,
-		      u16 type,
-		      u8 protocol,
-		      u16 port,
-		      u32 *out_sid)
+int security_port_sid(u8 protocol, u16 port, u32 *out_sid)
 {
 	struct ocontext *c;
 	int rc = 0;


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

Re: [PATCH 3/3] SELinux: Add network port SID cache

[Stephen Smalley]

On Mon, 2008-04-07 at 19:11 -0400, Paul Moore wrote:
> Much like we added a network node cache, this patch adds a network port cache.
> The design is taken almost completely from the network node cache which in
> turn was taken from the network interface cache.  The basic idea is to cache
> entries in a hash table based on protocol/port information.  The hash
> function only takes the port number into account since the number of different
> protocols in use at any one time is expected to be relatively small.

Not necessarily an obstacle to merging, but I was wondering if it would
be worthwhile to investigating unifying these caches into a single code
base with a more general interface, or if that would be a loss overall.

-- 
Stephen Smalley
National Security Agency


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

Re: [PATCH 3/3] SELinux: Add network port SID cache

[Paul Moore]

On Wednesday 09 April 2008 1:15:31 pm Stephen Smalley wrote:
> On Mon, 2008-04-07 at 19:11 -0400, Paul Moore wrote:
> > Much like we added a network node cache, this patch adds a network
> > port cache. The design is taken almost completely from the network
> > node cache which in turn was taken from the network interface
> > cache.  The basic idea is to cache entries in a hash table based on
> > protocol/port information.  The hash function only takes the port
> > number into account since the number of different protocols in use
> > at any one time is expected to be relatively small.
>
> Not necessarily an obstacle to merging, but I was wondering if it
> would be worthwhile to investigating unifying these caches into a
> single code base with a more general interface, or if that would be a
> loss overall.

Yeah, I thought of that too when I was doing the network node cache but 
I convinced myself it would probably do more harm than good.  The core 
problem is that all of the hashing and lookup functions are slightly 
different.  Not fatal, we can workaround it via case statments, a 
function table, or something similar but all of these solutions will 
impose some additional overhead.  For something which is supposed to be 
fast I thought it better to not go down this path.

I'm open to suggestions if you have any.

-- 
paul moore
linux @ hp

--
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: [PATCH 3/3] SELinux: Add network port SID cache

[Stephen Smalley]

On Wed, 2008-04-09 at 13:37 -0400, Paul Moore wrote:
> On Wednesday 09 April 2008 1:15:31 pm Stephen Smalley wrote:
> > On Mon, 2008-04-07 at 19:11 -0400, Paul Moore wrote:
> > > Much like we added a network node cache, this patch adds a network
> > > port cache. The design is taken almost completely from the network
> > > node cache which in turn was taken from the network interface
> > > cache.  The basic idea is to cache entries in a hash table based on
> > > protocol/port information.  The hash function only takes the port
> > > number into account since the number of different protocols in use
> > > at any one time is expected to be relatively small.
> >
> > Not necessarily an obstacle to merging, but I was wondering if it
> > would be worthwhile to investigating unifying these caches into a
> > single code base with a more general interface, or if that would be a
> > loss overall.
> 
> Yeah, I thought of that too when I was doing the network node cache but 
> I convinced myself it would probably do more harm than good.  The core 
> problem is that all of the hashing and lookup functions are slightly 
> different.  Not fatal, we can workaround it via case statments, a 
> function table, or something similar but all of these solutions will 
> impose some additional overhead.  For something which is supposed to be 
> fast I thought it better to not go down this path.

Ok - I'm fine with keeping them separate at present, but wanted to toss
out the idea for possible future investigation.  And we have examples
where we have gone the other direction, e.g. the avtab originally
re-used the hashtab code, but we eventually split it out into its own
direct implementation.

So other than that, my comments on the changes to the node cache also
apply to this code I think.

> I'm open to suggestions if you have any.

-- 
Stephen Smalley
National Security Agency


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