[PATCHv2] Smack: add support for modification of existing rules

[PATCHv2] Smack: add support for modification of existing rules

[Rafal Krypa]

The following patch introduces in-place modification of Smack rules.
Until now Smack supported only overwriting of existing rules.
To change permitted access for a given subject and object, user had
to read list of rules to get current accesses, modify it and write
modified rule back to kernel. This way was inefficient, non-atomic
and unnecessarily difficult.
New interface is intended to ease such modifications.

Changes in v2:
- dropped patches for smackfs seq list operations
- changed modification approach to simple integer assignment

Rafal Krypa (1):
  Smack: add support for modification of existing rules

 Documentation/security/Smack.txt |   11 ++
 security/smack/smackfs.c         |  249 ++++++++++++++++++++++++++------------
 2 files changed, 181 insertions(+), 79 deletions(-)

-- 
1.7.10.4

[PATCHv2] Smack: add support for modification of existing rules

[Rafal Krypa]

Rule modifications are enabled via /smack/change-rule. Format is as follows:
"Subject Object rwaxt rwaxt"

First two strings are subject and object labels up to 255 characters.
Third string contains permissions to enable.
Fourth string contains permissions to disable.

All unmentioned permissions will be left unchanged.
If no rule previously existed, it will be created.

Targeted for git://git.gitorious.org/smack-next/kernel.git

Signed-off-by: Rafal Krypa <r.krypa@samsung.com>
---
 Documentation/security/Smack.txt |   11 ++
 security/smack/smackfs.c         |  249 ++++++++++++++++++++++++++------------
 2 files changed, 181 insertions(+), 79 deletions(-)

diff --git a/Documentation/security/Smack.txt b/Documentation/security/Smack.txt
index 8a177e4..7a2d30c 100644
--- a/Documentation/security/Smack.txt
+++ b/Documentation/security/Smack.txt
@@ -117,6 +117,17 @@ access2
 ambient
 	This contains the Smack label applied to unlabeled network
 	packets.
+change-rule
+	This interface allows modification of existing access control rules.
+	The format accepted on write is:
+		"%s %s %s %s"
+	where the first string is the subject label, the second the
+	object label, the third the access to allow and the fourth the
+	access to deny. The access strings may contain only the characters
+	"rwxat-". If a rule for a given subject and object exists it will be
+	modified by enabling the permissions in the third string and disabling
+	those in the fourth string. If there is no such rule it will be
+	created using the access specified in the third and the fourth strings.
 cipso
 	This interface allows a specific CIPSO header to be assigned
 	to a Smack label. The format accepted on write is:
diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index 337e32c..2479a41 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -50,12 +50,12 @@ enum smk_inos {
 	SMK_ACCESS2	= 16,	/* make an access check with long labels */
 	SMK_CIPSO2	= 17,	/* load long label -> CIPSO mapping */
 	SMK_REVOKE_SUBJ	= 18,	/* set rules with subject label to '-' */
+	SMK_CHANGE_RULE	= 19,	/* change or add rules (long labels) */
 };
 
 /*
  * List locks
  */
-static DEFINE_MUTEX(smack_list_lock);
 static DEFINE_MUTEX(smack_cipso_lock);
 static DEFINE_MUTEX(smack_ambient_lock);
 static DEFINE_MUTEX(smk_netlbladdr_lock);
