diff mbox series

[RFC,02/20] selinux: avtab: avoid implicit conversions

Message ID 20230706132337.15924-2-cgzones@googlemail.com
State New
Headers
Series [RFC,01/20] selinux: check for multiplication overflow in put_entry() |

Commit Message

Christian Göttsche July 6, 2023, 1:23 p.m. UTC 
  Return u32 from avtab_hash() instead of int, since the hashing is done
on u32 and the result is used as an index on the hash array.

Use the type of the limit in for loops.

Avoid signed to unsigned conversion of multiplication result in
avtab_hash_eval().

Use unsigned loop iterator for index operations, to avoid sign
extension.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 security/selinux/ss/avtab.c | 38 ++++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 20 deletions(-)

Comments

Paul Moore July 18, 2023, 10:01 p.m. UTC | #1 
On Jul  6, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> wrote:
> 
> Return u32 from avtab_hash() instead of int, since the hashing is done
> on u32 and the result is used as an index on the hash array.
> 
> Use the type of the limit in for loops.
> 
> Avoid signed to unsigned conversion of multiplication result in
> avtab_hash_eval().
> 
> Use unsigned loop iterator for index operations, to avoid sign
> extension.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  security/selinux/ss/avtab.c | 38 ++++++++++++++++++-------------------
>  1 file changed, 18 insertions(+), 20 deletions(-)

See my previous comment about loop iterators.

--
paul-moore.com
diff mbox series

Patch

diff --git a/security/selinux/ss/avtab.c b/security/selinux/ss/avtab.c
index 6766edc0fe68..fbf51986afcf 100644
--- a/security/selinux/ss/avtab.c
+++ b/security/selinux/ss/avtab.c
@@ -29,7 +29,7 @@  static struct kmem_cache *avtab_xperms_cachep __ro_after_init;
 /* Based on MurmurHash3, written by Austin Appleby and placed in the
  * public domain.
  */
-static inline int avtab_hash(const struct avtab_key *keyp, u32 mask)
+static inline u32 avtab_hash(const struct avtab_key *keyp, u32 mask)
 {
 	static const u32 c1 = 0xcc9e2d51;
 	static const u32 c2 = 0x1b873593;
@@ -66,7 +66,7 @@  static inline int avtab_hash(const struct avtab_key *keyp, u32 mask)
 }
 
 static struct avtab_node*
