mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [RFC PATCH 01/20] selinux: check for multiplication overflow in put_entry()
@ 2023-07-06 13:23 Christian Göttsche
  2023-07-06 13:23 ` [RFC PATCH 02/20] selinux: avtab: avoid implicit conversions Christian Göttsche
                   ` (19 more replies)
  0 siblings, 20 replies; 42+ messages in thread
From: Christian Göttsche @ 2023-07-06 13:23 UTC (permalink / raw)
  To: selinux; +Cc: Paul Moore, Stephen Smalley, Eric Paris, linux-kernel

The function is always inlined and most of the time both relevant
arguments are compile time constants, allowing compilers to elide the
check.  Also the function is part of outputting the policy, which is not
performance critical.

Also convert the type of the third parameter into a size_t, since it
should always be a non-negative number of elements.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 security/selinux/ss/policydb.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
index 74b63ed1173f..6b4ad8e91265 100644
--- a/security/selinux/ss/policydb.h
+++ b/security/selinux/ss/policydb.h
@@ -366,9 +366,12 @@ static inline int next_entry(void *buf, struct policy_file *fp, size_t bytes)
 	return 0;
 }
 
-static inline int put_entry(const void *buf, size_t bytes, int num, struct policy_file *fp)
+static inline int put_entry(const void *buf, size_t bytes, size_t num, struct policy_file *fp)
 {
-	size_t len = bytes * num;
+	size_t len;
+
+	if (unlikely(check_mul_overflow(bytes, num, &len)))
+		return -EINVAL;
 
 	if (len > fp->len)
 		return -EINVAL;
-- 
2.40.1


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC PATCH 02/20] selinux: avtab: avoid implicit conversions
  2023-07-06 13:23 [RFC PATCH 01/20] selinux: check for multiplication overflow in put_entry() Christian Göttsche
@ 2023-07-06 13:23 ` Christian Göttsche
  2023-07-18 22:01   ` [PATCH RFC 2/20] " Paul Moore
  2023-07-06 13:23 ` [RFC PATCH 03/20] selinux: avoid avtab overflows Christian Göttsche
                   ` (18 subsequent siblings)
  19 siblings, 1 reply; 42+ messages in thread
From: Christian Göttsche @ 2023-07-06 13:23 UTC (permalink / raw)
  To: selinux; +Cc: Paul Moore, Stephen Smalley, Eric Paris, linux-kernel

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(-)

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);
-- 
2.40.1


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC PATCH 03/20] selinux: avoid avtab overflows
  2023-07-06 13:23 [RFC PATCH 01/20] selinux: check for multiplication overflow in put_entry() Christian Göttsche
  2023-07-06 13:23 ` [RFC PATCH 02/20] selinux: avtab: avoid implicit conversions Christian Göttsche
@ 2023-07-06 13:23 ` Christian Göttsche
  2023-07-18 22:01   ` [PATCH RFC 3/20] " Paul Moore
  2023-07-06 13:23 ` [RFC PATCH 04/20] selinux: ebitmap: use u32 as bit type Christian Göttsche
                   ` (17 subsequent siblings)
  19 siblings, 1 reply; 42+ messages in thread
From: Christian Göttsche @ 2023-07-06 13:23 UTC (permalink / raw)
  To: selinux; +Cc: Paul Moore, Stephen Smalley, Eric Paris, linux-kernel

Prevent inserting more than the supported U32_MAX number of entries.

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

diff --git a/security/selinux/ss/avtab.c b/security/selinux/ss/avtab.c
index fbf51986afcf..9c150fba3fa6 100644
--- a/security/selinux/ss/avtab.c
+++ b/security/selinux/ss/avtab.c
@@ -110,7 +110,7 @@ static int avtab_insert(struct avtab *h, const struct avtab_key *key,
 	struct avtab_node *prev, *cur, *newnode;
 	u16 specified = key->specified & ~(AVTAB_ENABLED|AVTAB_ENABLED_OLD);
 
-	if (!h || !h->nslot)
+	if (!h || !h->nslot || h->nel == U32_MAX)
 		return -EINVAL;
 
 	hvalue = avtab_hash(key, h->mask);
@@ -156,7 +156,7 @@ struct avtab_node *avtab_insert_nonunique(struct avtab *h,
 	struct avtab_node *prev, *cur;
 	u16 specified = key->specified & ~(AVTAB_ENABLED|AVTAB_ENABLED_OLD);
 
-	if (!h || !h->nslot)
+	if (!h || !h->nslot || h->nel == U32_MAX)
 		return NULL;
 	hvalue = avtab_hash(key, h->mask);
 	for (prev = NULL, cur = h->htable[hvalue];
-- 
2.40.1


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC PATCH 04/20] selinux: ebitmap: use u32 as bit type
  2023-07-06 13:23 [RFC PATCH 01/20] selinux: check for multiplication overflow in put_entry() Christian Göttsche
  2023-07-06 13:23 ` [RFC PATCH 02/20] selinux: avtab: avoid implicit conversions Christian Göttsche
  2023-07-06 13:23 ` [RFC PATCH 03/20] selinux: avoid avtab overflows Christian Göttsche
@ 2023-07-06 13:23 ` Christian Göttsche
  2023-07-18 22:01   ` [PATCH RFC 4/20] " Paul Moore
  2023-07-06 13:23 ` [RFC PATCH 05/20] selinux: hashtab: use identical iterator type Christian Göttsche
                   ` (16 subsequent siblings)
  19 siblings, 1 reply; 42+ messages in thread
From: Christian Göttsche @ 2023-07-06 13:23 UTC (permalink / raw)
  To: selinux; +Cc: Paul Moore, Stephen Smalley, Eric Paris, linux-kernel

The extensible bitmap supports bit positions up to U32_MAX due to the
type of the member highbit being u32.  Use u32 consistently as the type
for bit positions to announce to callers what range of values is
supported.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 security/selinux/ss/ebitmap.c | 32 ++++++++++++++++----------------
 security/selinux/ss/ebitmap.h | 32 ++++++++++++++++----------------
 2 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/security/selinux/ss/ebitmap.c b/security/selinux/ss/ebitmap.c
index d31b87be9a1e..17d2d9b0d444 100644
--- a/security/selinux/ss/ebitmap.c
+++ b/security/selinux/ss/ebitmap.c
@@ -24,7 +24,7 @@
 #include "ebitmap.h"
 #include "policydb.h"
 
-#define BITS_PER_U64	(sizeof(u64) * 8)
+#define BITS_PER_U64	((u32)(sizeof(u64) * 8))
 
 static struct kmem_cache *ebitmap_node_cachep __ro_after_init;
 
@@ -82,7 +82,8 @@ int ebitmap_cpy(struct ebitmap *dst, const struct ebitmap *src)
 int ebitmap_and(struct ebitmap *dst, const struct ebitmap *e1, const struct ebitmap *e2)
 {
 	struct ebitmap_node *n;
-	int bit, rc;
+	u32 bit;
+	int rc;
 
 	ebitmap_init(dst);
 
@@ -113,8 +114,7 @@ int ebitmap_netlbl_export(struct ebitmap *ebmap,
 {
 	struct ebitmap_node *e_iter = ebmap->node;
 	unsigned long e_map;
-	u32 offset;
-	unsigned int iter;
+	u32 offset, iter;
 	int rc;
 
 	if (e_iter == NULL) {
@@ -259,7 +259,7 @@ int ebitmap_contains(const struct ebitmap *e1, const struct ebitmap *e2, u32 las
 	return 1;
 }
 
-int ebitmap_get_bit(const struct ebitmap *e, unsigned long bit)
+int ebitmap_get_bit(const struct ebitmap *e, u32 bit)
 {
 	const struct ebitmap_node *n;
 
@@ -276,7 +276,7 @@ int ebitmap_get_bit(const struct ebitmap *e, unsigned long bit)
 	return 0;
 }
 
-int ebitmap_set_bit(struct ebitmap *e, unsigned long bit, int value)
+int ebitmap_set_bit(struct ebitmap *e, u32 bit, int value)
 {
 	struct ebitmap_node *n, *prev, *new;
 
@@ -287,7 +287,7 @@ int ebitmap_set_bit(struct ebitmap *e, unsigned long bit, int value)
 			if (value) {
 				ebitmap_node_set_bit(n, bit);
 			} else {
-				unsigned int s;
+				u32 s;
 
 				ebitmap_node_clr_bit(n, bit);
 
@@ -370,7 +370,7 @@ int ebitmap_read(struct ebitmap *e, void *fp)
 	u64 map;
 	__le64 mapbits;
 	__le32 buf[3];
-	int rc, i;
+	int rc;
 
 	ebitmap_init(e);
 
@@ -384,7 +384,7 @@ int ebitmap_read(struct ebitmap *e, void *fp)
 
 	if (mapunit != BITS_PER_U64) {
 		pr_err("SELinux: ebitmap: map size %u does not "
-		       "match my size %zd (high bit was %d)\n",
+		       "match my size %d (high bit was %d)\n",
 		       mapunit, BITS_PER_U64, e->highbit);
 		goto bad;
 	}
@@ -401,7 +401,7 @@ int ebitmap_read(struct ebitmap *e, void *fp)
 	if (e->highbit && !count)
 		goto bad;
 
-	for (i = 0; i < count; i++) {
+	for (u32 i = 0; i < count; i++) {
 		rc = next_entry(&ebitmap_start, fp, sizeof(u32));
 		if (rc < 0) {
 			pr_err("SELinux: ebitmap: truncated map\n");
@@ -471,18 +471,18 @@ int ebitmap_read(struct ebitmap *e, void *fp)
 int ebitmap_write(const struct ebitmap *e, void *fp)
 {
 	struct ebitmap_node *n;
-	u32 count;
+	u32 bit, count, last_bit, last_startbit;
 	__le32 buf[3];
 	u64 map;
-	int bit, last_bit, last_startbit, rc;
+	int rc;
 
 	buf[0] = cpu_to_le32(BITS_PER_U64);
 
 	count = 0;
 	last_bit = 0;
-	last_startbit = -1;
+	last_startbit = (u32)-1;
 	ebitmap_for_each_positive_bit(e, n, bit) {
-		if (rounddown(bit, (int)BITS_PER_U64) > last_startbit) {
+		if (last_startbit == (u32)-1 || rounddown(bit, BITS_PER_U64) > last_startbit) {
 			count++;
 			last_startbit = rounddown(bit, BITS_PER_U64);
 		}
@@ -496,9 +496,9 @@ int ebitmap_write(const struct ebitmap *e, void *fp)
 		return rc;
 
 	map = 0;
-	last_startbit = INT_MIN;
+	last_startbit = (u32)-1;
 	ebitmap_for_each_positive_bit(e, n, bit) {
-		if (rounddown(bit, (int)BITS_PER_U64) > last_startbit) {
+		if (last_startbit == (u32)-1 || rounddown(bit, BITS_PER_U64) > last_startbit) {
 			__le64 buf64[1];
 
 			/* this is the very first bit */
diff --git a/security/selinux/ss/ebitmap.h b/security/selinux/ss/ebitmap.h
index e5b57dc3fc53..fab3e5bef896 100644
--- a/security/selinux/ss/ebitmap.h
+++ b/security/selinux/ss/ebitmap.h
@@ -44,10 +44,10 @@ struct ebitmap {
 
 #define ebitmap_length(e) ((e)->highbit)
 
-static inline unsigned int ebitmap_start_positive(const struct ebitmap *e,
+static inline u32 ebitmap_start_positive(const struct ebitmap *e,
 						  struct ebitmap_node **n)
 {
-	unsigned int ofs;
+	u32 ofs;
 
 	for (*n = e->node; *n; *n = (*n)->next) {
 		ofs = find_first_bit((*n)->maps, EBITMAP_SIZE);
@@ -62,11 +62,11 @@ static inline void ebitmap_init(struct ebitmap *e)
 	memset(e, 0, sizeof(*e));
 }
 
-static inline unsigned int ebitmap_next_positive(const struct ebitmap *e,
+static inline u32 ebitmap_next_positive(const struct ebitmap *e,
 						 struct ebitmap_node **n,
-						 unsigned int bit)
+						 u32 bit)
 {
-	unsigned int ofs;
+	u32 ofs;
 
 	ofs = find_next_bit((*n)->maps, EBITMAP_SIZE, bit - (*n)->startbit + 1);
 	if (ofs < EBITMAP_SIZE)
@@ -86,10 +86,10 @@ static inline unsigned int ebitmap_next_positive(const struct ebitmap *e,
 	(((bit) - (node)->startbit) % EBITMAP_UNIT_SIZE)
 
 static inline int ebitmap_node_get_bit(const struct ebitmap_node *n,
-				       unsigned int bit)
+				       u32 bit)
 {
-	unsigned int index = EBITMAP_NODE_INDEX(n, bit);
-	unsigned int ofs = EBITMAP_NODE_OFFSET(n, bit);
+	u32 index = EBITMAP_NODE_INDEX(n, bit);
+	u32 ofs = EBITMAP_NODE_OFFSET(n, bit);
 
 	BUG_ON(index >= EBITMAP_UNIT_NUMS);
 	if ((n->maps[index] & (EBITMAP_BIT << ofs)))
@@ -98,20 +98,20 @@ static inline int ebitmap_node_get_bit(const struct ebitmap_node *n,
 }
 
 static inline void ebitmap_node_set_bit(struct ebitmap_node *n,
-					unsigned int bit)
+					u32 bit)
 {
-	unsigned int index = EBITMAP_NODE_INDEX(n, bit);
-	unsigned int ofs = EBITMAP_NODE_OFFSET(n, bit);
+	u32 index = EBITMAP_NODE_INDEX(n, bit);
+	u32 ofs = EBITMAP_NODE_OFFSET(n, bit);
 
 	BUG_ON(index >= EBITMAP_UNIT_NUMS);
 	n->maps[index] |= (EBITMAP_BIT << ofs);
 }
 
 static inline void ebitmap_node_clr_bit(struct ebitmap_node *n,
-					unsigned int bit)
+					u32 bit)
 {
-	unsigned int index = EBITMAP_NODE_INDEX(n, bit);
-	unsigned int ofs = EBITMAP_NODE_OFFSET(n, bit);
+	u32 index = EBITMAP_NODE_INDEX(n, bit);
+	u32 ofs = EBITMAP_NODE_OFFSET(n, bit);
 
 	BUG_ON(index >= EBITMAP_UNIT_NUMS);
 	n->maps[index] &= ~(EBITMAP_BIT << ofs);
@@ -126,8 +126,8 @@ int ebitmap_cmp(const struct ebitmap *e1, const struct ebitmap *e2);
 int ebitmap_cpy(struct ebitmap *dst, const struct ebitmap *src);
 int ebitmap_and(struct ebitmap *dst, const struct ebitmap *e1, const struct ebitmap *e2);
 int ebitmap_contains(const struct ebitmap *e1, const struct ebitmap *e2, u32 last_e2bit);
-int ebitmap_get_bit(const struct ebitmap *e, unsigned long bit);
-int ebitmap_set_bit(struct ebitmap *e, unsigned long bit, int value);
+int ebitmap_get_bit(const struct ebitmap *e, u32 bit);
+int ebitmap_set_bit(struct ebitmap *e, u32 bit, int value);
 void ebitmap_destroy(struct ebitmap *e);
 int ebitmap_read(struct ebitmap *e, void *fp);
 int ebitmap_write(const struct ebitmap *e, void *fp);
-- 
2.40.1


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC PATCH 05/20] selinux: hashtab: use identical iterator type
  2023-07-06 13:23 [RFC PATCH 01/20] selinux: check for multiplication overflow in put_entry() Christian Göttsche
                   ` (2 preceding siblings ...)
  2023-07-06 13:23 ` [RFC PATCH 04/20] selinux: ebitmap: use u32 as bit type Christian Göttsche
@ 2023-07-06 13:23 ` Christian Göttsche
  2023-07-18 22:01   ` [PATCH RFC 5/20] " Paul Moore
  2023-07-06 13:23 ` [RFC PATCH 06/20] selinux: mls: avoid implicit conversions Christian Göttsche
                   ` (15 subsequent siblings)
  19 siblings, 1 reply; 42+ messages in thread
From: Christian Göttsche @ 2023-07-06 13:23 UTC (permalink / raw)
  To: selinux; +Cc: Paul Moore, Stephen Smalley, Eric Paris, linux-kernel

Use the identical type u32 for the loop iterator.

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

diff --git a/security/selinux/ss/hashtab.c b/security/selinux/ss/hashtab.c
index 3fb8f9026e9b..ede3cc1bd204 100644
--- a/security/selinux/ss/hashtab.c
+++ b/security/selinux/ss/hashtab.c
@@ -137,7 +137,7 @@ int hashtab_duplicate(struct hashtab *new, struct hashtab *orig,
 		void *args)
 {
 	struct hashtab_node *cur, *tmp, *tail;
-	int i, rc;
+	int rc;
 
 	memset(new, 0, sizeof(*new));
 
@@ -147,7 +147,7 @@ int hashtab_duplicate(struct hashtab *new, struct hashtab *orig,
 
 	new->size = orig->size;
 
-	for (i = 0; i < orig->size; i++) {
+	for (u32 i = 0; i < orig->size; i++) {
 		tail = NULL;
 		for (cur = orig->htable[i]; cur; cur = cur->next) {
 			tmp = kmem_cache_zalloc(hashtab_node_cachep,
@@ -172,7 +172,7 @@ int hashtab_duplicate(struct hashtab *new, struct hashtab *orig,
 	return 0;
 
  error:
-	for (i = 0; i < new->size; i++) {
+	for (u32 i = 0; i < new->size; i++) {
 		for (cur = new->htable[i]; cur; cur = tmp) {
 			tmp = cur->next;
 			destroy(cur->key, cur->datum, args);
-- 
2.40.1


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC PATCH 06/20] selinux: mls: avoid implicit conversions
  2023-07-06 13:23 [RFC PATCH 01/20] selinux: check for multiplication overflow in put_entry() Christian Göttsche
                   ` (3 preceding siblings ...)
  2023-07-06 13:23 ` [RFC PATCH 05/20] selinux: hashtab: use identical iterator type Christian Göttsche
@ 2023-07-06 13:23 ` Christian Göttsche
  2023-07-18 22:01   ` [PATCH RFC 6/20] " Paul Moore
  2023-07-06 13:23 ` [RFC PATCH 07/20] selinux: services: update type for umber of class permissions Christian Göttsche
                   ` (14 subsequent siblings)
  19 siblings, 1 reply; 42+ messages in thread
From: Christian Göttsche @ 2023-07-06 13:23 UTC (permalink / raw)
  To: selinux; +Cc: Paul Moore, Stephen Smalley, Eric Paris, linux-kernel

Use u32 for ebitmap bits.

Use char for the default range of a class.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 security/selinux/ss/mls.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c
index 99571b19d4a9..1976f6b857e9 100644
--- a/security/selinux/ss/mls.c
+++ b/security/selinux/ss/mls.c
@@ -45,7 +45,7 @@ int mls_compute_context_len(struct policydb *p, struct context *context)
 
 	len = 1; /* for the beginning ":" */
 	for (l = 0; l < 2; l++) {
-		int index_sens = context->range.level[l].sens;
+		u32 index_sens = context->range.level[l].sens;
 		len += strlen(sym_name(p, SYM_LEVELS, index_sens - 1));
 
 		/* categories */
@@ -240,7 +240,7 @@ int mls_context_to_sid(struct policydb *pol,
 	char *sensitivity, *cur_cat, *next_cat, *rngptr;
 	struct level_datum *levdatum;
 	struct cat_datum *catdatum, *rngdatum;
-	int l, rc, i;
+	int l, rc;
 	char *rangep[2];
 
 	if (!pol->mls_enabled) {
@@ -331,7 +331,7 @@ int mls_context_to_sid(struct policydb *pol,
 			if (catdatum->value >= rngdatum->value)
 				return -EINVAL;
 
-			for (i = catdatum->value; i < rngdatum->value; i++) {
+			for (u32 i = catdatum->value; i < rngdatum->value; i++) {
 				rc = ebitmap_set_bit(&context->range.level[l].cat, i, 1);
 				if (rc)
 					return rc;
@@ -451,7 +451,8 @@ int mls_convert_context(struct policydb *oldp,
 	struct level_datum *levdatum;
 	struct cat_datum *catdatum;
 	struct ebitmap_node *node;
-	int l, i;
+	u32 i;
+	int l;
 
 	if (!oldp->mls_enabled || !newp->mls_enabled)
 		return 0;
@@ -495,7 +496,7 @@ int mls_compute_sid(struct policydb *p,
 	struct range_trans rtr;
 	struct mls_range *r;
 	struct class_datum *cladatum;
-	int default_range = 0;
+	char default_range = 0;
 
 	if (!p->mls_enabled)
 		return 0;
-- 
2.40.1


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC PATCH 07/20] selinux: services: update type for umber of class permissions
  2023-07-06 13:23 [RFC PATCH 01/20] selinux: check for multiplication overflow in put_entry() Christian Göttsche
                   ` (4 preceding siblings ...)
  2023-07-06 13:23 ` [RFC PATCH 06/20] selinux: mls: avoid implicit conversions Christian Göttsche
@ 2023-07-06 13:23 ` Christian Göttsche
  2023-07-07  2:27   ` Gong Ruiqi
  2023-07-18 22:01   ` [PATCH RFC 7/20] " Paul Moore
  2023-07-06 13:23 ` [RFC PATCH 08/20] selinux: services: avoid implicit conversions Christian Göttsche
                   ` (13 subsequent siblings)
  19 siblings, 2 replies; 42+ messages in thread
