Smack: Improve access check performance

Each Smack label that the kernel has seen is added to a
list of labels. The list of access rules for a given subject
label hangs off of the label list entry for the label.
This patch changes the structures that contain subject
labels to point at the label list entry rather that the
label itself. Doing so removes a label list lookup in
smk_access() that was accounting for the largest single
chunk of Smack overhead.

Targeted for git://git.gitorious.org/smack-next/kernel.gitSigned-off-by: NCasey Schaufler <casey@schaufler-ca.com>

security/smack/smack.h

@@ -28,6 +28,38 @@
 #define SMK_LABELLEN	24
 #define SMK_LONGLABEL	256
 
+/*
+ * This is the repository for labels seen so that it is
+ * not necessary to keep allocating tiny chuncks of memory
+ * and so that they can be shared.
+ *
+ * Labels are never modified in place. Anytime a label
+ * is imported (e.g. xattrset on a file) the list is checked
+ * for it and it is added if it doesn't exist. The address
+ * is passed out in either case. Entries are added, but
+ * never deleted.
+ *
+ * Since labels are hanging around anyway it doesn't
+ * hurt to maintain a secid for those awkward situations
+ * where kernel components that ought to use LSM independent
+ * interfaces don't. The secid should go away when all of
+ * these components have been repaired.
+ *
+ * The cipso value associated with the label gets stored here, too.
+ *
+ * Keep the access rules for this subject label here so that
+ * the entire set of rules does not need to be examined every
+ * time.
+ */
+struct smack_known {
+	struct list_head		list;
+	char				*smk_known;
+	u32				smk_secid;
+	struct netlbl_lsm_secattr	smk_netlabel;	/* on wire labels */
+	struct list_head		smk_rules;	/* access rules */
+	struct mutex			smk_rules_lock;	/* lock for rules */
+};
+
 /*
  * Maximum number of bytes for the levels in a CIPSO IP option.
  * Why 23? CIPSO is constrained to 30, so a 32 byte buffer is
@@ -46,25 +78,25 @@ struct superblock_smack {
 };
 
 struct socket_smack {
-	char		*smk_out;	/* outbound label */
-	char		*smk_in;	/* inbound label */
-	char		*smk_packet;	/* TCP peer label */
+	struct smack_known	*smk_out;	/* outbound label */
+	char			*smk_in;	/* inbound label */
+	char			*smk_packet;	/* TCP peer label */
 };
 
 /*
  * Inode smack data
  */
 struct inode_smack {
-	char		*smk_inode;	/* label of the fso */
-	char		*smk_task;	/* label of the task */
-	char		*smk_mmap;	/* label of the mmap domain */
-	struct mutex	smk_lock;	/* initialization lock */
-	int		smk_flags;	/* smack inode flags */
+	char			*smk_inode;	/* label of the fso */
+	struct smack_known	*smk_task;	/* label of the task */
+	struct smack_known	*smk_mmap;	/* label of the mmap domain */
+	struct mutex		smk_lock;	/* initialization lock */
+	int			smk_flags;	/* smack inode flags */
 };
 
 struct task_smack {
-	char			*smk_task;	/* label for access control */
-	char			*smk_forked;	/* label when forked */
+	struct smack_known	*smk_task;	/* label for access control */
+	struct smack_known	*smk_forked;	/* label when forked */
 	struct list_head	smk_rules;	/* per task access rules */
 	struct mutex		smk_rules_lock;	/* lock for the rules */
 };