@@ -110,6 +110,13 @@ struct smack_master_list {
 
 LIST_HEAD(smack_rule_list);
 
+struct smack_parsed_rule {
+	char			*smk_subject;
+	char			*smk_object;
+	int			smk_access1;
+	int			smk_access2;
+};
+
 static int smk_cipso_doi_value = SMACK_CIPSO_DOI_DEFAULT;
 
 const char *smack_cipso_option = SMACK_CIPSO_OPTION;
@@ -167,25 +174,28 @@ static void smk_netlabel_audit_set(struct netlbl_audit *nap)
 #define SMK_NETLBLADDRMIN	9
 
 /**
- * smk_set_access - add a rule to the rule list
- * @srp: the new rule to add
+ * smk_set_access - add a rule to the rule list or replace an old rule
+ * @srp: the rule to add or replace
  * @rule_list: the list of rules
  * @rule_lock: the rule list lock
+ * @global: if non-zero, indicates a global rule
  *
  * Looks through the current subject/object/access list for
  * the subject/object pair and replaces the access that was
  * there. If the pair isn't found add it with the specified
  * access.
  *
- * Returns 1 if a rule was found to exist already, 0 if it is new
  * Returns 0 if nothing goes wrong or -ENOMEM if it fails
  * during the allocation of the new pair to add.
  */
-static int smk_set_access(struct smack_rule *srp, struct list_head *rule_list,
-				struct mutex *rule_lock)
+static int smk_set_access(struct smack_parsed_rule *srp,
+				struct list_head *rule_list,
+				struct mutex *rule_lock, int global)
 {
 	struct smack_rule *sp;
+	struct smack_master_list *smlp;
 	int found = 0;
+	int rc = 0;
 
 	mutex_lock(rule_lock);
 
@@ -197,23 +207,89 @@ static int smk_set_access(struct smack_rule *srp, struct list_head *rule_list,
 		if (sp->smk_object == srp->smk_object &&
 		    sp->smk_subject == srp->smk_subject) {
 			found = 1;
-			sp->smk_access = srp->smk_access;
+			sp->smk_access |= srp->smk_access1;
+			sp->smk_access &= ~srp->smk_access2;
 			break;
 		}
 	}
-	if (found == 0)
-		list_add_rcu(&srp->list, rule_list);
 
+	if (found == 0) {
+		sp = kzalloc(sizeof(*sp), GFP_KERNEL);
+		if (sp == NULL) {
+			rc = -ENOMEM;
+			goto out;
+		}
+
+		sp->smk_subject = srp->smk_subject;
+		sp->smk_object = srp->smk_object;
+		sp->smk_access = srp->smk_access1 & ~srp->smk_access2;
+
+		list_add_rcu(&sp->list, rule_list);
+		/*
+		 * If this is a global as opposed to self and a new rule
+		 * it needs to get added for reporting.
+		 */
+		if (global) {
+			smlp = kzalloc(sizeof(*smlp), GFP_KERNEL);
+			if (smlp != NULL) {
+				smlp->smk_rule = sp;
+				list_add_rcu(&smlp->list, &smack_rule_list);
+			} else
+				rc = -ENOMEM;
+		}
+	}
+
+out:
 	mutex_unlock(rule_lock);
+	return rc;
+}
+
+/**
+ * smk_perm_from_str - parse smack accesses from a text string
+ * @string: a text string that contains a Smack accesses code
+ *
+ * Returns an integer with respective bits set for specified accesses.
+ */
+static int smk_perm_from_str(const char *string)
+{
+	int perm = 0;
+	const char *cp;
 
-	return found;
+	for (cp = string; ; cp++)
+		switch (*cp) {
+		case '-':
+			break;
+		case 'r':
+		case 'R':
+			perm |= MAY_READ;
+			break;
+		case 'w':
+		case 'W':
+			perm |= MAY_WRITE;
+			break;
+		case 'x':
+		case 'X':
+			perm |= MAY_EXEC;
+			break;
+		case 'a':
+		case 'A':
+			perm |= MAY_APPEND;
+			break;
+		case 't':
+		case 'T':
+			perm |= MAY_TRANSMUTE;
+			break;
+		default:
+			return perm;
+		}
 }
 
 /**
  * smk_fill_rule - Fill Smack rule from strings
  * @subject: subject label string
  * @object: object label string
- * @access: access string
+ * @access1: access string
+ * @access2: string with permissions to be removed
  * @rule: Smack rule
  * @import: if non-zero, import labels
  * @len: label length limit
@@ -221,8 +297,9 @@ static int smk_set_access(struct smack_rule *srp, struct list_head *rule_list,
  * Returns 0 on success, -1 on failure
  */
 static int smk_fill_rule(const char *subject, const char *object,
-				const char *access, struct smack_rule *rule,
-				int import, int len)
+				const char *access1, const char *access2,
+				struct smack_parsed_rule *rule, int import,
+				int len)
 {
 	const char *cp;
 	struct smack_known *skp;
@@ -255,36 +332,11 @@ static int smk_fill_rule(const char *subject, const char *object,
 		rule->smk_object = skp->smk_known;
 	}
 
-	rule->smk_access = 0;
-
-	for (cp = access; *cp != '\0'; cp++) {
-		switch (*cp) {
-		case '-':
-			break;
-		case 'r':
-		case 'R':
-			rule->smk_access |= MAY_READ;
-			break;
-		case 'w':
-		case 'W':
-			rule->smk_access |= MAY_WRITE;
-			break;
-		case 'x':
-		case 'X':
-			rule->smk_access |= MAY_EXEC;
-			break;
-		case 'a':
-		case 'A':
-			rule->smk_access |= MAY_APPEND;
-			break;
-		case 't':
-		case 'T':
-			rule->smk_access |= MAY_TRANSMUTE;
-			break;
-		default:
-			return 0;
-		}
-	}
+	rule->smk_access1 = smk_perm_from_str(access1);
+	if (access2)
+		rule->smk_access2 = smk_perm_from_str(access2);
+	else
+		rule->smk_access2 = ~rule->smk_access1;
 
 	return 0;
 }
