selinux: prepare for inlining of hashtab functions

Refactor searching and inserting into hashtabs to pave the way for
converting hashtab_search() and hashtab_insert() to inline functions in
the next patch. This will avoid indirect calls and allow the compiler to
better optimize individual callers, leading to a significant performance
improvement.

In order to avoid the indirect calls, the key hashing and comparison
callbacks need to be extracted from the hashtab struct and passed
directly to hashtab_search()/_insert() by the callers so that the
callback address is always known at compile time. The kernel's
rhashtable library (<linux/rhashtable*.h>) does the same thing.

This of course makes the hashtab functions slightly easier to misuse by
passing a wrong callback set, but unfortunately there is no better way
to implement a hash table that is both generic and efficient in C. This
patch tries to somewhat mitigate this by only calling the hashtab
functions in the same file where the corresponding callbacks are
defined (wrapping them into more specialized functions as needed).

Note that this patch doesn't bring any benefit without also moving the
definitions of hashtab_search() and -_insert() to the header file, which
is done in a follow-up patch for easier review of the hashtab.c changes
in this patch.

Signed-off-by: Ondrej Mosnacek <[e-mail]>
Acked-by: Stephen Smalley <[e-mail]>
Signed-off-by: Paul Moore <[e-mail]>

security/selinux/ss/hashtab.c

···
 	return nel == 0 ? 0 : roundup_pow_of_two(nel);
 }
 
-int hashtab_init(struct hashtab *h,
-		 u32 (*hash_value)(struct hashtab *h, const void *key),
-		 int (*keycmp)(struct hashtab *h, const void *key1,
-			       const void *key2),
-		 u32 nel_hint)
+int hashtab_init(struct hashtab *h, u32 nel_hint)
 {
 	h->size = hashtab_compute_size(nel_hint);
 	h->nel = 0;
-	h->hash_value = hash_value;
-	h->keycmp = keycmp;
 	if (!h->size)
 		return 0;
 
···
 	return h->htable ? 0 : -ENOMEM;
 }
 