@@ -78,7 +110,7 @@ struct task_smack {
  */
 struct smack_rule {
 	struct list_head	list;
-	char			*smk_subject;
+	struct smack_known	*smk_subject;
 	char			*smk_object;
 	int			smk_access;
 };
@@ -101,39 +133,7 @@ struct smk_port_label {
 	struct sock		*smk_sock;	/* socket initialized on */
 	unsigned short		smk_port;	/* the port number */
 	char			*smk_in;	/* incoming label */
-	char			*smk_out;	/* outgoing label */
-};
-
-/*
- * This is the repository for labels seen so that it is
- * not necessary to keep allocating tiny chuncks of memory
- * and so that they can be shared.
- *
- * Labels are never modified in place. Anytime a label
- * is imported (e.g. xattrset on a file) the list is checked
- * for it and it is added if it doesn't exist. The address
- * is passed out in either case. Entries are added, but
- * never deleted.
- *
- * Since labels are hanging around anyway it doesn't
- * hurt to maintain a secid for those awkward situations
- * where kernel components that ought to use LSM independent
- * interfaces don't. The secid should go away when all of
- * these components have been repaired.
- *
- * The cipso value associated with the label gets stored here, too.
- *
- * Keep the access rules for this subject label here so that
- * the entire set of rules does not need to be examined every
- * time.
- */
-struct smack_known {
-	struct list_head		list;
-	char				*smk_known;
-	u32				smk_secid;
-	struct netlbl_lsm_secattr	smk_netlabel;	/* on wire labels */
-	struct list_head		smk_rules;	/* access rules */
-	struct mutex			smk_rules_lock;	/* lock for rules */
+	struct smack_known	*smk_out;	/* outgoing label */
 };
 
 /*
@@ -214,9 +214,9 @@ struct inode_smack *new_inode_smack(char *);
  * These functions are in smack_access.c
  */
 int smk_access_entry(char *, char *, struct list_head *);
-int smk_access(char *, char *, int, struct smk_audit_info *);
+int smk_access(struct smack_known *, char *, int, struct smk_audit_info *);
 int smk_curacc(char *, u32, struct smk_audit_info *);
-char *smack_from_secid(const u32);
+struct smack_known *smack_from_secid(const u32);
 char *smk_parse_smack(const char *string, int len);
 int smk_netlbl_mls(int, char *, struct netlbl_lsm_secattr *, int);
 char *smk_import(const char *, int);
@@ -229,7 +229,7 @@ u32 smack_to_secid(const char *);
  */
 extern int smack_cipso_direct;
 extern int smack_cipso_mapped;
-extern char *smack_net_ambient;
+extern struct smack_known *smack_net_ambient;
 extern char *smack_onlycap;
 extern const char *smack_cipso_option;
 
@@ -265,17 +265,17 @@ static inline char *smk_of_inode(const struct inode *isp)
 }
 
 /*
- * Present a pointer to the smack label in an task blob.
+ * Present a pointer to the smack label entry in an task blob.
  */
-static inline char *smk_of_task(const struct task_smack *tsp)
+static inline struct smack_known *smk_of_task(const struct task_smack *tsp)
 {
 	return tsp->smk_task;
 }
 
 /*
- * Present a pointer to the forked smack label in an task blob.
+ * Present a pointer to the forked smack label entry in an task blob.
  */
-static inline char *smk_of_forked(const struct task_smack *tsp)
+static inline struct smack_known *smk_of_forked(const struct task_smack *tsp)
 {
 	return tsp->smk_forked;
 }
@@ -283,7 +283,7 @@ static inline char *smk_of_forked(const struct task_smack *tsp)
 /*
  * Present a pointer to the smack label in the current task blob.
  */
-static inline char *smk_of_current(void)
+static inline struct smack_known *smk_of_current(void)
 {
 	return smk_of_task(current_security());
 }
@@ -294,9 +294,11 @@ static inline char *smk_of_current(void)
  */
 static inline int smack_privileged(int cap)
 {
+	struct smack_known *skp = smk_of_current();
+
 	if (!capable(cap))
 		return 0;
-	if (smack_onlycap == NULL || smack_onlycap == smk_of_current())
+	if (smack_onlycap == NULL || smack_onlycap == skp->smk_known)
 		return 1;
 	return 0;
 }

security/smack/smack_access.c

