Smack: Rule list lookup performance

This patch is targeted for the smack-next tree.

Smack access checks suffer from two significant performance
issues. In cases where there are large numbers of rules the
search of the single list of rules is wasteful. Comparing the
string values of the smack labels is less efficient than a
numeric comparison would.

These changes take advantage of the Smack label list, which
maintains the mapping of Smack labels to secids and optional
CIPSO labels. Because the labels are kept perpetually, an
access check can be done strictly based on the address of the
label in the list without ever looking at the label itself.
Rather than keeping one global list of rules the rules with
a particular subject label can be based off of that label
list entry. The access check need never look at entries that
do not use the current subject label.

This requires that packets coming off the network with
CIPSO direct Smack labels that have never been seen before
be treated carefully. The only case where they could be
delivered is where the receiving socket has an IPIN star
label, so that case is explicitly addressed.

On a system with 39,800 rules (200 labels in all permutations)
a system with this patch runs an access speed test in 5% of
the time of the old version. That should be a best case
improvement. If all of the rules are associated with the
same subject label and all of the accesses are for processes
with that label (unlikely) the improvement is about 30%.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index b9c5e14..fb91516 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -516,6 +516,8 @@
 				     const struct qstr *qstr, char **name,
 				     void **value, size_t *len)
 {
+	struct smack_known *skp;
+	char *csp = smk_of_current();
 	char *isp = smk_of_inode(inode);
 	char *dsp = smk_of_inode(dir);
 	int may;
@@ -527,8 +529,9 @@
 	}
 
 	if (value) {
+		skp = smk_find_entry(csp);
 		rcu_read_lock();
-		may = smk_access_entry(smk_of_current(), dsp, &smack_rule_list);
+		may = smk_access_entry(csp, dsp, &skp->smk_rules);
 		rcu_read_unlock();
 
 		/*
@@ -1138,6 +1141,7 @@
 			   unsigned long flags, unsigned long addr,
 			   unsigned long addr_only)
 {
+	struct smack_known *skp;
 	struct smack_rule *srp;
 	struct task_smack *tsp;
 	char *sp;
@@ -1170,6 +1174,7 @@
 
 	tsp = current_security();
 	sp = smk_of_current();
+	skp = smk_find_entry(sp);
 	rc = 0;
 
 	rcu_read_lock();
@@ -1177,15 +1182,8 @@
 	 * For each Smack rule associated with the subject
 	 * label verify that the SMACK64MMAP also has access
 	 * to that rule's object label.
-	 *
-	 * Because neither of the labels comes
-	 * from the networking code it is sufficient
-	 * to compare pointers.
 	 */
-	list_for_each_entry_rcu(srp, &smack_rule_list, list) {
-		if (srp->smk_subject != sp)
-			continue;
-
+	list_for_each_entry_rcu(srp, &skp->smk_rules, list) {
 		osmack = srp->smk_object;
 		/*
 		 * Matching labels always allows access.
@@ -1214,7 +1212,8 @@
 		 * If there isn't one a SMACK64MMAP subject
 		 * can't have as much access as current.
 		 */
-		mmay = smk_access_entry(msmack, osmack, &smack_rule_list);
+		skp = smk_find_entry(msmack);
+		mmay = smk_access_entry(msmack, osmack, &skp->smk_rules);
 		if (mmay == -ENOENT) {
 			rc = -EACCES;
 			break;
@@ -1711,7 +1710,7 @@
 
 	ssp->smk_in = csp;
 	ssp->smk_out = csp;
-	ssp->smk_packet[0] = '\0';
+	ssp->smk_packet = NULL;
 
 	sk->sk_security = ssp;
 
@@ -2813,16 +2812,17 @@
 	return smack_netlabel_send(sock->sk, sip);
 }
 
-
 /**
  * smack_from_secattr - Convert a netlabel attr.mls.lvl/attr.mls.cat pair to smack
  * @sap: netlabel secattr
- * @sip: where to put the result
+ * @ssp: socket security information
  *
- * Copies a smack label into sip
+ * Returns a pointer to a Smack label found on the label list.
  */
-static void smack_from_secattr(struct netlbl_lsm_secattr *sap, char *sip)
+static char *smack_from_secattr(struct netlbl_lsm_secattr *sap,
+				struct socket_smack *ssp)
 {
+	struct smack_known *skp;
 	char smack[SMK_LABELLEN];
 	char *sp;
 	int pcat;
@@ -2852,15 +2852,43 @@
 		 * we are already done. WeeHee.
 		 */
 		if (sap->attr.mls.lvl == smack_cipso_direct) {
-			memcpy(sip, smack, SMK_MAXLEN);
-			return;
+			/*
+			 * The label sent is usually on the label list.
+			 *
+			 * If it is not we may still want to allow the
+			 * delivery.
+			 *
+			 * If the recipient is accepting all packets
+			 * because it is using the star ("*") label
+			 * for SMACK64IPIN provide the web ("@") label
+			 * so that a directed response will succeed.
+			 * This is not very correct from a MAC point
+			 * of view, but gets around the problem that
+			 * locking prevents adding the newly discovered
+			 * label to the list.
+			 * The case where the recipient is not using
+			 * the star label should obviously fail.
+			 * The easy way to do this is to provide the
+			 * star label as the subject label.
+			 */
+			skp = smk_find_entry(smack);
+			if (skp != NULL)
+				return skp->smk_known;
+			if (ssp != NULL &&
+			    ssp->smk_in == smack_known_star.smk_known)
+				return smack_known_web.smk_known;
+			return smack_known_star.smk_known;
 		}
 		/*
 		 * Look it up in the supplied table if it is not
 		 * a direct mapping.
 		 */
-		smack_from_cipso(sap->attr.mls.lvl, smack, sip);
-		return;
+		sp = smack_from_cipso(sap->attr.mls.lvl, smack);
+		if (sp != NULL)
+			return sp;
+		if (ssp != NULL && ssp->smk_in == smack_known_star.smk_known)
+			return smack_known_web.smk_known;
+		return smack_known_star.smk_known;
 	}
 	if ((sap->flags & NETLBL_SECATTR_SECID) != 0) {
 		/*
@@ -2875,16 +2903,14 @@
 		 * secid is from a fallback.
 		 */
 		BUG_ON(sp == NULL);
-		strncpy(sip, sp, SMK_MAXLEN);
-		return;
+		return sp;
 	}
 	/*
 	 * Without guidance regarding the smack value
 	 * for the packet fall back on the network
 	 * ambient value.
 	 */
-	strncpy(sip, smack_net_ambient, SMK_MAXLEN);
-	return;
+	return smack_net_ambient;
 }
 
 /**
@@ -2898,7 +2924,6 @@
 {
 	struct netlbl_lsm_secattr secattr;
 	struct socket_smack *ssp = sk->sk_security;
-	char smack[SMK_LABELLEN];
 	char *csp;
 	int rc;
 	struct smk_audit_info ad;
@@ -2911,10 +2936,9 @@
 	netlbl_secattr_init(&secattr);
 
 	rc = netlbl_skbuff_getattr(skb, sk->sk_family, &secattr);
-	if (rc == 0) {
-		smack_from_secattr(&secattr, smack);
-		csp = smack;
-	} else
+	if (rc == 0)
+		csp = smack_from_secattr(&secattr, ssp);
+	else
 		csp = smack_net_ambient;
 
 	netlbl_secattr_destroy(&secattr);
@@ -2951,15 +2975,19 @@
 					  int __user *optlen, unsigned len)
 {
 	struct socket_smack *ssp;
-	int slen;
+	char *rcp = "";
+	int slen = 1;
 	int rc = 0;
 
 	ssp = sock->sk->sk_security;
-	slen = strlen(ssp->smk_packet) + 1;
+	if (ssp->smk_packet != NULL) {
+		rcp = ssp->smk_packet;
+		slen = strlen(rcp) + 1;
+	}
 
 	if (slen > len)
 		rc = -ERANGE;
-	else if (copy_to_user(optval, ssp->smk_packet, slen) != 0)
+	else if (copy_to_user(optval, rcp, slen) != 0)
 		rc = -EFAULT;
 
 	if (put_user(slen, optlen) != 0)
@@ -2982,8 +3010,8 @@
 
 {
 	struct netlbl_lsm_secattr secattr;
-	struct socket_smack *sp;
-	char smack[SMK_LABELLEN];
+	struct socket_smack *ssp = NULL;
+	char *sp;
 	int family = PF_UNSPEC;
 	u32 s = 0;	/* 0 is the invalid secid */
 	int rc;
@@ -2998,17 +3026,19 @@
 		family = sock->sk->sk_family;
 
 	if (family == PF_UNIX) {
-		sp = sock->sk->sk_security;
-		s = smack_to_secid(sp->smk_out);
+		ssp = sock->sk->sk_security;
+		s = smack_to_secid(ssp->smk_out);
 	} else if (family == PF_INET || family == PF_INET6) {
 		/*
 		 * Translate what netlabel gave us.
 		 */
+		if (sock != NULL && sock->sk != NULL)
+			ssp = sock->sk->sk_security;
 		netlbl_secattr_init(&secattr);
 		rc = netlbl_skbuff_getattr(skb, family, &secattr);
 		if (rc == 0) {
-			smack_from_secattr(&secattr, smack);
-			s = smack_to_secid(smack);
+			sp = smack_from_secattr(&secattr, ssp);
+			s = smack_to_secid(sp);
 		}
 		netlbl_secattr_destroy(&secattr);
 	}
@@ -3056,7 +3086,7 @@
 	struct netlbl_lsm_secattr secattr;
 	struct sockaddr_in addr;
 	struct iphdr *hdr;
-	char smack[SMK_LABELLEN];
+	char *sp;
 	int rc;
 	struct smk_audit_info ad;
 
@@ -3067,9 +3097,9 @@
 	netlbl_secattr_init(&secattr);
 	rc = netlbl_skbuff_getattr(skb, family, &secattr);
 	if (rc == 0)
-		smack_from_secattr(&secattr, smack);
+		sp = smack_from_secattr(&secattr, ssp);
 	else
-		strncpy(smack, smack_known_huh.smk_known, SMK_MAXLEN);
+		sp = smack_known_huh.smk_known;
 	netlbl_secattr_destroy(&secattr);
 
 #ifdef CONFIG_AUDIT
@@ -3082,7 +3112,7 @@
 	 * Receiving a packet requires that the other end be able to write
 	 * here. Read access is not required.
 	 */
-	rc = smk_access(smack, ssp->smk_in, MAY_WRITE, &ad);
+	rc = smk_access(sp, ssp->smk_in, MAY_WRITE, &ad);
 	if (rc != 0)
 		return rc;
 
@@ -3090,7 +3120,7 @@
 	 * Save the peer's label in the request_sock so we can later setup
 	 * smk_packet in the child socket so that SO_PEERCRED can report it.
 	 */
-	req->peer_secid = smack_to_secid(smack);
+	req->peer_secid = smack_to_secid(sp);
 
 	/*
 	 * We need to decide if we want to label the incoming connection here
@@ -3103,7 +3133,7 @@
 	if (smack_host_label(&addr) == NULL) {
 		rcu_read_unlock();
 		netlbl_secattr_init(&secattr);
-		smack_to_secattr(smack, &secattr);
+		smack_to_secattr(sp, &secattr);
 		rc = netlbl_req_setattr(req, &secattr);
 		netlbl_secattr_destroy(&secattr);
 	} else {
@@ -3125,13 +3155,11 @@
 				 const struct request_sock *req)
 {
 	struct socket_smack *ssp = sk->sk_security;
-	char *smack;
 
-	if (req->peer_secid != 0) {
-		smack = smack_from_secid(req->peer_secid);
-		strncpy(ssp->smk_packet, smack, SMK_MAXLEN);
-	} else
-		ssp->smk_packet[0] = '\0';
+	if (req->peer_secid != 0)
+		ssp->smk_packet = smack_from_secid(req->peer_secid);
+	else
+		ssp->smk_packet = NULL;
 }
 
 /*