From: Christian Göttsche @ 2023-07-06 13:23 UTC (permalink / raw)
  To: selinux
  Cc: Paul Moore, Stephen Smalley, Eric Paris, Ondrej Mosnacek, GONG,
	Ruiqi, linux-kernel

Security classes have only up to 32 permissions, hence using an u16 is
sufficient (while improving padding).

Also use a fixed sized cast in a bit shift to work correctly on
architectures where sizeof(unsigned int) != sizeof(u32).

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 security/selinux/ss/services.c | 6 +++---
 security/selinux/ss/services.h | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 78946b71c1c1..3275cfe2c8f7 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -97,7 +97,6 @@ static int selinux_set_mapping(struct policydb *pol,
 			       struct selinux_map *out_map)
 {
 	u16 i, j;
-	unsigned k;
 	bool print_unknown_handle = false;
 
 	/* Find number of classes in the input mapping */
@@ -117,6 +116,7 @@ static int selinux_set_mapping(struct policydb *pol,
 	while (map[j].name) {
 		const struct security_class_mapping *p_in = map + (j++);
 		struct selinux_mapping *p_out = out_map->mapping + j;
+		u16 k;
 
 		/* An empty class string skips ahead */
 		if (!strcmp(p_in->name, "")) {
@@ -202,7 +202,7 @@ static void map_decision(struct selinux_map *map,
 {
 	if (tclass < map->size) {
 		struct selinux_mapping *mapping = &map->mapping[tclass];
-		unsigned int i, n = mapping->num_perms;
+		u16 i, n = mapping->num_perms;
 		u32 result;
 
 		for (i = 0, result = 0; i < n; i++) {
@@ -230,7 +230,7 @@ static void map_decision(struct selinux_map *map,
 		 * should audit that denial
 		 */
 		for (; i < (sizeof(u32)*8); i++)
-			result |= 1<<i;
+			result |= 1<<((u32)i);
 		avd->auditdeny = result;
 	}
 }
diff --git a/security/selinux/ss/services.h b/security/selinux/ss/services.h
index 8a9b85f44b66..b6f99353301e 100644
--- a/security/selinux/ss/services.h
+++ b/security/selinux/ss/services.h
@@ -12,7 +12,7 @@
 /* Mapping for a single class */
 struct selinux_mapping {
 	u16 value; /* policy value for class */
-	unsigned int num_perms; /* number of permissions in class */
+	u16 num_perms; /* number of permissions in class */
 	u32 perms[sizeof(u32) * 8]; /* policy values for permissions */
 };
 
-- 
2.40.1


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC PATCH 08/20] selinux: services: avoid implicit conversions
  2023-07-06 13:23 [RFC PATCH 01/20] selinux: check for multiplication overflow in put_entry() Christian Göttsche
                   ` (5 preceding siblings ...)
  2023-07-06 13:23 ` [RFC PATCH 07/20] selinux: services: update type for umber of class permissions Christian Göttsche
@ 2023-07-06 13:23 ` Christian Göttsche
  2023-07-18 22:01   ` [PATCH RFC 8/20] " Paul Moore
  2023-07-06 13:23 ` [RFC PATCH 09/20] selinux: status: consistently use u32 as sequence number type Christian Göttsche
                   ` (12 subsequent siblings)
  19 siblings, 1 reply; 42+ messages in thread
From: Christian Göttsche @ 2023-07-06 13:23 UTC (permalink / raw)
  To: selinux
  Cc: Paul Moore, Stephen Smalley, Eric Paris, Ondrej Mosnacek,
	Casey Schaufler, Xiu Jianfeng, GONG, Ruiqi, linux-kernel

Use u32 as the output parameter type in security_get_classes() and
security_get_permissions(), based on the type of the symtab nprim
member.

Declare the read-only class string parameter of
security_get_permissions() const.

Avoid several implicit conversions by using the identical type for the
destination.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 security/selinux/include/security.h |  4 ++--
 security/selinux/selinuxfs.c        |  7 ++++---
 security/selinux/ss/services.c      | 22 +++++++++-------------
 3 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index 665c4e5bae99..0f93fd019bb4 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -312,9 +312,9 @@ int security_net_peersid_resolve(u32 nlbl_sid, u32 nlbl_type,
 				 u32 *peer_sid);
 
 int security_get_classes(struct selinux_policy *policy,
-			 char ***classes, int *nclasses);
+			 char ***classes, u32 *nclasses);
 int security_get_permissions(struct selinux_policy *policy,
-			     char *class, char ***perms, int *nperms);
+			     const char *class, char ***perms, u32 *nperms);
 int security_get_reject_unknown(void);
 int security_get_allow_unknown(void);
 
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index bad1f6b685fd..16036633ddd3 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -1797,7 +1797,8 @@ static int sel_make_perm_files(struct selinux_policy *newpolicy,
 			char *objclass, int classvalue,
 			struct dentry *dir)
 {
-	int i, rc, nperms;
+	u32 i, nperms;
+	int rc;
 	char **perms;
 
 	rc = security_get_permissions(newpolicy, objclass, &perms, &nperms);
@@ -1867,8 +1868,8 @@ static int sel_make_classes(struct selinux_policy *newpolicy,
 			    struct dentry *class_dir,
 			    unsigned long *last_class_ino)
 {
-
-	int rc, nclasses, i;
+	u32 i, nclasses;
+	int rc;
 	char **classes;
 
 	rc = security_get_classes(newpolicy, &classes, &nclasses);
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 3275cfe2c8f7..2e2b17b00298 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -2822,7 +2822,6 @@ static inline int __security_genfs_sid(struct selinux_policy *policy,
 {
 	struct policydb *policydb = &policy->policydb;
 	struct sidtab *sidtab = policy->sidtab;
-	int len;
 	u16 sclass;
 	struct genfs *genfs;
 	struct ocontext *c;
@@ -2844,7 +2843,7 @@ static inline int __security_genfs_sid(struct selinux_policy *policy,
 		return -ENOENT;
 
 	for (c = genfs->head; c; c = c->next) {
-		len = strlen(c->u.name);
+		size_t len = strlen(c->u.name);
 		if ((!c->v.sclass || sclass == c->v.sclass) &&
 		    (strncmp(c->u.name, path, len) == 0))
 			break;
@@ -3332,7 +3331,7 @@ static int get_classes_callback(void *k, void *d, void *args)
 {
 	struct class_datum *datum = d;
 	char *name = k, **classes = args;
-	int value = datum->value - 1;
+	u32 value = datum->value - 1;
 
 	classes[value] = kstrdup(name, GFP_ATOMIC);
 	if (!classes[value])
@@ -3342,7 +3341,7 @@ static int get_classes_callback(void *k, void *d, void *args)
 }
 
 int security_get_classes(struct selinux_policy *policy,
-			 char ***classes, int *nclasses)
+			 char ***classes, u32 *nclasses)
 {
 	struct policydb *policydb;
 	int rc;
@@ -3358,8 +3357,7 @@ int security_get_classes(struct selinux_policy *policy,
 	rc = hashtab_map(&policydb->p_classes.table, get_classes_callback,
 			 *classes);
 	if (rc) {
-		int i;
-		for (i = 0; i < *nclasses; i++)
+		for (u32 i = 0; i < *nclasses; i++)
 			kfree((*classes)[i]);
 		kfree(*classes);
 	}
@@ -3372,7 +3370,7 @@ static int get_permissions_callback(void *k, void *d, void *args)
 {
 	struct perm_datum *datum = d;
 	char *name = k, **perms = args;
-	int value = datum->value - 1;
+	u32 value = datum->value - 1;
 
 	perms[value] = kstrdup(name, GFP_ATOMIC);
 	if (!perms[value])
@@ -3382,10 +3380,10 @@ static int get_permissions_callback(void *k, void *d, void *args)
 }
 
 int security_get_permissions(struct selinux_policy *policy,
-			     char *class, char ***perms, int *nperms)
+			     const char *class, char ***perms, u32 *nperms)
 {
 	struct policydb *policydb;
-	int rc, i;
+	int rc;
 	struct class_datum *match;
 
 	policydb = &policy->policydb;
@@ -3420,7 +3418,7 @@ int security_get_permissions(struct selinux_policy *policy,
 	return rc;
 
 err:
-	for (i = 0; i < *nperms; i++)
+	for (u32 i = 0; i < *nperms; i++)
 		kfree((*perms)[i]);
 	kfree(*perms);
 	return rc;
@@ -3600,9 +3598,7 @@ int selinux_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule)
 /* Check to see if the rule contains any selinux fields */
 int selinux_audit_rule_known(struct audit_krule *rule)
 {
-	int i;
-
-	for (i = 0; i < rule->field_count; i++) {
+	for (u32 i = 0; i < rule->field_count; i++) {
 		struct audit_field *f = &rule->fields[i];
 		switch (f->type) {
 		case AUDIT_SUBJ_USER:
-- 
2.40.1


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC PATCH 09/20] selinux: status: consistently use u32 as sequence number type
  2023-07-06 13:23 [RFC PATCH 01/20] selinux: check for multiplication overflow in put_entry() Christian Göttsche
                   ` (6 preceding siblings ...)
  2023-07-06 13:23 ` [RFC PATCH 08/20] selinux: services: avoid implicit conversions Christian Göttsche
@ 2023-07-06 13:23 ` Christian Göttsche
  2023-07-18 22:01   ` [PATCH RFC 9/20] " Paul Moore
  2023-07-06 13:23 ` [RFC PATCH 10/20] selinux: netif: avoid implicit conversions Christian Göttsche
                   ` (11 subsequent siblings)
  19 siblings, 1 reply; 42+ messages in thread
From: Christian Göttsche @ 2023-07-06 13:23 UTC (permalink / raw)
  To: selinux
  Cc: Paul Moore, Stephen Smalley, Eric Paris, Ondrej Mosnacek,
	Xiu Jianfeng, linux-kernel

Align the type with the one used in selinux_notify_policy_change() and
the sequence member of struct selinux_kernel_status.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 security/selinux/include/security.h | 2 +-
 security/selinux/status.c           | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index 0f93fd019bb4..a16c52d553e1 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -376,7 +376,7 @@ struct selinux_kernel_status {
 } __packed;
 
 extern void selinux_status_update_setenforce(int enforcing);
-extern void selinux_status_update_policyload(int seqno);
+extern void selinux_status_update_policyload(u32 seqno);
 extern void selinux_complete_init(void);
 extern struct path selinux_null;
 extern void selnl_notify_setenforce(int val);
diff --git a/security/selinux/status.c b/security/selinux/status.c
index 19ef929a075c..e436e4975adc 100644
--- a/security/selinux/status.c
+++ b/security/selinux/status.c
@@ -101,7 +101,7 @@ void selinux_status_update_setenforce(int enforcing)
  * It updates status of the times of policy reloaded, and current
  * setting of deny_unknown.
  */
-void selinux_status_update_policyload(int seqno)
+void selinux_status_update_policyload(u32 seqno)
 {
 	struct selinux_kernel_status   *status;
 
-- 
2.40.1


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC PATCH 10/20] selinux: netif: avoid implicit conversions
  2023-07-06 13:23 [RFC PATCH 01/20] selinux: check for multiplication overflow in put_entry() Christian Göttsche
                   ` (7 preceding siblings ...)
  2023-07-06 13:23 ` [RFC PATCH 09/20] selinux: status: consistently use u32 as sequence number type Christian Göttsche
@ 2023-07-06 13:23 ` Christian Göttsche
  2023-07-18 22:01   ` [PATCH RFC " Paul Moore
  2023-07-06 13:23 ` [RFC PATCH 11/20] selinux: avc: " Christian Göttsche
                   ` (10 subsequent siblings)
  19 siblings, 1 reply; 42+ messages in thread
From: Christian Göttsche @ 2023-07-06 13:23 UTC (permalink / raw)
  To: selinux; +Cc: Paul Moore, Stephen Smalley, Eric Paris, linux-kernel

Use the identical type sel_netif_hashfn() returns.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 security/selinux/netif.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/security/selinux/netif.c b/security/selinux/netif.c
index adbe9bea2d26..43a0d3594b72 100644
--- a/security/selinux/netif.c
+++ b/security/selinux/netif.c
@@ -67,7 +67,7 @@ static inline u32 sel_netif_hashfn(const struct net *ns, int ifindex)
 static inline struct sel_netif *sel_netif_find(const struct net *ns,
 					       int ifindex)
 {
-	int idx = sel_netif_hashfn(ns, ifindex);
+	u32 idx = sel_netif_hashfn(ns, ifindex);
 	struct sel_netif *netif;
 
 	list_for_each_entry_rcu(netif, &sel_netif_hash[idx], list)
@@ -89,7 +89,7 @@ static inline struct sel_netif *sel_netif_find(const struct net *ns,
  */
 static int sel_netif_insert(struct sel_netif *netif)
 {
-	int idx;
+	u32 idx;
 
 	if (sel_netif_total >= SEL_NETIF_HASH_MAX)
 		return -ENOSPC;
-- 
2.40.1


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC PATCH 11/20] selinux: avc: avoid implicit conversions
  2023-07-06 13:23 [RFC PATCH 01/20] selinux: check for multiplication overflow in put_entry() Christian Göttsche
                   ` (8 preceding siblings ...)
  2023-07-06 13:23 ` [RFC PATCH 10/20] selinux: netif: avoid implicit conversions Christian Göttsche
@ 2023-07-06 13:23 ` Christian Göttsche
  2023-07-18 22:01   ` [PATCH RFC " Paul Moore
  2023-07-06 13:23 ` [RFC PATCH 12/20] selinux: hooks: " Christian Göttsche
                   ` (9 subsequent siblings)
  19 siblings, 1 reply; 42+ messages in thread
From: Christian Göttsche @ 2023-07-06 13:23 UTC (permalink / raw)
  To: selinux; +Cc: Paul Moore, Stephen Smalley, Eric Paris, linux-kernel

Use a consistent type of u32 for sequence numbers.

Use a non-negative and input parameter matching type for the hash
result.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 security/selinux/avc.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/security/selinux/avc.c b/security/selinux/avc.c
index 1074db66e5ff..cd55479cce25 100644
--- a/security/selinux/avc.c
+++ b/security/selinux/avc.c
@@ -122,7 +122,7 @@ static struct kmem_cache *avc_xperms_data_cachep __ro_after_init;
 static struct kmem_cache *avc_xperms_decision_cachep __ro_after_init;
 static struct kmem_cache *avc_xperms_cachep __ro_after_init;
 
-static inline int avc_hash(u32 ssid, u32 tsid, u16 tclass)
+static inline u32 avc_hash(u32 ssid, u32 tsid, u16 tclass)
 {
 	return (ssid ^ (tsid<<2) ^ (tclass<<4)) & (AVC_CACHE_SLOTS - 1);
 }
@@ -523,7 +523,7 @@ static void avc_node_populate(struct avc_node *node, u32 ssid, u32 tsid, u16 tcl
 static inline struct avc_node *avc_search_node(u32 ssid, u32 tsid, u16 tclass)
 {
 	struct avc_node *node, *ret = NULL;
-	int hvalue;
+	u32 hvalue;
 	struct hlist_head *head;
 
 	hvalue = avc_hash(ssid, tsid, tclass);
@@ -566,7 +566,7 @@ static struct avc_node *avc_lookup(u32 ssid, u32 tsid, u16 tclass)
 	return NULL;
 }
 
-static int avc_latest_notif_update(int seqno, int is_insert)
+static int avc_latest_notif_update(u32 seqno, int is_insert)
 {
 	int ret = 0;
 	static DEFINE_SPINLOCK(notif_lock);
@@ -609,7 +609,7 @@ static void avc_insert(u32 ssid, u32 tsid, u16 tclass,
 		       struct av_decision *avd, struct avc_xperms_node *xp_node)
 {
 	struct avc_node *pos, *node = NULL;
-	int hvalue;
+	u32 hvalue;
 	unsigned long flag;
 	spinlock_t *lock;
 	struct hlist_head *head;
@@ -654,9 +654,9 @@ static void avc_audit_pre_callback(struct audit_buffer *ab, void *a)
 {
 	struct common_audit_data *ad = a;
 	struct selinux_audit_data *sad = ad->selinux_audit_data;
-	u32 av = sad->audited;
+	u32 av = sad->audited, perm;
 	const char *const *perms;
-	int i, perm;
+	u32 i;
 
 	audit_log_format(ab, "avc:  %s ", sad->denied ? "denied" : "granted");
 
@@ -833,7 +833,8 @@ static int avc_update_node(u32 event, u32 perms, u8 driver, u8 xperm, u32 ssid,
 			   struct extended_perms_decision *xpd,
 			   u32 flags)
 {
-	int hvalue, rc = 0;
+	u32 hvalue;
+	int rc = 0;
 	unsigned long flag;
 	struct avc_node *pos, *node, *orig = NULL;
 	struct hlist_head *head;
-- 
2.40.1


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC PATCH 12/20] selinux: hooks: avoid implicit conversions
  2023-07-06 13:23 [RFC PATCH 01/20] selinux: check for multiplication overflow in put_entry() Christian Göttsche
                   ` (9 preceding siblings ...)
  2023-07-06 13:23 ` [RFC PATCH 11/20] selinux: avc: " Christian Göttsche
@ 2023-07-06 13:23 ` Christian Göttsche
  2023-07-18 22:01   ` [PATCH RFC " Paul Moore
  2023-07-06 13:23 ` [RFC PATCH 13/20] selinux: selinuxfs: " Christian Göttsche
                   ` (8 subsequent siblings)
  19 siblings, 1 reply; 42+ messages in thread
From: Christian Göttsche @ 2023-07-06 13:23 UTC (permalink / raw)
  To: selinux; +Cc: Paul Moore, Stephen Smalley, Eric Paris, linux-kernel

Use the identical types in assignments of local variables for the
destination.

Merge tail calls into return statements.

Avoid using leading underscores for function local variable.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 security/selinux/hooks.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index b8a8a4f0f2ad..fff50604abce 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1125,7 +1125,7 @@ static inline int default_protocol_dgram(int protocol)
 
 static inline u16 socket_type_to_security_class(int family, int type, int protocol)
 {
-	int extsockclass = selinux_policycap_extsockclass();
+	bool extsockclass = selinux_policycap_extsockclass();
 
 	switch (family) {
 	case PF_UNIX:
@@ -5027,15 +5027,13 @@ static int selinux_sock_rcv_skb_compat(struct sock *sk, struct sk_buff *skb,
 
 static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
 {
-	int err;
+	int err, peerlbl_active, secmark_active;
 	struct sk_security_struct *sksec = sk->sk_security;
 	u16 family = sk->sk_family;
 	u32 sk_sid = sksec->sid;
 	struct common_audit_data ad;
 	struct lsm_network_audit net = {0,};
 	char *addrp;
-	u8 secmark_active;
-	u8 peerlbl_active;
 
 	if (family != PF_INET && family != PF_INET6)
 		return 0;
@@ -5498,11 +5496,11 @@ static void selinux_inet_conn_established(struct sock *sk, struct sk_buff *skb)
 
 static int selinux_secmark_relabel_packet(u32 sid)
 {
-	const struct task_security_struct *__tsec;
+	const struct task_security_struct *tsec;
 	u32 tsid;
 
-	__tsec = selinux_cred(current_cred());
-	tsid = __tsec->sid;
+	tsec = selinux_cred(current_cred());
+	tsid = tsec->sid;
 
 	return avc_has_perm(tsid, sid, SECCLASS_PACKET, PACKET__RELABELTO,
 			    NULL);
@@ -6000,8 +5998,7 @@ static int selinux_msg_queue_associate(struct kern_ipc_perm *msq, int msqflg)
 
 static int selinux_msg_queue_msgctl(struct kern_ipc_perm *msq, int cmd)
 {
-	int err;
-	int perms;
+	u32 perms;
 
 	switch (cmd) {
 	case IPC_INFO:
@@ -6024,8 +6021,7 @@ static int selinux_msg_queue_msgctl(struct kern_ipc_perm *msq, int cmd)
 		return 0;
 	}
 
-	err = ipc_has_perm(msq, perms);
-	return err;
+	return ipc_has_perm(msq, perms);
 }
 
 static int selinux_msg_queue_msgsnd(struct kern_ipc_perm *msq, struct msg_msg *msg, int msqflg)
@@ -6130,8 +6126,7 @@ static int selinux_shm_associate(struct kern_ipc_perm *shp, int shmflg)
 /* Note, at this point, shp is locked down */
 static int selinux_shm_shmctl(struct kern_ipc_perm *shp, int cmd)
 {
-	int perms;
-	int err;
+	u32 perms;
 
 	switch (cmd) {
 	case IPC_INFO:
@@ -6158,8 +6153,7 @@ static int selinux_shm_shmctl(struct kern_ipc_perm *shp, int cmd)
 		return 0;
 	}
 
-	err = ipc_has_perm(shp, perms);
-	return err;
+	return ipc_has_perm(shp, perms);
 }
 
 static int selinux_shm_shmat(struct kern_ipc_perm *shp,
@@ -6928,7 +6922,7 @@ static int selinux_uring_override_creds(const struct cred *new)
  */
 static int selinux_uring_sqpoll(void)
 {
-	int sid = current_sid();
+	u32 sid = current_sid();
 
 	return avc_has_perm(sid, sid,
 			    SECCLASS_IO_URING, IO_URING__SQPOLL, NULL);
-- 
2.40.1


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC PATCH 13/20] selinux: selinuxfs: avoid implicit conversions
  2023-07-06 13:23 [RFC PATCH 01/20] selinux: check for multiplication overflow in put_entry() Christian Göttsche
                   ` (10 preceding siblings ...)
  2023-07-06 13:23 ` [RFC PATCH 12/20] selinux: hooks: " Christian Göttsche
@ 2023-07-06 13:23 ` Christian Göttsche
  2023-07-18 22:01   ` [PATCH RFC " Paul Moore
  2023-07-06 13:23 ` [RFC PATCH 14/20] selinux: use consistent type for AV rule specifier Christian Göttsche
                   ` (7 subsequent siblings)
  19 siblings, 1 reply; 42+ messages in thread
From: Christian Göttsche @ 2023-07-06 13:23 UTC (permalink / raw)
  To: selinux; +Cc: Paul Moore, Stephen Smalley, Eric Paris, linux-kernel

Use umode_t as parameter type for sel_make_inode(), which assigns the
value to the member i_mode of struct inode.

Use identical type for loop iterator.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 security/selinux/selinuxfs.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index 16036633ddd3..c3ac0468f698 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -97,10 +97,9 @@ static int selinux_fs_info_create(struct super_block *sb)
 static void selinux_fs_info_free(struct super_block *sb)
 {
 	struct selinux_fs_info *fsi = sb->s_fs_info;
-	int i;
 
 	if (fsi) {
-		for (i = 0; i < fsi->bool_num; i++)
+		for (unsigned int i = 0; i < fsi->bool_num; i++)
 			kfree(fsi->bool_pending_names[i]);
 		kfree(fsi->bool_pending_names);
 		kfree(fsi->bool_pending_values);
@@ -1191,7 +1190,7 @@ static ssize_t sel_write_member(struct file *file, char *buf, size_t size)
 	return length;
 }
 
-static struct inode *sel_make_inode(struct super_block *sb, int mode)
+static struct inode *sel_make_inode(struct super_block *sb, umode_t mode)
 {
 	struct inode *ret = new_inode(sb);
 
-- 
2.40.1


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC PATCH 14/20] selinux: use consistent type for AV rule specifier
  2023-07-06 13:23 [RFC PATCH 01/20] selinux: check for multiplication overflow in put_entry() Christian Göttsche
                   ` (11 preceding siblings ...)
  2023-07-06 13:23 ` [RFC PATCH 13/20] selinux: selinuxfs: " Christian Göttsche
@ 2023-07-06 13:23 ` Christian Göttsche
  2023-07-18 22:01   ` [PATCH RFC " Paul Moore
  2023-07-06 13:23 ` [RFC PATCH 15/20] selinux: policydb: implicit conversions Christian Göttsche
                   ` (6 subsequent siblings)
  19 siblings, 1 reply; 42+ messages in thread
From: Christian Göttsche @ 2023-07-06 13:23 UTC (permalink / raw)
  To: selinux
  Cc: Paul Moore, Stephen Smalley, Eric Paris, Ondrej Mosnacek, GONG,
	Ruiqi, linux-kernel

The specifier for avtab keys is always supplied with a type of u16,
either as a macro to security_compute_sid() or the member specified of
the struct avtab_key.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 security/selinux/ss/avtab.c    | 2 +-
 security/selinux/ss/avtab.h    | 2 +-
 security/selinux/ss/services.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/security/selinux/ss/avtab.c b/security/selinux/ss/avtab.c
index 9c150fba3fa6..15a5d60fb1a5 100644
--- a/security/selinux/ss/avtab.c
+++ b/security/selinux/ss/avtab.c
@@ -248,7 +248,7 @@ struct avtab_node *avtab_search_node(struct avtab *h,
 }
 
 struct avtab_node*
-avtab_search_node_next(struct avtab_node *node, int specified)
+avtab_search_node_next(struct avtab_node *node, u16 specified)
 {
 	struct avtab_node *cur;
 
diff --git a/security/selinux/ss/avtab.h b/security/selinux/ss/avtab.h
index d6742fd9c560..f265e9da18e2 100644
--- a/security/selinux/ss/avtab.h
+++ b/security/selinux/ss/avtab.h
@@ -111,7 +111,7 @@ struct avtab_node *avtab_insert_nonunique(struct avtab *h,
 struct avtab_node *avtab_search_node(struct avtab *h,
 				     const struct avtab_key *key);
 
-struct avtab_node *avtab_search_node_next(struct avtab_node *node, int specified);
+struct avtab_node *avtab_search_node_next(struct avtab_node *node, u16 specified);
 
 #define MAX_AVTAB_HASH_BITS 16
 #define MAX_AVTAB_HASH_BUCKETS (1 << MAX_AVTAB_HASH_BITS)
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 2e2b17b00298..823b000381a4 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -1694,7 +1694,7 @@ static void filename_compute_type(struct policydb *policydb,
 static int security_compute_sid(u32 ssid,
 				u32 tsid,
 				u16 orig_tclass,
-				u32 specified,
+				u16 specified,
 				const char *objname,
 				u32 *out_sid,
 				bool kern)
-- 
2.40.1


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC PATCH 15/20] selinux: policydb: implicit conversions
  2023-07-06 13:23 [RFC PATCH 01/20] selinux: check for multiplication overflow in put_entry() Christian Göttsche
                   ` (12 preceding siblings ...)
  2023-07-06 13:23 ` [RFC PATCH 14/20] selinux: use consistent type for AV rule specifier Christian Göttsche
@ 2023-07-06 13:23 ` Christian Göttsche
  2023-07-18 22:01   ` [PATCH RFC " Paul Moore
  2023-07-06 13:23 ` [RFC PATCH 16/20] selinux: symtab: implicit conversion Christian Göttsche
                   ` (5 subsequent siblings)
  19 siblings, 1 reply; 42+ messages in thread
From: Christian Göttsche @ 2023-07-06 13:23 UTC (permalink / raw)
  To: selinux
  Cc: Paul Moore, Stephen Smalley, Eric Paris, Ondrej Mosnacek, linux-kernel

Use the identical type for local variables, e.g. loop counters.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 security/selinux/ss/policydb.c | 112 +++++++++++++++++++--------------
 1 file changed, 65 insertions(+), 47 deletions(-)

diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index cfe77ef24ee2..9d0a3dab80d5 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -161,9 +161,7 @@ static const struct policydb_compat_info policydb_compat[] = {
 
 static const struct policydb_compat_info *policydb_lookup_compat(int version)
 {
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(policydb_compat); i++) {
+	for (u32 i = 0; i < ARRAY_SIZE(policydb_compat); i++) {
 		if (policydb_compat[i].version == version)
 			return &policydb_compat[i];
 	}
@@ -359,7 +357,7 @@ static int role_tr_destroy(void *key, void *datum, void *p)
 	return 0;
 }
 
-static void ocontext_destroy(struct ocontext *c, int i)
+static void ocontext_destroy(struct ocontext *c, u32 i)
 {
 	if (!c)
 		return;
@@ -781,7 +779,7 @@ void policydb_destroy(struct policydb *p)
 {
 	struct ocontext *c, *ctmp;
 	struct genfs *g, *gtmp;
-	int i;
+	u32 i;
 	struct role_allow *ra, *lra = NULL;
 
 	for (i = 0; i < SYM_NUM; i++) {
@@ -1155,7 +1153,7 @@ static int common_read(struct policydb *p, struct symtab *s, void *fp)
 	struct common_datum *comdatum;
 	__le32 buf[4];
 	u32 len, nel;
-	int i, rc;
+	int rc;
 
 	comdatum = kzalloc(sizeof(*comdatum), GFP_KERNEL);
 	if (!comdatum)
@@ -1178,7 +1176,7 @@ static int common_read(struct policydb *p, struct symtab *s, void *fp)
 	if (rc)
 		goto bad;
 
-	for (i = 0; i < nel; i++) {
+	for (u32 i = 0; i < nel; i++) {
 		rc = perm_read(p, &comdatum->permissions, fp);
 		if (rc)
 			goto bad;
@@ -1220,16 +1218,16 @@ static int type_set_read(struct type_set *t, void *fp)
 
 static int read_cons_helper(struct policydb *p,
 				struct constraint_node **nodep,
-				int ncons, int allowxtarget, void *fp)
+				u32 ncons, int allowxtarget, void *fp)
 {
 	struct constraint_node *c, *lc;
 	struct constraint_expr *e, *le;
 	__le32 buf[3];
 	u32 nexpr;
-	int rc, i, j, depth;
+	int rc, depth;
 
 	lc = NULL;
-	for (i = 0; i < ncons; i++) {
+	for (u32 i = 0; i < ncons; i++) {
 		c = kzalloc(sizeof(*c), GFP_KERNEL);
 		if (!c)
 			return -ENOMEM;
@@ -1246,7 +1244,7 @@ static int read_cons_helper(struct policydb *p,
 		nexpr = le32_to_cpu(buf[1]);
 		le = NULL;
 		depth = -1;
-		for (j = 0; j < nexpr; j++) {
+		for (u32 j = 0; j < nexpr; j++) {
 			e = kzalloc(sizeof(*e), GFP_KERNEL);
 			if (!e)
 				return -ENOMEM;
@@ -1319,7 +1317,7 @@ static int class_read(struct policydb *p, struct symtab *s, void *fp)
 	struct class_datum *cladatum;
 	__le32 buf[6];
 	u32 len, len2, ncons, nel;
-	int i, rc;
+	int rc;
 
 	cladatum = kzalloc(sizeof(*cladatum), GFP_KERNEL);
 	if (!cladatum)
@@ -1359,7 +1357,7 @@ static int class_read(struct policydb *p, struct symtab *s, void *fp)
 			goto bad;
 		}
 	}
-	for (i = 0; i < nel; i++) {
+	for (u32 i = 0; i < nel; i++) {
 		rc = perm_read(p, &cladatum->permissions, fp);
 		if (rc)
 			goto bad;
@@ -1412,7 +1410,8 @@ static int role_read(struct policydb *p, struct symtab *s, void *fp)
 {
 	char *key = NULL;
 	struct role_datum *role;
-	int rc, to_read = 2;
+	int rc;
+	unsigned int to_read = 2;
 	__le32 buf[3];
 	u32 len;
 
@@ -1468,7 +1467,8 @@ static int type_read(struct policydb *p, struct symtab *s, void *fp)
 {
 	char *key = NULL;
 	struct type_datum *typdatum;
-	int rc, to_read = 3;
+	int rc;
+	unsigned int to_read = 3;
 	__le32 buf[4];
 	u32 len;
 
@@ -1542,7 +1542,8 @@ static int user_read(struct policydb *p, struct symtab *s, void *fp)
 {
 	char *key = NULL;
 	struct user_datum *usrdatum;
-	int rc, to_read = 2;
+	int rc;
+	unsigned int to_read = 2;
 	__le32 buf[3];
 	u32 len;
 
@@ -1683,7 +1684,7 @@ static int user_bounds_sanity_check(void *key, void *datum, void *datap)
 	upper = user = datum;
 	while (upper->bounds) {
 		struct ebitmap_node *node;
-		unsigned long bit;
+		u32 bit;
 
 		if (++depth == POLICYDB_BOUNDS_MAXDEPTH) {
 			pr_err("SELinux: user %s: "
@@ -1719,7 +1720,7 @@ static int role_bounds_sanity_check(void *key, void *datum, void *datap)
 	upper = role = datum;
 	while (upper->bounds) {
 		struct ebitmap_node *node;
-		unsigned long bit;
+		u32 bit;
 
 		if (++depth == POLICYDB_BOUNDS_MAXDEPTH) {
 			pr_err("SELinux: role %s: "
@@ -1834,7 +1835,7 @@ static int range_read(struct policydb *p, void *fp)
 {
 	struct range_trans *rt = NULL;
 	struct mls_range *r = NULL;
-	int i, rc;
+	int rc;
 	__le32 buf[2];
 	u32 nel;
 
@@ -1851,7 +1852,7 @@ static int range_read(struct policydb *p, void *fp)
 	if (rc)
 		return rc;
 
-	for (i = 0; i < nel; i++) {
+	for (u32 i = 0; i < nel; i++) {
 		rc = -ENOMEM;
 		rt = kzalloc(sizeof(*rt), GFP_KERNEL);
 		if (!rt)
@@ -1996,7 +1997,7 @@ static int filename_trans_read_helper(struct policydb *p, void *fp)
 	struct filename_trans_key *ft = NULL;
 	struct filename_trans_datum **dst, *datum, *first = NULL;
 	char *name = NULL;
-	u32 len, ttype, tclass, ndatum, i;
+	u32 len, ttype, ndatum, tclass;
 	__le32 buf[3];
 	int rc;
 
@@ -2026,7 +2027,7 @@ static int filename_trans_read_helper(struct policydb *p, void *fp)
 	}
 
 	dst = &first;
-	for (i = 0; i < ndatum; i++) {
+	for (u32 i = 0; i < ndatum; i++) {
 		rc = -ENOMEM;
 		datum = kmalloc(sizeof(*datum), GFP_KERNEL);
 		if (!datum)
@@ -2082,9 +2083,9 @@ static int filename_trans_read_helper(struct policydb *p, void *fp)
 
 static int filename_trans_read(struct policydb *p, void *fp)
 {
-	u32 nel;
+	u32 nel, i;
 	__le32 buf[1];
-	int rc, i;
+	int rc;
 
 	if (p->policyvers < POLICYDB_VERSION_FILENAME_TRANS)
 		return 0;
@@ -2123,7 +2124,7 @@ static int filename_trans_read(struct policydb *p, void *fp)
 
 static int genfs_read(struct policydb *p, void *fp)
 {
-	int i, j, rc;
+	int rc;
 	u32 nel, nel2, len, len2;
 	__le32 buf[1];
 	struct ocontext *l, *c;
@@ -2136,7 +2137,7 @@ static int genfs_read(struct policydb *p, void *fp)
 		return rc;
 	nel = le32_to_cpu(buf[0]);
 
-	for (i = 0; i < nel; i++) {
+	for (u32 i = 0; i < nel; i++) {
 		rc = next_entry(buf, fp, sizeof(u32));
 		if (rc)
 			goto out;
@@ -2175,7 +2176,7 @@ static int genfs_read(struct policydb *p, void *fp)
 			goto out;
 
 		nel2 = le32_to_cpu(buf[0]);
-		for (j = 0; j < nel2; j++) {
+		for (u32 j = 0; j < nel2; j++) {
 			rc = next_entry(buf, fp, sizeof(u32));
 			if (rc)
 				goto out;
@@ -2237,8 +2238,8 @@ static int genfs_read(struct policydb *p, void *fp)
 static int ocontext_read(struct policydb *p, const struct policydb_compat_info *info,
 			 void *fp)
 {
-	int i, j, rc;
-	u32 nel, len;
+	int i, rc;
+	u32 nel, len, val;
 	__be64 prefixbuf[1];
 	__le32 buf[3];
 	struct ocontext *l, *c;
@@ -2251,7 +2252,7 @@ static int ocontext_read(struct policydb *p, const struct policydb_compat_info *
 		nel = le32_to_cpu(buf[0]);
 
 		l = NULL;
-		for (j = 0; j < nel; j++) {
+		for (u32 j = 0; j < nel; j++) {
 			rc = -ENOMEM;
 			c = kzalloc(sizeof(*c), GFP_KERNEL);
 			if (!c)
@@ -2299,9 +2300,27 @@ static int ocontext_read(struct policydb *p, const struct policydb_compat_info *
 				rc = next_entry(buf, fp, sizeof(u32)*3);
 				if (rc)
 					goto out;
-				c->u.port.protocol = le32_to_cpu(buf[0]);
-				c->u.port.low_port = le32_to_cpu(buf[1]);
-				c->u.port.high_port = le32_to_cpu(buf[2]);
+
+				rc = -EINVAL;
+
+				val = le32_to_cpu(buf[0]);
+				if (val > U8_MAX)
+					goto out;
+				c->u.port.protocol = val;
+
+				val = le32_to_cpu(buf[1]);
+				if (val > U16_MAX)
+					goto out;
+				c->u.port.low_port = val;
+
+				val = le32_to_cpu(buf[2]);
+				if (val > U16_MAX)
+					goto out;
+				c->u.port.high_port = val;
+
+				if (c->u.port.low_port > c->u.port.high_port)
+					goto out;
+
 				rc = context_read_and_validate(&c->context[0], p, fp);
 				if (rc)
 					goto out;
@@ -2429,7 +2448,7 @@ int policydb_read(struct policydb *p, void *fp)
 	struct role_allow *ra, *lra;
 	struct role_trans_key *rtk = NULL;
 	struct role_trans_datum *rtd = NULL;
-	int i, j, rc;
+	int rc;
 	__le32 buf[4];
 	u32 len, nprim, nel, perm;
 
@@ -2546,7 +2565,7 @@ int policydb_read(struct policydb *p, void *fp)
 		goto bad;
 	}
 
-	for (i = 0; i < info->sym_num; i++) {
+	for (int i = 0; i < info->sym_num; i++) {
 		rc = next_entry(buf, fp, sizeof(u32)*2);
 		if (rc)
 			goto bad;
@@ -2563,7 +2582,7 @@ int policydb_read(struct policydb *p, void *fp)
 				goto out;
 		}
 
-		for (j = 0; j < nel; j++) {
+		for (u32 j = 0; j < nel; j++) {
 			rc = read_f[i](p, &p->symtab[i], fp);
 			if (rc)
 				goto bad;
@@ -2597,7 +2616,7 @@ int policydb_read(struct policydb *p, void *fp)
 	rc = hashtab_init(&p->role_tr, nel);
 	if (rc)
 		goto bad;
-	for (i = 0; i < nel; i++) {
+	for (u32 i = 0; i < nel; i++) {
 		rc = -ENOMEM;
 		rtk = kmalloc(sizeof(*rtk), GFP_KERNEL);
 		if (!rtk)
@@ -2643,7 +2662,7 @@ int policydb_read(struct policydb *p, void *fp)
 		goto bad;
 	nel = le32_to_cpu(buf[0]);
 	lra = NULL;
-	for (i = 0; i < nel; i++) {
+	for (u32 i = 0; i < nel; i++) {
 		rc = -ENOMEM;
 		ra = kzalloc(sizeof(*ra), GFP_KERNEL);
 		if (!ra)
@@ -2707,10 +2726,10 @@ int policydb_read(struct policydb *p, void *fp)
 		goto bad;
 
 	/* just in case ebitmap_init() becomes more than just a memset(0): */
-	for (i = 0; i < p->p_types.nprim; i++)
+	for (u32 i = 0; i < p->p_types.nprim; i++)
 		ebitmap_init(&p->type_attr_map_array[i]);
 
-	for (i = 0; i < p->p_types.nprim; i++) {
+	for (u32 i = 0; i < p->p_types.nprim; i++) {
 		struct ebitmap *e = &p->type_attr_map_array[i];
 
 		if (p->policyvers >= POLICYDB_VERSION_AVTAB) {
@@ -3282,7 +3301,7 @@ static int (*const write_f[SYM_NUM]) (void *key, void *datum, void *datap) = {
 static int ocontext_write(struct policydb *p, const struct policydb_compat_info *info,
 			  void *fp)
 {
-	unsigned int i, j, rc;
+	int i, rc;
 	size_t nel, len;
 	__be64 prefixbuf[1];
 	__le32 buf[3];
@@ -3360,9 +3379,9 @@ static int ocontext_write(struct policydb *p, const struct policydb_compat_info
 					return rc;
 				break;
 			case OCON_NODE6:
-				for (j = 0; j < 4; j++)
+				for (unsigned int j = 0; j < 4; j++)
 					nodebuf[j] = c->u.node6.addr[j]; /* network order */
-				for (j = 0; j < 4; j++)
+				for (unsigned int j = 0; j < 4; j++)
 					nodebuf[j + 4] = c->u.node6.mask[j]; /* network order */
 				rc = put_entry(nodebuf, sizeof(u32), 8, fp);
 				if (rc)
@@ -3631,8 +3650,7 @@ static int filename_trans_write(struct policydb *p, void *fp)
  */
 int policydb_write(struct policydb *p, void *fp)
 {
-	unsigned int i, num_syms;
-	int rc;
+	int rc, num_syms;
 	__le32 buf[4];
 	u32 config;
 	size_t len;
@@ -3701,7 +3719,7 @@ int policydb_write(struct policydb *p, void *fp)
 	}
 
 	num_syms = info->sym_num;
-	for (i = 0; i < num_syms; i++) {
+	for (int i = 0; i < num_syms; i++) {
 		struct policy_data pd;
 
 		pd.fp = fp;
@@ -3750,7 +3768,7 @@ int policydb_write(struct policydb *p, void *fp)
 	if (rc)
 		return rc;
 
-	for (i = 0; i < p->p_types.nprim; i++) {
+	for (u32 i = 0; i < p->p_types.nprim; i++) {
 		struct ebitmap *e = &p->type_attr_map_array[i];
 
 		rc = ebitmap_write(e, fp);
-- 
2.40.1


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC PATCH 16/20] selinux: symtab: implicit conversion
  2023-07-06 13:23 [RFC PATCH 01/20] selinux: check for multiplication overflow in put_entry() Christian Göttsche
                   ` (13 preceding siblings ...)
  2023-07-06 13:23 ` [RFC PATCH 15/20] selinux: policydb: implicit conversions Christian Göttsche
@ 2023-07-06 13:23 ` Christian Göttsche
  2023-07-18 22:01   ` [PATCH RFC " Paul Moore
  2023-07-06 13:23 ` [RFC PATCH 17/20] selinux: services: implicit conversions Christian Göttsche
                   ` (4 subsequent siblings)
  19 siblings, 1 reply; 42+ messages in thread
From: Christian Göttsche @ 2023-07-06 13:23 UTC (permalink / raw)
  To: selinux; +Cc: Paul Moore, Stephen Smalley, Eric Paris, linux-kernel

hashtab_init() takes an u32 as size parameter type.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 security/selinux/ss/symtab.c | 2 +-
 security/selinux/ss/symtab.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/security/selinux/ss/symtab.c b/security/selinux/ss/symtab.c
index c42a6648a07d..7a77571fb275 100644
--- a/security/selinux/ss/symtab.c
+++ b/security/selinux/ss/symtab.c
@@ -37,7 +37,7 @@ static const struct hashtab_key_params symtab_key_params = {
 	.cmp = symcmp,
 };
 
-int symtab_init(struct symtab *s, unsigned int size)
+int symtab_init(struct symtab *s, u32 size)
 {
 	s->nprim = 0;
 	return hashtab_init(&s->table, size);
diff --git a/security/selinux/ss/symtab.h b/security/selinux/ss/symtab.h
index f2614138d0cd..3033c4db6cb6 100644
--- a/security/selinux/ss/symtab.h
+++ b/security/selinux/ss/symtab.h
@@ -17,7 +17,7 @@ struct symtab {
 	u32 nprim;		/* number of primary names in table */
 };
 
-int symtab_init(struct symtab *s, unsigned int size);
+int symtab_init(struct symtab *s, u32 size);
 
 int symtab_insert(struct symtab *s, char *name, void *datum);
 void *symtab_search(struct symtab *s, const char *name);
-- 
2.40.1


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC PATCH 17/20] selinux: services: implicit conversions
  2023-07-06 13:23 [RFC PATCH 01/20] selinux: check for multiplication overflow in put_entry() Christian Göttsche
                   ` (14 preceding siblings ...)
  2023-07-06 13:23 ` [RFC PATCH 16/20] selinux: symtab: implicit conversion Christian Göttsche
@ 2023-07-06 13:23 ` Christian Göttsche
  2023-07-18 22:01   ` [PATCH RFC " Paul Moore
  2023-07-06 13:23 ` [RFC PATCH 18/20] selinux: nlmsgtab: implicit conversion Christian Göttsche
                   ` (3 subsequent siblings)
  19 siblings, 1 reply; 42+ messages in thread
From: Christian Göttsche @ 2023-07-06 13:23 UTC (permalink / raw)
  To: selinux
  Cc: Paul Moore, Stephen Smalley, Eric Paris, Ondrej Mosnacek, GONG,
	Ruiqi, linux-kernel

Use the type identical to the source for local variables.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 security/selinux/ss/services.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 823b000381a4..e2cd6d7ea7cc 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -856,7 +856,7 @@ int security_bounded_transition(u32 old_sid, u32 new_sid)
 	struct sidtab *sidtab;
 	struct sidtab_entry *old_entry, *new_entry;
 	struct type_datum *type;
-	int index;
+	u32 index;
 	int rc;
 
 	if (!selinux_initialized())
@@ -1511,9 +1511,7 @@ static int security_context_to_sid_core(const char *scontext, u32 scontext_len,
 		return -ENOMEM;
 
 	if (!selinux_initialized()) {
-		int i;
-
-		for (i = 1; i < SECINITSID_NUM; i++) {
+		for (u32 i = 1; i < SECINITSID_NUM; i++) {
 			const char *s = initial_sid_to_string[i];
 
 			if (s && !strcmp(s, scontext2)) {
-- 
2.40.1


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC PATCH 18/20] selinux: nlmsgtab: implicit conversion
  2023-07-06 13:23 [RFC PATCH 01/20] selinux: check for multiplication overflow in put_entry() Christian Göttsche
                   ` (15 preceding siblings ...)
  2023-07-06 13:23 ` [RFC PATCH 17/20] selinux: services: implicit conversions Christian Göttsche
@ 2023-07-06 13:23 ` Christian Göttsche
  2023-07-18 22:01   ` [PATCH RFC " Paul Moore
  2023-07-06 13:23 ` [RFC PATCH 19/20] selinux: status: avoid implicit conversions regarding enforcing status Christian Göttsche
                   ` (2 subsequent siblings)
  19 siblings, 1 reply; 42+ messages in thread
From: Christian Göttsche @ 2023-07-06 13:23 UTC (permalink / raw)
  To: selinux; +Cc: Paul Moore, Stephen Smalley, Eric Paris, linux-kernel

Use an unsigned type as loop iterator.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 security/selinux/nlmsgtab.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/security/selinux/nlmsgtab.c b/security/selinux/nlmsgtab.c
index 2ee7b4ed43ef..b36623d5cf11 100644
--- a/security/selinux/nlmsgtab.c
+++ b/security/selinux/nlmsgtab.c
@@ -153,9 +153,9 @@ static const struct nlmsg_perm nlmsg_audit_perms[] = {
 
 static int nlmsg_perm(u16 nlmsg_type, u32 *perm, const struct nlmsg_perm *tab, size_t tabsize)
 {
-	int i, err = -EINVAL;
+	int err = -EINVAL;
 
-	for (i = 0; i < tabsize/sizeof(struct nlmsg_perm); i++)
+	for (u32 i = 0; i < tabsize/sizeof(struct nlmsg_perm); i++)
 		if (nlmsg_type == tab[i].nlmsg_type) {
 			*perm = tab[i].perm;
 			err = 0;
-- 
2.40.1


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC PATCH 19/20] selinux: status: avoid implicit conversions regarding enforcing status
  2023-07-06 13:23 [RFC PATCH 01/20] selinux: check for multiplication overflow in put_entry() Christian Göttsche
                   ` (16 preceding siblings ...)
  2023-07-06 13:23 ` [RFC PATCH 18/20] selinux: nlmsgtab: implicit conversion Christian Göttsche
@ 2023-07-06 13:23 ` Christian Göttsche
  2023-07-18 22:01   ` [PATCH RFC " Paul Moore
  2023-07-06 13:23 ` [RFC PATCH 20/20] selinux: selinuxfs: avoid implicit conversions Christian Göttsche
  2023-07-18 22:01 ` [PATCH RFC 1/20] selinux: check for multiplication overflow in put_entry() Paul Moore
  19 siblings, 1 reply; 42+ messages in thread
From: Christian Göttsche @ 2023-07-06 13:23 UTC (permalink / raw)
  To: selinux
  Cc: Paul Moore, Stephen Smalley, Eric Paris, Ondrej Mosnacek,
	Xiu Jianfeng, linux-kernel

Use the type bool as parameter type in
selinux_status_update_setenforce().  The related function
enforcing_enabled() returns the type bool, while the struct
selinux_kernel_status member enforcing uses an u32.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 security/selinux/include/security.h | 2 +-
 security/selinux/selinuxfs.c        | 7 ++++---
 security/selinux/status.c           | 4 ++--
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index a16c52d553e1..d0837efde62b 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -375,7 +375,7 @@ struct selinux_kernel_status {
 	 */
 } __packed;
 
-extern void selinux_status_update_setenforce(int enforcing);
+extern void selinux_status_update_setenforce(bool enforcing);
 extern void selinux_status_update_policyload(u32 seqno);
 extern void selinux_complete_init(void);
 extern struct path selinux_null;
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index c3ac0468f698..88d856f5c6bc 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -137,7 +137,8 @@ static ssize_t sel_write_enforce(struct file *file, const char __user *buf,
 {
 	char *page = NULL;
 	ssize_t length;
-	int old_value, new_value;
+	int scan_value;
+	bool old_value, new_value;
 
 	if (count >= PAGE_SIZE)
 		return -ENOMEM;
@@ -151,10 +152,10 @@ static ssize_t sel_write_enforce(struct file *file, const char __user *buf,
 		return PTR_ERR(page);
 
 	length = -EINVAL;
-	if (sscanf(page, "%d", &new_value) != 1)
+	if (sscanf(page, "%d", &scan_value) != 1)
 		goto out;
 
-	new_value = !!new_value;
+	new_value = !!scan_value;
 
 	old_value = enforcing_enabled();
 	if (new_value != old_value) {
diff --git a/security/selinux/status.c b/security/selinux/status.c
index e436e4975adc..dffca22ce6f7 100644
--- a/security/selinux/status.c
+++ b/security/selinux/status.c
@@ -76,7 +76,7 @@ struct page *selinux_kernel_status_page(void)
  *
  * It updates status of the current enforcing/permissive mode.
  */
-void selinux_status_update_setenforce(int enforcing)
+void selinux_status_update_setenforce(bool enforcing)
 {
 	struct selinux_kernel_status   *status;
 
@@ -87,7 +87,7 @@ void selinux_status_update_setenforce(int enforcing)
 		status->sequence++;
 		smp_wmb();
 
-		status->enforcing = enforcing;
+		status->enforcing = enforcing ? 1 : 0;
 
 		smp_wmb();
 		status->sequence++;
-- 
2.40.1


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC PATCH 20/20] selinux: selinuxfs: avoid implicit conversions
  2023-07-06 13:23 [RFC PATCH 01/20] selinux: check for multiplication overflow in put_entry() Christian Göttsche
                   ` (17 preceding siblings ...)
  2023-07-06 13:23 ` [RFC PATCH 19/20] selinux: status: avoid implicit conversions regarding enforcing status Christian Göttsche
@ 2023-07-06 13:23 ` Christian Göttsche
  2023-07-18 22:01   ` [PATCH RFC " Paul Moore
  2023-07-18 22:01 ` [PATCH RFC 1/20] selinux: check for multiplication overflow in put_entry() Paul Moore
  19 siblings, 1 reply; 42+ messages in thread
From: Christian Göttsche @ 2023-07-06 13:23 UTC (permalink / raw)
  To: selinux; +Cc: Paul Moore, Stephen Smalley, Eric Paris, linux-kernel

Use unsigned loop counters where the upper bound is of unsigned
type.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 security/selinux/selinuxfs.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index 88d856f5c6bc..a2dc415779ae 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -1074,7 +1074,7 @@ static ssize_t sel_write_user(struct file *file, char *buf, size_t size)
 	u32 sid, *sids = NULL;
 	ssize_t length;
 	char *newcon;
-	int i, rc;
+	int rc;
 	u32 len, nsids;
 
 	length = avc_has_perm(current_sid(), SECINITSID_SECURITY,
@@ -1107,7 +1107,7 @@ static ssize_t sel_write_user(struct file *file, char *buf, size_t size)
 
 	length = sprintf(buf, "%u", nsids) + 1;
 	ptr = buf + length;
-	for (i = 0; i < nsids; i++) {
+	for (u32 i = 0; i < nsids; i++) {
 		rc = security_sid_to_context(sids[i], &newcon, &len);
 		if (rc) {
 			length = rc;
@@ -1612,7 +1612,6 @@ static int sel_make_avc_files(struct dentry *dir)
 {
 	struct super_block *sb = dir->d_sb;
 	struct selinux_fs_info *fsi = sb->s_fs_info;
-	int i;
 	static const struct tree_descr files[] = {
 		{ "cache_threshold",
 		  &sel_avc_cache_threshold_ops, S_IRUGO|S_IWUSR },
@@ -1622,7 +1621,7 @@ static int sel_make_avc_files(struct dentry *dir)
 #endif
 	};
 
-	for (i = 0; i < ARRAY_SIZE(files); i++) {
+	for (u32 i = 0; i < ARRAY_SIZE(files); i++) {
 		struct inode *inode;
 		struct dentry *dentry;
 
@@ -1648,12 +1647,11 @@ static int sel_make_ss_files(struct dentry *dir)
 {
 	struct super_block *sb = dir->d_sb;
 	struct selinux_fs_info *fsi = sb->s_fs_info;
-	int i;
 	static const struct tree_descr files[] = {
 		{ "sidtab_hash_stats", &sel_sidtab_hash_stats_ops, S_IRUGO },
 	};
 
-	for (i = 0; i < ARRAY_SIZE(files); i++) {
+	for (u32 i = 0; i < ARRAY_SIZE(files); i++) {
 		struct inode *inode;
 		struct dentry *dentry;
 
@@ -1699,9 +1697,7 @@ static const struct file_operations sel_initcon_ops = {
 
 static int sel_make_initcon_files(struct dentry *dir)
 {
-	int i;
-
-	for (i = 1; i <= SECINITSID_NUM; i++) {
+	for (u32 i = 1; i <= SECINITSID_NUM; i++) {
 		struct inode *inode;
 		struct dentry *dentry;
 		const char *s = security_get_initial_sid_context(i);
-- 
2.40.1


^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC PATCH 07/20] selinux: services: update type for umber of class permissions
  2023-07-06 13:23 ` [RFC PATCH 07/20] selinux: services: update type for umber of class permissions Christian Göttsche
@ 2023-07-07  2:27   ` Gong Ruiqi
  2023-07-18 22:01   ` [PATCH RFC 7/20] " Paul Moore
  1 sibling, 0 replies; 42+ messages in thread
From: Gong Ruiqi @ 2023-07-07  2:27 UTC (permalink / raw)
  To: Christian Göttsche
  Cc: Paul Moore, Stephen Smalley, Eric Paris, Ondrej Mosnacek,
	linux-kernel, selinux

Hi Christian,

First of all, there's a typo in the subject: umber -> number ;)

On 2023/07/06 21:23, Christian Göttsche wrote:
> Security classes have only up to 32 permissions, hence using an u16 is
> sufficient (while improving padding).
> 
> Also use a fixed sized cast in a bit shift to work correctly on
> architectures where sizeof(unsigned int) != sizeof(u32).
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  security/selinux/ss/services.c | 6 +++---
>  security/selinux/ss/services.h | 2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index 78946b71c1c1..3275cfe2c8f7 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -97,7 +97,6 @@ static int selinux_set_mapping(struct policydb *pol,
>  			       struct selinux_map *out_map)
>  {
>  	u16 i, j;
> -	unsigned k;
>  	bool print_unknown_handle = false;
>  
>  	/* Find number of classes in the input mapping */
> @@ -117,6 +116,7 @@ static int selinux_set_mapping(struct policydb *pol,
>  	while (map[j].name) {
>  		const struct security_class_mapping *p_in = map + (j++);
>  		struct selinux_mapping *p_out = out_map->mapping + j;
> +		u16 k;
>  
>  		/* An empty class string skips ahead */
>  		if (!strcmp(p_in->name, "")) {
> @@ -202,7 +202,7 @@ static void map_decision(struct selinux_map *map,
>  {
>  	if (tclass < map->size) {
>  		struct selinux_mapping *mapping = &map->mapping[tclass];
> -		unsigned int i, n = mapping->num_perms;
> +		u16 i, n = mapping->num_perms;
>  		u32 result;
>  
>  		for (i = 0, result = 0; i < n; i++) {
> @@ -230,7 +230,7 @@ static void map_decision(struct selinux_map *map,
>  		 * should audit that denial
>  		 */
>  		for (; i < (sizeof(u32)*8); i++)
> -			result |= 1<<i;
> +			result |= 1<<((u32)i);

Is it really necessary to do explicit conversion here? Its value is
known to be small, and IIUC, u16 will be implicitly promoted to int as
an operand of <<, as described here:

https://en.cppreference.com/w/c/language/conversion#Integer_promotions

>  		avd->auditdeny = result;
>  	}
>  }
> diff --git a/security/selinux/ss/services.h b/security/selinux/ss/services.h
> index 8a9b85f44b66..b6f99353301e 100644
> --- a/security/selinux/ss/services.h
> +++ b/security/selinux/ss/services.h
> @@ -12,7 +12,7 @@
>  /* Mapping for a single class */
>  struct selinux_mapping {
>  	u16 value; /* policy value for class */
> -	unsigned int num_perms; /* number of permissions in class */
> +	u16 num_perms; /* number of permissions in class */
>  	u32 perms[sizeof(u32) * 8]; /* policy values for permissions */
>  };
>  

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH RFC 1/20] selinux: check for multiplication overflow in  put_entry()
  2023-07-06 13:23 [RFC PATCH 01/20] selinux: check for multiplication overflow in put_entry() Christian Göttsche
                   ` (18 preceding siblings ...)
  2023-07-06 13:23 ` [RFC PATCH 20/20] selinux: selinuxfs: avoid implicit conversions Christian Göttsche
@ 2023-07-18 22:01 ` Paul Moore
  19 siblings, 0 replies; 42+ messages in thread
From: Paul Moore @ 2023-07-18 22:01 UTC (permalink / raw)
  To: Gong Ruiqi, Christian Göttsche
  Cc: Stephen Smalley, Eric Paris, Ondrej Mosnacek, linux-kernel, selinux

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 650 bytes --]

On Jul  6, 2023 Gong Ruiqi <gongruiqi1@huawei.com> wrote:
> 
> The function is always inlined and most of the time both relevant
> arguments are compile time constants, allowing compilers to elide the
> check.  Also the function is part of outputting the policy, which is not
> performance critical.
> 
> Also convert the type of the third parameter into a size_t, since it
> should always be a non-negative number of elements.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  security/selinux/ss/policydb.h | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)

Merged into selinux/next, thanks.

--
paul-moore.com

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH RFC 2/20] selinux: avtab: avoid implicit conversions
  2023-07-06 13:23 ` [RFC PATCH 02/20] selinux: avtab: avoid implicit conversions Christian Göttsche
@ 2023-07-18 22:01   ` Paul Moore
  0 siblings, 0 replies; 42+ messages in thread
From: Paul Moore @ 2023-07-18 22:01 UTC (permalink / raw)
  To: Christian Göttsche, selinux
  Cc: Stephen Smalley, Eric Paris, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 705 bytes --]

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

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH RFC 3/20] selinux: avoid avtab overflows
  2023-07-06 13:23 ` [RFC PATCH 03/20] selinux: avoid avtab overflows Christian Göttsche
@ 2023-07-18 22:01   ` Paul Moore
  0 siblings, 0 replies; 42+ messages in thread
From: Paul Moore @ 2023-07-18 22:01 UTC (permalink / raw)
  To: Christian Göttsche, selinux
  Cc: Stephen Smalley, Eric Paris, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 376 bytes --]

On Jul  6, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> wrote:
> 
> Prevent inserting more than the supported U32_MAX number of entries.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  security/selinux/ss/avtab.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Merged into selinux/next, thanks!

--
paul-moore.com

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH RFC 4/20] selinux: ebitmap: use u32 as bit type
  2023-07-06 13:23 ` [RFC PATCH 04/20] selinux: ebitmap: use u32 as bit type Christian Göttsche
@ 2023-07-18 22:01   ` Paul Moore
  0 siblings, 0 replies; 42+ messages in thread
From: Paul Moore @ 2023-07-18 22:01 UTC (permalink / raw)
  To: Christian Göttsche, selinux
  Cc: Stephen Smalley, Eric Paris, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 648 bytes --]

On Jul  6, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> wrote:
> 
> The extensible bitmap supports bit positions up to U32_MAX due to the
> type of the member highbit being u32.  Use u32 consistently as the type
> for bit positions to announce to callers what range of values is
> supported.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  security/selinux/ss/ebitmap.c | 32 ++++++++++++++++----------------
>  security/selinux/ss/ebitmap.h | 32 ++++++++++++++++----------------
>  2 files changed, 32 insertions(+), 32 deletions(-)

See my previous comment about loop iterators.

--
paul-moore.com

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH RFC 5/20] selinux: hashtab: use identical iterator type
  2023-07-06 13:23 ` [RFC PATCH 05/20] selinux: hashtab: use identical iterator type Christian Göttsche
@ 2023-07-18 22:01   ` Paul Moore
  0 siblings, 0 replies; 42+ messages in thread
From: Paul Moore @ 2023-07-18 22:01 UTC (permalink / raw)
  To: Christian Göttsche, selinux
  Cc: Stephen Smalley, Eric Paris, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 374 bytes --]

On Jul  6, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> wrote:
> 
> Use the identical type u32 for the loop iterator.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  security/selinux/ss/hashtab.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

No to the loop iterators declared in the loop.

--
paul-moore.com

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH RFC 6/20] selinux: mls: avoid implicit conversions
  2023-07-06 13:23 ` [RFC PATCH 06/20] selinux: mls: avoid implicit conversions Christian Göttsche
@ 2023-07-18 22:01   ` Paul Moore
  0 siblings, 0 replies; 42+ messages in thread
From: Paul Moore @ 2023-07-18 22:01 UTC (permalink / raw)
  To: Christian Göttsche, selinux
  Cc: Stephen Smalley, Eric Paris, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 381 bytes --]

On Jul  6, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> wrote:
> 
> Use u32 for ebitmap bits.
> 
> Use char for the default range of a class.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  security/selinux/ss/mls.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)

Two words: "loop iterators"

--
paul-moore.com

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH RFC 7/20] selinux: services: update type for umber of class  permissions
  2023-07-06 13:23 ` [RFC PATCH 07/20] selinux: services: update type for umber of class permissions Christian Göttsche
  2023-07-07  2:27   ` Gong Ruiqi
@ 2023-07-18 22:01   ` Paul Moore
  2023-07-19  1:45     ` Gong Ruiqi
  1 sibling, 1 reply; 42+ messages in thread
From: Paul Moore @ 2023-07-18 22:01 UTC (permalink / raw)
  To: Gong Ruiqi, Christian Göttsche
  Cc: Stephen Smalley, Eric Paris, Ondrej Mosnacek, linux-kernel, selinux

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2629 bytes --]

On Jul  6, 2023 Gong Ruiqi <gongruiqi1@huawei.com> wrote:
> 
> Security classes have only up to 32 permissions, hence using an u16 is
> sufficient (while improving padding).

Can you explain the improved padding comment?  It looks like you are
only changing the iterator's type so the struct should remain
unchanged, and FWIW, it looks like security_class_wrapping allocates
space for 33 permission strings.

> Also use a fixed sized cast in a bit shift to work correctly on
> architectures where sizeof(unsigned int) != sizeof(u32).
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  security/selinux/ss/services.c | 6 +++---
>  security/selinux/ss/services.h | 2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index 78946b71c1c1..3275cfe2c8f7 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -97,7 +97,6 @@ static int selinux_set_mapping(struct policydb *pol,
>  			       struct selinux_map *out_map)
>  {
>  	u16 i, j;
> -	unsigned k;
>  	bool print_unknown_handle = false;
>  
>  	/* Find number of classes in the input mapping */
> @@ -117,6 +116,7 @@ static int selinux_set_mapping(struct policydb *pol,
>  	while (map[j].name) {
>  		const struct security_class_mapping *p_in = map + (j++);
>  		struct selinux_mapping *p_out = out_map->mapping + j;
> +		u16 k;
>  
>  		/* An empty class string skips ahead */
>  		if (!strcmp(p_in->name, "")) {
> @@ -202,7 +202,7 @@ static void map_decision(struct selinux_map *map,
>  {
>  	if (tclass < map->size) {
>  		struct selinux_mapping *mapping = &map->mapping[tclass];
> -		unsigned int i, n = mapping->num_perms;
> +		u16 i, n = mapping->num_perms;
>  		u32 result;
>  
>  		for (i = 0, result = 0; i < n; i++) {
> @@ -230,7 +230,7 @@ static void map_decision(struct selinux_map *map,
>  		 * should audit that denial
>  		 */
>  		for (; i < (sizeof(u32)*8); i++)
> -			result |= 1<<i;
> +			result |= 1<<((u32)i);
>  		avd->auditdeny = result;
>  	}
>  }
> diff --git a/security/selinux/ss/services.h b/security/selinux/ss/services.h
> index 8a9b85f44b66..b6f99353301e 100644
> --- a/security/selinux/ss/services.h
> +++ b/security/selinux/ss/services.h
> @@ -12,7 +12,7 @@
>  /* Mapping for a single class */
>  struct selinux_mapping {
>  	u16 value; /* policy value for class */
> -	unsigned int num_perms; /* number of permissions in class */
> +	u16 num_perms; /* number of permissions in class */
>  	u32 perms[sizeof(u32) * 8]; /* policy values for permissions */
>  };
>  
> -- 
> 2.40.1

--
paul-moore.com

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH RFC 8/20] selinux: services: avoid implicit conversions
  2023-07-06 13:23 ` [RFC PATCH 08/20] selinux: services: avoid implicit conversions Christian Göttsche
@ 2023-07-18 22:01   ` Paul Moore
  0 siblings, 0 replies; 42+ messages in thread
From: Paul Moore @ 2023-07-18 22:01 UTC (permalink / raw)
  To: Christian Göttsche, selinux
  Cc: Stephen Smalley, Eric Paris, Ondrej Mosnacek, Casey Schaufler,
	Xiu Jianfeng, GONG, Ruiqi, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 753 bytes --]

On Jul  6, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> wrote:
> 
> Use u32 as the output parameter type in security_get_classes() and
> security_get_permissions(), based on the type of the symtab nprim
> member.
> 
> Declare the read-only class string parameter of
> security_get_permissions() const.
> 
> Avoid several implicit conversions by using the identical type for the
> destination.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  security/selinux/include/security.h |  4 ++--
>  security/selinux/selinuxfs.c        |  7 ++++---
>  security/selinux/ss/services.c      | 22 +++++++++-------------
>  3 files changed, 15 insertions(+), 18 deletions(-)

More loop iterators ...

--
paul-moore.com

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH RFC 9/20] selinux: status: consistently use u32 as sequence  number type
  2023-07-06 13:23 ` [RFC PATCH 09/20] selinux: status: consistently use u32 as sequence number type Christian Göttsche
@ 2023-07-18 22:01   ` Paul Moore
  0 siblings, 0 replies; 42+ messages in thread
From: Paul Moore @ 2023-07-18 22:01 UTC (permalink / raw)
  To: Christian Göttsche, selinux
  Cc: Stephen Smalley, Eric Paris, Ondrej Mosnacek, Xiu Jianfeng, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1748 bytes --]

On Jul  6, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> wrote:
> 
> Align the type with the one used in selinux_notify_policy_change() and
> the sequence member of struct selinux_kernel_status.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  security/selinux/include/security.h | 2 +-
>  security/selinux/status.c           | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

I was going to suggest you also update avc_latest_notif_update(), but
it looks like you tackle that later in the patchset.

Merged into selinux/next, thanks.

> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> index 0f93fd019bb4..a16c52d553e1 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -376,7 +376,7 @@ struct selinux_kernel_status {
>  } __packed;
>  
>  extern void selinux_status_update_setenforce(int enforcing);
> -extern void selinux_status_update_policyload(int seqno);
> +extern void selinux_status_update_policyload(u32 seqno);
>  extern void selinux_complete_init(void);
>  extern struct path selinux_null;
>  extern void selnl_notify_setenforce(int val);
> diff --git a/security/selinux/status.c b/security/selinux/status.c
> index 19ef929a075c..e436e4975adc 100644
> --- a/security/selinux/status.c
> +++ b/security/selinux/status.c
> @@ -101,7 +101,7 @@ void selinux_status_update_setenforce(int enforcing)
>   * It updates status of the times of policy reloaded, and current
>   * setting of deny_unknown.
>   */
> -void selinux_status_update_policyload(int seqno)
> +void selinux_status_update_policyload(u32 seqno)
>  {
>  	struct selinux_kernel_status   *status;
>  
> -- 
> 2.40.1

--
paul-moore.com

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH RFC 10/20] selinux: netif: avoid implicit conversions
  2023-07-06 13:23 ` [RFC PATCH 10/20] selinux: netif: avoid implicit conversions Christian Göttsche
@ 2023-07-18 22:01   ` Paul Moore
  0 siblings, 0 replies; 42+ messages in thread
From: Paul Moore @ 2023-07-18 22:01 UTC (permalink / raw)
  To: Christian Göttsche, selinux
  Cc: Stephen Smalley, Eric Paris, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 355 bytes --]

On Jul  6, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> wrote:
> 
> Use the identical type sel_netif_hashfn() returns.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  security/selinux/netif.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Merged into selinux/next, thanks.

--
paul-moore.com

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH RFC 11/20] selinux: avc: avoid implicit conversions
  2023-07-06 13:23 ` [RFC PATCH 11/20] selinux: avc: " Christian Göttsche
@ 2023-07-18 22:01   ` Paul Moore
  0 siblings, 0 replies; 42+ messages in thread
From: Paul Moore @ 2023-07-18 22:01 UTC (permalink / raw)
  To: Christian Göttsche, selinux
  Cc: Stephen Smalley, Eric Paris, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1469 bytes --]

On Jul  6, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> wrote:
> 
> Use a consistent type of u32 for sequence numbers.
> 
> Use a non-negative and input parameter matching type for the hash
> result.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  security/selinux/avc.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)

...

> diff --git a/security/selinux/avc.c b/security/selinux/avc.c
> index 1074db66e5ff..cd55479cce25 100644
> --- a/security/selinux/avc.c
> +++ b/security/selinux/avc.c
> @@ -654,9 +654,9 @@ static void avc_audit_pre_callback(struct audit_buffer *ab, void *a)
>  {
>  	struct common_audit_data *ad = a;
>  	struct selinux_audit_data *sad = ad->selinux_audit_data;
> -	u32 av = sad->audited;
> +	u32 av = sad->audited, perm;
>  	const char *const *perms;
> -	int i, perm;
> +	u32 i;

Technically the perm type change doesn't fit with the description, but
it's minor enough that it shouldn't be an issue.

Merged into selinux/next.

>  	audit_log_format(ab, "avc:  %s ", sad->denied ? "denied" : "granted");
>  
> @@ -833,7 +833,8 @@ static int avc_update_node(u32 event, u32 perms, u8 driver, u8 xperm, u32 ssid,
>  			   struct extended_perms_decision *xpd,
>  			   u32 flags)
>  {
> -	int hvalue, rc = 0;
> +	u32 hvalue;
> +	int rc = 0;
>  	unsigned long flag;
>  	struct avc_node *pos, *node, *orig = NULL;
>  	struct hlist_head *head;
> -- 
> 2.40.1

--
paul-moore.com

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH RFC 12/20] selinux: hooks: avoid implicit conversions
  2023-07-06 13:23 ` [RFC PATCH 12/20] selinux: hooks: " Christian Göttsche
@ 2023-07-18 22:01   ` Paul Moore
  0 siblings, 0 replies; 42+ messages in thread
From: Paul Moore @ 2023-07-18 22:01 UTC (permalink / raw)
  To: Christian Göttsche, selinux
  Cc: Stephen Smalley, Eric Paris, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 522 bytes --]

On Jul  6, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> wrote:
> 
> Use the identical types in assignments of local variables for the
> destination.
> 
> Merge tail calls into return statements.
> 
> Avoid using leading underscores for function local variable.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  security/selinux/hooks.c | 26 ++++++++++----------------
>  1 file changed, 10 insertions(+), 16 deletions(-)

Merged into selinux/next, thanks.

--
paul-moore.com

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH RFC 13/20] selinux: selinuxfs: avoid implicit conversions
  2023-07-06 13:23 ` [RFC PATCH 13/20] selinux: selinuxfs: " Christian Göttsche
@ 2023-07-18 22:01   ` Paul Moore
  0 siblings, 0 replies; 42+ messages in thread
From: Paul Moore @ 2023-07-18 22:01 UTC (permalink / raw)
  To: Christian Göttsche, selinux
  Cc: Stephen Smalley, Eric Paris, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 483 bytes --]

On Jul  6, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> wrote:
> 
> Use umode_t as parameter type for sel_make_inode(), which assigns the
> value to the member i_mode of struct inode.
> 
> Use identical type for loop iterator.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  security/selinux/selinuxfs.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)

No declaration of iterators inside loops please.

--
paul-moore.com

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH RFC 14/20] selinux: use consistent type for AV rule  specifier
  2023-07-06 13:23 ` [RFC PATCH 14/20] selinux: use consistent type for AV rule specifier Christian Göttsche
@ 2023-07-18 22:01   ` Paul Moore
  0 siblings, 0 replies; 42+ messages in thread
From: Paul Moore @ 2023-07-18 22:01 UTC (permalink / raw)
  To: Christian Göttsche, selinux
  Cc: Stephen Smalley, Eric Paris, Ondrej Mosnacek, GONG, Ruiqi, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 556 bytes --]

On Jul  6, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> wrote:
> 
> The specifier for avtab keys is always supplied with a type of u16,
> either as a macro to security_compute_sid() or the member specified of
> the struct avtab_key.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  security/selinux/ss/avtab.c    | 2 +-
>  security/selinux/ss/avtab.h    | 2 +-
>  security/selinux/ss/services.c | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)

Merged into selinux/next, thanks.

--
paul-moore.com

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH RFC 15/20] selinux: policydb: implicit conversions
  2023-07-06 13:23 ` [RFC PATCH 15/20] selinux: policydb: implicit conversions Christian Göttsche
@ 2023-07-18 22:01   ` Paul Moore
  0 siblings, 0 replies; 42+ messages in thread
From: Paul Moore @ 2023-07-18 22:01 UTC (permalink / raw)
  To: Christian Göttsche, selinux
  Cc: Stephen Smalley, Eric Paris, Ondrej Mosnacek, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 392 bytes --]

On Jul  6, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> wrote:
> 
> Use the identical type for local variables, e.g. loop counters.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  security/selinux/ss/policydb.c | 112 +++++++++++++++++++--------------
>  1 file changed, 65 insertions(+), 47 deletions(-)

Loop iterators ...

--
paul-moore.com

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH RFC 16/20] selinux: symtab: implicit conversion
  2023-07-06 13:23 ` [RFC PATCH 16/20] selinux: symtab: implicit conversion Christian Göttsche
@ 2023-07-18 22:01   ` Paul Moore
  0 siblings, 0 replies; 42+ messages in thread
From: Paul Moore @ 2023-07-18 22:01 UTC (permalink / raw)
  To: Christian Göttsche, selinux
  Cc: Stephen Smalley, Eric Paris, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 398 bytes --]

On Jul  6, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> wrote:
> 
> hashtab_init() takes an u32 as size parameter type.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  security/selinux/ss/symtab.c | 2 +-
>  security/selinux/ss/symtab.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Merged into selinux/next, thanks.

--
paul-moore.com

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH RFC 17/20] selinux: services: implicit conversions
  2023-07-06 13:23 ` [RFC PATCH 17/20] selinux: services: implicit conversions Christian Göttsche
@ 2023-07-18 22:01   ` Paul Moore
  0 siblings, 0 replies; 42+ messages in thread
From: Paul Moore @ 2023-07-18 22:01 UTC (permalink / raw)
  To: Christian Göttsche, selinux
  Cc: Stephen Smalley, Eric Paris, Ondrej Mosnacek, GONG, Ruiqi, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 383 bytes --]

On Jul  6, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> wrote:
> 
> Use the type identical to the source for local variables.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  security/selinux/ss/services.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)

More loop iterators, see my previous comments.

--
paul-moore.com

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH RFC 18/20] selinux: nlmsgtab: implicit conversion
  2023-07-06 13:23 ` [RFC PATCH 18/20] selinux: nlmsgtab: implicit conversion Christian Göttsche
@ 2023-07-18 22:01   ` Paul Moore
  0 siblings, 0 replies; 42+ messages in thread
From: Paul Moore @ 2023-07-18 22:01 UTC (permalink / raw)
  To: Christian Göttsche, selinux
  Cc: Stephen Smalley, Eric Paris, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 331 bytes --]

On Jul  6, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> wrote:
> 
> Use an unsigned type as loop iterator.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  security/selinux/nlmsgtab.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Loop iterators ...

--
paul-moore.com

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH RFC 19/20] selinux: status: avoid implicit conversions  regarding enforcing status
  2023-07-06 13:23 ` [RFC PATCH 19/20] selinux: status: avoid implicit conversions regarding enforcing status Christian Göttsche
@ 2023-07-18 22:01   ` Paul Moore
  0 siblings, 0 replies; 42+ messages in thread
From: Paul Moore @ 2023-07-18 22:01 UTC (permalink / raw)
  To: Christian Göttsche, selinux
  Cc: Stephen Smalley, Eric Paris, Ondrej Mosnacek, Xiu Jianfeng, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 628 bytes --]

On Jul  6, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> wrote:
> 
> Use the type bool as parameter type in
> selinux_status_update_setenforce().  The related function
> enforcing_enabled() returns the type bool, while the struct
> selinux_kernel_status member enforcing uses an u32.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  security/selinux/include/security.h | 2 +-
>  security/selinux/selinuxfs.c        | 7 ++++---
>  security/selinux/status.c           | 4 ++--
>  3 files changed, 7 insertions(+), 6 deletions(-)

Merged into selinux/next, thanks.

--
paul-moore.com

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH RFC 20/20] selinux: selinuxfs: avoid implicit conversions
  2023-07-06 13:23 ` [RFC PATCH 20/20] selinux: selinuxfs: avoid implicit conversions Christian Göttsche
@ 2023-07-18 22:01   ` Paul Moore
  0 siblings, 0 replies; 42+ messages in thread
From: Paul Moore @ 2023-07-18 22:01 UTC (permalink / raw)
  To: Christian Göttsche, selinux
  Cc: Stephen Smalley, Eric Paris, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 432 bytes --]

On Jul  6, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com> wrote:
> 
> Use unsigned loop counters where the upper bound is of unsigned
> type.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  security/selinux/selinuxfs.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)

More loop iterator declarations inside the loop, see my previous
comments.

--
paul-moore.com

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH RFC 7/20] selinux: services: update type for umber of class permissions
  2023-07-18 22:01   ` [PATCH RFC 7/20] " Paul Moore
@ 2023-07-19  1:45     ` Gong Ruiqi
  0 siblings, 0 replies; 42+ messages in thread
From: Gong Ruiqi @ 2023-07-19  1:45 UTC (permalink / raw)
  To: Paul Moore, Christian Göttsche
  Cc: Stephen Smalley, Eric Paris, Ondrej Mosnacek, linux-kernel, selinux



On 2023/07/19 6:01, Paul Moore wrote:
> On Jul  6, 2023 Gong Ruiqi <gongruiqi1@huawei.com> wrote:
>>
>> Security classes have only up to 32 permissions, hence using an u16 is
>> sufficient (while improving padding).
> 
> Can you explain the improved padding comment?  

I think what Christian means is that struct selinux_mapping will occupy
less memory since num_perms is changed from uint (32 bits) to u16, which
saves 16 bits of space due to padding.

> [...]
>
>> diff --git a/security/selinux/ss/services.h b/security/selinux/ss/services.h
>> index 8a9b85f44b66..b6f99353301e 100644
>> --- a/security/selinux/ss/services.h
>> +++ b/security/selinux/ss/services.h
>> @@ -12,7 +12,7 @@
>>  /* Mapping for a single class */
>>  struct selinux_mapping {
>>  	u16 value; /* policy value for class */
>> -	unsigned int num_perms; /* number of permissions in class */
>> +	u16 num_perms; /* number of permissions in class */
>>  	u32 perms[sizeof(u32) * 8]; /* policy values for permissions */
>>  };

Check here.

>>  
>> -- 
>> 2.40.1
> 
> --
> paul-moore.com

^ permalink raw reply	[flat|nested] 42+ messages in thread

end of thread, other threads:[~2023-07-19  1:45 UTC | newest]

Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-06 13:23 [RFC PATCH 01/20] selinux: check for multiplication overflow in put_entry() Christian Göttsche
2023-07-06 13:23 ` [RFC PATCH 02/20] selinux: avtab: avoid implicit conversions Christian Göttsche
2023-07-18 22:01   ` [PATCH RFC 2/20] " Paul Moore
2023-07-06 13:23 ` [RFC PATCH 03/20] selinux: avoid avtab overflows Christian Göttsche
2023-07-18 22:01   ` [PATCH RFC 3/20] " Paul Moore
2023-07-06 13:23 ` [RFC PATCH 04/20] selinux: ebitmap: use u32 as bit type Christian Göttsche
2023-07-18 22:01   ` [PATCH RFC 4/20] " Paul Moore
2023-07-06 13:23 ` [RFC PATCH 05/20] selinux: hashtab: use identical iterator type Christian Göttsche
2023-07-18 22:01   ` [PATCH RFC 5/20] " Paul Moore
2023-07-06 13:23 ` [RFC PATCH 06/20] selinux: mls: avoid implicit conversions Christian Göttsche
2023-07-18 22:01   ` [PATCH RFC 6/20] " Paul Moore
2023-07-06 13:23 ` [RFC PATCH 07/20] selinux: services: update type for umber of class permissions Christian Göttsche
2023-07-07  2:27   ` Gong Ruiqi
2023-07-18 22:01   ` [PATCH RFC 7/20] " Paul Moore
2023-07-19  1:45     ` Gong Ruiqi
2023-07-06 13:23 ` [RFC PATCH 08/20] selinux: services: avoid implicit conversions Christian Göttsche
2023-07-18 22:01   ` [PATCH RFC 8/20] " Paul Moore
2023-07-06 13:23 ` [RFC PATCH 09/20] selinux: status: consistently use u32 as sequence number type Christian Göttsche
2023-07-18 22:01   ` [PATCH RFC 9/20] " Paul Moore
2023-07-06 13:23 ` [RFC PATCH 10/20] selinux: netif: avoid implicit conversions Christian Göttsche
2023-07-18 22:01   ` [PATCH RFC " Paul Moore
2023-07-06 13:23 ` [RFC PATCH 11/20] selinux: avc: " Christian Göttsche
2023-07-18 22:01   ` [PATCH RFC " Paul Moore
2023-07-06 13:23 ` [RFC PATCH 12/20] selinux: hooks: " Christian Göttsche
2023-07-18 22:01   ` [PATCH RFC " Paul Moore
2023-07-06 13:23 ` [RFC PATCH 13/20] selinux: selinuxfs: " Christian Göttsche
2023-07-18 22:01   ` [PATCH RFC " Paul Moore
2023-07-06 13:23 ` [RFC PATCH 14/20] selinux: use consistent type for AV rule specifier Christian Göttsche
2023-07-18 22:01   ` [PATCH RFC " Paul Moore
2023-07-06 13:23 ` [RFC PATCH 15/20] selinux: policydb: implicit conversions Christian Göttsche
2023-07-18 22:01   ` [PATCH RFC " Paul Moore
2023-07-06 13:23 ` [RFC PATCH 16/20] selinux: symtab: implicit conversion Christian Göttsche
2023-07-18 22:01   ` [PATCH RFC " Paul Moore
2023-07-06 13:23 ` [RFC PATCH 17/20] selinux: services: implicit conversions Christian Göttsche
2023-07-18 22:01   ` [PATCH RFC " Paul Moore
2023-07-06 13:23 ` [RFC PATCH 18/20] selinux: nlmsgtab: implicit conversion Christian Göttsche
2023-07-18 22:01   ` [PATCH RFC " Paul Moore
2023-07-06 13:23 ` [RFC PATCH 19/20] selinux: status: avoid implicit conversions regarding enforcing status Christian Göttsche
2023-07-18 22:01   ` [PATCH RFC " Paul Moore
2023-07-06 13:23 ` [RFC PATCH 20/20] selinux: selinuxfs: avoid implicit conversions Christian Göttsche
2023-07-18 22:01   ` [PATCH RFC " Paul Moore
2023-07-18 22:01 ` [PATCH RFC 1/20] selinux: check for multiplication overflow in put_entry() Paul Moore

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome