selinux: declare path parameters of _genfs_sid const

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

security/selinux/selinuxfs.c +2 -1

index e4cd7cb856f3..f2f6203e0fff 100644
@@ -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

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

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

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

security/selinux/include/security.h +2 -2

index 6482e0efb368..d91a5672de99 100644
@@ -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];
 };

security/selinux/ss/ebitmap.h +3 -3

index 9eb2d0af2805..58eb822f11ee 100644
@@ -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);

security/selinux/ss/sidtab.c +2 -3

index 293ec048af08..a54b8652bfb5 100644
@@ -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

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

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

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

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

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

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

security/selinux/ss/policydb.c +0 -3

index 67e03f6e8966..d036e1238e77 100644
@@ -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

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

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