-avtab_insert_node(struct avtab *h, int hvalue,
+avtab_insert_node(struct avtab *h, u32 hvalue,
 		  struct avtab_node *prev,
 		  const struct avtab_key *key, const struct avtab_datum *datum)
 {
@@ -106,7 +106,7 @@  avtab_insert_node(struct avtab *h, int hvalue,
 static int avtab_insert(struct avtab *h, const struct avtab_key *key,
 			const struct avtab_datum *datum)
 {
-	int hvalue;
+	u32 hvalue;
 	struct avtab_node *prev, *cur, *newnode;
 	u16 specified = key->specified & ~(AVTAB_ENABLED|AVTAB_ENABLED_OLD);
 
@@ -152,7 +152,7 @@  struct avtab_node *avtab_insert_nonunique(struct avtab *h,
 					  const struct avtab_key *key,
 					  const struct avtab_datum *datum)
 {
-	int hvalue;
+	u32 hvalue;
 	struct avtab_node *prev, *cur;
 	u16 specified = key->specified & ~(AVTAB_ENABLED|AVTAB_ENABLED_OLD);
 
@@ -182,7 +182,7 @@  struct avtab_node *avtab_insert_nonunique(struct avtab *h,
 
 struct avtab_datum *avtab_search(struct avtab *h, const struct avtab_key *key)
 {
-	int hvalue;
+	u32 hvalue;
 	struct avtab_node *cur;
 	u16 specified = key->specified & ~(AVTAB_ENABLED|AVTAB_ENABLED_OLD);
 
@@ -218,7 +218,7 @@  struct avtab_datum *avtab_search(struct avtab *h, const struct avtab_key *key)
 struct avtab_node *avtab_search_node(struct avtab *h,
 				     const struct avtab_key *key)
 {
-	int hvalue;
+	u32 hvalue;
 	struct avtab_node *cur;
 	u16 specified = key->specified & ~(AVTAB_ENABLED|AVTAB_ENABLED_OLD);
 
@@ -278,13 +278,12 @@  avtab_search_node_next(struct avtab_node *node, int specified)
 
 void avtab_destroy(struct avtab *h)
 {
-	int i;
 	struct avtab_node *cur, *temp;
 
 	if (!h)
 		return;
 
-	for (i = 0; i < h->nslot; i++) {
+	for (u32 i = 0; i < h->nslot; i++) {
 		cur = h->htable[i];
 		while (cur) {
 			temp = cur;
@@ -356,14 +355,14 @@  int avtab_alloc_dup(struct avtab *new, const struct avtab *orig)
 
 void avtab_hash_eval(struct avtab *h, const char *tag)
 {
-	int i, chain_len, slots_used, max_chain_len;
+	unsigned int chain_len, slots_used, max_chain_len;
 	unsigned long long chain2_len_sum;
 	struct avtab_node *cur;
 
 	slots_used = 0;
 	max_chain_len = 0;
 	chain2_len_sum = 0;
-	for (i = 0; i < h->nslot; i++) {
+	for (u32 i = 0; i < h->nslot; i++) {
 		cur = h->htable[i];
 		if (cur) {
 			slots_used++;
@@ -404,13 +403,13 @@  int avtab_read_item(struct avtab *a, void *fp, struct policydb *pol,
 {
 	__le16 buf16[4];
 	u16 enabled;
-	u32 items, items2, val, vers = pol->policyvers;
+	u32 items, items2, val;
 	struct avtab_key key;
 	struct avtab_datum datum;
 	struct avtab_extended_perms xperms;
 	__le32 buf32[ARRAY_SIZE(xperms.perms.p)];
-	int i, rc;
-	unsigned set;
+	int rc;
+	unsigned int set, vers = pol->policyvers;
 
 	memset(&key, 0, sizeof(struct avtab_key));
 	memset(&datum, 0, sizeof(struct avtab_datum));
@@ -470,7 +469,7 @@  int avtab_read_item(struct avtab *a, void *fp, struct policydb *pol,
 			return -EINVAL;
 		}
 
-		for (i = 0; i < ARRAY_SIZE(spec_order); i++) {
+		for (u32 i = 0; i < ARRAY_SIZE(spec_order); i++) {
 			if (val & spec_order[i]) {
 				key.specified = spec_order[i] | enabled;
 				datum.u.data = le32_to_cpu(buf32[items++]);
@@ -508,7 +507,7 @@  int avtab_read_item(struct avtab *a, void *fp, struct policydb *pol,
 	}
 
 	set = 0;
-	for (i = 0; i < ARRAY_SIZE(spec_order); i++) {
+	for (u32 i = 0; i < ARRAY_SIZE(spec_order); i++) {
 		if (key.specified & spec_order[i])
 			set++;
 	}
@@ -540,7 +539,7 @@  int avtab_read_item(struct avtab *a, void *fp, struct policydb *pol,
 			pr_err("SELinux: avtab: truncated entry\n");
 			return rc;
 		}
-		for (i = 0; i < ARRAY_SIZE(xperms.perms.p); i++)
+		for (u32 i = 0; i < ARRAY_SIZE(xperms.perms.p); i++)
 			xperms.perms.p[i] = le32_to_cpu(buf32[i]);
 		datum.u.xperms = &xperms;
 	} else {
@@ -569,7 +568,7 @@  int avtab_read(struct avtab *a, void *fp, struct policydb *pol)
 {
 	int rc;
 	__le32 buf[1];
-	u32 nel, i;
+	u32 nel;
 
 
 	rc = next_entry(buf, fp, sizeof(u32));
@@ -588,7 +587,7 @@  int avtab_read(struct avtab *a, void *fp, struct policydb *pol)
 	if (rc)
 		goto bad;
 
-	for (i = 0; i < nel; i++) {
+	for (u32 i = 0; i < nel; i++) {
 		rc = avtab_read_item(a, fp, pol, avtab_insertf, NULL);
 		if (rc) {
 			if (rc == -ENOMEM)
@@ -646,7 +645,6 @@  int avtab_write_item(struct policydb *p, const struct avtab_node *cur, void *fp)
 
 int avtab_write(struct policydb *p, struct avtab *a, void *fp)
 {
-	unsigned int i;
 	int rc = 0;
 	struct avtab_node *cur;
 	__le32 buf[1];
@@ -656,7 +654,7 @@  int avtab_write(struct policydb *p, struct avtab *a, void *fp)
 	if (rc)
 		return rc;
 
-	for (i = 0; i < a->nslot; i++) {
+	for (u32 i = 0; i < a->nslot; i++) {
 		for (cur = a->htable[i]; cur;
 		     cur = cur->next) {
 			rc = avtab_write_item(p, cur, fp);