[PATCH 1/3] SELinux: fix selinux to safely handle any bugs even when not CONFIG_BUG

[PATCH 1/3] SELinux: fix selinux to safely handle any bugs even when not CONFIG_BUG

[Eric Paris]

If CONFIG_BUG is not set places in the code where BUG is called will not
end execution.  Look for places where this might result is system problems
and keep the system running.

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

 kernel/capability.c            |    1 +
 security/selinux/avc.c         |   13 +++++++++++--
 security/selinux/hooks.c       |    9 ++++++++-
 security/selinux/netnode.c     |    3 +++
 security/selinux/ss/ebitmap.h  |   15 ++++++++++++---
 security/selinux/ss/services.c |   38 +++++++++++++++++++++++++++++++-------
 security/selinux/xfrm.c        |   15 ++++++++++++---
 7 files changed, 78 insertions(+), 16 deletions(-)

diff --git a/kernel/capability.c b/kernel/capability.c
index 4e17041..e8bec6a 100644
--- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -304,6 +304,7 @@ int capable(int cap)
 	if (unlikely(!cap_valid(cap))) {
 		printk(KERN_CRIT "capable() called with invalid cap=%u\n", cap);
 		BUG();
+		return 0;
 	}
 
 	if (security_capable(cap) == 0) {
diff --git a/security/selinux/avc.c b/security/selinux/avc.c
index eb41f43..c22db1e 100644
--- a/security/selinux/avc.c
+++ b/security/selinux/avc.c
@@ -202,6 +202,7 @@ static void avc_dump_query(struct audit_buffer *ab, u32 ssid, u32 tsid, u16 tcla
 {
 	int rc;
 	char *scontext;
+	const char *class_string;
 	u32 scontext_len;
 
 	rc = security_sid_to_context(ssid, &scontext, &scontext_len);
@@ -220,8 +221,16 @@ static void avc_dump_query(struct audit_buffer *ab, u32 ssid, u32 tsid, u16 tcla
 		kfree(scontext);
 	}
 
-	BUG_ON(tclass >= ARRAY_SIZE(class_to_string) || !class_to_string[tclass]);
-	audit_log_format(ab, " tclass=%s", class_to_string[tclass]);
+	if (unlikely(tclass >= ARRAY_SIZE(class_to_string) || !class_to_string[tclass])) {
+		class_string = "unknown";
+		if (tclass >= ARRAY_SIZE(class_to_string))
+			BUG();
+		else
+			BUG();
+	} else {
+		class_string = class_to_string[tclass];
+	}
+	audit_log_format(ab, " tclass=%s", class_string);
 }
 
 /**
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index ae5bb64..6e6847d 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -995,7 +995,11 @@ static int superblock_doinit(struct super_block *sb, void *data)
 	if (!data)
 		goto out;
 
-	BUG_ON(sb->s_type->fs_flags & FS_BINARY_MOUNTDATA);
+	if (unlikely(sb->s_type->fs_flags & FS_BINARY_MOUNTDATA)) {
+		BUG();
+		rc = -EINVAL;
+		goto out_err;
+	}
 
 	rc = selinux_parse_opts_str(options, &opts);
 	if (rc)
@@ -1042,6 +1046,7 @@ static void selinux_write_opts(struct seq_file *m,
 			continue;
 		default:
 			BUG();
+			continue;
 		};
 		/* we need a comma before each option */
 		seq_putc(m, ',');
@@ -1500,6 +1505,8 @@ static int task_has_capability(struct task_struct *tsk,
 		printk(KERN_ERR
 		       "SELinux:  out of range capability %d\n", cap);
 		BUG();
+		/* should I audit somehow? */
+		return -EPERM;
 	}
 
 	rc = avc_has_perm_noaudit(sid, sid, sclass, av, 0, &avd);
diff --git a/security/selinux/netnode.c b/security/selinux/netnode.c
index 7100072..e002d5a 100644
--- a/security/selinux/netnode.c
+++ b/security/selinux/netnode.c
@@ -140,6 +140,7 @@ static struct sel_netnode *sel_netnode_find(const void *addr, u16 family)
 		break;
 	default:
 		BUG();
+		return NULL;
 	}
 
 	list_for_each_entry_rcu(node, &sel_netnode_hash[idx].list, list)
@@ -180,6 +181,7 @@ static void sel_netnode_insert(struct sel_netnode *node)
 		break;
 	default:
 		BUG();
+		return;
 	}
 
 	INIT_RCU_HEAD(&node->rcu);
@@ -240,6 +242,7 @@ static int sel_netnode_sid_slow(void *addr, u16 family, u32 *sid)
 		break;
 	default:
 		BUG();
+		ret = -EINVAL;
 	}
 	if (ret != 0)
 		goto out;