@@ -93,7 +93,7 @@ int smk_access_entry(char *subject_label, char *object_label,
 
 	list_for_each_entry_rcu(srp, rule_list, list) {
 		if (srp->smk_object == object_label &&
-		    srp->smk_subject == subject_label) {
+		    srp->smk_subject->smk_known == subject_label) {
 			may = srp->smk_access;
 			break;
 		}
@@ -104,7 +104,7 @@ int smk_access_entry(char *subject_label, char *object_label,
 
 /**
  * smk_access - determine if a subject has a specific access to an object
- * @subject_label: a pointer to the subject's Smack label
+ * @subject_known: a pointer to the subject's Smack label entry
  * @object_label: a pointer to the object's Smack label
  * @request: the access requested, in "MAY" format
  * @a : a pointer to the audit data
@@ -115,10 +115,9 @@ int smk_access_entry(char *subject_label, char *object_label,
  *
  * Smack labels are shared on smack_list
  */
-int smk_access(char *subject_label, char *object_label, int request,
-	       struct smk_audit_info *a)
+int smk_access(struct smack_known *subject_known, char *object_label,
+		int request, struct smk_audit_info *a)
 {
-	struct smack_known *skp;
 	int may = MAY_NOT;
 	int rc = 0;
 
@@ -127,7 +126,7 @@ int smk_access(char *subject_label, char *object_label, int request,
 	 *
 	 * A star subject can't access any object.
 	 */
-	if (subject_label == smack_known_star.smk_known) {
+	if (subject_known == &smack_known_star) {
 		rc = -EACCES;
 		goto out_audit;
 	}
@@ -137,7 +136,7 @@ int smk_access(char *subject_label, char *object_label, int request,
 	 * An internet subject can access any object.
 	 */
 	if (object_label == smack_known_web.smk_known ||
-	    subject_label == smack_known_web.smk_known)
+	    subject_known == &smack_known_web)
 		goto out_audit;
 	/*
 	 * A star object can be accessed by any subject.
@@ -148,7 +147,7 @@ int smk_access(char *subject_label, char *object_label, int request,
 	 * An object can be accessed in any way by a subject
 	 * with the same label.
 	 */
-	if (subject_label == object_label)
+	if (subject_known->smk_known == object_label)
 		goto out_audit;
 	/*
 	 * A hat subject can read any object.
@@ -157,7 +156,7 @@ int smk_access(char *subject_label, char *object_label, int request,
 	if ((request & MAY_ANYREAD) == request) {
 		if (object_label == smack_known_floor.smk_known)
 			goto out_audit;
-		if (subject_label == smack_known_hat.smk_known)
+		if (subject_known == &smack_known_hat)
 			goto out_audit;
 	}
 	/*
@@ -167,9 +166,9 @@ int smk_access(char *subject_label, char *object_label, int request,
 	 * good. A negative response from smk_access_entry()
 	 * indicates there is no entry for this pair.
 	 */
-	skp = smk_find_entry(subject_label);
 	rcu_read_lock();
-	may = smk_access_entry(subject_label, object_label, &skp->smk_rules);
+	may = smk_access_entry(subject_known->smk_known, object_label,
+				&subject_known->smk_rules);
 	rcu_read_unlock();
 
 	if (may > 0 && (request & may) == request)
@@ -179,7 +178,8 @@ int smk_access(char *subject_label, char *object_label, int request,
 out_audit:
 #ifdef CONFIG_AUDIT
 	if (a)
-		smack_log(subject_label, object_label, request, rc, a);
+		smack_log(subject_known->smk_known, object_label, request,
+				rc, a);
 #endif
 	return rc;
 }
@@ -198,20 +198,21 @@ int smk_access(char *subject_label, char *object_label, int request,
 int smk_curacc(char *obj_label, u32 mode, struct smk_audit_info *a)
 {
 	struct task_smack *tsp = current_security();
-	char *sp = smk_of_task(tsp);
+	struct smack_known *skp = smk_of_task(tsp);
 	int may;
 	int rc;
 
 	/*
 	 * Check the global rule list
 	 */
-	rc = smk_access(sp, obj_label, mode, NULL);
+	rc = smk_access(skp, obj_label, mode, NULL);
 	if (rc == 0) {
 		/*
 		 * If there is an entry in the task's rule list
 		 * it can further restrict access.
 		 */
-		may = smk_access_entry(sp, obj_label, &tsp->smk_rules);
+		may = smk_access_entry(skp->smk_known, obj_label,
+					&tsp->smk_rules);
 		if (may < 0)
 			goto out_audit;
 		if ((mode & may) == mode)
@@ -228,7 +229,7 @@ int smk_curacc(char *obj_label, u32 mode, struct smk_audit_info *a)
 out_audit:
 #ifdef CONFIG_AUDIT
 	if (a)
-		smack_log(sp, obj_label, mode, rc, a);
+		smack_log(skp->smk_known, obj_label, mode, rc, a);
 #endif
 	return rc;
 }
@@ -513,10 +514,10 @@ char *smk_import(const char *string, int len)
  * smack_from_secid - find the Smack label associated with a secid
  * @secid: an integer that might be associated with a Smack label
  *
- * Returns a pointer to the appropriate Smack label if there is one,
+ * Returns a pointer to the appropriate Smack label entry if there is one,
  * otherwise a pointer to the invalid Smack label.
  */
-char *smack_from_secid(const u32 secid)
+struct smack_known *smack_from_secid(const u32 secid)
 {
 	struct smack_known *skp;
 
@@ -524,7 +525,7 @@ char *smack_from_secid(const u32 secid)
 	list_for_each_entry_rcu(skp, &smack_known_list, list) {
 		if (skp->smk_secid == secid) {
 			rcu_read_unlock();
-			return skp->smk_known;
+			return skp;
 		}
 	}
 
@@ -533,7 +534,7 @@ char *smack_from_secid(const u32 secid)
 	 * of a secid that is not on the list.
 	 */
 	rcu_read_unlock();
-	return smack_known_invalid.smk_known;
+	return &smack_known_invalid;
 }
 
 /**

security/smack/smackfs.c

@@ -66,7 +66,7 @@ static DEFINE_MUTEX(smk_netlbladdr_lock);
  * If it isn't somehow marked, use this.
  * It can be reset via smackfs/ambient
  */
-char *smack_net_ambient;
+struct smack_known *smack_net_ambient;
 
 /*
  * This is the level in a CIPSO header that indicates a
@@ -112,7 +112,7 @@ struct smack_master_list {
 LIST_HEAD(smack_rule_list);
 
 struct smack_parsed_rule {
-	char			*smk_subject;
+	struct smack_known	*smk_subject;
 	char			*smk_object;
 	int			smk_access1;
 	int			smk_access2;
@@ -163,9 +163,11 @@ static inline void smack_catset_bit(unsigned int cat, char *catsetp)
  */
 static void smk_netlabel_audit_set(struct netlbl_audit *nap)
 {
+	struct smack_known *skp = smk_of_current();
+
 	nap->loginuid = audit_get_loginuid(current);
 	nap->sessionid = audit_get_sessionid(current);
-	nap->secid = smack_to_secid(smk_of_current());
+	nap->secid = skp->smk_secid;
 }
 
 /*
@@ -306,7 +308,7 @@ static int smk_fill_rule(const char *subject, const char *object,
 	struct smack_known *skp;
 
 	if (import) {
-		rule->smk_subject = smk_import(subject, len);
+		rule->smk_subject = smk_import_entry(subject, len);
 		if (rule->smk_subject == NULL)
 			return -1;
 
@@ -321,7 +323,7 @@ static int smk_fill_rule(const char *subject, const char *object,
 		kfree(cp);
 		if (skp == NULL)
 			return -1;
-		rule->smk_subject = skp->smk_known;
+		rule->smk_subject = skp;
 
 		cp = smk_parse_smack(object, len);
 		if (cp == NULL)
@@ -445,7 +447,6 @@ static ssize_t smk_write_rules_list(struct file *file, const char __user *buf,
 					struct list_head *rule_list,
 					struct mutex *rule_lock, int format)
 {
-	struct smack_known *skp;
 	struct smack_parsed_rule *rule;
 	char *data;
 	int datalen;
@@ -505,12 +506,10 @@ static ssize_t smk_write_rules_list(struct file *file, const char __user *buf,
 			goto out_free_rule;
 	}
 
-
 	if (rule_list == NULL) {
 		load = 1;
-		skp = smk_find_entry(rule->smk_subject);
-		rule_list = &skp->smk_rules;
-		rule_lock = &skp->smk_rules_lock;
+		rule_list = &rule->smk_subject->smk_rules;
+		rule_lock = &rule->smk_subject->smk_rules_lock;
 	}
 
 	rc = smk_set_access(rule, rule_list, rule_lock, load);
@@ -579,13 +578,14 @@ static void smk_rule_show(struct seq_file *s, struct smack_rule *srp, int max)
 	 * because you should expect to be able to write
 	 * anything you read back.
 	 */
-	if (strlen(srp->smk_subject) >= max || strlen(srp->smk_object) >= max)
+	if (strlen(srp->smk_subject->smk_known) >= max ||
+	    strlen(srp->smk_object) >= max)
 		return;
 
 	if (srp->smk_access == 0)
 		return;
 
-	seq_printf(s, "%s %s", srp->smk_subject, srp->smk_object);
+	seq_printf(s, "%s %s", srp->smk_subject->smk_known, srp->smk_object);
 
 	seq_putc(s, ' ');
 
@@ -738,9 +738,9 @@ static void smk_unlbl_ambient(char *oldambient)
 			       __func__, __LINE__, rc);
 	}
 	if (smack_net_ambient == NULL)
-		smack_net_ambient = smack_known_floor.smk_known;
+		smack_net_ambient = &smack_known_floor;
 
-	rc = netlbl_cfg_unlbl_map_add(smack_net_ambient, PF_INET,
+	rc = netlbl_cfg_unlbl_map_add(smack_net_ambient->smk_known, PF_INET,
 				      NULL, NULL, &nai);
 	if (rc != 0)
 		printk(KERN_WARNING "%s:%d add rc = %d\n",
@@ -1535,11 +1535,12 @@ static ssize_t smk_read_ambient(struct file *filp, char __user *buf,
 	 */
 	mutex_lock(&smack_ambient_lock);
 
-	asize = strlen(smack_net_ambient) + 1;
+	asize = strlen(smack_net_ambient->smk_known) + 1;
 
 	if (cn >= asize)
 		rc = simple_read_from_buffer(buf, cn, ppos,
-					     smack_net_ambient, asize);
+					     smack_net_ambient->smk_known,
+					     asize);
 	else
 		rc = -EINVAL;
 
@@ -1560,8 +1561,8 @@ static ssize_t smk_read_ambient(struct file *filp, char __user *buf,
 static ssize_t smk_write_ambient(struct file *file, const char __user *buf,
 				 size_t count, loff_t *ppos)
 {
+	struct smack_known *skp;
 	char *oldambient;
-	char *smack = NULL;
 	char *data;
 	int rc = count;
 
@@ -1577,16 +1578,16 @@ static ssize_t smk_write_ambient(struct file *file, const char __user *buf,
 		goto out;
 	}
 
-	smack = smk_import(data, count);
-	if (smack == NULL) {
+	skp = smk_import_entry(data, count);
+	if (skp == NULL) {
 		rc = -EINVAL;
 		goto out;
 	}
 
 	mutex_lock(&smack_ambient_lock);
 
-	oldambient = smack_net_ambient;
-	smack_net_ambient = smack;
+	oldambient = smack_net_ambient->smk_known;
+	smack_net_ambient = skp;
 	smk_unlbl_ambient(oldambient);
 
 	mutex_unlock(&smack_ambient_lock);
@@ -1645,7 +1646,7 @@ static ssize_t smk_write_onlycap(struct file *file, const char __user *buf,
 				 size_t count, loff_t *ppos)
 {
 	char *data;
-	char *sp = smk_of_task(current->cred->security);
+	struct smack_known *skp = smk_of_task(current->cred->security);
 	int rc = count;
 
 	if (!smack_privileged(CAP_MAC_ADMIN))
@@ -1656,7 +1657,7 @@ static ssize_t smk_write_onlycap(struct file *file, const char __user *buf,
 	 * explicitly for clarity. The smk_access() implementation
 	 * would use smk_access(smack_onlycap, MAY_WRITE)
 	 */
-	if (smack_onlycap != NULL && smack_onlycap != sp)
+	if (smack_onlycap != NULL && smack_onlycap != skp->smk_known)
 		return -EPERM;
 
 	data = kzalloc(count, GFP_KERNEL);
@@ -1866,8 +1867,8 @@ static ssize_t smk_user_access(struct file *file, const char __user *buf,
 	if (res)
 		return -EINVAL;
 
-	res = smk_access(rule.smk_subject, rule.smk_object, rule.smk_access1,
-			  NULL);
+	res = smk_access(rule.smk_subject, rule.smk_object,
+				rule.smk_access1, NULL);
 	data[0] = res == 0 ? '1' : '0';
 	data[1] = '\0';