2022-01-25 20:01:53

by Christian Göttsche

[permalink] [raw]
Subject: [PATCH 2/9] selinux: declare path parameters of _genfs_sid const

The path parameter is only read from in security_genfs_sid(),
selinux_policy_genfs_sid() and __security_genfs_sid(). Since a string
literal is passed as argument, declare the parameter const.
Also align the parameter names in the declaration and definition.

Reported by clang [-Wwrite-strings]:

security/selinux/hooks.c:553:60: error: passing 'const char [2]' to parameter of type 'char *' discards qualifiers [-Wincompatible-pointer-types-discards-qualifiers]
rc = security_genfs_sid(&selinux_state, sb->s_type->name, /,
^~~
./security/selinux/include/security.h:389:36: note: passing argument to parameter 'name' here
const char *fstype, char *name, u16 sclass,
^

Signed-off-by: Christian Göttsche <[email protected]>
---
security/selinux/include/security.h | 4 ++--
security/selinux/ss/services.c | 6 +++---
2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index ac0ece01305a..6482e0efb368 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -386,11 +386,11 @@ int security_get_allow_unknown(struct selinux_state *state);
int security_fs_use(struct selinux_state *state, struct super_block *sb);

int security_genfs_sid(struct selinux_state *state,
- const char *fstype, char *name, u16 sclass,
+ const char *fstype, const char *path, u16 sclass,
u32 *sid);

int selinux_policy_genfs_sid(struct selinux_policy *policy,
- const char *fstype, char *name, u16 sclass,
+ const char *fstype, const char *path, u16 sclass,
u32 *sid);

#ifdef CONFIG_NETLABEL
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 8e92af7dd284..5a7df45bdab1 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -2875,7 +2875,7 @@ int security_get_user_sids(struct selinux_state *state,
*/
static inline int __security_genfs_sid(struct selinux_policy *policy,
const char *fstype,
- char *path,
+ const char *path,
u16 orig_sclass,
u32 *sid)
{
@@ -2928,7 +2928,7 @@ static inline int __security_genfs_sid(struct selinux_policy *policy,
*/
int security_genfs_sid(struct selinux_state *state,
const char *fstype,
- char *path,
+ const char *path,
u16 orig_sclass,
u32 *sid)
{
@@ -2952,7 +2952,7 @@ int security_genfs_sid(struct selinux_state *state,

int selinux_policy_genfs_sid(struct selinux_policy *policy,
const char *fstype,
- char *path,
+ const char *path,
u16 orig_sclass,
u32 *sid)
{
--
2.34.1


2022-01-25 20:02:01

by Christian Göttsche

[permalink] [raw]
Subject: [PATCH 7/9] selinux: do not discard const qualifier in cast

Do not discard the const qualifier on the cast from const void* to
__be32*; the addressed value is not modified.

Reported by clang [-Wcast-qual]

Signed-off-by: Christian Göttsche <[email protected]>
---
security/selinux/netnode.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/security/selinux/netnode.c b/security/selinux/netnode.c
index 4a7d2ab5b960..889552db0d31 100644
--- a/security/selinux/netnode.c
+++ b/security/selinux/netnode.c
@@ -107,7 +107,7 @@ static struct sel_netnode *sel_netnode_find(const void *addr, u16 family)

switch (family) {
case PF_INET:
- idx = sel_netnode_hashfn_ipv4(*(__be32 *)addr);
+ idx = sel_netnode_hashfn_ipv4(*(const __be32 *)addr);
break;
case PF_INET6:
idx = sel_netnode_hashfn_ipv6(addr);
@@ -121,7 +121,7 @@ static struct sel_netnode *sel_netnode_find(const void *addr, u16 family)
if (node->nsec.family == family)
switch (family) {
case PF_INET:
- if (node->nsec.addr.ipv4 == *(__be32 *)addr)
+ if (node->nsec.addr.ipv4 == *(const __be32 *)addr)
return node;
break;
case PF_INET6:
--
2.34.1

2022-01-25 20:02:02

by Christian Göttsche

[permalink] [raw]
Subject: [PATCH 3/9] selinux: declare name parameter of hash_eval const

String literals are passed as second argument to hash_eval(). Also the
parameter is already declared const in the DEBUG_HASHES configuration.

Reported by clang [-Wwrite-strings]:

security/selinux/ss/policydb.c:1881:26: error: passing 'const char [8]' to parameter of type 'char *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
hash_eval(&p->range_tr, rangetr);
^~~~~~~~~
security/selinux/ss/policydb.c:707:55: note: passing argument to parameter 'hash_name' here
static inline void hash_eval(struct hashtab *h, char *hash_name)
^
security/selinux/ss/policydb.c:2099:32: error: passing 'const char [11]' to parameter of type 'char *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
hash_eval(&p->filename_trans, filenametr);
^~~~~~~~~~~~
security/selinux/ss/policydb.c:707:55: note: passing argument to parameter 'hash_name' here
static inline void hash_eval(struct hashtab *h, char *hash_name)
^

Signed-off-by: Christian Göttsche <[email protected]>
---
security/selinux/ss/policydb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index 0ae1b718194a..67e03f6e8966 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -704,7 +704,7 @@ static void symtab_hash_eval(struct symtab *s)
}

#else
-static inline void hash_eval(struct hashtab *h, char *hash_name)
+static inline void hash_eval(struct hashtab *h, const char *hash_name)
{
}
#endif
--
2.34.1

2022-01-25 20:02:06

by Christian Göttsche

[permalink] [raw]
Subject: [PATCH 8/9] selinux: simplify cred_init_security

The parameter of selinux_cred() is declared const, so an explicit cast
dropping the const qualifier is not necessary. Without the cast the
local variable cred serves no purpose.

Reported by clang [-Wcast-qual]

Signed-off-by: Christian Göttsche <[email protected]>
---
security/selinux/hooks.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 5b6895e4fc29..a840c8c1ec35 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -211,10 +211,9 @@ static int selinux_lsm_notifier_avc_callback(u32 event)
*/
static void cred_init_security(void)
{
- struct cred *cred = (struct cred *) current->real_cred;
struct task_security_struct *tsec;

- tsec = selinux_cred(cred);
+ tsec = selinux_cred(current->real_cred);
tsec->osid = tsec->sid = SECINITSID_KERNEL;
}

--
2.34.1

2022-01-25 20:02:40

by Christian Göttsche

[permalink] [raw]
Subject: [PATCH 6/9] selinux: drop unused parameter of avtab_insert_node

The parameter cur is not used in avtab_insert_node().

Reported by clang [-Wunused-parameter]

Signed-off-by: Christian Göttsche <[email protected]>
---
security/selinux/ss/avtab.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/security/selinux/ss/avtab.c b/security/selinux/ss/avtab.c
index c97695ae508f..cfdae20792e1 100644
--- a/security/selinux/ss/avtab.c
+++ b/security/selinux/ss/avtab.c
@@ -67,7 +67,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,
- struct avtab_node *prev, struct avtab_node *cur,
+ struct avtab_node *prev,
const struct avtab_key *key, const struct avtab_datum *datum)
{
struct avtab_node *newnode;
@@ -137,7 +137,7 @@ static int avtab_insert(struct avtab *h, const struct avtab_key *key,
break;
}

- newnode = avtab_insert_node(h, hvalue, prev, cur, key, datum);
+ newnode = avtab_insert_node(h, hvalue, prev, key, datum);
if (!newnode)
return -ENOMEM;

@@ -177,7 +177,7 @@ struct avtab_node *avtab_insert_nonunique(struct avtab *h,
key->target_class < cur->key.target_class)
break;
}
- return avtab_insert_node(h, hvalue, prev, cur, key, datum);
+ return avtab_insert_node(h, hvalue, prev, key, datum);
}

struct avtab_datum *avtab_search(struct avtab *h, const struct avtab_key *key)
--
2.34.1

2022-01-25 20:03:45

by Christian Göttsche

[permalink] [raw]
Subject: [PATCH 5/9] selinux: drop cast to same type

Both the lvalue scontextp and rvalue scontext are of the type char*.
Drop the redundant explicit cast not needed since commit 9a59daa03df7
("SELinux: fix sleeping allocation in security_context_to_sid"), where
the type of scontext changed from const char* to char*.

Signed-off-by: Christian Göttsche <[email protected]>
---
security/selinux/ss/services.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 5a7df45bdab1..2f8db93e53b2 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -1452,7 +1452,7 @@ static int string_to_context_struct(struct policydb *pol,
/* Parse the security context. */

rc = -EINVAL;
- scontextp = (char *) scontext;
+ scontextp = scontext;

/* Extract the user. */
p = scontextp;
--
2.34.1

2022-01-25 20:03:46

by Christian Göttsche

[permalink] [raw]
Subject: [PATCH 1/9] selinux: check return value of sel_make_avc_files

sel_make_avc_files() might fail and return a negative errno value on
memory allocation failures. Re-add the check of the return value,
dropped in 66f8e2f03c02.

Reported by clang-analyzer:

security/selinux/selinuxfs.c:2129:2: warning: Value stored to 'ret' is never read [deadcode.DeadStores]
ret = sel_make_avc_files(dentry);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~

Fixes: 66f8e2f03c02 ("selinux: sidtab reverse lookup hash table")
Signed-off-by: Christian Göttsche <[email protected]>
---
security/selinux/selinuxfs.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index e4cd7cb856f3..f2f6203e0fff 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -2127,6 +2127,8 @@ static int sel_fill_super(struct super_block *sb, struct fs_context *fc)
}

ret = sel_make_avc_files(dentry);
+ if (ret)
+ goto err;

dentry = sel_make_dir(sb->s_root, "ss", &fsi->last_ino);
if (IS_ERR(dentry)) {
--
2.34.1

2022-01-25 20:04:00

by Christian Göttsche

[permalink] [raw]
Subject: [PATCH 9/9] selinux: drop unused macro

The macro _DEBUG_HASHES is nowhere used. The configuration DEBUG_HASHES
enables debugging of the SELinux hash tables, but the with an underscore
prefixed macro definition has no direct impact or any documentation.

Reported by clang [-Wunused-macros]

Signed-off-by: Christian Göttsche <[email protected]>
---
security/selinux/ss/policydb.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index 67e03f6e8966..d036e1238e77 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -41,8 +41,6 @@
#include "mls.h"
#include "services.h"

-#define _DEBUG_HASHES
-
#ifdef DEBUG_HASHES
static const char *symtab_name[SYM_NUM] = {
"common prefixes",
--
2.34.1

2022-01-25 20:05:40

by Christian Göttsche

[permalink] [raw]
Subject: [PATCH 4/9] selinux: enclose macro arguments in parenthesis

Enclose the macro arguments in parenthesis to avoid potential evaluation
order issues.

Note the xperm and ebitmap macros are still not side-effect safe due to
double evaluation.

Reported by clang-tidy [bugprone-macro-parentheses]

Signed-off-by: Christian Göttsche <[email protected]>
---
security/selinux/include/security.h | 4 ++--
security/selinux/ss/ebitmap.h | 6 +++---
security/selinux/ss/sidtab.c | 4 ++--
3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index 6482e0efb368..d91a5672de99 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -254,8 +254,8 @@ struct av_decision {
#define XPERMS_AUDITALLOW 2
#define XPERMS_DONTAUDIT 4

-#define security_xperm_set(perms, x) (perms[x >> 5] |= 1 << (x & 0x1f))
-#define security_xperm_test(perms, x) (1 & (perms[x >> 5] >> (x & 0x1f)))
+#define security_xperm_set(perms, x) ((perms)[(x) >> 5] |= 1 << ((x) & 0x1f))
+#define security_xperm_test(perms, x) (1 & ((perms)[(x) >> 5] >> ((x) & 0x1f)))
struct extended_perms_data {
u32 p[8];
};
diff --git a/security/selinux/ss/ebitmap.h b/security/selinux/ss/ebitmap.h
index 9eb2d0af2805..58eb822f11ee 100644
--- a/security/selinux/ss/ebitmap.h
+++ b/security/selinux/ss/ebitmap.h
@@ -118,9 +118,9 @@ static inline void ebitmap_node_clr_bit(struct ebitmap_node *n,
}

#define ebitmap_for_each_positive_bit(e, n, bit) \
- for (bit = ebitmap_start_positive(e, &n); \
- bit < ebitmap_length(e); \
- bit = ebitmap_next_positive(e, &n, bit)) \
+ for ((bit) = ebitmap_start_positive(e, &(n)); \
+ (bit) < ebitmap_length(e); \
+ (bit) = ebitmap_next_positive(e, &(n), bit)) \

int ebitmap_cmp(struct ebitmap *e1, struct ebitmap *e2);
int ebitmap_cpy(struct ebitmap *dst, struct ebitmap *src);
diff --git a/security/selinux/ss/sidtab.c b/security/selinux/ss/sidtab.c
index 293ec048af08..a54b8652bfb5 100644
--- a/security/selinux/ss/sidtab.c
+++ b/security/selinux/ss/sidtab.c
@@ -27,8 +27,8 @@ struct sidtab_str_cache {
char str[];
};

-#define index_to_sid(index) (index + SECINITSID_NUM + 1)
-#define sid_to_index(sid) (sid - (SECINITSID_NUM + 1))
+#define index_to_sid(index) ((index) + SECINITSID_NUM + 1)
+#define sid_to_index(sid) ((sid) - (SECINITSID_NUM + 1))

int sidtab_init(struct sidtab *s)
{
--
2.34.1

2022-01-26 08:31:53

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH 1/9] selinux: check return value of sel_make_avc_files

On Tue, Jan 25, 2022 at 6:15 AM Christian Göttsche
<[email protected]> wrote:
>
> sel_make_avc_files() might fail and return a negative errno value on
> memory allocation failures. Re-add the check of the return value,
> dropped in 66f8e2f03c02.
>
> Reported by clang-analyzer:
>
> security/selinux/selinuxfs.c:2129:2: warning: Value stored to 'ret' is never read [deadcode.DeadStores]
> ret = sel_make_avc_files(dentry);
> ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Fixes: 66f8e2f03c02 ("selinux: sidtab reverse lookup hash table")
> Signed-off-by: Christian Göttsche <[email protected]>

Reviewed-by: Nick Desaulniers <[email protected]>

> ---
> security/selinux/selinuxfs.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
> index e4cd7cb856f3..f2f6203e0fff 100644
> --- a/security/selinux/selinuxfs.c
> +++ b/security/selinux/selinuxfs.c
> @@ -2127,6 +2127,8 @@ static int sel_fill_super(struct super_block *sb, struct fs_context *fc)
> }
>
> ret = sel_make_avc_files(dentry);
> + if (ret)
> + goto err;
>
> dentry = sel_make_dir(sb->s_root, "ss", &fsi->last_ino);
> if (IS_ERR(dentry)) {
> --
> 2.34.1
>


--
Thanks,
~Nick Desaulniers

2022-01-26 09:42:57

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH 4/9] selinux: enclose macro arguments in parenthesis

On Tue, Jan 25, 2022 at 6:14 AM Christian Göttsche
<[email protected]> wrote:
>
> Enclose the macro arguments in parenthesis to avoid potential evaluation
> order issues.
>
> Note the xperm and ebitmap macros are still not side-effect safe due to
> double evaluation.
>
> Reported by clang-tidy [bugprone-macro-parentheses]

Good idea to run clang-tidy :)
Reviewed-by: Nick Desaulniers <[email protected]>

>
> Signed-off-by: Christian Göttsche <[email protected]>
> ---
> security/selinux/include/security.h | 4 ++--
> security/selinux/ss/ebitmap.h | 6 +++---
> security/selinux/ss/sidtab.c | 4 ++--
> 3 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> index 6482e0efb368..d91a5672de99 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -254,8 +254,8 @@ struct av_decision {
> #define XPERMS_AUDITALLOW 2
> #define XPERMS_DONTAUDIT 4
>
> -#define security_xperm_set(perms, x) (perms[x >> 5] |= 1 << (x & 0x1f))
> -#define security_xperm_test(perms, x) (1 & (perms[x >> 5] >> (x & 0x1f)))
> +#define security_xperm_set(perms, x) ((perms)[(x) >> 5] |= 1 << ((x) & 0x1f))
> +#define security_xperm_test(perms, x) (1 & ((perms)[(x) >> 5] >> ((x) & 0x1f)))
> struct extended_perms_data {
> u32 p[8];
> };
> diff --git a/security/selinux/ss/ebitmap.h b/security/selinux/ss/ebitmap.h
> index 9eb2d0af2805..58eb822f11ee 100644
> --- a/security/selinux/ss/ebitmap.h
> +++ b/security/selinux/ss/ebitmap.h
> @@ -118,9 +118,9 @@ static inline void ebitmap_node_clr_bit(struct ebitmap_node *n,
> }
>
> #define ebitmap_for_each_positive_bit(e, n, bit) \
> - for (bit = ebitmap_start_positive(e, &n); \
> - bit < ebitmap_length(e); \
> - bit = ebitmap_next_positive(e, &n, bit)) \
> + for ((bit) = ebitmap_start_positive(e, &(n)); \
> + (bit) < ebitmap_length(e); \
> + (bit) = ebitmap_next_positive(e, &(n), bit)) \
>
> int ebitmap_cmp(struct ebitmap *e1, struct ebitmap *e2);
> int ebitmap_cpy(struct ebitmap *dst, struct ebitmap *src);
> diff --git a/security/selinux/ss/sidtab.c b/security/selinux/ss/sidtab.c
> index 293ec048af08..a54b8652bfb5 100644
> --- a/security/selinux/ss/sidtab.c
> +++ b/security/selinux/ss/sidtab.c
> @@ -27,8 +27,8 @@ struct sidtab_str_cache {
> char str[];
> };
>
> -#define index_to_sid(index) (index + SECINITSID_NUM + 1)
> -#define sid_to_index(sid) (sid - (SECINITSID_NUM + 1))
> +#define index_to_sid(index) ((index) + SECINITSID_NUM + 1)
> +#define sid_to_index(sid) ((sid) - (SECINITSID_NUM + 1))
>
> int sidtab_init(struct sidtab *s)
> {
> --
> 2.34.1
>


--
Thanks,
~Nick Desaulniers

2022-01-26 12:37:48

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH 9/9] selinux: drop unused macro

On Tue, Jan 25, 2022 at 6:15 AM Christian Göttsche
<[email protected]> wrote:
>
> The macro _DEBUG_HASHES is nowhere used. The configuration DEBUG_HASHES
> enables debugging of the SELinux hash tables, but the with an underscore
> prefixed macro definition has no direct impact or any documentation.
>
> Reported by clang [-Wunused-macros]
>
> Signed-off-by: Christian Göttsche <[email protected]>

Reviewed-by: Nick Desaulniers <[email protected]>

> ---
> security/selinux/ss/policydb.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index 67e03f6e8966..d036e1238e77 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -41,8 +41,6 @@
> #include "mls.h"
> #include "services.h"
>
> -#define _DEBUG_HASHES
> -
> #ifdef DEBUG_HASHES
> static const char *symtab_name[SYM_NUM] = {
> "common prefixes",
> --
> 2.34.1
>


--
Thanks,
~Nick Desaulniers

2022-01-26 13:45:31

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH 1/9] selinux: check return value of sel_make_avc_files

On Tue, Jan 25, 2022 at 9:15 AM Christian Göttsche
<[email protected]> wrote:
>
> sel_make_avc_files() might fail and return a negative errno value on
> memory allocation failures. Re-add the check of the return value,
> dropped in 66f8e2f03c02.
>
> Reported by clang-analyzer:
>
> security/selinux/selinuxfs.c:2129:2: warning: Value stored to 'ret' is never read [deadcode.DeadStores]
> ret = sel_make_avc_files(dentry);
> ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Fixes: 66f8e2f03c02 ("selinux: sidtab reverse lookup hash table")
> Signed-off-by: Christian Göttsche <[email protected]>
> ---
> security/selinux/selinuxfs.c | 2 ++
> 1 file changed, 2 insertions(+)

Merged into selinux/next, thanks Christian.

--
paul-moore.com

2022-01-26 14:15:55

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH 2/9] selinux: declare path parameters of _genfs_sid const

On Tue, Jan 25, 2022 at 9:14 AM Christian Göttsche
<[email protected]> wrote:
>
> The path parameter is only read from in security_genfs_sid(),
> selinux_policy_genfs_sid() and __security_genfs_sid(). Since a string
> literal is passed as argument, declare the parameter const.
> Also align the parameter names in the declaration and definition.
>
> Reported by clang [-Wwrite-strings]:
>
> security/selinux/hooks.c:553:60: error: passing 'const char [2]' to parameter of type 'char *' discards qualifiers [-Wincompatible-pointer-types-discards-qualifiers]
> rc = security_genfs_sid(&selinux_state, sb->s_type->name, /,
> ^~~
> ./security/selinux/include/security.h:389:36: note: passing argument to parameter 'name' here
> const char *fstype, char *name, u16 sclass,
> ^
>
> Signed-off-by: Christian Göttsche <[email protected]>
> ---
> security/selinux/include/security.h | 4 ++--
> security/selinux/ss/services.c | 6 +++---
> 2 files changed, 5 insertions(+), 5 deletions(-)

Merged, thanks!

--
paul-moore.com

2022-01-26 22:32:30

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH 3/9] selinux: declare name parameter of hash_eval const

On Tue, Jan 25, 2022 at 9:14 AM Christian Göttsche
<[email protected]> wrote:
>
> String literals are passed as second argument to hash_eval(). Also the
> parameter is already declared const in the DEBUG_HASHES configuration.
>
> Reported by clang [-Wwrite-strings]:
>
> security/selinux/ss/policydb.c:1881:26: error: passing 'const char [8]' to parameter of type 'char *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
> hash_eval(&p->range_tr, rangetr);
> ^~~~~~~~~
> security/selinux/ss/policydb.c:707:55: note: passing argument to parameter 'hash_name' here
> static inline void hash_eval(struct hashtab *h, char *hash_name)
> ^
> security/selinux/ss/policydb.c:2099:32: error: passing 'const char [11]' to parameter of type 'char *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
> hash_eval(&p->filename_trans, filenametr);
> ^~~~~~~~~~~~
> security/selinux/ss/policydb.c:707:55: note: passing argument to parameter 'hash_name' here
> static inline void hash_eval(struct hashtab *h, char *hash_name)
> ^
>
> Signed-off-by: Christian Göttsche <[email protected]>
> ---
> security/selinux/ss/policydb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

Merged into selinux/next, thanks!

--
paul-moore.com

2022-01-26 22:37:48

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH 4/9] selinux: enclose macro arguments in parenthesis

On Tue, Jan 25, 2022 at 9:14 AM Christian Göttsche
<[email protected]> wrote:
>
> Enclose the macro arguments in parenthesis to avoid potential evaluation
> order issues.
>
> Note the xperm and ebitmap macros are still not side-effect safe due to
> double evaluation.
>
> Reported by clang-tidy [bugprone-macro-parentheses]
>
> Signed-off-by: Christian Göttsche <[email protected]>
> ---
> security/selinux/include/security.h | 4 ++--
> security/selinux/ss/ebitmap.h | 6 +++---
> security/selinux/ss/sidtab.c | 4 ++--
> 3 files changed, 7 insertions(+), 7 deletions(-)

Merged, thanks.

--
paul-moore.com

2022-01-26 22:40:12

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH 5/9] selinux: drop cast to same type

On Tue, Jan 25, 2022 at 9:14 AM Christian Göttsche
<[email protected]> wrote:
>
> Both the lvalue scontextp and rvalue scontext are of the type char*.
> Drop the redundant explicit cast not needed since commit 9a59daa03df7
> ("SELinux: fix sleeping allocation in security_context_to_sid"), where
> the type of scontext changed from const char* to char*.
>
> Signed-off-by: Christian Göttsche <[email protected]>
> ---
> security/selinux/ss/services.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

Merged, thanks.

--
paul-moore.com

2022-01-26 22:44:59

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH 6/9] selinux: drop unused parameter of avtab_insert_node

On Tue, Jan 25, 2022 at 9:14 AM Christian Göttsche
<[email protected]> wrote:
>
> The parameter cur is not used in avtab_insert_node().
>
> Reported by clang [-Wunused-parameter]
>
> Signed-off-by: Christian Göttsche <[email protected]>
> ---
> security/selinux/ss/avtab.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)

Merged, thanks.

--
paul-moore.com

2022-01-27 00:38:44

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH 8/9] selinux: simplify cred_init_security

On Tue, Jan 25, 2022 at 9:15 AM Christian Göttsche
<[email protected]> wrote:
>
> The parameter of selinux_cred() is declared const, so an explicit cast
> dropping the const qualifier is not necessary. Without the cast the
> local variable cred serves no purpose.
>
> Reported by clang [-Wcast-qual]
>
> Signed-off-by: Christian Göttsche <[email protected]>
> ---
> security/selinux/hooks.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)

Merged, thanks.

--
paul-moore.com

2022-01-27 00:38:48

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH 7/9] selinux: do not discard const qualifier in cast

