[PATCH] SELinux: eliminate unaligned accesses by policy loading code

This patch rewrites the SELinux next_entry() function and all callers to
copy entry data from the binary policy into properly aligned buffers,
eliminating unaligned accesses.  This patch is in response to a bug report
from Prarit Bhargava for SELinux and ia64, and he has confirmed that this
patch eliminates the unaligned access warnings.
Signed-off-by: Stephen Smalley <sds@epoch.ncsc.mil>
Signed-off-by: James Morris <jmorris@redhat.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

security/selinux/ss/avtab.c

@@ -303,20 +303,25 @@ void avtab_hash_eval(struct avtab *h, char *tag)
 
 int avtab_read_item(void *fp, struct avtab_datum *avdatum, struct avtab_key *avkey)
 {
-	__u32 *buf;
-	__u32 items, items2;
+	u32 buf[7];
+	u32 items, items2;
+	int rc;
 
 	memset(avkey, 0, sizeof(struct avtab_key));
 	memset(avdatum, 0, sizeof(struct avtab_datum));
 
-	buf = next_entry(fp, sizeof(__u32));
-	if (!buf) {
+	rc = next_entry(buf, fp, sizeof(u32));
+	if (rc < 0) {
 		printk(KERN_ERR "security: avtab: truncated entry\n");
 		goto bad;
 	}
 	items2 = le32_to_cpu(buf[0]);
-	buf = next_entry(fp, sizeof(__u32)*items2);
-	if (!buf) {
+	if (items2 > ARRAY_SIZE(buf)) {
+		printk(KERN_ERR "security: avtab: entry overflow\n");
+		goto bad;
+	}
+	rc = next_entry(buf, fp, sizeof(u32)*items2);
+	if (rc < 0) {
 		printk(KERN_ERR "security: avtab: truncated entry\n");
 		goto bad;
 	}
@@ -362,21 +367,22 @@ int avtab_read_item(void *fp, struct avtab_datum *avdatum, struct avtab_key *avk
 
 int avtab_read(struct avtab *a, void *fp, u32 config)
 {
-	int i, rc = -EINVAL;
+	int rc;
 	struct avtab_key avkey;
 	struct avtab_datum avdatum;
-	u32 *buf;
-	u32 nel;
+	u32 buf[1];
+	u32 nel, i;
 
 
-	buf = next_entry(fp, sizeof(u32));
-	if (!buf) {
+	rc = next_entry(buf, fp, sizeof(u32));
+	if (rc < 0) {
 		printk(KERN_ERR "security: avtab: truncated table\n");
 		goto bad;
 	}
 	nel = le32_to_cpu(buf[0]);
 	if (!nel) {
 		printk(KERN_ERR "security: avtab: table is empty\n");
+		rc = -EINVAL;
 		goto bad;
 	}
 	for (i = 0; i < nel; i++) {

security/selinux/ss/conditional.c

@@ -219,15 +219,16 @@ int cond_read_bool(struct policydb *p, struct hashtab *h, void *fp)
 {
 	char *key = NULL;
 	struct cond_bool_datum *booldatum;
-	__u32 *buf, len;
+	u32 buf[3], len;
+	int rc;
 
 	booldatum = kmalloc(sizeof(struct cond_bool_datum), GFP_KERNEL);
 	if (!booldatum)
 		return -1;
 	memset(booldatum, 0, sizeof(struct cond_bool_datum));
 
-	buf = next_entry(fp, sizeof(__u32) * 3);
-	if (!buf)
+	rc = next_entry(buf, fp, sizeof buf);
+	if (rc < 0)
 		goto err;
 
 	booldatum->value = le32_to_cpu(buf[0]);
@@ -238,13 +239,12 @@ int cond_read_bool(struct policydb *p, struct hashtab *h, void *fp)
 
 	len = le32_to_cpu(buf[2]);
 
-	buf = next_entry(fp, len);
-	if (!buf)
-		goto err;
 	key = kmalloc(len + 1, GFP_KERNEL);
 	if (!key)
 		goto err;
-	memcpy(key, buf, len);
+	rc = next_entry(key, fp, len);
+	if (rc < 0)
+		goto err;
 	key[len] = 0;
 	if (hashtab_insert(h, key, booldatum))
 		goto err;
@@ -262,15 +262,15 @@ static int cond_read_av_list(struct policydb *p, void *fp, struct cond_av_list *
 	struct avtab_key key;
 	struct avtab_datum datum;
 	struct avtab_node *node_ptr;
-	int len, i;
-	__u32 *buf;
-	__u8 found;
+	int rc;
+	u32 buf[1], i, len;
+	u8 found;
 
 	*ret_list = NULL;
 
 	len = 0;
-	buf = next_entry(fp, sizeof(__u32));
-	if (!buf)
+	rc = next_entry(buf, fp, sizeof buf);
+	if (rc < 0)
 		return -1;
 
 	len = le32_to_cpu(buf[0]);
@@ -369,27 +369,27 @@ static int expr_isvalid(struct policydb *p, struct cond_expr *expr)
 
 static int cond_read_node(struct policydb *p, struct cond_node *node, void *fp)
 {
-	__u32 *buf;
-	int len, i;
+	u32 buf[2], len, i;
+	int rc;
 	struct cond_expr *expr = NULL, *last = NULL;
 
-	buf = next_entry(fp, sizeof(__u32));
-	if (!buf)
+	rc = next_entry(buf, fp, sizeof(u32));
+	if (rc < 0)
 		return -1;
 
 	node->cur_state = le32_to_cpu(buf[0]);
 
 	len = 0;
-	buf = next_entry(fp, sizeof(__u32));
-	if (!buf)
+	rc = next_entry(buf, fp, sizeof(u32));
+	if (rc < 0)
 		return -1;
 
 	/* expr */
 	len = le32_to_cpu(buf[0]);
 
 	for (i = 0; i < len; i++ ) {
-		buf = next_entry(fp, sizeof(__u32) * 2);
-		if (!buf)
+		rc = next_entry(buf, fp, sizeof(u32) * 2);
+		if (rc < 0)
 			goto err;
 
 		expr = kmalloc(sizeof(struct cond_expr), GFP_KERNEL);
@@ -425,11 +425,11 @@ static int cond_read_node(struct policydb *p, struct cond_node *node, void *fp)
 int cond_read_list(struct policydb *p, void *fp)
 {
 	struct cond_node *node, *last = NULL;
-	__u32 *buf;
-	int i, len;
+	u32 buf[1], i, len;
+	int rc;
 
-	buf = next_entry(fp, sizeof(__u32));
-	if (!buf)
+	rc = next_entry(buf, fp, sizeof buf);
+	if (rc < 0)
 		return -1;
 
 	len = le32_to_cpu(buf[0]);

security/selinux/ss/ebitmap.c

@@ -239,13 +239,13 @@ int ebitmap_read(struct ebitmap *e, void *fp)
 {
 	int rc = -EINVAL;
 	struct ebitmap_node *n, *l;
-	u32 *buf, mapsize, count, i;
+	u32 buf[3], mapsize, count, i;
 	u64 map;
 
 	ebitmap_init(e);
 
-	buf = next_entry(fp, sizeof(u32)*3);
-	if (!buf)
+	rc = next_entry(buf, fp, sizeof buf);
+	if (rc < 0)
 		goto out;
 
 	mapsize = le32_to_cpu(buf[0]);
@@ -269,8 +269,8 @@ int ebitmap_read(struct ebitmap *e, void *fp)
 	}
 	l = NULL;
 	for (i = 0; i < count; i++) {
-		buf = next_entry(fp, sizeof(u32));
-		if (!buf) {
+		rc = next_entry(buf, fp, sizeof(u32));
+		if (rc < 0) {
 			printk(KERN_ERR "security: ebitmap: truncated map\n");
 			goto bad;
 		}
@@ -296,12 +296,11 @@ int ebitmap_read(struct ebitmap *e, void *fp)
 			       n->startbit, (e->highbit - MAPSIZE));
 			goto bad_free;
 		}
-		buf = next_entry(fp, sizeof(u64));
-		if (!buf) {
+		rc = next_entry(&map, fp, sizeof(u64));
+		if (rc < 0) {
 			printk(KERN_ERR "security: ebitmap: truncated map\n");
 			goto bad_free;
 		}
-		memcpy(&map, buf, sizeof(u64));
 		n->map = le64_to_cpu(map);
 
 		if (!n->map) {

security/selinux/ss/mls.c

@@ -402,10 +402,11 @@ void mls_user_destroy(struct user_datum *usrdatum)
 
 int mls_read_perm(struct perm_datum *perdatum, void *fp)
 {
-	u32 *buf;
+	u32 buf[1];
+	int rc;
 
-	buf = next_entry(fp, sizeof(u32));
-	if (!buf)
+	rc = next_entry(buf, fp, sizeof buf);
+	if (rc < 0)
 		return -EINVAL;
 	perdatum->base_perms = le32_to_cpu(buf[0]);
 	return 0;
@@ -418,7 +419,8 @@ int mls_read_perm(struct perm_datum *perdatum, void *fp)
 struct mls_level *mls_read_level(void *fp)
 {
 	struct mls_level *l;
-	u32 *buf;
+	u32 buf[1];
+	int rc;
 
 	l = kmalloc(sizeof(*l), GFP_ATOMIC);
 	if (!l) {
@@ -427,8 +429,8 @@ struct mls_level *mls_read_level(void *fp)
 	}
 	memset(l, 0, sizeof(*l));
 
-	buf = next_entry(fp, sizeof(u32));
-	if (!buf) {
+	rc = next_entry(buf, fp, sizeof buf);
+	if (rc < 0) {
 		printk(KERN_ERR "security: mls: truncated level\n");
 		goto bad;
 	}
@@ -453,16 +455,21 @@ struct mls_level *mls_read_level(void *fp)
  */
 static int mls_read_range_helper(struct mls_range *r, void *fp)
 {
-	u32 *buf;
-	int items, rc = -EINVAL;
+	u32 buf[2], items;
+	int rc;
 
-	buf = next_entry(fp, sizeof(u32));
-	if (!buf)
+	rc = next_entry(buf, fp, sizeof(u32));
+	if (rc < 0)
 		goto out;
 
 	items = le32_to_cpu(buf[0]);
-	buf = next_entry(fp, sizeof(u32) * items);
-	if (!buf) {
+	if (items > ARRAY_SIZE(buf)) {
+		printk(KERN_ERR "security: mls:  range overflow\n");
+		rc = -EINVAL;
+		goto out;
+	}
+	rc = next_entry(buf, fp, sizeof(u32) * items);
+	if (rc < 0) {
 		printk(KERN_ERR "security: mls:  truncated range\n");
 		goto out;
 	}
@@ -515,10 +522,11 @@ int mls_read_range(struct context *c, void *fp)
 int mls_read_class(struct class_datum *cladatum, void *fp)
 {
 	struct mls_perms *p = &cladatum->mlsperms;
-	u32 *buf;
+	u32 buf[4];
+	int rc;
 
-	buf = next_entry(fp, sizeof(u32)*4);
-	if (!buf) {
+	rc = next_entry(buf, fp, sizeof buf);
+	if (rc < 0) {
 		printk(KERN_ERR "security: mls:  truncated mls permissions\n");
 		return -EINVAL;
 	}
@@ -532,15 +540,13 @@ int mls_read_class(struct class_datum *cladatum, void *fp)
 int mls_read_user(struct user_datum *usrdatum, void *fp)
 {
 	struct mls_range_list *r, *l;
-	int rc = 0;
+	int rc;
 	u32 nel, i;
-	u32 *buf;
+	u32 buf[1];
 
-	buf = next_entry(fp, sizeof(u32));
-	if (!buf) {
-		rc = -EINVAL;
+	rc = next_entry(buf, fp, sizeof buf);
+	if (rc < 0)
 		goto out;
-	}
 	nel = le32_to_cpu(buf[0]);
 	l = NULL;
 	for (i = 0; i < nel; i++) {
@@ -569,10 +575,11 @@ int mls_read_user(struct user_datum *usrdatum, void *fp)
 
 int mls_read_nlevels(struct policydb *p, void *fp)
 {
-	u32 *buf;
+	u32 buf[1];
+	int rc;
 
-	buf = next_entry(fp, sizeof(u32));
-	if (!buf)
+	rc = next_entry(buf, fp, sizeof buf);
+	if (rc < 0)
 		return -EINVAL;
 	p->nlevels = le32_to_cpu(buf[0]);
 	return 0;
@@ -657,7 +664,7 @@ int sens_read(struct policydb *p, struct hashtab *h, void *fp)
 	char *key = NULL;
 	struct level_datum *levdatum;
 	int rc;
-	u32 *buf, len;
+	u32 buf[2], len;
 
 	levdatum = kmalloc(sizeof(*levdatum), GFP_ATOMIC);
 	if (!levdatum) {
@@ -666,26 +673,21 @@ int sens_read(struct policydb *p, struct hashtab *h, void *fp)
 	}
 	memset(levdatum, 0, sizeof(*levdatum));
 
-	buf = next_entry(fp, sizeof(u32)*2);
-	if (!buf) {
-		rc = -EINVAL;
+	rc = next_entry(buf, fp, sizeof buf);
+	if (rc < 0)
 		goto bad;
-	}
 
 	len = le32_to_cpu(buf[0]);
 	levdatum->isalias = le32_to_cpu(buf[1]);
 
-	buf = next_entry(fp, len);
-	if (!buf) {
-		rc = -EINVAL;
-		goto bad;
-	}
 	key = kmalloc(len + 1,GFP_ATOMIC);
 	if (!key) {
 		rc = -ENOMEM;
 		goto bad;
 	}
-	memcpy(key, buf, len);
+	rc = next_entry(key, fp, len);
+	if (rc < 0)
+		goto bad;
 	key[len] = 0;
 
 	levdatum->level = mls_read_level(fp);
@@ -710,7 +712,7 @@ int cat_read(struct policydb *p, struct hashtab *h, void *fp)
 	char *key = NULL;
 	struct cat_datum *catdatum;
 	int rc;
-	u32 *buf, len;
+	u32 buf[3], len;
 
 	catdatum = kmalloc(sizeof(*catdatum), GFP_ATOMIC);
 	if (!catdatum) {
@@ -719,27 +721,22 @@ int cat_read(struct policydb *p, struct hashtab *h, void *fp)
 	}
 	memset(catdatum, 0, sizeof(*catdatum));
 
-	buf = next_entry(fp, sizeof(u32)*3);
-	if (!buf) {
-		rc = -EINVAL;
+	rc = next_entry(buf, fp, sizeof buf);
+	if (rc < 0)
 		goto bad;
-	}
 
 	len = le32_to_cpu(buf[0]);
 	catdatum->value = le32_to_cpu(buf[1]);
 	catdatum->isalias = le32_to_cpu(buf[2]);
 
-	buf = next_entry(fp, len);
-	if (!buf) {
-		rc = -EINVAL;
-		goto bad;
-	}
 	key = kmalloc(len + 1,GFP_ATOMIC);
 	if (!key) {
 		rc = -ENOMEM;
 		goto bad;
 	}
-	memcpy(key, buf, len);
+	rc = next_entry(key, fp, len);
+	if (rc < 0)
+		goto bad;
 	key[len] = 0;
 
 	rc = hashtab_insert(h, key, catdatum);

security/selinux/ss/policydb.h

@@ -271,17 +271,15 @@ struct policy_file {
 	size_t len;
 };
 
-static inline void *next_entry(struct policy_file *fp, size_t bytes)
+static inline int next_entry(void *buf, struct policy_file *fp, size_t bytes)
 {
-	void *buf;
-
 	if (bytes > fp->len)
-		return NULL;
+		return -EINVAL;
 
-	buf = fp->data;
+	memcpy(buf, fp->data, bytes);
 	fp->data += bytes;
 	fp->len -= bytes;
-	return buf;
+	return 0;
 }
 
 #endif	/* _SS_POLICYDB_H_ */