-int hashtab_insert(struct hashtab *h, void *key, void *datum)
+int hashtab_insert(struct hashtab *h, void *key, void *datum,
+		   struct hashtab_key_params key_params)
 {
 	u32 hvalue;
 	struct hashtab_node *prev, *cur, *newnode;
···
 	if (!h->size || h->nel == HASHTAB_MAX_NODES)
 		return -EINVAL;
 
-	hvalue = h->hash_value(h, key);
+	hvalue = key_params.hash(key) & (h->size - 1);
 	prev = NULL;
 	cur = h->htable[hvalue];
-	while (cur && h->keycmp(h, key, cur->key) > 0) {
+	while (cur) {
+		int cmp = key_params.cmp(key, cur->key);
+
+		if (cmp == 0)
+			return -EEXIST;
+		if (cmp < 0)
+			break;
 		prev = cur;
 		cur = cur->next;
 	}
-
-	if (cur && (h->keycmp(h, key, cur->key) == 0))
-		return -EEXIST;
 
 	newnode = kmem_cache_zalloc(hashtab_node_cachep, GFP_KERNEL);
 	if (!newnode)
···
 	return 0;
 }
 
-void *hashtab_search(struct hashtab *h, const void *key)
+void *hashtab_search(struct hashtab *h, const void *key,
+		     struct hashtab_key_params key_params)
 {
 	u32 hvalue;
 	struct hashtab_node *cur;
···
 	if (!h->size)
 		return NULL;
 
-	hvalue = h->hash_value(h, key);
+	hvalue = key_params.hash(key) & (h->size - 1);
 	cur = h->htable[hvalue];
-	while (cur && h->keycmp(h, key, cur->key) > 0)
+	while (cur) {
+		int cmp = key_params.cmp(key, cur->key);
+
+		if (cmp == 0)
+			return cur->datum;
+		if (cmp < 0)
+			break;
 		cur = cur->next;
-
-	if (!cur || (h->keycmp(h, key, cur->key) != 0))
-		return NULL;
-
-	return cur->datum;
+	}
+	return NULL;
 }
 
 void hashtab_destroy(struct hashtab *h)

security/selinux/ss/hashtab.h

···
 
 #define HASHTAB_MAX_NODES	0xffffffff
 
+struct hashtab_key_params {
+	u32 (*hash)(const void *key);	/* hash function */
+	int (*cmp)(const void *key1, const void *key2);
+					/* key comparison function */
+};
+
 struct hashtab_node {
 	void *key;
 	void *datum;
···
 	struct hashtab_node **htable;	/* hash table */
 	u32 size;			/* number of slots in hash table */
 	u32 nel;			/* number of elements in hash table */
-	u32 (*hash_value)(struct hashtab *h, const void *key);
-					/* hash function */
-	int (*keycmp)(struct hashtab *h, const void *key1, const void *key2);
-					/* key comparison function */
 };
 
 struct hashtab_info {
···
  *
  * Returns -ENOMEM if insufficient space is available or 0 otherwise.
  */
-int hashtab_init(struct hashtab *h,
-		 u32 (*hash_value)(struct hashtab *h, const void *key),
-		 int (*keycmp)(struct hashtab *h, const void *key1,
-			       const void *key2),
-		 u32 nel_hint);
+int hashtab_init(struct hashtab *h, u32 nel_hint);
 
 /*
  * Inserts the specified (key, datum) pair into the specified hash table.
···
  * -EINVAL for general errors or
   0 otherwise.
  */
-int hashtab_insert(struct hashtab *h, void *k, void *d);
+int hashtab_insert(struct hashtab *h, void *k, void *d,
+		   struct hashtab_key_params key_params);
 
 /*
  * Searches for the entry with the specified key in the hash table.
···
  * Returns NULL if no entry has the specified key or
  * the datum of the entry otherwise.
  */
-void *hashtab_search(struct hashtab *h, const void *k);
+void *hashtab_search(struct hashtab *h, const void *k,
+		     struct hashtab_key_params key_params);
 
 /*
  * Destroys the specified hash table.

security/selinux/ss/mls.c

···
 		rtr.source_type = scontext->type;
 		rtr.target_type = tcontext->type;
 		rtr.target_class = tclass;
-		r = hashtab_search(&p->range_tr, &rtr);
+		r = policydb_rangetr_search(p, &rtr);
 		if (r)
 			return mls_range_set(newcontext, r);
 

security/selinux/ss/policydb.c

···
 	return rc;
 }
 
-static u32 filenametr_hash(struct hashtab *h, const void *k)
+static u32 filenametr_hash(const void *k)
 {
 	const struct filename_trans_key *ft = k;
 	unsigned long hash;
···
 	byte_num = 0;
 	while ((focus = ft->name[byte_num++]))
 		hash = partial_name_hash(focus, hash);
-	return hash & (h->size - 1);
+	return hash;
 }
 
-static int filenametr_cmp(struct hashtab *h, const void *k1, const void *k2)
+static int filenametr_cmp(const void *k1, const void *k2)
 {
 	const struct filename_trans_key *ft1 = k1;
 	const struct filename_trans_key *ft2 = k2;
···
 
 }
 
-static u32 rangetr_hash(struct hashtab *h, const void *k)
+static const struct hashtab_key_params filenametr_key_params = {
+	.hash = filenametr_hash,
+	.cmp = filenametr_cmp,
+};
+
+struct filename_trans_datum *policydb_filenametr_search(
+	struct policydb *p, struct filename_trans_key *key)
+{
+	return hashtab_search(&p->filename_trans, key, filenametr_key_params);
+}
+
+static u32 rangetr_hash(const void *k)
 {
 	const struct range_trans *key = k;
 
-	return (key->source_type + (key->target_type << 3) +
-		(key->target_class << 5)) & (h->size - 1);
+	return key->source_type + (key->target_type << 3) +
+		(key->target_class << 5);
 }
 
-static int rangetr_cmp(struct hashtab *h, const void *k1, const void *k2)
+static int rangetr_cmp(const void *k1, const void *k2)
 {
 	const struct range_trans *key1 = k1, *key2 = k2;
 	int v;
···
 	return v;
 }
 
-static u32 role_trans_hash(struct hashtab *h, const void *k)
+static const struct hashtab_key_params rangetr_key_params = {
+	.hash = rangetr_hash,
+	.cmp = rangetr_cmp,
+};
+
+struct mls_range *policydb_rangetr_search(struct policydb *p,
+					  struct range_trans *key)
+{
+	return hashtab_search(&p->range_tr, key, rangetr_key_params);
+}
+
+static u32 role_trans_hash(const void *k)
 {
 	const struct role_trans_key *key = k;
 
-	return (key->role + (key->type << 3) + (key->tclass << 5)) &
-		(h->size - 1);
+	return key->role + (key->type << 3) + (key->tclass << 5);
 }
 
-static int role_trans_cmp(struct hashtab *h, const void *k1, const void *k2)
+static int role_trans_cmp(const void *k1, const void *k2)
 {
 	const struct role_trans_key *key1 = k1, *key2 = k2;
 	int v;
···
 		return v;
 
 	return key1->tclass - key2->tclass;
+}
+
+static const struct hashtab_key_params roletr_key_params = {
+	.hash = role_trans_hash,
+	.cmp = role_trans_cmp,
+};
+
+struct role_trans_datum *policydb_roletr_search(struct policydb *p,
+						struct role_trans_key *key)
+{
+	return hashtab_search(&p->role_tr, key, roletr_key_params);
 }
 
 /*
···
 
 	nel = le32_to_cpu(buf[0]);
 
-	rc = hashtab_init(&p->range_tr, rangetr_hash, rangetr_cmp, nel);
+	rc = hashtab_init(&p->range_tr, nel);
 	if (rc)
 		return rc;
 
···
 			goto out;
 		}
 
-		rc = hashtab_insert(&p->range_tr, rt, r);
+		rc = hashtab_insert(&p->range_tr, rt, r, rangetr_key_params);
 		if (rc)
 			goto out;
 
···
 	otype = le32_to_cpu(buf[3]);
 
 	last = NULL;
-	datum = hashtab_search(&p->filename_trans, &key);
+	datum = policydb_filenametr_search(p, &key);
 	while (datum) {
 		if (unlikely(ebitmap_get_bit(&datum->stypes, stype - 1))) {
 			/* conflicting/duplicate rules are ignored */
···
 			if (!ft)
 				goto out;
 
-			rc = hashtab_insert(&p->filename_trans, ft, datum);
+			rc = hashtab_insert(&p->filename_trans, ft, datum,
+					    filenametr_key_params);
 			if (rc)
 				goto out;
 			name = NULL;
···
 	ft->tclass = tclass;
 	ft->name = name;
 
-	rc = hashtab_insert(&p->filename_trans, ft, first);
+	rc = hashtab_insert(&p->filename_trans, ft, first,
+			    filenametr_key_params);
 	if (rc == -EEXIST)
 		pr_err("SELinux:  Duplicate filename transition key\n");
 	if (rc)
···
 	if (p->policyvers < POLICYDB_VERSION_COMP_FTRANS) {
 		p->compat_filename_trans_count = nel;
 
-		rc = hashtab_init(&p->filename_trans, filenametr_hash,
-				  filenametr_cmp, (1 << 11));
+		rc = hashtab_init(&p->filename_trans, (1 << 11));
 		if (rc)
 			return rc;
 
···
 				return rc;
 		}
 	} else {
-		rc = hashtab_init(&p->filename_trans, filenametr_hash,
-				  filenametr_cmp, nel);
+		rc = hashtab_init(&p->filename_trans, nel);
 		if (rc)
 			return rc;
 
···
 		goto bad;
 	nel = le32_to_cpu(buf[0]);
 
-	rc = hashtab_init(&p->role_tr, role_trans_hash, role_trans_cmp, nel);
+	rc = hashtab_init(&p->role_tr, nel);
 	if (rc)
 		goto bad;
 	for (i = 0; i < nel; i++) {
···
 		    !policydb_role_isvalid(p, rtd->new_role))
 			goto bad;
 
-		rc = hashtab_insert(&p->role_tr, rtk, rtd);
+		rc = hashtab_insert(&p->role_tr, rtk, rtd, roletr_key_params);
 		if (rc)
 			goto bad;
 

security/selinux/ss/policydb.h

···
 extern int policydb_read(struct policydb *p, void *fp);
 extern int policydb_write(struct policydb *p, void *fp);
 
+extern struct filename_trans_datum *policydb_filenametr_search(
+	struct policydb *p, struct filename_trans_key *key);
+
+extern struct mls_range *policydb_rangetr_search(
+	struct policydb *p, struct range_trans *key);
+
+extern struct role_trans_datum *policydb_roletr_search(
+	struct policydb *p, struct role_trans_key *key);
+
 #define POLICYDB_CONFIG_MLS    1
 
 /* the config flags related to unknown classes/perms are bits 2 and 3 */

security/selinux/ss/services.c

···
 	ft.tclass = tclass;
 	ft.name = objname;
 
-	datum = hashtab_search(&policydb->filename_trans, &ft);
+	datum = policydb_filenametr_search(policydb, &ft);
 	while (datum) {
 		if (ebitmap_get_bit(&datum->stypes, stype - 1)) {
 			newcontext->type = datum->otype;
···
 			.tclass = tclass,
 		};
 
-		rtd = hashtab_search(&policydb->role_tr, &rtk);
+		rtd = policydb_roletr_search(policydb, &rtk);
 		if (rtd)
 			newcontext.role = rtd->new_role;
 	}

security/selinux/ss/symtab.c

···
 #include <linux/errno.h>
 #include "symtab.h"
 
-static unsigned int symhash(struct hashtab *h, const void *key)
+static unsigned int symhash(const void *key)
 {
 	const char *p, *keyp;
 	unsigned int size;
···
 	size = strlen(keyp);
 	for (p = keyp; (p - keyp) < size; p++)
 		val = (val << 4 | (val >> (8*sizeof(unsigned int)-4))) ^ (*p);
-	return val & (h->size - 1);
+	return val;
 }
 
-static int symcmp(struct hashtab *h, const void *key1, const void *key2)
+static int symcmp(const void *key1, const void *key2)
 {
 	const char *keyp1, *keyp2;
 
···
 	return strcmp(keyp1, keyp2);
 }
 
+static const struct hashtab_key_params symtab_key_params = {
+	.hash = symhash,
+	.cmp = symcmp,
+};
 
 int symtab_init(struct symtab *s, unsigned int size)
 {
 	s->nprim = 0;
-	return hashtab_init(&s->table, symhash, symcmp, size);
+	return hashtab_init(&s->table, size);
 }
 
 int symtab_insert(struct symtab *s, char *name, void *datum)
 {
-	return hashtab_insert(&s->table, name, datum);
+	return hashtab_insert(&s->table, name, datum, symtab_key_params);
 }
 
 void *symtab_search(struct symtab *s, const char *name)
 {
-	return hashtab_search(&s->table, name);
+	return hashtab_search(&s->table, name, symtab_key_params);
 }