Reported by sparse:
security/selinux/selinuxfs.c:1483:30: warning: incorrect type in assignment (different signedness)
security/selinux/selinuxfs.c:1483:30: expected unsigned int *
security/selinux/selinuxfs.c:1483:30: got int *[addressable] values
security/selinux/selinuxfs.c:1400:48: warning: incorrect type in argument 3 (different signedness)
security/selinux/selinuxfs.c:1400:48: expected int *values
security/selinux/selinuxfs.c:1400:48: got unsigned int *bool_pending_values
Also mark the read-only boolean array parameter of security_set_bools()
const.
Signed-off-by: Christian Göttsche <[email protected]>
---
security/selinux/include/conditional.h | 4 ++--
security/selinux/selinuxfs.c | 12 ++++++------
security/selinux/ss/policydb.h | 2 +-
security/selinux/ss/services.c | 13 +++++++------
4 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/security/selinux/include/conditional.h b/security/selinux/include/conditional.h
index b09343346e3f..f1b52115e0a3 100644
--- a/security/selinux/include/conditional.h
+++ b/security/selinux/include/conditional.h
@@ -14,9 +14,9 @@
#include "security.h"
int security_get_bools(struct selinux_policy *policy,
- u32 *len, char ***names, int **values);
+ u32 *len, char ***names, unsigned char **values);
-int security_set_bools(struct selinux_state *state, u32 len, int *values);
+int security_set_bools(struct selinux_state *state, u32 len, const unsigned char *values);
int security_get_bool_value(struct selinux_state *state, u32 index);
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index 8fcdd494af27..404b4561f8b0 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -71,7 +71,7 @@ struct selinux_fs_info {
struct dentry *bool_dir;
unsigned int bool_num;
char **bool_pending_names;
- unsigned int *bool_pending_values;
+ unsigned char *bool_pending_values;
struct dentry *class_dir;
unsigned long last_class_ino;
bool policy_opened;
@@ -356,7 +356,7 @@ static const struct file_operations sel_policyvers_ops = {
/* declaration for sel_write_load */
static int sel_make_bools(struct selinux_policy *newpolicy, struct dentry *bool_dir,
unsigned int *bool_num, char ***bool_pending_names,
- unsigned int **bool_pending_values);
+ unsigned char **bool_pending_values);
static int sel_make_classes(struct selinux_policy *newpolicy,
struct dentry *class_dir,
unsigned long *last_class_ino);
@@ -527,7 +527,7 @@ static const struct file_operations sel_policy_ops = {
};
static void sel_remove_old_bool_data(unsigned int bool_num, char **bool_names,
- unsigned int *bool_values)
+ unsigned char *bool_values)
{
u32 i;
@@ -545,7 +545,7 @@ static int sel_make_policy_nodes(struct selinux_fs_info *fsi,
struct dentry *tmp_parent, *tmp_bool_dir, *tmp_class_dir, *old_dentry;
unsigned int tmp_bool_num, old_bool_num;
char **tmp_bool_names, **old_bool_names;
- unsigned int *tmp_bool_values, *old_bool_values;
+ unsigned char *tmp_bool_values, *old_bool_values;
unsigned long tmp_ino = fsi->last_ino; /* Don't increment last_ino in this function */
tmp_parent = sel_make_disconnected_dir(fsi->sb, &tmp_ino);
@@ -1423,7 +1423,7 @@ static void sel_remove_entries(struct dentry *de)
static int sel_make_bools(struct selinux_policy *newpolicy, struct dentry *bool_dir,
unsigned int *bool_num, char ***bool_pending_names,
- unsigned int **bool_pending_values)
+ unsigned char **bool_pending_values)
{
int ret;
ssize_t len;
@@ -1432,7 +1432,7 @@ static int sel_make_bools(struct selinux_policy *newpolicy, struct dentry *bool_
struct inode_security_struct *isec;
char **names = NULL, *page;
u32 i, num;
- int *values = NULL;
+ unsigned char *values = NULL;
u32 sid;
ret = -ENOMEM;
diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
index c24d4e1063ea..f85e875a7799 100644
--- a/security/selinux/ss/policydb.h
+++ b/security/selinux/ss/policydb.h
@@ -148,7 +148,7 @@ struct range_trans {
/* Boolean data type */
struct cond_bool_datum {
__u32 value; /* internal type value */
- int state;
+ unsigned char state;
};
struct cond_node;
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 802a80648c6c..7349ed4a4d0d 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -3022,7 +3022,7 @@ int security_fs_use(struct selinux_state *state, struct super_block *sb)
}
int security_get_bools(struct selinux_policy *policy,
- u32 *len, char ***names, int **values)
+ u32 *len, char ***names, unsigned char **values)
{
struct policydb *policydb;
u32 i;
@@ -3044,7 +3044,7 @@ int security_get_bools(struct selinux_policy *policy,
goto err;
rc = -ENOMEM;
- *values = kcalloc(*len, sizeof(int), GFP_ATOMIC);
+ *values = kcalloc(*len, sizeof(unsigned char), GFP_ATOMIC);
if (!*values)
goto err;
@@ -3074,7 +3074,7 @@ int security_get_bools(struct selinux_policy *policy,
}
-int security_set_bools(struct selinux_state *state, u32 len, int *values)
+int security_set_bools(struct selinux_state *state, u32 len, const unsigned char *values)
{
struct selinux_policy *newpolicy, *oldpolicy;
int rc;
@@ -3106,8 +3106,8 @@ int security_set_bools(struct selinux_state *state, u32 len, int *values)
/* Update the boolean states in the copy */
for (i = 0; i < len; i++) {
- int new_state = !!values[i];
- int old_state = newpolicy->policydb.bool_val_to_struct[i]->state;
+ unsigned char new_state = !!values[i];
+ unsigned char old_state = newpolicy->policydb.bool_val_to_struct[i]->state;
if (new_state != old_state) {
audit_log(audit_context(), GFP_ATOMIC,
@@ -3174,7 +3174,8 @@ int security_get_bool_value(struct selinux_state *state,
static int security_preserve_bools(struct selinux_policy *oldpolicy,
struct selinux_policy *newpolicy)
{
- int rc, *bvalues = NULL;
+ int rc;
+ unsigned char *bvalues = NULL;
char **bnames = NULL;
struct cond_bool_datum *booldatum;
u32 i, nbools = 0;
--
2.36.0
On Mon, May 2, 2022 at 9:59 AM Christian Göttsche
<[email protected]> wrote:
>
> Reported by sparse:
>
> security/selinux/selinuxfs.c:1483:30: warning: incorrect type in assignment (different signedness)
> security/selinux/selinuxfs.c:1483:30: expected unsigned int *
> security/selinux/selinuxfs.c:1483:30: got int *[addressable] values
> security/selinux/selinuxfs.c:1400:48: warning: incorrect type in argument 3 (different signedness)
> security/selinux/selinuxfs.c:1400:48: expected int *values
> security/selinux/selinuxfs.c:1400:48: got unsigned int *bool_pending_values
>
> Also mark the read-only boolean array parameter of security_set_bools()
> const.
>
> Signed-off-by: Christian Göttsche <[email protected]>
> ---
> security/selinux/include/conditional.h | 4 ++--
> security/selinux/selinuxfs.c | 12 ++++++------
> security/selinux/ss/policydb.h | 2 +-
> security/selinux/ss/services.c | 13 +++++++------
> 4 files changed, 16 insertions(+), 15 deletions(-)
I could understand fixing the signed/unsigned type mismatch, but I
don't quite understand the move from an int type to a char; is it
simply to save space, i.e. 32-bits vs 8-bits? I think I would prefer
either simply fixing the signed/unsigned mismatch and leaving the
booleans as ints, or moving completely to a bool type, although that
is likely to be much more involved.
--
paul-moore.com