On Tue, Jan 25, 2022 at 9:14 AM Christian Göttsche
<[email protected]> wrote:
>
> Do not discard the const qualifier on the cast from const void* to
> __be32*; the addressed value is not modified.
>
> Reported by clang [-Wcast-qual]
>
> Signed-off-by: Christian Göttsche <[email protected]>
> ---
> security/selinux/netnode.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)

Merged, thanks.

--
paul-moore.com

2022-01-27 01:13:56

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH 9/9] selinux: drop unused macro

On Tue, Jan 25, 2022 at 9:15 AM Christian Göttsche
<[email protected]> wrote:
>
> The macro _DEBUG_HASHES is nowhere used. The configuration DEBUG_HASHES
> enables debugging of the SELinux hash tables, but the with an underscore
> prefixed macro definition has no direct impact or any documentation.
>
> Reported by clang [-Wunused-macros]
>
> Signed-off-by: Christian Göttsche <[email protected]>
> ---
> security/selinux/ss/policydb.c | 2 --
> 1 file changed, 2 deletions(-)

Merged into selinux/next, thanks Christian. This macro definition
predates the move to git so there is no quick answer to "why is this
here?", but my best guess is that it is an artifact of a developer
"disabling" the DEBUG_HASHES macro by adding an underscore to the
front. Regardless of the reason behind it, I agree it should be
removed.

--
paul-moore.com