@@ -297,30 +349,33 @@ static int smk_fill_rule(const char *subject, const char *object,
  *
  * Returns 0 on success, -1 on errors.
  */
-static int smk_parse_rule(const char *data, struct smack_rule *rule, int import)
+static int smk_parse_rule(const char *data, struct smack_parsed_rule *rule,
+				int import)
 {
 	int rc;
 
 	rc = smk_fill_rule(data, data + SMK_LABELLEN,
-			   data + SMK_LABELLEN + SMK_LABELLEN, rule, import,
-			   SMK_LABELLEN);
+			   data + SMK_LABELLEN + SMK_LABELLEN, NULL, rule,
+			   import, SMK_LABELLEN);
 	return rc;
 }
 
 /**
  * smk_parse_long_rule - parse Smack rule from rule string
  * @data: string to be parsed, null terminated
- * @rule: Smack rule
+ * @rule: Will be filled with Smack parsed rule
  * @import: if non-zero, import labels
+ * @change: if non-zero, data is from /smack/change-rule
  *
  * Returns 0 on success, -1 on failure
  */
-static int smk_parse_long_rule(const char *data, struct smack_rule *rule,
-				int import)
+static int smk_parse_long_rule(const char *data, struct smack_parsed_rule *rule,
+				int import, int change)
 {
 	char *subject;
 	char *object;
-	char *access;
+	char *access1;
+	char *access2;
 	int datalen;
 	int rc = -1;
 
@@ -334,14 +389,27 @@ static int smk_parse_long_rule(const char *data, struct smack_rule *rule,
 	object = kzalloc(datalen, GFP_KERNEL);
 	if (object == NULL)
 		goto free_out_s;
-	access = kzalloc(datalen, GFP_KERNEL);
-	if (access == NULL)
+	access1 = kzalloc(datalen, GFP_KERNEL);
+	if (access1 == NULL)
 		goto free_out_o;
+	access2 = kzalloc(datalen, GFP_KERNEL);
+	if (access2 == NULL)
+		goto free_out_a;
+
+	if (change) {
+		if (sscanf(data, "%s %s %s %s",
+			subject, object, access1, access2) == 4)
+			rc = smk_fill_rule(subject, object, access1, access2,
+				rule, import, 0);
+	} else {
+		if (sscanf(data, "%s %s %s", subject, object, access1) == 3)
+			rc = smk_fill_rule(subject, object, access1, NULL,
+				rule, import, 0);
+	}
 
-	if (sscanf(data, "%s %s %s", subject, object, access) == 3)
-		rc = smk_fill_rule(subject, object, access, rule, import, 0);
-
-	kfree(access);
+	kfree(access2);
+free_out_a:
+	kfree(access1);
 free_out_o:
 	kfree(object);
 free_out_s:
@@ -351,6 +419,7 @@ free_out_s:
 
 #define SMK_FIXED24_FMT	0	/* Fixed 24byte label format */
 #define SMK_LONG_FMT	1	/* Variable long label format */
+#define SMK_CHANGE_FMT	2	/* Rule modification format */
 /**
  * smk_write_rules_list - write() for any /smack rule file
  * @file: file pointer, not actually used
@@ -359,22 +428,24 @@ free_out_s:
  * @ppos: where to start - must be 0
  * @rule_list: the list of rules to write to
  * @rule_lock: lock for the rule list
- * @format: /smack/load or /smack/load2 format.
+ * @format: /smack/load or /smack/load2 or /smack/change-rule format.
  *
  * Get one smack access rule from above.
  * The format for SMK_LONG_FMT is:
  *	"subject<whitespace>object<whitespace>access[<whitespace>...]"
  * The format for SMK_FIXED24_FMT is exactly:
  *	"subject                 object                  rwxat"
+ * The format for SMK_CHANGE_FMT is:
+ *	"subject<whitespace>object<whitespace>
+ *	 acc_enable<whitespace>acc_disable[<whitespace>...]"
  */
 static ssize_t smk_write_rules_list(struct file *file, const char __user *buf,
 					size_t count, loff_t *ppos,
 					struct list_head *rule_list,
 					struct mutex *rule_lock, int format)
 {
-	struct smack_master_list *smlp;
 	struct smack_known *skp;
-	struct smack_rule *rule;
+	struct smack_parsed_rule *rule;
 	char *data;
 	int datalen;
 	int rc = -EINVAL;
@@ -417,7 +488,11 @@ static ssize_t smk_write_rules_list(struct file *file, const char __user *buf,
 		 * Be sure the data string is terminated.
 		 */
 		data[count] = '\0';
-		if (smk_parse_long_rule(data, rule, 1))
+		if (smk_parse_long_rule(data, rule, 1, 0))
+			goto out_free_rule;
+	} else if (format == SMK_CHANGE_FMT) {
+		data[count] = '\0';
+		if (smk_parse_long_rule(data, rule, 1, 1))
 			goto out_free_rule;
 	} else {
 		/*
@@ -437,22 +512,9 @@ static ssize_t smk_write_rules_list(struct file *file, const char __user *buf,
 		rule_lock = &skp->smk_rules_lock;
 	}
 
-	rc = count;
-	/*
-	 * If this is a global as opposed to self and a new rule
-	 * it needs to get added for reporting.
-	 * smk_set_access returns true if there was already a rule
-	 * for the subject/object pair, and false if it was new.
-	 */
-	if (!smk_set_access(rule, rule_list, rule_lock)) {
-		if (load) {
-			smlp = kzalloc(sizeof(*smlp), GFP_KERNEL);
-			if (smlp != NULL) {
-				smlp->smk_rule = rule;
-				list_add_rcu(&smlp->list, &smack_rule_list);
-			} else
-				rc = -ENOMEM;
-		}
+	rc = smk_set_access(rule, rule_list, rule_lock, load);
+	if (rc == 0) {
+		rc = count;
 		goto out;
 	}
 
@@ -1774,7 +1836,7 @@ static const struct file_operations smk_load_self_ops = {
 static ssize_t smk_user_access(struct file *file, const char __user *buf,
 				size_t count, loff_t *ppos, int format)
 {
-	struct smack_rule rule;
+	struct smack_parsed_rule rule;
 	char *data;
 	char *cod;
 	int res;
@@ -1796,14 +1858,14 @@ static ssize_t smk_user_access(struct file *file, const char __user *buf,
 			return -ENOMEM;
 		memcpy(cod, data, count);
 		cod[count] = '\0';
-		res = smk_parse_long_rule(cod, &rule, 0);
+		res = smk_parse_long_rule(cod, &rule, 0, 0);
 		kfree(cod);
 	}
 
 	if (res)
 		return -EINVAL;
 
-	res = smk_access(rule.smk_subject, rule.smk_object, rule.smk_access,
+	res = smk_access(rule.smk_subject, rule.smk_object, rule.smk_access1,
 			  NULL);
 	data[0] = res == 0 ? '1' : '0';
 	data[1] = '\0';
@@ -2075,6 +2137,33 @@ static int smk_init_sysfs(void)
 }
 
 /**
+ * smk_write_change_rule - write() for /smack/change-rule
+ * @file: file pointer
+ * @buf: data from user space
+ * @count: bytes sent
+ * @ppos: where to start - must be 0
+ */
+static ssize_t smk_write_change_rule(struct file *file, const char __user *buf,
+				size_t count, loff_t *ppos)
+{
+	/*
+	 * Must have privilege.
+	 */
+	if (!capable(CAP_MAC_ADMIN))
+		return -EPERM;
+
+	return smk_write_rules_list(file, buf, count, ppos, NULL, NULL,
+				    SMK_CHANGE_FMT);
+}
+
+static const struct file_operations smk_change_rule_ops = {
+	.write		= smk_write_change_rule,
+	.read		= simple_transaction_read,
+	.release	= simple_transaction_release,
+	.llseek		= generic_file_llseek,
+};
+
+/**
  * smk_fill_super - fill the /smackfs superblock
  * @sb: the empty superblock
  * @data: unused
@@ -2123,6 +2212,8 @@ static int smk_fill_super(struct super_block *sb, void *data, int silent)
 		[SMK_REVOKE_SUBJ] = {
 			"revoke-subject", &smk_revoke_subj_ops,
 			S_IRUGO|S_IWUSR},
+		[SMK_CHANGE_RULE] = {
+			"change-rule", &smk_change_rule_ops, S_IRUGO|S_IWUSR},
 		/* last one */
 			{""}
 	};
-- 
1.7.10.4

Re: [PATCHv2] Smack: add support for modification of existing rules

[Mimi Zohar]

On Thu, 2013-01-10 at 19:42 +0100, Rafal Krypa wrote:
> Rule modifications are enabled via /smack/change-rule. Format is as follows:
> "Subject Object rwaxt rwaxt"
> 
> First two strings are subject and object labels up to 255 characters.
> Third string contains permissions to enable.
> Fourth string contains permissions to disable.
> 
> All unmentioned permissions will be left unchanged.
> If no rule previously existed, it will be created.

Changing rules on a running system could affect IMA, if the IMA policy
contains LSM based rules.  Patch "[PATCH 1/9] ima: re-initialize IMA
policy LSM info" addresses this issue.  It assumes existing LSM rules
have not been dropped.

thanks,

Mimi

Re: [PATCHv2] Smack: add support for modification of existing rules

[Casey Schaufler]

On 1/10/2013 10:42 AM, Rafal Krypa wrote:
> Rule modifications are enabled via /smack/change-rule. Format is as follows:
> "Subject Object rwaxt rwaxt"
>
> First two strings are subject and object labels up to 255 characters.
> Third string contains permissions to enable.
> Fourth string contains permissions to disable.
>
> All unmentioned permissions will be left unchanged.
> If no rule previously existed, it will be created.
>
> Targeted for git://git.gitorious.org/smack-next/kernel.git

Applied to git://git.gitorious.org/smack-next/kernel.git#stage-for-3.9


>
> Signed-off-by: Rafal Krypa <r.krypa@samsung.com>
> ---
>  Documentation/security/Smack.txt |   11 ++
>  security/smack/smackfs.c         |  249 ++++++++++++++++++++++++++------------
>  2 files changed, 181 insertions(+), 79 deletions(-)
>
> diff --git a/Documentation/security/Smack.txt b/Documentation/security/Smack.txt
> index 8a177e4..7a2d30c 100644
> --- a/Documentation/security/Smack.txt
> +++ b/Documentation/security/Smack.txt
> @@ -117,6 +117,17 @@ access2
>  ambient
>  	This contains the Smack label applied to unlabeled network
>  	packets.
> +change-rule
> +	This interface allows modification of existing access control rules.
> +	The format accepted on write is:
> +		"%s %s %s %s"
> +	where the first string is the subject label, the second the
> +	object label, the third the access to allow and the fourth the
> +	access to deny. The access strings may contain only the characters
> +	"rwxat-". If a rule for a given subject and object exists it will be
> +	modified by enabling the permissions in the third string and disabling
> +	those in the fourth string. If there is no such rule it will be
> +	created using the access specified in the third and the fourth strings.
>  cipso
>  	This interface allows a specific CIPSO header to be assigned
>  	to a Smack label. The format accepted on write is:
> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index 337e32c..2479a41 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c
> @@ -50,12 +50,12 @@ enum smk_inos {
>  	SMK_ACCESS2	= 16,	/* make an access check with long labels */
>  	SMK_CIPSO2	= 17,	/* load long label -> CIPSO mapping */
>  	SMK_REVOKE_SUBJ	= 18,	/* set rules with subject label to '-' */
> +	SMK_CHANGE_RULE	= 19,	/* change or add rules (long labels) */
>  };
>  
>  /*
>   * List locks
>   */
> -static DEFINE_MUTEX(smack_list_lock);
>  static DEFINE_MUTEX(smack_cipso_lock);
>  static DEFINE_MUTEX(smack_ambient_lock);
>  static DEFINE_MUTEX(smk_netlbladdr_lock);
> @@ -110,6 +110,13 @@ struct smack_master_list {
>  
>  LIST_HEAD(smack_rule_list);
>  
> +struct smack_parsed_rule {
> +	char			*smk_subject;
> +	char			*smk_object;
> +	int			smk_access1;
> +	int			smk_access2;
> +};
> +
>  static int smk_cipso_doi_value = SMACK_CIPSO_DOI_DEFAULT;
>  
>  const char *smack_cipso_option = SMACK_CIPSO_OPTION;
> @@ -167,25 +174,28 @@ static void smk_netlabel_audit_set(struct netlbl_audit *nap)
>  #define SMK_NETLBLADDRMIN	9
>  
>  /**
> - * smk_set_access - add a rule to the rule list
> - * @srp: the new rule to add
> + * smk_set_access - add a rule to the rule list or replace an old rule
> + * @srp: the rule to add or replace
>   * @rule_list: the list of rules
>   * @rule_lock: the rule list lock
> + * @global: if non-zero, indicates a global rule
>   *
>   * Looks through the current subject/object/access list for
>   * the subject/object pair and replaces the access that was
>   * there. If the pair isn't found add it with the specified
>   * access.
>   *
> - * Returns 1 if a rule was found to exist already, 0 if it is new
>   * Returns 0 if nothing goes wrong or -ENOMEM if it fails
>   * during the allocation of the new pair to add.
>   */
> -static int smk_set_access(struct smack_rule *srp, struct list_head *rule_list,
> -				struct mutex *rule_lock)
> +static int smk_set_access(struct smack_parsed_rule *srp,
> +				struct list_head *rule_list,
> +				struct mutex *rule_lock, int global)
>  {
>  	struct smack_rule *sp;
> +	struct smack_master_list *smlp;
>  	int found = 0;
> +	int rc = 0;
>  
>  	mutex_lock(rule_lock);
>  
> @@ -197,23 +207,89 @@ static int smk_set_access(struct smack_rule *srp, struct list_head *rule_list,
>  		if (sp->smk_object == srp->smk_object &&
>  		    sp->smk_subject == srp->smk_subject) {
>  			found = 1;
> -			sp->smk_access = srp->smk_access;
> +			sp->smk_access |= srp->smk_access1;
> +			sp->smk_access &= ~srp->smk_access2;
>  			break;
>  		}
>  	}
> -	if (found == 0)
> -		list_add_rcu(&srp->list, rule_list);
>  
> +	if (found == 0) {
> +		sp = kzalloc(sizeof(*sp), GFP_KERNEL);
> +		if (sp == NULL) {
> +			rc = -ENOMEM;
> +			goto out;
> +		}
> +
> +		sp->smk_subject = srp->smk_subject;
> +		sp->smk_object = srp->smk_object;
> +		sp->smk_access = srp->smk_access1 & ~srp->smk_access2;
> +
> +		list_add_rcu(&sp->list, rule_list);
> +		/*
> +		 * If this is a global as opposed to self and a new rule
> +		 * it needs to get added for reporting.
> +		 */
> +		if (global) {
> +			smlp = kzalloc(sizeof(*smlp), GFP_KERNEL);
> +			if (smlp != NULL) {
> +				smlp->smk_rule = sp;
> +				list_add_rcu(&smlp->list, &smack_rule_list);
> +			} else
> +				rc = -ENOMEM;
> +		}
> +	}
> +
> +out:
>  	mutex_unlock(rule_lock);
> +	return rc;
> +}
> +
> +/**
> + * smk_perm_from_str - parse smack accesses from a text string
> + * @string: a text string that contains a Smack accesses code
> + *
> + * Returns an integer with respective bits set for specified accesses.
> + */
> +static int smk_perm_from_str(const char *string)
> +{
> +	int perm = 0;
> +	const char *cp;
>  
> -	return found;
> +	for (cp = string; ; cp++)
> +		switch (*cp) {
> +		case '-':
> +			break;
> +		case 'r':
> +		case 'R':
> +			perm |= MAY_READ;
> +			break;
> +		case 'w':
> +		case 'W':
> +			perm |= MAY_WRITE;
> +			break;
> +		case 'x':
> +		case 'X':
> +			perm |= MAY_EXEC;
> +			break;
> +		case 'a':
> +		case 'A':
> +			perm |= MAY_APPEND;
> +			break;
> +		case 't':
> +		case 'T':
> +			perm |= MAY_TRANSMUTE;
> +			break;
> +		default:
> +			return perm;
> +		}
>  }
>  
>  /**
>   * smk_fill_rule - Fill Smack rule from strings
>   * @subject: subject label string
>   * @object: object label string
> - * @access: access string
> + * @access1: access string
> + * @access2: string with permissions to be removed
>   * @rule: Smack rule
>   * @import: if non-zero, import labels
>   * @len: label length limit
> @@ -221,8 +297,9 @@ static int smk_set_access(struct smack_rule *srp, struct list_head *rule_list,
>   * Returns 0 on success, -1 on failure
>   */
>  static int smk_fill_rule(const char *subject, const char *object,
> -				const char *access, struct smack_rule *rule,
> -				int import, int len)
> +				const char *access1, const char *access2,
> +				struct smack_parsed_rule *rule, int import,
> +				int len)
>  {
>  	const char *cp;
>  	struct smack_known *skp;
> @@ -255,36 +332,11 @@ static int smk_fill_rule(const char *subject, const char *object,
>  		rule->smk_object = skp->smk_known;
>  	}
>  
> -	rule->smk_access = 0;
> -
> -	for (cp = access; *cp != '\0'; cp++) {
> -		switch (*cp) {
> -		case '-':
> -			break;
> -		case 'r':
> -		case 'R':
> -			rule->smk_access |= MAY_READ;
> -			break;
> -		case 'w':
> -		case 'W':
> -			rule->smk_access |= MAY_WRITE;
> -			break;
> -		case 'x':
> -		case 'X':
> -			rule->smk_access |= MAY_EXEC;
> -			break;
> -		case 'a':
> -		case 'A':
> -			rule->smk_access |= MAY_APPEND;
> -			break;
> -		case 't':
> -		case 'T':
> -			rule->smk_access |= MAY_TRANSMUTE;
> -			break;
> -		default:
> -			return 0;
> -		}
> -	}
> +	rule->smk_access1 = smk_perm_from_str(access1);
> +	if (access2)
> +		rule->smk_access2 = smk_perm_from_str(access2);
> +	else
> +		rule->smk_access2 = ~rule->smk_access1;
>  
>  	return 0;
>  }
> @@ -297,30 +349,33 @@ static int smk_fill_rule(const char *subject, const char *object,
>   *
>   * Returns 0 on success, -1 on errors.
>   */
> -static int smk_parse_rule(const char *data, struct smack_rule *rule, int import)
> +static int smk_parse_rule(const char *data, struct smack_parsed_rule *rule,
> +				int import)
>  {
>  	int rc;
>  
>  	rc = smk_fill_rule(data, data + SMK_LABELLEN,
> -			   data + SMK_LABELLEN + SMK_LABELLEN, rule, import,
> -			   SMK_LABELLEN);
> +			   data + SMK_LABELLEN + SMK_LABELLEN, NULL, rule,
> +			   import, SMK_LABELLEN);
>  	return rc;
>  }
>  
>  /**
>   * smk_parse_long_rule - parse Smack rule from rule string
>   * @data: string to be parsed, null terminated
> - * @rule: Smack rule
> + * @rule: Will be filled with Smack parsed rule
>   * @import: if non-zero, import labels
> + * @change: if non-zero, data is from /smack/change-rule
>   *
>   * Returns 0 on success, -1 on failure
>   */
> -static int smk_parse_long_rule(const char *data, struct smack_rule *rule,
> -				int import)
> +static int smk_parse_long_rule(const char *data, struct smack_parsed_rule *rule,
> +				int import, int change)
>  {
>  	char *subject;
>  	char *object;
> -	char *access;
> +	char *access1;
> +	char *access2;
>  	int datalen;
>  	int rc = -1;
>  
> @@ -334,14 +389,27 @@ static int smk_parse_long_rule(const char *data, struct smack_rule *rule,
>  	object = kzalloc(datalen, GFP_KERNEL);
>  	if (object == NULL)
>  		goto free_out_s;
> -	access = kzalloc(datalen, GFP_KERNEL);
> -	if (access == NULL)
> +	access1 = kzalloc(datalen, GFP_KERNEL);
> +	if (access1 == NULL)
>  		goto free_out_o;
> +	access2 = kzalloc(datalen, GFP_KERNEL);
> +	if (access2 == NULL)
> +		goto free_out_a;
> +
> +	if (change) {
> +		if (sscanf(data, "%s %s %s %s",
> +			subject, object, access1, access2) == 4)
> +			rc = smk_fill_rule(subject, object, access1, access2,
> +				rule, import, 0);
> +	} else {
> +		if (sscanf(data, "%s %s %s", subject, object, access1) == 3)
> +			rc = smk_fill_rule(subject, object, access1, NULL,
> +				rule, import, 0);
> +	}
>  
> -	if (sscanf(data, "%s %s %s", subject, object, access) == 3)
> -		rc = smk_fill_rule(subject, object, access, rule, import, 0);
> -
> -	kfree(access);
> +	kfree(access2);
> +free_out_a:
> +	kfree(access1);
>  free_out_o:
>  	kfree(object);
>  free_out_s:
> @@ -351,6 +419,7 @@ free_out_s:
>  
>  #define SMK_FIXED24_FMT	0	/* Fixed 24byte label format */
>  #define SMK_LONG_FMT	1	/* Variable long label format */
> +#define SMK_CHANGE_FMT	2	/* Rule modification format */
>  /**
>   * smk_write_rules_list - write() for any /smack rule file
>   * @file: file pointer, not actually used
> @@ -359,22 +428,24 @@ free_out_s:
>   * @ppos: where to start - must be 0
>   * @rule_list: the list of rules to write to
>   * @rule_lock: lock for the rule list
> - * @format: /smack/load or /smack/load2 format.
> + * @format: /smack/load or /smack/load2 or /smack/change-rule format.
>   *
>   * Get one smack access rule from above.
>   * The format for SMK_LONG_FMT is:
>   *	"subject<whitespace>object<whitespace>access[<whitespace>...]"
>   * The format for SMK_FIXED24_FMT is exactly:
>   *	"subject                 object                  rwxat"
> + * The format for SMK_CHANGE_FMT is:
> + *	"subject<whitespace>object<whitespace>
> + *	 acc_enable<whitespace>acc_disable[<whitespace>...]"
>   */
>  static ssize_t smk_write_rules_list(struct file *file, const char __user *buf,
>  					size_t count, loff_t *ppos,
>  					struct list_head *rule_list,
>  					struct mutex *rule_lock, int format)
>  {
> -	struct smack_master_list *smlp;
>  	struct smack_known *skp;
> -	struct smack_rule *rule;
> +	struct smack_parsed_rule *rule;
>  	char *data;
>  	int datalen;
>  	int rc = -EINVAL;
> @@ -417,7 +488,11 @@ static ssize_t smk_write_rules_list(struct file *file, const char __user *buf,
>  		 * Be sure the data string is terminated.
>  		 */
>  		data[count] = '\0';
> -		if (smk_parse_long_rule(data, rule, 1))
> +		if (smk_parse_long_rule(data, rule, 1, 0))
> +			goto out_free_rule;
> +	} else if (format == SMK_CHANGE_FMT) {
> +		data[count] = '\0';
> +		if (smk_parse_long_rule(data, rule, 1, 1))
>  			goto out_free_rule;
>  	} else {
>  		/*
> @@ -437,22 +512,9 @@ static ssize_t smk_write_rules_list(struct file *file, const char __user *buf,
>  		rule_lock = &skp->smk_rules_lock;
>  	}
>  
> -	rc = count;
> -	/*
> -	 * If this is a global as opposed to self and a new rule
> -	 * it needs to get added for reporting.
> -	 * smk_set_access returns true if there was already a rule
> -	 * for the subject/object pair, and false if it was new.
> -	 */
> -	if (!smk_set_access(rule, rule_list, rule_lock)) {
> -		if (load) {
> -			smlp = kzalloc(sizeof(*smlp), GFP_KERNEL);
> -			if (smlp != NULL) {
> -				smlp->smk_rule = rule;
> -				list_add_rcu(&smlp->list, &smack_rule_list);
> -			} else
> -				rc = -ENOMEM;
> -		}
> +	rc = smk_set_access(rule, rule_list, rule_lock, load);
> +	if (rc == 0) {
> +		rc = count;
>  		goto out;
>  	}
>  
> @@ -1774,7 +1836,7 @@ static const struct file_operations smk_load_self_ops = {
>  static ssize_t smk_user_access(struct file *file, const char __user *buf,
>  				size_t count, loff_t *ppos, int format)
>  {
> -	struct smack_rule rule;
> +	struct smack_parsed_rule rule;
>  	char *data;
>  	char *cod;
>  	int res;
> @@ -1796,14 +1858,14 @@ static ssize_t smk_user_access(struct file *file, const char __user *buf,
>  			return -ENOMEM;
>  		memcpy(cod, data, count);
>  		cod[count] = '\0';
> -		res = smk_parse_long_rule(cod, &rule, 0);
> +		res = smk_parse_long_rule(cod, &rule, 0, 0);
>  		kfree(cod);
>  	}
>  
>  	if (res)
>  		return -EINVAL;
>  
> -	res = smk_access(rule.smk_subject, rule.smk_object, rule.smk_access,
> +	res = smk_access(rule.smk_subject, rule.smk_object, rule.smk_access1,
>  			  NULL);
>  	data[0] = res == 0 ? '1' : '0';
>  	data[1] = '\0';
> @@ -2075,6 +2137,33 @@ static int smk_init_sysfs(void)
>  }
>  
>  /**
> + * smk_write_change_rule - write() for /smack/change-rule
> + * @file: file pointer
> + * @buf: data from user space
> + * @count: bytes sent
> + * @ppos: where to start - must be 0
> + */
> +static ssize_t smk_write_change_rule(struct file *file, const char __user *buf,
> +				size_t count, loff_t *ppos)
> +{
> +	/*
> +	 * Must have privilege.
> +	 */
> +	if (!capable(CAP_MAC_ADMIN))
> +		return -EPERM;
> +
> +	return smk_write_rules_list(file, buf, count, ppos, NULL, NULL,
> +				    SMK_CHANGE_FMT);
> +}
> +
> +static const struct file_operations smk_change_rule_ops = {
> +	.write		= smk_write_change_rule,
> +	.read		= simple_transaction_read,
> +	.release	= simple_transaction_release,
> +	.llseek		= generic_file_llseek,
> +};
> +
> +/**
>   * smk_fill_super - fill the /smackfs superblock
>   * @sb: the empty superblock
>   * @data: unused
> @@ -2123,6 +2212,8 @@ static int smk_fill_super(struct super_block *sb, void *data, int silent)
>  		[SMK_REVOKE_SUBJ] = {
>  			"revoke-subject", &smk_revoke_subj_ops,
>  			S_IRUGO|S_IWUSR},
> +		[SMK_CHANGE_RULE] = {
> +			"change-rule", &smk_change_rule_ops, S_IRUGO|S_IWUSR},
>  		/* last one */
>  			{""}
>  	};

[PATCHv2] Smack: add support for modification of existing rules

[Rafal Krypa]

The following patch introduces in-place modification of Smack rules.
Until now Smack supported only overwriting of existing rules.
To change permitted access for a given subject and object, user had
to read list of rules to get current accesses, modify it and write
modified rule back to kernel. This way was inefficient, non-atomic
and unnecessarily difficult.
New interface is intended to ease such modifications.

Changes in v2:
- dropped patches for smackfs seq list operations
- changed modification approach to simple integer assignment

Rafal Krypa (1):

  Smack: add support for modification of existing rules

 Documentation/security/Smack.txt |   11 ++
 security/smack/smackfs.c         |  249 ++++++++++++++++++++++++++------------
 2 files changed, 181 insertions(+), 79 deletions(-)

-- 
1.7.10.4