diff --git a/security/selinux/ss/ebitmap.h b/security/selinux/ss/ebitmap.h
index f283b43..425f8b8 100644
--- a/security/selinux/ss/ebitmap.h
+++ b/security/selinux/ss/ebitmap.h
@@ -85,7 +85,10 @@ static inline int ebitmap_node_get_bit(struct ebitmap_node *n,
 	unsigned int index = EBITMAP_NODE_INDEX(n, bit);
 	unsigned int ofs = EBITMAP_NODE_OFFSET(n, bit);
 
-	BUG_ON(index >= EBITMAP_UNIT_NUMS);
+	if (unlikely((index >= EBITMAP_UNIT_NUMS))) {
+		BUG();
+		return 0;
+	}
 	if ((n->maps[index] & (EBITMAP_BIT << ofs)))
 		return 1;
 	return 0;
@@ -97,7 +100,10 @@ static inline void ebitmap_node_set_bit(struct ebitmap_node *n,
 	unsigned int index = EBITMAP_NODE_INDEX(n, bit);
 	unsigned int ofs = EBITMAP_NODE_OFFSET(n, bit);
 
-	BUG_ON(index >= EBITMAP_UNIT_NUMS);
+	if (unlikely((index >= EBITMAP_UNIT_NUMS))) {
+		BUG();
+		return;
+	}
 	n->maps[index] |= (EBITMAP_BIT << ofs);
 }
 
@@ -107,7 +113,10 @@ static inline void ebitmap_node_clr_bit(struct ebitmap_node *n,
 	unsigned int index = EBITMAP_NODE_INDEX(n, bit);
 	unsigned int ofs = EBITMAP_NODE_OFFSET(n, bit);
 
-	BUG_ON(index >= EBITMAP_UNIT_NUMS);
+	if (unlikely((index >= EBITMAP_UNIT_NUMS))) {
+		BUG();
+		return;
+	}
 	n->maps[index] &= ~(EBITMAP_BIT << ofs);
 }
 
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index c65e4fe..cfb0769 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -120,16 +120,25 @@ static int constraint_expr_eval(struct context *scontext,
 	for (e = cexpr; e; e = e->next) {
 		switch (e->expr_type) {
 		case CEXPR_NOT:
-			BUG_ON(sp < 0);
+			if (unlikely(sp < 0)) {
+				BUG();
+				return 0;
+			}
 			s[sp] = !s[sp];
 			break;
 		case CEXPR_AND:
-			BUG_ON(sp < 1);
+			if (unlikely(sp < 1)) {
+				BUG();
+				return 0;
+			}
 			sp--;
 			s[sp] &= s[sp+1];
 			break;
 		case CEXPR_OR:
-			BUG_ON(sp < 1);
+			if (unlikely(sp < 1)) {
+				BUG();
+				return 0;
+			}
 			sp--;
 			s[sp] |= s[sp+1];
 			break;
@@ -541,7 +550,11 @@ int security_permissive_sid(u32 sid)
 	read_lock(&policy_rwlock);
 
 	context = sidtab_search(&sidtab, sid);
-	BUG_ON(!context);
+	if (unlikely(!context)) {
+		BUG();
+		rc = 0;
+		goto out;
+	}
 
 	type = context->type;
 	/*
@@ -550,6 +563,7 @@ int security_permissive_sid(u32 sid)
 	 */
 	rc = ebitmap_get_bit(&policydb.permissive_map, type);
 
+out:
 	read_unlock(&policy_rwlock);
 	return rc;
 }
@@ -697,7 +711,11 @@ int security_bounded_transition(u32 old_sid, u32 new_sid)
 	index = new_context->type;
 	while (true) {
 		type = policydb.type_val_to_struct[index - 1];
-		BUG_ON(!type);
+		if (unlikely(!type)) {
+			BUG();
+			rc = -EPERM;
+			goto out;
+		}
 
 		/* not bounded anymore */
 		if (!type->bounds) {
@@ -1381,7 +1399,10 @@ static int validate_classes(struct policydb *p)
 			continue;
 		pol_class = p->p_class_val_to_name[class_val-1];
 		cladatum = hashtab_search(p->p_classes.table, pol_class);
-		BUG_ON(!cladatum);
+		if (unlikely(!cladatum)) {
+			BUG();
+			return -EINVAL;
+		}
 		perms = &cladatum->permissions;
 		nprim = 1 << (perms->nprim - 1);
 		if (perm_val > nprim) {
@@ -1416,7 +1437,10 @@ static int validate_classes(struct policydb *p)
 			continue;
 		pol_class = p->p_class_val_to_name[class_val-1];
 		cladatum = hashtab_search(p->p_classes.table, pol_class);
-		BUG_ON(!cladatum);
+		if (unlikely(!cladatum)) {
+			BUG();
+			return -EINVAL;
+		}
 		if (!cladatum->comdatum) {
 			printk(KERN_ERR
 			       "SELinux:  class %s should have an inherits clause but does not\n",
diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c
index c0eb720..22413ec 100644
--- a/security/selinux/xfrm.c
+++ b/security/selinux/xfrm.c
@@ -202,7 +202,10 @@ static int selinux_xfrm_sec_ctx_alloc(struct xfrm_sec_ctx **ctxp,
 	char *ctx_str = NULL;
 	u32 str_len;
 
-	BUG_ON(uctx && sid);
+	if (unlikely(ctx && sid)) {
+		BUG();
+		return -EINVAL;
+	}
 
 	if (!uctx)
 		goto not_from_user;
@@ -288,7 +291,10 @@ int selinux_xfrm_policy_alloc(struct xfrm_sec_ctx **ctxp,
 {
 	int err;
 
-	BUG_ON(!uctx);
+	if (unlikely(!uctx)) {
+		BUG();
+		return -EINVAL;
+	}
 
 	err = selinux_xfrm_sec_ctx_alloc(ctxp, uctx, 0);
 	if (err == 0)
@@ -356,7 +362,10 @@ int selinux_xfrm_state_alloc(struct xfrm_state *x, struct xfrm_user_sec_ctx *uct
 {
 	int err;
 
-	BUG_ON(!x);
+	if (unlikely(!x)) {
+		BUG();
+		return -EINVAL;
+	}
 
 	err = selinux_xfrm_sec_ctx_alloc(&x->security, uctx, secid);
 	if (err == 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.

[PATCH 2/3] SELinux: call capabilities code directory

[Eric Paris]

For cleanliness and efficiency remove all calls to secondary-> and instead
call capabilities code directly.  capabilities are the only module that
selinux stacks with and so the code should not indicate that other stacking
might be possible.

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

 security/selinux/hooks.c |   28 ++++++++++++++--------------
 1 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 6e6847d..e2bdb28 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1864,7 +1864,7 @@ static int selinux_ptrace_may_access(struct task_struct *child,
 {
 	int rc;
 
-	rc = secondary_ops->ptrace_may_access(child, mode);
+	rc = cap_ptrace_may_access(child, mode);
 	if (rc)
 		return rc;
 
@@ -1881,7 +1881,7 @@ static int selinux_ptrace_traceme(struct task_struct *parent)
 {
 	int rc;
 
-	rc = secondary_ops->ptrace_traceme(parent);
+	rc = cap_ptrace_traceme(parent);
 	if (rc)
 		return rc;
 
@@ -1897,7 +1897,7 @@ static int selinux_capget(struct task_struct *target, kernel_cap_t *effective,
 	if (error)
 		return error;
 
-	return secondary_ops->capget(target, effective, inheritable, permitted);
+	return cap_capget(target, effective, inheritable, permitted);
 }
 
 static int selinux_capset(struct cred *new, const struct cred *old,
@@ -1907,7 +1907,7 @@ static int selinux_capset(struct cred *new, const struct cred *old,
 {
 	int error;
 
-	error = secondary_ops->capset(new, old,
+	error = cap_capset(new, old,
 				      effective, inheritable, permitted);
 	if (error)
 		return error;
@@ -1930,7 +1930,7 @@ static int selinux_capable(struct task_struct *tsk, const struct cred *cred,
 {
 	int rc;
 
-	rc = secondary_ops->capable(tsk, cred, cap, audit);
+	rc = cap_capable(tsk, cred, cap, audit);
 	if (rc)
 		return rc;
 
@@ -2056,7 +2056,7 @@ static int selinux_syslog(int type)
 {
 	int rc;
 
-	rc = secondary_ops->syslog(type);
+	rc = cap_syslog(type);
 	if (rc)
 		return rc;
 
@@ -2087,7 +2087,7 @@ static int selinux_syslog(int type)
  * mapping. 0 means there is enough memory for the allocation to
  * succeed and -ENOMEM implies there is not.
  *
- * Note that secondary_ops->capable and task_has_perm_noaudit return 0
+ * Note that cap_capable and task_has_perm_noaudit return 0
  * if the capability is granted, but __vm_enough_memory requires 1 if
  * the capability is granted.
  *
@@ -2117,7 +2117,7 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm)
 	struct inode *inode = bprm->file->f_path.dentry->d_inode;
 	int rc;
 
-	rc = secondary_ops->bprm_set_creds(bprm);
+	rc = cap_bprm_set_creds(bprm);
 	if (rc)
 		return rc;
 
@@ -2234,7 +2234,7 @@ static int selinux_bprm_secureexec(struct linux_binprm *bprm)
 					PROCESS__NOATSECURE, NULL);
 	}
 
-	return (atsecure || secondary_ops->bprm_secureexec(bprm));
+	return (atsecure || cap_bprm_secureexec(bprm));
 }
 
 extern struct vfsmount *selinuxfs_mount;
@@ -3335,7 +3335,7 @@ static int selinux_task_setnice(struct task_struct *p, int nice)
 {
 	int rc;
 
-	rc = secondary_ops->task_setnice(p, nice);
+	rc = cap_task_setnice(p, nice);
 	if (rc)
 		return rc;
 
@@ -3346,7 +3346,7 @@ static int selinux_task_setioprio(struct task_struct *p, int ioprio)
 {
 	int rc;
 
-	rc = secondary_ops->task_setioprio(p, ioprio);
+	rc = cap_task_setioprio(p, ioprio);
 	if (rc)
 		return rc;
 
@@ -3376,7 +3376,7 @@ static int selinux_task_setscheduler(struct task_struct *p, int policy, struct s
 {
 	int rc;
 
-	rc = secondary_ops->task_setscheduler(p, policy, lp);
+	rc = cap_task_setscheduler(p, policy, lp);
 	if (rc)
 		return rc;
 
@@ -4634,7 +4634,7 @@ static int selinux_netlink_send(struct sock *sk, struct sk_buff *skb)
 {
 	int err;
 
-	err = secondary_ops->netlink_send(sk, skb);
+	err = cap_netlink_send(sk, skb);
 	if (err)
 		return err;
 
@@ -4649,7 +4649,7 @@ static int selinux_netlink_recv(struct sk_buff *skb, int capability)
 	int err;
 	struct avc_audit_data ad;
 
-	err = secondary_ops->netlink_recv(skb, capability);
+	err = cap_netlink_recv(skb, capability);
 	if (err)
 		return err;
 


--
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: better printk when file with invalid label found

[Eric Paris]

Currently when an inode is read into the kernel with an invalid label
string (can often happen with removable media) we output a string like:

SELinux: inode_doinit_with_dentry:  context_to_sid([SOME INVALID LABEL])
returned -22 dor dev=[blah] ino=[blah]

Which is all but incomprehensible to all but a couple of us.  Instead, on
EINVAL only, I plan to output a much more user friendly string and I plan to
ratelimit the printk since many of these could be generated very rapidly.

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

 security/selinux/hooks.c |   17 +++++++++++++----
 1 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index e2bdb28..7fe870d 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1335,10 +1335,19 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
 							     sbsec->def_sid,
 							     GFP_NOFS);
 			if (rc) {
-				printk(KERN_WARNING "SELinux: %s:  context_to_sid(%s) "
-				       "returned %d for dev=%s ino=%ld\n",
-				       __func__, context, -rc,
-				       inode->i_sb->s_id, inode->i_ino);
+				char *dev = inode->i_sb->s_id;
+				unsigned long ino = inode->i_ino;
+
+				if (rc == -EINVAL) {
+					if (printk_ratelimit())
+						printk(KERN_NOTICE "SELinux: inode=%lu on dev=%s was found to have an invalid "
+							"context=%s.  This indicates you may need to relabel the inode or the "
+							"filesystem in question.\n", ino, dev, context);
+				} else {
+					printk(KERN_WARNING "SELinux: %s:  context_to_sid(%s) "
+					       "returned %d for dev=%s ino=%ld\n",
+					       __func__, context, -rc, dev, ino);
+				}
 				kfree(context);
 				/* Leave with the unlabeled SID */
 				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 1/3] SELinux: fix selinux to safely handle any bugs even when not CONFIG_BUG

[Stephen Smalley]

On Mon, 2009-02-09 at 16:37 -0500, Eric Paris wrote:
> If CONFIG_BUG is not set places in the code where BUG is called will not
> end execution.  Look for places where this might result is system problems
> and keep the system running.
> 
> Signed-off-by: Eric Paris <eparis@redhat.com>
> ---
> 
>  kernel/capability.c            |    1 +
>  security/selinux/avc.c         |   13 +++++++++++--
>  security/selinux/hooks.c       |    9 ++++++++-
>  security/selinux/netnode.c     |    3 +++
>  security/selinux/ss/ebitmap.h  |   15 ++++++++++++---
>  security/selinux/ss/services.c |   38 +++++++++++++++++++++++++++++++-------
>  security/selinux/xfrm.c        |   15 ++++++++++++---
>  7 files changed, 78 insertions(+), 16 deletions(-)
> 
<snip>
> diff --git a/security/selinux/avc.c b/security/selinux/avc.c
> index eb41f43..c22db1e 100644
> --- a/security/selinux/avc.c
> +++ b/security/selinux/avc.c
> @@ -202,6 +202,7 @@ static void avc_dump_query(struct audit_buffer *ab, u32 ssid, u32 tsid, u16 tcla
>  {
>  	int rc;
>  	char *scontext;
> +	const char *class_string;
>  	u32 scontext_len;
>  
>  	rc = security_sid_to_context(ssid, &scontext, &scontext_len);
> @@ -220,8 +221,16 @@ static void avc_dump_query(struct audit_buffer *ab, u32 ssid, u32 tsid, u16 tcla
>  		kfree(scontext);
>  	}
>  
> -	BUG_ON(tclass >= ARRAY_SIZE(class_to_string) || !class_to_string[tclass]);
> -	audit_log_format(ab, " tclass=%s", class_to_string[tclass]);
> +	if (unlikely(tclass >= ARRAY_SIZE(class_to_string) || !class_to_string[tclass])) {
> +		class_string = "unknown";
> +		if (tclass >= ARRAY_SIZE(class_to_string))
> +			BUG();
> +		else
> +			BUG();

Reduce that if statement to just BUG().
A possible alternative to logging the "unknown" string would be to log
the value of tclass; that would provide more information for debugging.

> +	} else {
> +		class_string = class_to_string[tclass];
> +	}
> +	audit_log_format(ab, " tclass=%s", class_string);
>  }
>  
>  /**
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index ae5bb64..6e6847d 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -995,7 +995,11 @@ static int superblock_doinit(struct super_block *sb, void *data)
>  	if (!data)
>  		goto out;
>  
> -	BUG_ON(sb->s_type->fs_flags & FS_BINARY_MOUNTDATA);
> +	if (unlikely(sb->s_type->fs_flags & FS_BINARY_MOUNTDATA)) {
> +		BUG();
> +		rc = -EINVAL;
> +		goto out_err;

Why out_err vs. out?  No allocation yet, right?  Or alternatively, if
all calls to security_init_mnt_opts() should be balanced with calls to
security_free_mnt_opts(), why don't we goto out_err in the prior error
path?

> @@ -1500,6 +1505,8 @@ static int task_has_capability(struct task_struct *tsk,
>  		printk(KERN_ERR
>  		       "SELinux:  out of range capability %d\n", cap);
>  		BUG();
> +		/* should I audit somehow? */
> +		return -EPERM;

I think the printk KERN_ERR suffices, or turn it into a audit_log
AUDIT_SELINUX_ERR call.

> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index c65e4fe..cfb0769 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -120,16 +120,25 @@ static int constraint_expr_eval(struct context *scontext,
>  	for (e = cexpr; e; e = e->next) {
>  		switch (e->expr_type) {
>  		case CEXPR_NOT:
> -			BUG_ON(sp < 0);
> +			if (unlikely(sp < 0)) {
> +				BUG();
> +				return 0;
> +			}

General question:  Should we in fact be panic'ing in these cases where
we cannot return a value that will in fact abort the computation and
guarantee that the operation will not proceed?  Same applies to the
ebitmap code.  Just returning 0 (false) doesn't necessarily mean that we
won't grant a permission, as the result may get negated.

-- 
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: call capabilities code directory

[Stephen Smalley]

On Mon, 2009-02-09 at 16:37 -0500, Eric Paris wrote:
> For cleanliness and efficiency remove all calls to secondary-> and instead
> call capabilities code directly.  capabilities are the only module that
> selinux stacks with and so the code should not indicate that other stacking
> might be possible.
> 
> Signed-off-by: Eric Paris <eparis@redhat.com>
> ---
> 
>  security/selinux/hooks.c |   28 ++++++++++++++--------------
>  1 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 6e6847d..e2bdb28 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2087,7 +2087,7 @@ static int selinux_syslog(int type)
>   * mapping. 0 means there is enough memory for the allocation to
>   * succeed and -ENOMEM implies there is not.
>   *
> - * Note that secondary_ops->capable and task_has_perm_noaudit return 0
> + * Note that cap_capable and task_has_perm_noaudit return 0

This part of the comment is a bit out of date - at this point we are
just calling selinux_capable(...SECURITY_CAP_NOAUDIT) rather than
separately calling cap_capable() and task_has_perm_noaudit().

>   * if the capability is granted, but __vm_enough_memory requires 1 if
>   * the capability is granted.
>   *

-- 
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: better printk when file with invalid label found

[Stephen Smalley]

On Mon, 2009-02-09 at 16:37 -0500, Eric Paris wrote:
> Currently when an inode is read into the kernel with an invalid label
> string (can often happen with removable media) we output a string like:
> 
> SELinux: inode_doinit_with_dentry:  context_to_sid([SOME INVALID LABEL])
> returned -22 dor dev=[blah] ino=[blah]
> 
> Which is all but incomprehensible to all but a couple of us.  Instead, on
> EINVAL only, I plan to output a much more user friendly string and I plan to
> ratelimit the printk since many of these could be generated very rapidly.
> 
> Signed-off-by: Eric Paris <eparis@redhat.com>

You could likely further drop the function name in all cases, and maybe
even the error code (is there no strerror() equivalent in the kernel?).


> ---
> 
>  security/selinux/hooks.c |   17 +++++++++++++----
>  1 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index e2bdb28..7fe870d 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1335,10 +1335,19 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
>  							     sbsec->def_sid,
>  							     GFP_NOFS);
>  			if (rc) {
> -				printk(KERN_WARNING "SELinux: %s:  context_to_sid(%s) "
> -				       "returned %d for dev=%s ino=%ld\n",
> -				       __func__, context, -rc,
> -				       inode->i_sb->s_id, inode->i_ino);
> +				char *dev = inode->i_sb->s_id;
> +				unsigned long ino = inode->i_ino;
> +
> +				if (rc == -EINVAL) {
> +					if (printk_ratelimit())
> +						printk(KERN_NOTICE "SELinux: inode=%lu on dev=%s was found to have an invalid "
> +							"context=%s.  This indicates you may need to relabel the inode or the "
> +							"filesystem in question.\n", ino, dev, context);
> +				} else {
> +					printk(KERN_WARNING "SELinux: %s:  context_to_sid(%s) "
> +					       "returned %d for dev=%s ino=%ld\n",
> +					       __func__, context, -rc, dev, ino);
> +				}
>  				kfree(context);
>  				/* Leave with the unlabeled SID */
>  				rc = 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 3/3] SELinux: better printk when file with invalid label found

[Eric Paris]

On Tue, 2009-02-10 at 09:14 -0500, Stephen Smalley wrote:
> On Mon, 2009-02-09 at 16:37 -0500, Eric Paris wrote:
> > Currently when an inode is read into the kernel with an invalid label
> > string (can often happen with removable media) we output a string like:
> > 
> > SELinux: inode_doinit_with_dentry:  context_to_sid([SOME INVALID LABEL])
> > returned -22 dor dev=[blah] ino=[blah]
> > 
> > Which is all but incomprehensible to all but a couple of us.  Instead, on
> > EINVAL only, I plan to output a much more user friendly string and I plan to
> > ratelimit the printk since many of these could be generated very rapidly.
> > 
> > Signed-off-by: Eric Paris <eparis@redhat.com>
> 
> You could likely further drop the function name in all cases, and maybe
> even the error code (is there no strerror() equivalent in the kernel?).

I'd say that anyone getting the other message needs to report it to a
developer and I kinda like return codes, function names, and everything
else that normal users consider cryptic.  It really shouldn't be popping
out, so I hoped to leave it as is.

If you really want me to simply that one as well, do you have a text
suggestion?  If not, I'll leave this patch as is.

-Eric



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

Re: [PATCH 2/3] SELinux: call capabilities code directory

[Eric Paris]

On Tue, 2009-02-10 at 09:06 -0500, Stephen Smalley wrote:
> On Mon, 2009-02-09 at 16:37 -0500, Eric Paris wrote:
> > For cleanliness and efficiency remove all calls to secondary-> and instead
> > call capabilities code directly.  capabilities are the only module that
> > selinux stacks with and so the code should not indicate that other stacking
> > might be possible.
> > 
> > Signed-off-by: Eric Paris <eparis@redhat.com>
> > ---
> > 
> >  security/selinux/hooks.c |   28 ++++++++++++++--------------
> >  1 files changed, 14 insertions(+), 14 deletions(-)
> > 
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index 6e6847d..e2bdb28 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -2087,7 +2087,7 @@ static int selinux_syslog(int type)
> >   * mapping. 0 means there is enough memory for the allocation to
> >   * succeed and -ENOMEM implies there is not.
> >   *
> > - * Note that secondary_ops->capable and task_has_perm_noaudit return 0
> > + * Note that cap_capable and task_has_perm_noaudit return 0
> 
> This part of the comment is a bit out of date - at this point we are
> just calling selinux_capable(...SECURITY_CAP_NOAUDIT) rather than
> separately calling cap_capable() and task_has_perm_noaudit().

version 2 will redo the comment completely.


--
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: fix selinux to safely handle any bugs even when not CONFIG_BUG

[Eric Paris]

On Tue, 2009-02-10 at 09:00 -0500, Stephen Smalley wrote:
> On Mon, 2009-02-09 at 16:37 -0500, Eric Paris wrote:
> > If CONFIG_BUG is not set places in the code where BUG is called will not
> > end execution.  Look for places where this might result is system problems
> > and keep the system running.
> > 
> > Signed-off-by: Eric Paris <eparis@redhat.com>
> > ---
> > 
> >  kernel/capability.c            |    1 +
> >  security/selinux/avc.c         |   13 +++++++++++--
> >  security/selinux/hooks.c       |    9 ++++++++-
> >  security/selinux/netnode.c     |    3 +++
> >  security/selinux/ss/ebitmap.h  |   15 ++++++++++++---
> >  security/selinux/ss/services.c |   38 +++++++++++++++++++++++++++++++-------
> >  security/selinux/xfrm.c        |   15 ++++++++++++---
> >  7 files changed, 78 insertions(+), 16 deletions(-)
> > 
> <snip>
> > diff --git a/security/selinux/avc.c b/security/selinux/avc.c
> > index eb41f43..c22db1e 100644
> > --- a/security/selinux/avc.c
> > +++ b/security/selinux/avc.c
> > @@ -202,6 +202,7 @@ static void avc_dump_query(struct audit_buffer *ab, u32 ssid, u32 tsid, u16 tcla
> >  {
> >  	int rc;
> >  	char *scontext;
> > +	const char *class_string;
> >  	u32 scontext_len;
> >  
> >  	rc = security_sid_to_context(ssid, &scontext, &scontext_len);
> > @@ -220,8 +221,16 @@ static void avc_dump_query(struct audit_buffer *ab, u32 ssid, u32 tsid, u16 tcla
> >  		kfree(scontext);
> >  	}
> >  
> > -	BUG_ON(tclass >= ARRAY_SIZE(class_to_string) || !class_to_string[tclass]);
> > -	audit_log_format(ab, " tclass=%s", class_to_string[tclass]);
> > +	if (unlikely(tclass >= ARRAY_SIZE(class_to_string) || !class_to_string[tclass])) {
> > +		class_string = "unknown";
> > +		if (tclass >= ARRAY_SIZE(class_to_string))
> > +			BUG();
> > +		else
> > +			BUG();
> 
> Reduce that if statement to just BUG().
> A possible alternative to logging the "unknown" string would be to log
> the value of tclass; that would provide more information for debugging.

Took a second to remember why I did that.  I don't like BUG_ON(A || B)
since you don't know which it was.  If I output the value of tclass
before I BUG, maybe in a printk I'd be happy.  I look at this section
again.

> 
> > +	} else {
> > +		class_string = class_to_string[tclass];
> > +	}
> > +	audit_log_format(ab, " tclass=%s", class_string);
> >  }
> >  
> >  /**
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index ae5bb64..6e6847d 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -995,7 +995,11 @@ static int superblock_doinit(struct super_block *sb, void *data)
> >  	if (!data)
> >  		goto out;
> >  
> > -	BUG_ON(sb->s_type->fs_flags & FS_BINARY_MOUNTDATA);
> > +	if (unlikely(sb->s_type->fs_flags & FS_BINARY_MOUNTDATA)) {
> > +		BUG();
> > +		rc = -EINVAL;
> > +		goto out_err;
> 
> Why out_err vs. out?  No allocation yet, right?  Or alternatively, if
> all calls to security_init_mnt_opts() should be balanced with calls to
> security_free_mnt_opts(), why don't we goto out_err in the prior error
> path?

Will look.

> > @@ -1500,6 +1505,8 @@ static int task_has_capability(struct task_struct *tsk,
> >  		printk(KERN_ERR
> >  		       "SELinux:  out of range capability %d\n", cap);
> >  		BUG();
> > +		/* should I audit somehow? */
> > +		return -EPERM;
> 
> I think the printk KERN_ERR suffices, or turn it into a audit_log
> AUDIT_SELINUX_ERR call.

Ok.

> > diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> > index c65e4fe..cfb0769 100644
> > --- a/security/selinux/ss/services.c
> > +++ b/security/selinux/ss/services.c
> > @@ -120,16 +120,25 @@ static int constraint_expr_eval(struct context *scontext,
> >  	for (e = cexpr; e; e = e->next) {
> >  		switch (e->expr_type) {
> >  		case CEXPR_NOT:
> > -			BUG_ON(sp < 0);
> > +			if (unlikely(sp < 0)) {
> > +				BUG();
> > +				return 0;
> > +			}
> 
> General question:  Should we in fact be panic'ing in these cases where
> we cannot return a value that will in fact abort the computation and
> guarantee that the operation will not proceed?  Same applies to the
> ebitmap code.  Just returning 0 (false) doesn't necessarily mean that we
> won't grant a permission, as the result may get negated.

James?


--
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: fix selinux to safely handle any bugs even when not CONFIG_BUG

[James Morris]

On Tue, 10 Feb 2009, Eric Paris wrote:

> > > +++ b/security/selinux/ss/services.c
> > > @@ -120,16 +120,25 @@ static int constraint_expr_eval(struct context *scontext,
> > >  	for (e = cexpr; e; e = e->next) {
> > >  		switch (e->expr_type) {
> > >  		case CEXPR_NOT:
> > > -			BUG_ON(sp < 0);
> > > +			if (unlikely(sp < 0)) {
> > > +				BUG();
> > > +				return 0;
> > > +			}
> > 
> > General question:  Should we in fact be panic'ing in these cases where
> > we cannot return a value that will in fact abort the computation and
> > guarantee that the operation will not proceed?  Same applies to the
> > ebitmap code.  Just returning 0 (false) doesn't necessarily mean that we
> > won't grant a permission, as the result may get negated.
> 
> James?

Perhaps panic() in enforcing mode, otherwise print a warning.


- James
-- 
James Morris
<jmorris@namei.org>

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

Re: [PATCH 3/3] SELinux: better printk when file with invalid label found

[Stephen Smalley]

On Tue, 2009-02-10 at 09:30 -0500, Eric Paris wrote:
> On Tue, 2009-02-10 at 09:14 -0500, Stephen Smalley wrote:
> > On Mon, 2009-02-09 at 16:37 -0500, Eric Paris wrote:
> > > Currently when an inode is read into the kernel with an invalid label
> > > string (can often happen with removable media) we output a string like:
> > > 
> > > SELinux: inode_doinit_with_dentry:  context_to_sid([SOME INVALID LABEL])
> > > returned -22 dor dev=[blah] ino=[blah]
> > > 
> > > Which is all but incomprehensible to all but a couple of us.  Instead, on
> > > EINVAL only, I plan to output a much more user friendly string and I plan to
> > > ratelimit the printk since many of these could be generated very rapidly.
> > > 
> > > Signed-off-by: Eric Paris <eparis@redhat.com>
> > 
> > You could likely further drop the function name in all cases, and maybe
> > even the error code (is there no strerror() equivalent in the kernel?).
> 
> I'd say that anyone getting the other message needs to report it to a
> developer and I kinda like return codes, function names, and everything
> else that normal users consider cryptic.  It really shouldn't be popping
> out, so I hoped to leave it as is.
> 
> If you really want me to simply that one as well, do you have a text
> suggestion?  If not, I'll leave this patch as is.

Shrug.  That's fine.

-- 
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: fix selinux to safely handle any bugs even when not CONFIG_BUG

[Stephen Smalley]

On Wed, 2009-02-11 at 01:52 +1100, James Morris wrote:
> On Tue, 10 Feb 2009, Eric Paris wrote:
> 
> > > > +++ b/security/selinux/ss/services.c
> > > > @@ -120,16 +120,25 @@ static int constraint_expr_eval(struct context *scontext,
> > > >  	for (e = cexpr; e; e = e->next) {
> > > >  		switch (e->expr_type) {
> > > >  		case CEXPR_NOT:
> > > > -			BUG_ON(sp < 0);
> > > > +			if (unlikely(sp < 0)) {
> > > > +				BUG();
> > > > +				return 0;
> > > > +			}
> > > 
> > > General question:  Should we in fact be panic'ing in these cases where
> > > we cannot return a value that will in fact abort the computation and
> > > guarantee that the operation will not proceed?  Same applies to the
> > > ebitmap code.  Just returning 0 (false) doesn't necessarily mean that we
> > > won't grant a permission, as the result may get negated.
> > 
> > James?
> 
> Perhaps panic() in enforcing mode, otherwise print a warning.

Yuck.

These are cases where we were already fine with an undefined opcode from
BUG() happening in the usual CONFIG_BUG=y case, so I'd think they could
be made unconditional panic() calls.

-- 
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: fix selinux to safely handle any bugs even when not CONFIG_BUG

[Paul Moore]

On Monday 09 February 2009 04:37:14 pm Eric Paris wrote:
> If CONFIG_BUG is not set places in the code where BUG is called will not
> end execution.  Look for places where this might result is system problems
> and keep the system running.
>
> Signed-off-by: Eric Paris <eparis@redhat.com>

...

> diff --git a/security/selinux/netnode.c b/security/selinux/netnode.c
> index 7100072..e002d5a 100644
> --- a/security/selinux/netnode.c
> +++ b/security/selinux/netnode.c

...

> @@ -240,6 +242,7 @@ static int sel_netnode_sid_slow(void *addr, u16 family,
> u32 *sid) break;
>  	default:
>  		BUG();
> +		ret = -EINVAL;
>  	}
>  	if (ret != 0)
>  		goto out;

I'm just being picky here, but how about EPROTONOSUPPORT instead of EINVAL 
here?  Otherwise it looks good to me.

-- 
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: fix selinux to safely handle any bugs even when not CONFIG_BUG

[James Morris]

On Mon, 9 Feb 2009, Eric Paris wrote:

> If CONFIG_BUG is not set places in the code where BUG is called will not
> end execution.  Look for places where this might result is system problems
> and keep the system running.

I wonder if we should make SELinux depend on CONFIG_BUG ?

This is pointlessly ugly, and I can't see why you'd disable CONFIG_BUG 
when running SELinux.


> Signed-off-by: Eric Paris <eparis@redhat.com>
> ---
> 
>  kernel/capability.c            |    1 +
>  security/selinux/avc.c         |   13 +++++++++++--
>  security/selinux/hooks.c       |    9 ++++++++-
>  security/selinux/netnode.c     |    3 +++
>  security/selinux/ss/ebitmap.h  |   15 ++++++++++++---
>  security/selinux/ss/services.c |   38 +++++++++++++++++++++++++++++++-------
>  security/selinux/xfrm.c        |   15 ++++++++++++---
>  7 files changed, 78 insertions(+), 16 deletions(-)
> 
> diff --git a/kernel/capability.c b/kernel/capability.c
> index 4e17041..e8bec6a 100644
> --- a/kernel/capability.c
> +++ b/kernel/capability.c
> @@ -304,6 +304,7 @@ int capable(int cap)
>  	if (unlikely(!cap_valid(cap))) {
>  		printk(KERN_CRIT "capable() called with invalid cap=%u\n", cap);
>  		BUG();
> +		return 0;
>  	}
>  
>  	if (security_capable(cap) == 0) {
> diff --git a/security/selinux/avc.c b/security/selinux/avc.c
> index eb41f43..c22db1e 100644
> --- a/security/selinux/avc.c
> +++ b/security/selinux/avc.c
> @@ -202,6 +202,7 @@ static void avc_dump_query(struct audit_buffer *ab, u32 ssid, u32 tsid, u16 tcla
>  {
>  	int rc;
>  	char *scontext;
> +	const char *class_string;
>  	u32 scontext_len;
>  
>  	rc = security_sid_to_context(ssid, &scontext, &scontext_len);
> @@ -220,8 +221,16 @@ static void avc_dump_query(struct audit_buffer *ab, u32 ssid, u32 tsid, u16 tcla
>  		kfree(scontext);
>  	}
>  
> -	BUG_ON(tclass >= ARRAY_SIZE(class_to_string) || !class_to_string[tclass]);
> -	audit_log_format(ab, " tclass=%s", class_to_string[tclass]);
> +	if (unlikely(tclass >= ARRAY_SIZE(class_to_string) || !class_to_string[tclass])) {
> +		class_string = "unknown";
> +		if (tclass >= ARRAY_SIZE(class_to_string))
> +			BUG();
> +		else
> +			BUG();
> +	} else {
> +		class_string = class_to_string[tclass];
> +	}
> +	audit_log_format(ab, " tclass=%s", class_string);
>  }
>  
>  /**
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index ae5bb64..6e6847d 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -995,7 +995,11 @@ static int superblock_doinit(struct super_block *sb, void *data)
>  	if (!data)
>  		goto out;
>  
> -	BUG_ON(sb->s_type->fs_flags & FS_BINARY_MOUNTDATA);
> +	if (unlikely(sb->s_type->fs_flags & FS_BINARY_MOUNTDATA)) {
> +		BUG();
> +		rc = -EINVAL;
> +		goto out_err;
> +	}
>  
>  	rc = selinux_parse_opts_str(options, &opts);
>  	if (rc)
> @@ -1042,6 +1046,7 @@ static void selinux_write_opts(struct seq_file *m,
>  			continue;
>  		default:
>  			BUG();
> +			continue;
>  		};
>  		/* we need a comma before each option */
>  		seq_putc(m, ',');
> @@ -1500,6 +1505,8 @@ static int task_has_capability(struct task_struct *tsk,
>  		printk(KERN_ERR
>  		       "SELinux:  out of range capability %d\n", cap);
>  		BUG();
> +		/* should I audit somehow? */
> +		return -EPERM;
>  	}
>  
>  	rc = avc_has_perm_noaudit(sid, sid, sclass, av, 0, &avd);
> diff --git a/security/selinux/netnode.c b/security/selinux/netnode.c
> index 7100072..e002d5a 100644
> --- a/security/selinux/netnode.c
> +++ b/security/selinux/netnode.c
> @@ -140,6 +140,7 @@ static struct sel_netnode *sel_netnode_find(const void *addr, u16 family)
>  		break;
>  	default:
>  		BUG();
> +		return NULL;
>  	}
>  
>  	list_for_each_entry_rcu(node, &sel_netnode_hash[idx].list, list)
> @@ -180,6 +181,7 @@ static void sel_netnode_insert(struct sel_netnode *node)
>  		break;
>  	default:
>  		BUG();
> +		return;
>  	}
>  
>  	INIT_RCU_HEAD(&node->rcu);
> @@ -240,6 +242,7 @@ static int sel_netnode_sid_slow(void *addr, u16 family, u32 *sid)
>  		break;
>  	default:
>  		BUG();
> +		ret = -EINVAL;
>  	}
>  	if (ret != 0)
>  		goto out;
> diff --git a/security/selinux/ss/ebitmap.h b/security/selinux/ss/ebitmap.h
> index f283b43..425f8b8 100644
> --- a/security/selinux/ss/ebitmap.h
> +++ b/security/selinux/ss/ebitmap.h
> @@ -85,7 +85,10 @@ static inline int ebitmap_node_get_bit(struct ebitmap_node *n,
>  	unsigned int index = EBITMAP_NODE_INDEX(n, bit);
>  	unsigned int ofs = EBITMAP_NODE_OFFSET(n, bit);
>  
> -	BUG_ON(index >= EBITMAP_UNIT_NUMS);
> +	if (unlikely((index >= EBITMAP_UNIT_NUMS))) {
> +		BUG();
> +		return 0;
> +	}
>  	if ((n->maps[index] & (EBITMAP_BIT << ofs)))
>  		return 1;
>  	return 0;
> @@ -97,7 +100,10 @@ static inline void ebitmap_node_set_bit(struct ebitmap_node *n,
>  	unsigned int index = EBITMAP_NODE_INDEX(n, bit);
>  	unsigned int ofs = EBITMAP_NODE_OFFSET(n, bit);
>  
> -	BUG_ON(index >= EBITMAP_UNIT_NUMS);
> +	if (unlikely((index >= EBITMAP_UNIT_NUMS))) {
> +		BUG();
> +		return;
> +	}
>  	n->maps[index] |= (EBITMAP_BIT << ofs);
>  }
>  
> @@ -107,7 +113,10 @@ static inline void ebitmap_node_clr_bit(struct ebitmap_node *n,
>  	unsigned int index = EBITMAP_NODE_INDEX(n, bit);
>  	unsigned int ofs = EBITMAP_NODE_OFFSET(n, bit);
>  
> -	BUG_ON(index >= EBITMAP_UNIT_NUMS);
> +	if (unlikely((index >= EBITMAP_UNIT_NUMS))) {
> +		BUG();
> +		return;
> +	}
>  	n->maps[index] &= ~(EBITMAP_BIT << ofs);
>  }
>  
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index c65e4fe..cfb0769 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -120,16 +120,25 @@ static int constraint_expr_eval(struct context *scontext,
>  	for (e = cexpr; e; e = e->next) {
>  		switch (e->expr_type) {
>  		case CEXPR_NOT:
> -			BUG_ON(sp < 0);
> +			if (unlikely(sp < 0)) {
> +				BUG();
> +				return 0;
> +			}
>  			s[sp] = !s[sp];
>  			break;
>  		case CEXPR_AND:
> -			BUG_ON(sp < 1);
> +			if (unlikely(sp < 1)) {
> +				BUG();
> +				return 0;
> +			}
>  			sp--;
>  			s[sp] &= s[sp+1];
>  			break;
>  		case CEXPR_OR:
> -			BUG_ON(sp < 1);
> +			if (unlikely(sp < 1)) {
> +				BUG();
> +				return 0;
> +			}
>  			sp--;
>  			s[sp] |= s[sp+1];
>  			break;
> @@ -541,7 +550,11 @@ int security_permissive_sid(u32 sid)
>  	read_lock(&policy_rwlock);
>  
>  	context = sidtab_search(&sidtab, sid);
> -	BUG_ON(!context);
> +	if (unlikely(!context)) {
> +		BUG();
> +		rc = 0;
> +		goto out;
> +	}
>  
>  	type = context->type;
>  	/*
> @@ -550,6 +563,7 @@ int security_permissive_sid(u32 sid)
>  	 */
>  	rc = ebitmap_get_bit(&policydb.permissive_map, type);
>  
> +out:
>  	read_unlock(&policy_rwlock);
>  	return rc;
>  }
> @@ -697,7 +711,11 @@ int security_bounded_transition(u32 old_sid, u32 new_sid)
>  	index = new_context->type;
>  	while (true) {
>  		type = policydb.type_val_to_struct[index - 1];
> -		BUG_ON(!type);
> +		if (unlikely(!type)) {
> +			BUG();
> +			rc = -EPERM;
> +			goto out;
> +		}
>  
>  		/* not bounded anymore */
>  		if (!type->bounds) {
> @@ -1381,7 +1399,10 @@ static int validate_classes(struct policydb *p)
>  			continue;
>  		pol_class = p->p_class_val_to_name[class_val-1];
>  		cladatum = hashtab_search(p->p_classes.table, pol_class);
> -		BUG_ON(!cladatum);
> +		if (unlikely(!cladatum)) {
> +			BUG();
> +			return -EINVAL;
> +		}
>  		perms = &cladatum->permissions;
>  		nprim = 1 << (perms->nprim - 1);
>  		if (perm_val > nprim) {
> @@ -1416,7 +1437,10 @@ static int validate_classes(struct policydb *p)
>  			continue;
>  		pol_class = p->p_class_val_to_name[class_val-1];
>  		cladatum = hashtab_search(p->p_classes.table, pol_class);
> -		BUG_ON(!cladatum);
> +		if (unlikely(!cladatum)) {
> +			BUG();
> +			return -EINVAL;
> +		}
>  		if (!cladatum->comdatum) {
>  			printk(KERN_ERR
>  			       "SELinux:  class %s should have an inherits clause but does not\n",
> diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c
> index c0eb720..22413ec 100644
> --- a/security/selinux/xfrm.c
> +++ b/security/selinux/xfrm.c
> @@ -202,7 +202,10 @@ static int selinux_xfrm_sec_ctx_alloc(struct xfrm_sec_ctx **ctxp,
>  	char *ctx_str = NULL;
>  	u32 str_len;
>  
> -	BUG_ON(uctx && sid);
> +	if (unlikely(ctx && sid)) {
> +		BUG();
> +		return -EINVAL;
> +	}
>  
>  	if (!uctx)
>  		goto not_from_user;
> @@ -288,7 +291,10 @@ int selinux_xfrm_policy_alloc(struct xfrm_sec_ctx **ctxp,
>  {
>  	int err;
>  
> -	BUG_ON(!uctx);
> +	if (unlikely(!uctx)) {
> +		BUG();
> +		return -EINVAL;
> +	}
>  
>  	err = selinux_xfrm_sec_ctx_alloc(ctxp, uctx, 0);
>  	if (err == 0)
> @@ -356,7 +362,10 @@ int selinux_xfrm_state_alloc(struct xfrm_state *x, struct xfrm_user_sec_ctx *uct
>  {
>  	int err;
>  
> -	BUG_ON(!x);
> +	if (unlikely(!x)) {
> +		BUG();
> +		return -EINVAL;
> +	}
>  
>  	err = selinux_xfrm_sec_ctx_alloc(&x->security, uctx, secid);
>  	if (err == 0)
> 

-- 
James Morris
<jmorris@namei.org>

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