2023-07-06 13:31:06

by Christian Göttsche

[permalink] [raw]
Subject: [RFC PATCH 01/20] selinux: check for multiplication overflow in put_entry()

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 <[email protected]>
---
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



2023-07-06 13:31:18

by Christian Göttsche

[permalink] [raw]
Subject: [RFC PATCH 16/20] selinux: symtab: implicit conversion

hashtab_init() takes an u32 as size parameter type.

Signed-off-by: Christian Göttsche <[email protected]>
---
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


2023-07-06 13:31:27

by Christian Göttsche

[permalink] [raw]
Subject: [RFC PATCH 10/20] selinux: netif: avoid implicit conversions

Use the identical type sel_netif_hashfn() returns.

Signed-off-by: Christian Göttsche <[email protected]>
---
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


2023-07-06 13:31:59

by Christian Göttsche

[permalink] [raw]
Subject: [RFC PATCH 06/20] selinux: mls: avoid implicit conversions

Use u32 for ebitmap bits.

Use char for the default range of a class.

Signed-off-by: Christian Göttsche <[email protected]>
---
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


2023-07-06 13:32:06

by Christian Göttsche

[permalink] [raw]
Subject: [RFC PATCH 15/20] selinux: policydb: implicit conversions

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

Signed-off-by: Christian Göttsche <[email protected]>
---
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


2023-07-06 13:32:10

by Christian Göttsche

[permalink] [raw]
Subject: [RFC PATCH 17/20] selinux: services: implicit conversions

Use the type identical to the source for local variables.

Signed-off-by: Christian Göttsche <[email protected]>
---
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


2023-07-06 13:32:51

by Christian Göttsche

[permalink] [raw]
Subject: [RFC PATCH 09/20] selinux: status: consistently use u32 as sequence number type

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 <[email protected]>
---
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


2023-07-06 13:33:24

by Christian Göttsche

[permalink] [raw]
Subject: [RFC PATCH 13/20] selinux: selinuxfs: avoid implicit conversions

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 <[email protected]>
---
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


2023-07-18 22:02:53

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH RFC 1/20] selinux: check for multiplication overflow in put_entry()

On Jul 6, 2023 Gong Ruiqi <[email protected]> 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 <[email protected]>
> ---
> security/selinux/ss/policydb.h | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)

Merged into selinux/next, thanks.

--
paul-moore.com

2023-07-18 22:03:30

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH RFC 10/20] selinux: netif: avoid implicit conversions

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

Merged into selinux/next, thanks.

--
paul-moore.com

2023-07-18 22:03:40

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH RFC 15/20] selinux: policydb: implicit conversions

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

Loop iterators ...

--
paul-moore.com

2023-07-18 22:03:53

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH RFC 16/20] selinux: symtab: implicit conversion

On Jul 6, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <[email protected]> wrote:
>
> hashtab_init() takes an u32 as size parameter type.
>
> Signed-off-by: Christian Göttsche <[email protected]>
> ---
> 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

2023-07-18 22:04:02

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH RFC 17/20] selinux: services: implicit conversions

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

More loop iterators, see my previous comments.

--
paul-moore.com

2023-07-18 22:16:24

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH RFC 13/20] selinux: selinuxfs: avoid implicit conversions

On Jul 6, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <[email protected]> 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 <[email protected]>
> ---
> security/selinux/selinuxfs.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)

No declaration of iterators inside loops please.

--
paul-moore.com

2023-07-18 22:19:54

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH RFC 6/20] selinux: mls: avoid implicit conversions

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

Two words: "loop iterators"

--
paul-moore.com

2023-07-18 22:22:24

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH RFC 9/20] selinux: status: consistently use u32 as sequence number type

On Jul 6, 2023 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <[email protected]> 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 <[email protected]>
> ---
> 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