2023-07-06 13:54:04

by Christian Göttsche

Subject: [RFC PATCH] selinux: disable debug functions by default

avtab_hash_eval() and hashtab_stat() are only used in policydb.c when
the debug macro DEBUG_HASHES is defined.

Signed-off-by: Christian Göttsche <[email protected]>
---
security/selinux/ss/avtab.c | 2 ++
security/selinux/ss/avtab.h | 3 +++
security/selinux/ss/hashtab.c | 3 ++-
security/selinux/ss/hashtab.h | 2 ++
4 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/security/selinux/ss/avtab.c b/security/selinux/ss/avtab.c
index 6766edc0fe68..2fd1a21b4428 100644
--- a/security/selinux/ss/avtab.c
+++ b/security/selinux/ss/avtab.c
@@ -354,6 +354,7 @@ int avtab_alloc_dup(struct avtab *new, const struct avtab *orig)
return avtab_alloc_common(new, orig->nslot);
}

+#ifdef DEBUG_HASHES
void avtab_hash_eval(struct avtab *h, const char *tag)
{
int i, chain_len, slots_used, max_chain_len;
@@ -384,6 +385,7 @@ void avtab_hash_eval(struct avtab *h, const char *tag)
tag, h->nel, slots_used, h->nslot, max_chain_len,
chain2_len_sum);
}
+#endif

static const uint16_t spec_order[] = {
AVTAB_ALLOWED,
diff --git a/security/selinux/ss/avtab.h b/security/selinux/ss/avtab.h
index d6742fd9c560..66c9077b7098 100644
--- a/security/selinux/ss/avtab.h
+++ b/security/selinux/ss/avtab.h
@@ -92,7 +92,10 @@ int avtab_alloc(struct avtab *, u32);
int avtab_alloc_dup(struct avtab *new, const struct avtab *orig);
struct avtab_datum *avtab_search(struct avtab *h, const struct avtab_key *k);
void avtab_destroy(struct avtab *h);
+
+#ifdef DEBUG_HASHES
void avtab_hash_eval(struct avtab *h, const char *tag);
+#endif

struct policydb;
int avtab_read_item(struct avtab *a, void *fp, struct policydb *pol,
diff --git a/security/selinux/ss/hashtab.c b/security/selinux/ss/hashtab.c
index 3fb8f9026e9b..672ea20ad1bb 100644
--- a/security/selinux/ss/hashtab.c
+++ b/security/selinux/ss/hashtab.c
@@ -103,7 +103,7 @@ int hashtab_map(struct hashtab *h,
return 0;
}

-
+#ifdef DEBUG_HASHES
void hashtab_stat(struct hashtab *h, struct hashtab_info *info)
{
u32 i, chain_len, slots_used, max_chain_len;
@@ -129,6 +129,7 @@ void hashtab_stat(struct hashtab *h, struct hashtab_info *info)
info->slots_used = slots_used;
info->max_chain_len = max_chain_len;
}
+#endif

int hashtab_duplicate(struct hashtab *new, struct hashtab *orig,
int (*copy)(struct hashtab_node *new,
diff --git a/security/selinux/ss/hashtab.h b/security/selinux/ss/hashtab.h
index 043a773bf0b7..64010a7f01a1 100644
--- a/security/selinux/ss/hashtab.h
+++ b/security/selinux/ss/hashtab.h
@@ -143,6 +143,8 @@ int hashtab_duplicate(struct hashtab *new, struct hashtab *orig,
void *args);

/* Fill info with some hash table statistics */
+#ifdef DEBUG_HASHES
void hashtab_stat(struct hashtab *h, struct hashtab_info *info);
+#endif

#endif /* _SS_HASHTAB_H */
--
2.40.1

2023-07-17 20:59:39

by Paul Moore

Subject: Re: [RFC PATCH] selinux: disable debug functions by default

On Thu, Jul 6, 2023 at 9:37 AM Christian Göttsche
<[email protected]> wrote:
>
> avtab_hash_eval() and hashtab_stat() are only used in policydb.c when
> the debug macro DEBUG_HASHES is defined.
>
> Signed-off-by: Christian Göttsche <[email protected]>
> ---
> security/selinux/ss/avtab.c | 2 ++
> security/selinux/ss/avtab.h | 3 +++
> security/selinux/ss/hashtab.c | 3 ++-
> security/selinux/ss/hashtab.h | 2 ++
> 4 files changed, 9 insertions(+), 1 deletion(-)

This reminds me that I don't really like the "hidden" and kludgy
nature of DEBUG_HASHES. What if we created a proper SELinux debug
Kconfig flag and used it in place of DEBUG_HASHES? I'm thinking of
something like this:

config SECURITY_SELINUX_DEBUG
bool "NSA SELinux kernel debugging support"
depends on SECURITY_SELINUX
default n
help
This enables debugging code designed to help SELinux kernel developers,
unless you know what this does in the kernel code you should leave this
disabled.

... and then we do all of the usual Kconfig triggered dummy funcs,
etc. Thoughts?

--
paul-moore.com