[PATCH 0/2] Smack: allow multiple labels in onlycap

[PATCH 0/2] Smack: allow multiple labels in onlycap

[Rafal Krypa]

It has been pointed out in Tizen recently, that onlycap feature might be
impractical with possibility of only one label to set.
If there is a desire to limit CAP_MAC_ADMIN and CAP_MAC_OVERRIDE in the
system, all privileged processes would have to run with a single label.
If multiple modules need those capabilities, they cannot have different
policy rules.

This patch set aims for making onlycap more flexible. Multiple labels can
be configured for privilege.

To support that, the first patch does some preparation in smackfs. Until
now all lists in Smack were add-only. No elements were ever removed from
lists of labels, rules and all other. Therefore Smack uses RCU lists with
non-standard, relaxed usage of critical sections. But extended onlycap
functionality will require both addition and removal of elements. To
support that, smackfs must be adapted first for correct usage of RCU read
critical sections. Failing to do that could lead to memory races and
undefined behaviour in smackfs.
As a bonus, first patch also fixes a bug in smackfs that was found by
coincidence.


Rafal Krypa (2):
  Smack: fix seq operations in smackfs
  Smack: allow multiple labels in onlycap

 Documentation/security/Smack.txt |   6 +-
 security/smack/smack.h           |  25 ++---
 security/smack/smack_access.c    |  41 ++++++++
 security/smack/smackfs.c         | 214 ++++++++++++++++++++++++---------------
 4 files changed, 184 insertions(+), 102 deletions(-)

-- 
2.1.4

[PATCH 1/2] Smack: fix seq operations in smackfs

[Rafal Krypa]

Use proper RCU functions and read locking in smackfs seq_operations.

Smack gets away with not using proper RCU functions in smackfs, because
it never removes entries from these lists. But now one list will be
needed (with interface in smackfs) that will have both elements added and
removed to it.
This change will also help any future changes implementing removal of
unneeded entries from other Smack lists.

The patch also fixes handling of pos argument in smk_seq_start and
smk_seq_next. This fixes a bug in case when smackfs is read with a small
buffer:

Kernel panic - not syncing: Kernel mode fault at addr 0xfa0000011b
CPU: 0 PID: 1292 Comm: dd Not tainted 4.1.0-rc1-00012-g98179b8 #13
Stack:
 00000003 0000000d 7ff39e48 7f69fd00
 7ff39ce0 601ae4b0 7ff39d50 600e587b
 00000010 6039f690 7f69fd40 00612003
Call Trace:
 [<601ae4b0>] load2_seq_show+0x19/0x1d
 [<600e587b>] seq_read+0x168/0x331
 [<600c5943>] __vfs_read+0x21/0x101
 [<601a595e>] ? security_file_permission+0xf8/0x105
 [<600c5ec6>] ? rw_verify_area+0x86/0xe2
 [<600c5fc3>] vfs_read+0xa1/0x14c
 [<600c68e2>] SyS_read+0x57/0xa0
 [<6001da60>] handle_syscall+0x60/0x80
 [<6003087d>] userspace+0x442/0x548
 [<6001aa77>] ? interrupt_end+0x0/0x80
 [<6001daae>] ? copy_chunk_to_user+0x0/0x2b
 [<6002cb6b>] ? save_registers+0x1f/0x39
 [<60032ef7>] ? arch_prctl+0xf5/0x170
 [<6001a92d>] fork_handler+0x85/0x87

Signed-off-by: Rafal Krypa <r.krypa@samsung.com>
---
 security/smack/smackfs.c | 52 ++++++++++++++++++++----------------------------
 1 file changed, 22 insertions(+), 30 deletions(-)

diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index 3e42426..e40dc48 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -566,23 +566,17 @@ static void *smk_seq_start(struct seq_file *s, loff_t *pos,
 				struct list_head *head)
 {
 	struct list_head *list;
+	int i = *pos;
+
+	rcu_read_lock();
+	for (list = rcu_dereference(list_next_rcu(head));
+		list != head;
+		list = rcu_dereference(list_next_rcu(list))) {
+		if (i-- == 0)
+			return list;
+	}
 
-	/*
-	 * This is 0 the first time through.
-	 */
-	if (s->index == 0)
-		s->private = head;
-
-	if (s->private == NULL)
-		return NULL;
-
-	list = s->private;
-	if (list_empty(list))
-		return NULL;
-
-	if (s->index == 0)
-		return list->next;
-	return list;
+	return NULL;
 }
 
 static void *smk_seq_next(struct seq_file *s, void *v, loff_t *pos,
@@ -590,17 +584,15 @@ static void *smk_seq_next(struct seq_file *s, void *v, loff_t *pos,
 {
 	struct list_head *list = v;
 
-	if (list_is_last(list, head)) {
-		s->private = NULL;
-		return NULL;
-	}
-	s->private = list->next;
-	return list->next;
+	++*pos;
+	list = rcu_dereference(list_next_rcu(list));
+
+	return (list == head) ? NULL : list;
 }
 
 static void smk_seq_stop(struct seq_file *s, void *v)
 {
-	/* No-op */
+	rcu_read_unlock();
 }
 
 static void smk_rule_show(struct seq_file *s, struct smack_rule *srp, int max)
@@ -660,7 +652,7 @@ static int load_seq_show(struct seq_file *s, void *v)
 {
 	struct list_head *list = v;
 	struct smack_master_list *smlp =
-		 list_entry(list, struct smack_master_list, list);
+		list_entry_rcu(list, struct smack_master_list, list);
 
 	smk_rule_show(s, smlp->smk_rule, SMK_LABELLEN);
 
@@ -808,7 +800,7 @@ static int cipso_seq_show(struct seq_file *s, void *v)
 {
 	struct list_head  *list = v;
 	struct smack_known *skp =
-		 list_entry(list, struct smack_known, list);
+		list_entry_rcu(list, struct smack_known, list);
 	struct netlbl_lsm_catmap *cmp = skp->smk_netlabel.attr.mls.cat;
 	char sep = '/';
 	int i;
@@ -999,7 +991,7 @@ static int cipso2_seq_show(struct seq_file *s, void *v)
 {
 	struct list_head  *list = v;
 	struct smack_known *skp =
-		 list_entry(list, struct smack_known, list);
+		list_entry_rcu(list, struct smack_known, list);
 	struct netlbl_lsm_catmap *cmp = skp->smk_netlabel.attr.mls.cat;
 	char sep = '/';
 	int i;
@@ -1083,7 +1075,7 @@ static int netlbladdr_seq_show(struct seq_file *s, void *v)
 {
 	struct list_head *list = v;
 	struct smk_netlbladdr *skp =
-			 list_entry(list, struct smk_netlbladdr, list);
+			list_entry_rcu(list, struct smk_netlbladdr, list);
 	unsigned char *hp = (char *) &skp->smk_host.sin_addr.s_addr;
 	int maskn;
 	u32 temp_mask = be32_to_cpu(skp->smk_mask.s_addr);
@@ -1917,7 +1909,7 @@ static int load_self_seq_show(struct seq_file *s, void *v)
 {
 	struct list_head *list = v;
 	struct smack_rule *srp =
-		 list_entry(list, struct smack_rule, list);
+		list_entry_rcu(list, struct smack_rule, list);
 
 	smk_rule_show(s, srp, SMK_LABELLEN);
 
@@ -2046,7 +2038,7 @@ static int load2_seq_show(struct seq_file *s, void *v)
 {
 	struct list_head *list = v;
 	struct smack_master_list *smlp =
-		 list_entry(list, struct smack_master_list, list);
+		list_entry_rcu(list, struct smack_master_list, list);
 
 	smk_rule_show(s, smlp->smk_rule, SMK_LONGLABEL);
 
@@ -2123,7 +2115,7 @@ static int load_self2_seq_show(struct seq_file *s, void *v)
 {
 	struct list_head *list = v;
 	struct smack_rule *srp =
-		 list_entry(list, struct smack_rule, list);
+		list_entry_rcu(list, struct smack_rule, list);
 
 	smk_rule_show(s, srp, SMK_LONGLABEL);
 
-- 
2.1.4

[PATCH 2/2] Smack: allow multiple labels in onlycap

[Rafal Krypa]

Smack onlycap allows limiting of CAP_MAC_ADMIN and CAP_MAC_OVERRIDE to
processes running with the configured label. But having single privileged
label is not enough in some real use cases. On a complex system like Tizen,
there maybe few programs that need to configure Smack policy in run-time
and running them all with a single label is not always practical.
This patch extends onlycap feature for multiple labels. They are configured
in the same smackfs "onlycap" interface, separated by spaces.

Signed-off-by: Rafal Krypa <r.krypa@samsung.com>
---
 Documentation/security/Smack.txt |   6 +-
 security/smack/smack.h           |  25 +++---
 security/smack/smack_access.c    |  41 ++++++++++
 security/smack/smackfs.c         | 162 ++++++++++++++++++++++++++-------------
 4 files changed, 162 insertions(+), 72 deletions(-)

diff --git a/Documentation/security/Smack.txt b/Documentation/security/Smack.txt
index abc82f8..de5e1ae 100644
--- a/Documentation/security/Smack.txt
+++ b/Documentation/security/Smack.txt
@@ -206,11 +206,11 @@ netlabel
 	label. The format accepted on write is:
 		"%d.%d.%d.%d label" or "%d.%d.%d.%d/%d label".
 onlycap
-	This contains the label processes must have for CAP_MAC_ADMIN
+	This contains labels processes must have for CAP_MAC_ADMIN
 	and CAP_MAC_OVERRIDE to be effective. If this file is empty
 	these capabilities are effective at for processes with any
-	label. The value is set by writing the desired label to the
-	file or cleared by writing "-" to the file.
+	label. The values are set by writing the desired labels, separated
+	by spaces, to the file or cleared by writing "-" to the file.
 ptrace
 	This is used to define the current ptrace policy
 	0 - default: this is the policy that relies on Smack access rules.
diff --git a/security/smack/smack.h b/security/smack/smack.h
index b8c1a86..244e035 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -138,6 +138,11 @@ struct smk_port_label {
 	struct smack_known	*smk_out;	/* outgoing label */
 };
 
+struct smack_onlycap {
+	struct list_head	list;
+	struct smack_known	*smk_label;
+};
+
 /*
  * Mount options
  */
@@ -249,6 +254,7 @@ int smk_netlbl_mls(int, char *, struct netlbl_lsm_secattr *, int);
 struct smack_known *smk_import_entry(const char *, int);
 void smk_insert_entry(struct smack_known *skp);
 struct smack_known *smk_find_entry(const char *);
+int smack_privileged(int cap);
 
 /*
  * Shared data.
@@ -257,7 +263,6 @@ extern int smack_enabled;
 extern int smack_cipso_direct;
 extern int smack_cipso_mapped;
 extern struct smack_known *smack_net_ambient;
-extern struct smack_known *smack_onlycap;
 extern struct smack_known *smack_syslog_label;
 #ifdef CONFIG_SECURITY_SMACK_BRINGUP
 extern struct smack_known *smack_unconfined;
@@ -276,6 +281,9 @@ extern struct mutex	smack_known_lock;
 extern struct list_head smack_known_list;
 extern struct list_head smk_netlbladdr_list;
 
+extern struct mutex     smack_onlycap_lock;
+extern struct list_head smack_onlycap_list;
+
 #define SMACK_HASH_SLOTS 16
 extern struct hlist_head smack_known_hash[SMACK_HASH_SLOTS];
 
@@ -332,21 +340,6 @@ static inline struct smack_known *smk_of_current(void)
 }
 
 /*
- * Is the task privileged and allowed to be privileged
- * by the onlycap rule.
- */
-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 == skp)
-		return 1;
-	return 0;
-}
-
-/*
  * logging functions
  */
 #define SMACK_AUDIT_DENIED 0x1
diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c
index 408e20b..00f6b38 100644
--- a/security/smack/smack_access.c
+++ b/security/smack/smack_access.c
@@ -617,3 +617,44 @@ struct smack_known *smack_from_secid(const u32 secid)
 	rcu_read_unlock();
 	return &smack_known_invalid;
 }
+
+/*
+ * Unless a process is running with one of these labels
+ * even having CAP_MAC_OVERRIDE isn't enough to grant
+ * privilege to violate MAC policy. If no labels are
+ * designated (the empty list case) capabilities apply to
+ * everyone.
+ */
+LIST_HEAD(smack_onlycap_list);
+DEFINE_MUTEX(smack_onlycap_lock);
+
+/*
+ * Is the task privileged and allowed to be privileged
+ * by the onlycap rule.
+ *
+ * Returns 1 if the task is allowed to be privileged, 0 if it's not.
+ */
+int smack_privileged(int cap)
+{
+	struct smack_known *skp = smk_of_current();
+	struct smack_onlycap *sop;
+
+	if (!capable(cap))
+		return 0;
+
+	rcu_read_lock();
+	if (list_empty(&smack_onlycap_list)) {
+		rcu_read_unlock();
+		return 1;
+	}
+
+	list_for_each_entry_rcu(sop, &smack_onlycap_list, list) {
+		if (sop->smk_label == skp) {
+			rcu_read_unlock();
+			return 1;
+		}
+	}
+	rcu_read_unlock();
+
+	return 0;
+}
diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index e40dc48..2aabcb2 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -87,16 +87,6 @@ int smack_cipso_direct = SMACK_CIPSO_DIRECT_DEFAULT;
  */
 int smack_cipso_mapped = SMACK_CIPSO_MAPPED_DEFAULT;
 
-/*
- * Unless a process is running with this label even
- * having CAP_MAC_OVERRIDE isn't enough to grant
- * privilege to violate MAC policy. If no label is
- * designated (the NULL case) capabilities apply to
- * everyone. It is expected that the hat (^) label
- * will be used if any label is used.
- */
-struct smack_known *smack_onlycap;
-
 #ifdef CONFIG_SECURITY_SMACK_BRINGUP
 /*
  * Allow one label to be unconfined. This is for
@@ -1636,34 +1626,78 @@ static const struct file_operations smk_ambient_ops = {
 	.llseek		= default_llseek,
 };
 
-/**
- * smk_read_onlycap - read() for smackfs/onlycap
- * @filp: file pointer, not actually used
- * @buf: where to put the result
- * @cn: maximum to send along
- * @ppos: where to start
- *
- * Returns number of bytes read or error code, as appropriate
+/*
+ * Seq_file operations for /smack/onlycap
  */
-static ssize_t smk_read_onlycap(struct file *filp, char __user *buf,
-				size_t cn, loff_t *ppos)
+static void *onlycap_seq_start(struct seq_file *s, loff_t *pos)
 {
-	char *smack = "";
-	ssize_t rc = -EINVAL;
-	int asize;
+	return smk_seq_start(s, pos, &smack_onlycap_list);
+}
 
-	if (*ppos != 0)
-		return 0;
+static void *onlycap_seq_next(struct seq_file *s, void *v, loff_t *pos)
+{
+	return smk_seq_next(s, v, pos, &smack_onlycap_list);
+}
 
-	if (smack_onlycap != NULL)
-		smack = smack_onlycap->smk_known;
+static int onlycap_seq_show(struct seq_file *s, void *v)
+{
+	struct list_head *list = v;
+	struct smack_onlycap *sop =
+		list_entry_rcu(list, struct smack_onlycap, list);
 
-	asize = strlen(smack) + 1;
+	seq_puts(s, sop->smk_label->smk_known);
+	seq_putc(s, ' ');
 
-	if (cn >= asize)
-		rc = simple_read_from_buffer(buf, cn, ppos, smack, asize);
+	return 0;
+}
 
-	return rc;
+static const struct seq_operations onlycap_seq_ops = {
+	.start = onlycap_seq_start,
+	.next  = onlycap_seq_next,
+	.show  = onlycap_seq_show,
+	.stop  = smk_seq_stop,
+};
+
+static int smk_open_onlycap(struct inode *inode, struct file *file)
+{
+	return seq_open(file, &onlycap_seq_ops);
+}
+
+/**
+ * list_swap_rcu - swap public list with a private one in RCU-safe way
+ * The caller must hold appropriate mutex to prevent concurrent modifications
+ * to the public list.
+ * Private list is assumed to be not accessible to other threads yet.
+ *
+ * @public: public list
+ * @private: private list
+ */
+static void list_swap_rcu(struct list_head *public, struct list_head *private)
+{
+	struct list_head *first, *last;
+
+	if (list_empty(public)) {
+		list_splice_init_rcu(private, public, synchronize_rcu);
+	} else {
+		/* Remember public list before replacing it */
+		first = public->next;
+		last = public->prev;
+
+		/* Publish private list in place of public in RCU-safe way */
+		private->prev->next = public;
+		private->next->prev = public;
+		rcu_assign_pointer(public->next, private->next);
+		public->prev = private->prev;
+
+		synchronize_rcu();
+
+		/* When all readers are done with the old public list,
+		 * attach it in place of private */
+		private->next = first;
+		private->prev = last;
+		first->prev = private;
+		last->next = private;
+	}
 }
 
 /**
@@ -1679,28 +1713,47 @@ static ssize_t smk_write_onlycap(struct file *file, const char __user *buf,
 				 size_t count, loff_t *ppos)
 {
 	char *data;
-	struct smack_known *skp = smk_of_task(current->cred->security);
+	char *data_parse
+	char *tok;
+	struct smack_known *skp;
+	struct smack_onlycap *sop;
+	struct smack_onlycap *sop2;
+	LIST_HEAD(list_tmp);
 	int rc = count;
 
 	if (!smack_privileged(CAP_MAC_ADMIN))
 		return -EPERM;
 
-	/*
-	 * This can be done using smk_access() but is done
-	 * explicitly for clarity. The smk_access() implementation
-	 * would use smk_access(smack_onlycap, MAY_WRITE)
-	 */
-	if (smack_onlycap != NULL && smack_onlycap != skp)
-		return -EPERM;
-
 	data = kzalloc(count + 1, GFP_KERNEL);
 	if (data == NULL)
 		return -ENOMEM;
 
 	if (copy_from_user(data, buf, count) != 0) {
-		rc = -EFAULT;
-		goto freeout;
+		kfree(data);
+		return -EFAULT;
+	}
+
+	data_parse = data;
+	while ((tok = strsep(&data_parse, " ")) != NULL) {
+		if (!*tok)
+			continue;
+
+		skp = smk_import_entry(tok, 0);
+		if (IS_ERR(skp)) {
+			rc = PTR_ERR(skp);
+			break;
+		}
+
+		sop = kzalloc(sizeof(*sop), GFP_KERNEL);
+		if (sop == NULL) {
+			rc = -ENOMEM;
+			break;
+		}
+
+		sop->smk_label = skp;
+		list_add_rcu(&sop->list, &list_tmp);
 	}
+	kfree(data);
 
 	/*
 	 * Clear the smack_onlycap on invalid label errors. This means
@@ -1710,26 +1763,29 @@ static ssize_t smk_write_onlycap(struct file *file, const char __user *buf,
 	 * so "-usecapabilities" will also work.
 	 *
 	 * But do so only on invalid label, not on system errors.
+	 * The invalid label must be first to count as clearing attempt.
 	 */
-	skp = smk_import_entry(data, count);
-	if (PTR_ERR(skp) == -EINVAL)
-		skp = NULL;
-	else if (IS_ERR(skp)) {
-		rc = PTR_ERR(skp);
-		goto freeout;
+	if (rc == -EINVAL && list_empty(&list_tmp))
+		rc = count;
+
+	if (rc >= 0) {
+		mutex_lock(&smack_onlycap_lock);
+		list_swap_rcu(&smack_onlycap_list, &list_tmp);
+		mutex_unlock(&smack_onlycap_lock);
 	}
 
-	smack_onlycap = skp;
+	list_for_each_entry_safe(sop, sop2, &list_tmp, list)
+		kfree(sop);
 
-freeout:
-	kfree(data);
 	return rc;
 }
 
 static const struct file_operations smk_onlycap_ops = {
-	.read		= smk_read_onlycap,
+	.open		= smk_open_onlycap,
+	.read		= seq_read,
 	.write		= smk_write_onlycap,
-	.llseek		= default_llseek,
+	.llseek		= seq_lseek,
+	.release	= seq_release,
 };
 
 #ifdef CONFIG_SECURITY_SMACK_BRINGUP
-- 
2.1.4

Re: [PATCH 2/2] Smack: allow multiple labels in onlycap

[Casey Schaufler]

On 5/21/2015 9:24 AM, Rafal Krypa wrote:
> Smack onlycap allows limiting of CAP_MAC_ADMIN and CAP_MAC_OVERRIDE to
> processes running with the configured label. But having single privileged
> label is not enough in some real use cases. On a complex system like Tizen,
> there maybe few programs that need to configure Smack policy in run-time
> and running them all with a single label is not always practical.
> This patch extends onlycap feature for multiple labels. They are configured
> in the same smackfs "onlycap" interface, separated by spaces.
>
> Signed-off-by: Rafal Krypa <r.krypa@samsung.com>

I have a comment below. I will test this out while you work
on the (simple) revision.

> ---
>  Documentation/security/Smack.txt |   6 +-
>  security/smack/smack.h           |  25 +++---
>  security/smack/smack_access.c    |  41 ++++++++++
>  security/smack/smackfs.c         | 162 ++++++++++++++++++++++++++-------------
>  4 files changed, 162 insertions(+), 72 deletions(-)
>
> diff --git a/Documentation/security/Smack.txt b/Documentation/security/Smack.txt
> index abc82f8..de5e1ae 100644
> --- a/Documentation/security/Smack.txt
> +++ b/Documentation/security/Smack.txt
> @@ -206,11 +206,11 @@ netlabel
>  	label. The format accepted on write is:
>  		"%d.%d.%d.%d label" or "%d.%d.%d.%d/%d label".
>  onlycap
> -	This contains the label processes must have for CAP_MAC_ADMIN
> +	This contains labels processes must have for CAP_MAC_ADMIN
>  	and CAP_MAC_OVERRIDE to be effective. If this file is empty
>  	these capabilities are effective at for processes with any
> -	label. The value is set by writing the desired label to the
> -	file or cleared by writing "-" to the file.
> +	label. The values are set by writing the desired labels, separated
> +	by spaces, to the file or cleared by writing "-" to the file.
>  ptrace
>  	This is used to define the current ptrace policy
>  	0 - default: this is the policy that relies on Smack access rules.
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index b8c1a86..244e035 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -138,6 +138,11 @@ struct smk_port_label {
>  	struct smack_known	*smk_out;	/* outgoing label */
>  };
>  
> +struct smack_onlycap {
> +	struct list_head	list;
> +	struct smack_known	*smk_label;
> +};
> +
>  /*
>   * Mount options
>   */
> @@ -249,6 +254,7 @@ int smk_netlbl_mls(int, char *, struct netlbl_lsm_secattr *, int);
>  struct smack_known *smk_import_entry(const char *, int);
>  void smk_insert_entry(struct smack_known *skp);
>  struct smack_known *smk_find_entry(const char *);
> +int smack_privileged(int cap);
>  
>  /*
>   * Shared data.
> @@ -257,7 +263,6 @@ extern int smack_enabled;
>  extern int smack_cipso_direct;
>  extern int smack_cipso_mapped;
>  extern struct smack_known *smack_net_ambient;
> -extern struct smack_known *smack_onlycap;
>  extern struct smack_known *smack_syslog_label;
>  #ifdef CONFIG_SECURITY_SMACK_BRINGUP
>  extern struct smack_known *smack_unconfined;
> @@ -276,6 +281,9 @@ extern struct mutex	smack_known_lock;
>  extern struct list_head smack_known_list;
>  extern struct list_head smk_netlbladdr_list;
>  
> +extern struct mutex     smack_onlycap_lock;
> +extern struct list_head smack_onlycap_list;
> +
>  #define SMACK_HASH_SLOTS 16
>  extern struct hlist_head smack_known_hash[SMACK_HASH_SLOTS];
>  
> @@ -332,21 +340,6 @@ static inline struct smack_known *smk_of_current(void)
>  }
>  
>  /*
> - * Is the task privileged and allowed to be privileged
> - * by the onlycap rule.
> - */
> -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 == skp)
> -		return 1;
> -	return 0;
> -}
> -
> -/*
>   * logging functions
>   */
>  #define SMACK_AUDIT_DENIED 0x1
> diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c
> index 408e20b..00f6b38 100644
> --- a/security/smack/smack_access.c
> +++ b/security/smack/smack_access.c
> @@ -617,3 +617,44 @@ struct smack_known *smack_from_secid(const u32 secid)
>  	rcu_read_unlock();
>  	return &smack_known_invalid;
>  }
> +
> +/*
> + * Unless a process is running with one of these labels
> + * even having CAP_MAC_OVERRIDE isn't enough to grant
> + * privilege to violate MAC policy. If no labels are
> + * designated (the empty list case) capabilities apply to
> + * everyone.
> + */
> +LIST_HEAD(smack_onlycap_list);
> +DEFINE_MUTEX(smack_onlycap_lock);
> +
> +/*
> + * Is the task privileged and allowed to be privileged
> + * by the onlycap rule.
> + *
> + * Returns 1 if the task is allowed to be privileged, 0 if it's not.
> + */
> +int smack_privileged(int cap)
> +{
> +	struct smack_known *skp = smk_of_current();
> +	struct smack_onlycap *sop;
> +
> +	if (!capable(cap))
> +		return 0;
> +
> +	rcu_read_lock();
> +	if (list_empty(&smack_onlycap_list)) {
> +		rcu_read_unlock();
> +		return 1;
> +	}
> +
> +	list_for_each_entry_rcu(sop, &smack_onlycap_list, list) {
> +		if (sop->smk_label == skp) {
> +			rcu_read_unlock();
> +			return 1;
> +		}
> +	}
> +	rcu_read_unlock();
> +
> +	return 0;
> +}
> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index e40dc48..2aabcb2 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c
> @@ -87,16 +87,6 @@ int smack_cipso_direct = SMACK_CIPSO_DIRECT_DEFAULT;
>   */
>  int smack_cipso_mapped = SMACK_CIPSO_MAPPED_DEFAULT;
>  
> -/*
> - * Unless a process is running with this label even
> - * having CAP_MAC_OVERRIDE isn't enough to grant
> - * privilege to violate MAC policy. If no label is
> - * designated (the NULL case) capabilities apply to
> - * everyone. It is expected that the hat (^) label
> - * will be used if any label is used.
> - */
> -struct smack_known *smack_onlycap;
> -
>  #ifdef CONFIG_SECURITY_SMACK_BRINGUP
>  /*
>   * Allow one label to be unconfined. This is for
> @@ -1636,34 +1626,78 @@ static const struct file_operations smk_ambient_ops = {
>  	.llseek		= default_llseek,
>  };
>  
> -/**
> - * smk_read_onlycap - read() for smackfs/onlycap
> - * @filp: file pointer, not actually used
> - * @buf: where to put the result
> - * @cn: maximum to send along
> - * @ppos: where to start
> - *
> - * Returns number of bytes read or error code, as appropriate
> +/*
> + * Seq_file operations for /smack/onlycap
>   */
> -static ssize_t smk_read_onlycap(struct file *filp, char __user *buf,
> -				size_t cn, loff_t *ppos)
> +static void *onlycap_seq_start(struct seq_file *s, loff_t *pos)
>  {
> -	char *smack = "";
> -	ssize_t rc = -EINVAL;
> -	int asize;
> +	return smk_seq_start(s, pos, &smack_onlycap_list);
> +}
>  
> -	if (*ppos != 0)
> -		return 0;
> +static void *onlycap_seq_next(struct seq_file *s, void *v, loff_t *pos)
> +{
> +	return smk_seq_next(s, v, pos, &smack_onlycap_list);
> +}
>  
> -	if (smack_onlycap != NULL)
> -		smack = smack_onlycap->smk_known;
> +static int onlycap_seq_show(struct seq_file *s, void *v)
> +{
> +	struct list_head *list = v;
> +	struct smack_onlycap *sop =
> +		list_entry_rcu(list, struct smack_onlycap, list);
>  
> -	asize = strlen(smack) + 1;
> +	seq_puts(s, sop->smk_label->smk_known);
> +	seq_putc(s, ' ');
>  
> -	if (cn >= asize)
> -		rc = simple_read_from_buffer(buf, cn, ppos, smack, asize);
> +	return 0;
> +}
>  
> -	return rc;
> +static const struct seq_operations onlycap_seq_ops = {
> +	.start = onlycap_seq_start,
> +	.next  = onlycap_seq_next,
> +	.show  = onlycap_seq_show,
> +	.stop  = smk_seq_stop,
> +};
> +
> +static int smk_open_onlycap(struct inode *inode, struct file *file)
> +{
> +	return seq_open(file, &onlycap_seq_ops);
> +}
> +
> +/**
> + * list_swap_rcu - swap public list with a private one in RCU-safe way
> + * The caller must hold appropriate mutex to prevent concurrent modifications
> + * to the public list.
> + * Private list is assumed to be not accessible to other threads yet.
> + *
> + * @public: public list
> + * @private: private list
> + */
> +static void list_swap_rcu(struct list_head *public, struct list_head *private)

Let's not get in the habit of using names that reasonably belong to someone else.
Change this to smk_list_swap_rcu() or some such name. Starting with "list_" is
asking from confusion.

> +{
> +	struct list_head *first, *last;
> +
> +	if (list_empty(public)) {
> +		list_splice_init_rcu(private, public, synchronize_rcu);
> +	} else {
> +		/* Remember public list before replacing it */
> +		first = public->next;
> +		last = public->prev;
> +
> +		/* Publish private list in place of public in RCU-safe way */
> +		private->prev->next = public;
> +		private->next->prev = public;
> +		rcu_assign_pointer(public->next, private->next);
> +		public->prev = private->prev;
> +
> +		synchronize_rcu();
> +
> +		/* When all readers are done with the old public list,
> +		 * attach it in place of private */
> +		private->next = first;
> +		private->prev = last;
> +		first->prev = private;
> +		last->next = private;
> +	}
>  }
>  
>  /**
> @@ -1679,28 +1713,47 @@ static ssize_t smk_write_onlycap(struct file *file, const char __user *buf,
>  				 size_t count, loff_t *ppos)
>  {
>  	char *data;
> -	struct smack_known *skp = smk_of_task(current->cred->security);
> +	char *data_parse
> +	char *tok;
> +	struct smack_known *skp;
> +	struct smack_onlycap *sop;
> +	struct smack_onlycap *sop2;
> +	LIST_HEAD(list_tmp);
>  	int rc = count;
>  
>  	if (!smack_privileged(CAP_MAC_ADMIN))
>  		return -EPERM;
>  
> -	/*
> -	 * This can be done using smk_access() but is done
> -	 * explicitly for clarity. The smk_access() implementation
> -	 * would use smk_access(smack_onlycap, MAY_WRITE)
> -	 */
> -	if (smack_onlycap != NULL && smack_onlycap != skp)
> -		return -EPERM;
> -
>  	data = kzalloc(count + 1, GFP_KERNEL);
>  	if (data == NULL)
>  		return -ENOMEM;
>  
>  	if (copy_from_user(data, buf, count) != 0) {
> -		rc = -EFAULT;
> -		goto freeout;
> +		kfree(data);
> +		return -EFAULT;
> +	}
> +
> +	data_parse = data;
> +	while ((tok = strsep(&data_parse, " ")) != NULL) {
> +		if (!*tok)
> +			continue;
> +
> +		skp = smk_import_entry(tok, 0);
> +		if (IS_ERR(skp)) {
> +			rc = PTR_ERR(skp);
> +			break;
> +		}
> +
> +		sop = kzalloc(sizeof(*sop), GFP_KERNEL);
> +		if (sop == NULL) {
> +			rc = -ENOMEM;
> +			break;
> +		}
> +
> +		sop->smk_label = skp;
> +		list_add_rcu(&sop->list, &list_tmp);
>  	}
> +	kfree(data);
>  
>  	/*
>  	 * Clear the smack_onlycap on invalid label errors. This means
> @@ -1710,26 +1763,29 @@ static ssize_t smk_write_onlycap(struct file *file, const char __user *buf,
>  	 * so "-usecapabilities" will also work.
>  	 *
>  	 * But do so only on invalid label, not on system errors.
> +	 * The invalid label must be first to count as clearing attempt.
>  	 */
> -	skp = smk_import_entry(data, count);
> -	if (PTR_ERR(skp) == -EINVAL)
> -		skp = NULL;
> -	else if (IS_ERR(skp)) {
> -		rc = PTR_ERR(skp);
> -		goto freeout;
> +	if (rc == -EINVAL && list_empty(&list_tmp))
> +		rc = count;
> +
> +	if (rc >= 0) {
> +		mutex_lock(&smack_onlycap_lock);
> +		list_swap_rcu(&smack_onlycap_list, &list_tmp);
> +		mutex_unlock(&smack_onlycap_lock);
>  	}
>  
> -	smack_onlycap = skp;
> +	list_for_each_entry_safe(sop, sop2, &list_tmp, list)
> +		kfree(sop);
>  
> -freeout:
> -	kfree(data);
>  	return rc;
>  }
>  
>  static const struct file_operations smk_onlycap_ops = {
> -	.read		= smk_read_onlycap,
> +	.open		= smk_open_onlycap,
> +	.read		= seq_read,
>  	.write		= smk_write_onlycap,
> -	.llseek		= default_llseek,
> +	.llseek		= seq_lseek,
> +	.release	= seq_release,
>  };
>  
>  #ifdef CONFIG_SECURITY_SMACK_BRINGUP

[PATCH 2/2 v2] Smack: allow multiple labels in onlycap

[Rafal Krypa]

Smack onlycap allows limiting of CAP_MAC_ADMIN and CAP_MAC_OVERRIDE to
processes running with the configured label. But having single privileged
label is not enough in some real use cases. On a complex system like Tizen,
there maybe few programs that need to configure Smack policy in run-time
and running them all with a single label is not always practical.
This patch extends onlycap feature for multiple labels. They are configured
in the same smackfs "onlycap" interface, separated by spaces.

Signed-off-by: Rafal Krypa <r.krypa@samsung.com>
---
 Documentation/security/Smack.txt |   6 +-
 security/smack/smack.h           |  25 +++---
 security/smack/smack_access.c    |  41 ++++++++++
 security/smack/smackfs.c         | 163 ++++++++++++++++++++++++++-------------
 4 files changed, 163 insertions(+), 72 deletions(-)

diff --git a/Documentation/security/Smack.txt b/Documentation/security/Smack.txt
index abc82f8..de5e1ae 100644
--- a/Documentation/security/Smack.txt
+++ b/Documentation/security/Smack.txt
@@ -206,11 +206,11 @@ netlabel
 	label. The format accepted on write is:
 		"%d.%d.%d.%d label" or "%d.%d.%d.%d/%d label".
 onlycap
-	This contains the label processes must have for CAP_MAC_ADMIN
+	This contains labels processes must have for CAP_MAC_ADMIN
 	and CAP_MAC_OVERRIDE to be effective. If this file is empty
 	these capabilities are effective at for processes with any
-	label. The value is set by writing the desired label to the
-	file or cleared by writing "-" to the file.
+	label. The values are set by writing the desired labels, separated
+	by spaces, to the file or cleared by writing "-" to the file.
 ptrace
 	This is used to define the current ptrace policy
 	0 - default: this is the policy that relies on Smack access rules.
diff --git a/security/smack/smack.h b/security/smack/smack.h
index b8c1a86..244e035 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -138,6 +138,11 @@ struct smk_port_label {
 	struct smack_known	*smk_out;	/* outgoing label */
 };
 
+struct smack_onlycap {
+	struct list_head	list;
+	struct smack_known	*smk_label;
+};
+
 /*
  * Mount options
  */
@@ -249,6 +254,7 @@ int smk_netlbl_mls(int, char *, struct netlbl_lsm_secattr *, int);
 struct smack_known *smk_import_entry(const char *, int);
 void smk_insert_entry(struct smack_known *skp);
 struct smack_known *smk_find_entry(const char *);
+int smack_privileged(int cap);
 
 /*
  * Shared data.
@@ -257,7 +263,6 @@ extern int smack_enabled;
 extern int smack_cipso_direct;
 extern int smack_cipso_mapped;
 extern struct smack_known *smack_net_ambient;
-extern struct smack_known *smack_onlycap;
 extern struct smack_known *smack_syslog_label;
 #ifdef CONFIG_SECURITY_SMACK_BRINGUP
 extern struct smack_known *smack_unconfined;
@@ -276,6 +281,9 @@ extern struct mutex	smack_known_lock;
 extern struct list_head smack_known_list;
 extern struct list_head smk_netlbladdr_list;
 
+extern struct mutex     smack_onlycap_lock;
+extern struct list_head smack_onlycap_list;
+
 #define SMACK_HASH_SLOTS 16
 extern struct hlist_head smack_known_hash[SMACK_HASH_SLOTS];
 
@@ -332,21 +340,6 @@ static inline struct smack_known *smk_of_current(void)
 }
 
 /*
- * Is the task privileged and allowed to be privileged
- * by the onlycap rule.
- */
-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 == skp)
-		return 1;
-	return 0;
-}
-
-/*
  * logging functions
  */
 #define SMACK_AUDIT_DENIED 0x1
diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c
index 408e20b..00f6b38 100644
--- a/security/smack/smack_access.c
+++ b/security/smack/smack_access.c
@@ -617,3 +617,44 @@ struct smack_known *smack_from_secid(const u32 secid)
 	rcu_read_unlock();
 	return &smack_known_invalid;
 }
+
+/*
+ * Unless a process is running with one of these labels
+ * even having CAP_MAC_OVERRIDE isn't enough to grant
+ * privilege to violate MAC policy. If no labels are
+ * designated (the empty list case) capabilities apply to
+ * everyone.
+ */
+LIST_HEAD(smack_onlycap_list);
+DEFINE_MUTEX(smack_onlycap_lock);
+
+/*
+ * Is the task privileged and allowed to be privileged
+ * by the onlycap rule.
+ *
+ * Returns 1 if the task is allowed to be privileged, 0 if it's not.
+ */
+int smack_privileged(int cap)
+{
+	struct smack_known *skp = smk_of_current();
+	struct smack_onlycap *sop;
+
+	if (!capable(cap))
+		return 0;
+
+	rcu_read_lock();
+	if (list_empty(&smack_onlycap_list)) {
+		rcu_read_unlock();
+		return 1;
+	}
+
+	list_for_each_entry_rcu(sop, &smack_onlycap_list, list) {
+		if (sop->smk_label == skp) {
+			rcu_read_unlock();
+			return 1;
+		}
+	}
+	rcu_read_unlock();
+
+	return 0;
+}
diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index e40dc48..f1c22a8 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -87,16 +87,6 @@ int smack_cipso_direct = SMACK_CIPSO_DIRECT_DEFAULT;
  */
 int smack_cipso_mapped = SMACK_CIPSO_MAPPED_DEFAULT;
 
-/*
- * Unless a process is running with this label even
- * having CAP_MAC_OVERRIDE isn't enough to grant
- * privilege to violate MAC policy. If no label is
- * designated (the NULL case) capabilities apply to
- * everyone. It is expected that the hat (^) label
- * will be used if any label is used.
- */
-struct smack_known *smack_onlycap;
-
 #ifdef CONFIG_SECURITY_SMACK_BRINGUP
 /*
  * Allow one label to be unconfined. This is for
@@ -1636,34 +1626,79 @@ static const struct file_operations smk_ambient_ops = {
 	.llseek		= default_llseek,
 };
 
-/**
- * smk_read_onlycap - read() for smackfs/onlycap
- * @filp: file pointer, not actually used
- * @buf: where to put the result
- * @cn: maximum to send along
- * @ppos: where to start
- *
- * Returns number of bytes read or error code, as appropriate
+/*
+ * Seq_file operations for /smack/onlycap
  */
-static ssize_t smk_read_onlycap(struct file *filp, char __user *buf,
-				size_t cn, loff_t *ppos)
+static void *onlycap_seq_start(struct seq_file *s, loff_t *pos)
 {
-	char *smack = "";
-	ssize_t rc = -EINVAL;
-	int asize;
+	return smk_seq_start(s, pos, &smack_onlycap_list);
+}
 
-	if (*ppos != 0)
-		return 0;
+static void *onlycap_seq_next(struct seq_file *s, void *v, loff_t *pos)
+{
+	return smk_seq_next(s, v, pos, &smack_onlycap_list);
+}
 
-	if (smack_onlycap != NULL)
-		smack = smack_onlycap->smk_known;
+static int onlycap_seq_show(struct seq_file *s, void *v)
+{
+	struct list_head *list = v;
+	struct smack_onlycap *sop =
+		list_entry_rcu(list, struct smack_onlycap, list);
 
-	asize = strlen(smack) + 1;
+	seq_puts(s, sop->smk_label->smk_known);
+	seq_putc(s, ' ');
 
-	if (cn >= asize)
-		rc = simple_read_from_buffer(buf, cn, ppos, smack, asize);
+	return 0;
+}
 
-	return rc;
+static const struct seq_operations onlycap_seq_ops = {
+	.start = onlycap_seq_start,
+	.next  = onlycap_seq_next,
+	.show  = onlycap_seq_show,
+	.stop  = smk_seq_stop,
+};
+
+static int smk_open_onlycap(struct inode *inode, struct file *file)
+{
+	return seq_open(file, &onlycap_seq_ops);
+}
+
+/**
+ * smk_list_swap_rcu - swap public list with a private one in RCU-safe way
+ * The caller must hold appropriate mutex to prevent concurrent modifications
+ * to the public list.
+ * Private list is assumed to be not accessible to other threads yet.
+ *
+ * @public: public list
+ * @private: private list
+ */
+static void smk_list_swap_rcu(struct list_head *public,
+			      struct list_head *private)
+{
+	struct list_head *first, *last;
+
+	if (list_empty(public)) {
+		list_splice_init_rcu(private, public, synchronize_rcu);
+	} else {
+		/* Remember public list before replacing it */
+		first = public->next;
+		last = public->prev;
+
+		/* Publish private list in place of public in RCU-safe way */
+		private->prev->next = public;
+		private->next->prev = public;
+		rcu_assign_pointer(public->next, private->next);
+		public->prev = private->prev;
+
+		synchronize_rcu();
+
+		/* When all readers are done with the old public list,
+		 * attach it in place of private */
+		private->next = first;
+		private->prev = last;
+		first->prev = private;
+		last->next = private;
+	}
 }
 
 /**
@@ -1679,28 +1714,47 @@ static ssize_t smk_write_onlycap(struct file *file, const char __user *buf,
 				 size_t count, loff_t *ppos)
 {
 	char *data;
-	struct smack_known *skp = smk_of_task(current->cred->security);
+	char *data_parse;
+	char *tok;
+	struct smack_known *skp;
+	struct smack_onlycap *sop;
+	struct smack_onlycap *sop2;
+	LIST_HEAD(list_tmp);
 	int rc = count;
 
 	if (!smack_privileged(CAP_MAC_ADMIN))
 		return -EPERM;
 
-	/*
-	 * This can be done using smk_access() but is done
-	 * explicitly for clarity. The smk_access() implementation
-	 * would use smk_access(smack_onlycap, MAY_WRITE)
-	 */
-	if (smack_onlycap != NULL && smack_onlycap != skp)
-		return -EPERM;
-
 	data = kzalloc(count + 1, GFP_KERNEL);
 	if (data == NULL)
 		return -ENOMEM;
 
 	if (copy_from_user(data, buf, count) != 0) {
-		rc = -EFAULT;
-		goto freeout;
+		kfree(data);
+		return -EFAULT;
+	}
+
+	data_parse = data;
+	while ((tok = strsep(&data_parse, " ")) != NULL) {
+		if (!*tok)
+			continue;
+
+		skp = smk_import_entry(tok, 0);
+		if (IS_ERR(skp)) {
+			rc = PTR_ERR(skp);
+			break;
+		}
+
+		sop = kzalloc(sizeof(*sop), GFP_KERNEL);
+		if (sop == NULL) {
+			rc = -ENOMEM;
+			break;
+		}
+
+		sop->smk_label = skp;
+		list_add_rcu(&sop->list, &list_tmp);
 	}
+	kfree(data);
 
 	/*
 	 * Clear the smack_onlycap on invalid label errors. This means
@@ -1710,26 +1764,29 @@ static ssize_t smk_write_onlycap(struct file *file, const char __user *buf,
 	 * so "-usecapabilities" will also work.
 	 *
 	 * But do so only on invalid label, not on system errors.
+	 * The invalid label must be first to count as clearing attempt.
 	 */
-	skp = smk_import_entry(data, count);
-	if (PTR_ERR(skp) == -EINVAL)
-		skp = NULL;
-	else if (IS_ERR(skp)) {
-		rc = PTR_ERR(skp);
-		goto freeout;
+	if (rc == -EINVAL && list_empty(&list_tmp))
+		rc = count;
+
+	if (rc >= 0) {
+		mutex_lock(&smack_onlycap_lock);
+		smk_list_swap_rcu(&smack_onlycap_list, &list_tmp);
+		mutex_unlock(&smack_onlycap_lock);
 	}
 
-	smack_onlycap = skp;
+	list_for_each_entry_safe(sop, sop2, &list_tmp, list)
+		kfree(sop);
 
-freeout:
-	kfree(data);
 	return rc;
 }
 
 static const struct file_operations smk_onlycap_ops = {
-	.read		= smk_read_onlycap,
+	.open		= smk_open_onlycap,
+	.read		= seq_read,
 	.write		= smk_write_onlycap,
-	.llseek		= default_llseek,
+	.llseek		= seq_lseek,
+	.release	= seq_release,
 };
 
 #ifdef CONFIG_SECURITY_SMACK_BRINGUP
-- 
2.1.4

Re: [PATCH 1/2] Smack: fix seq operations in smackfs

[Casey Schaufler]

On 5/21/2015 9:24 AM, Rafal Krypa wrote:
> Use proper RCU functions and read locking in smackfs seq_operations.
>
> Smack gets away with not using proper RCU functions in smackfs, because
> it never removes entries from these lists. But now one list will be
> needed (with interface in smackfs) that will have both elements added and
> removed to it.
> This change will also help any future changes implementing removal of
> unneeded entries from other Smack lists.
>
> The patch also fixes handling of pos argument in smk_seq_start and
> smk_seq_next. This fixes a bug in case when smackfs is read with a small
> buffer:
>
> Kernel panic - not syncing: Kernel mode fault at addr 0xfa0000011b
> CPU: 0 PID: 1292 Comm: dd Not tainted 4.1.0-rc1-00012-g98179b8 #13
> Stack:
>  00000003 0000000d 7ff39e48 7f69fd00
>  7ff39ce0 601ae4b0 7ff39d50 600e587b
>  00000010 6039f690 7f69fd40 00612003
> Call Trace:
>  [<601ae4b0>] load2_seq_show+0x19/0x1d
>  [<600e587b>] seq_read+0x168/0x331
>  [<600c5943>] __vfs_read+0x21/0x101
>  [<601a595e>] ? security_file_permission+0xf8/0x105
>  [<600c5ec6>] ? rw_verify_area+0x86/0xe2
>  [<600c5fc3>] vfs_read+0xa1/0x14c
>  [<600c68e2>] SyS_read+0x57/0xa0
>  [<6001da60>] handle_syscall+0x60/0x80
>  [<6003087d>] userspace+0x442/0x548
>  [<6001aa77>] ? interrupt_end+0x0/0x80
>  [<6001daae>] ? copy_chunk_to_user+0x0/0x2b
>  [<6002cb6b>] ? save_registers+0x1f/0x39
>  [<60032ef7>] ? arch_prctl+0xf5/0x170
>  [<6001a92d>] fork_handler+0x85/0x87
>
> Signed-off-by: Rafal Krypa <r.krypa@samsung.com>

Applied to https://github.com/cschaufler/smack-next.git#smack-for-4.2-stacked

> ---
>  security/smack/smackfs.c | 52 ++++++++++++++++++++----------------------------
>  1 file changed, 22 insertions(+), 30 deletions(-)
>
> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index 3e42426..e40dc48 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c
> @@ -566,23 +566,17 @@ static void *smk_seq_start(struct seq_file *s, loff_t *pos,
>  				struct list_head *head)
>  {
>  	struct list_head *list;
> +	int i = *pos;
> +
> +	rcu_read_lock();
> +	for (list = rcu_dereference(list_next_rcu(head));
> +		list != head;
> +		list = rcu_dereference(list_next_rcu(list))) {
> +		if (i-- == 0)
> +			return list;
> +	}
>  
> -	/*
> -	 * This is 0 the first time through.
> -	 */
> -	if (s->index == 0)
> -		s->private = head;
> -
> -	if (s->private == NULL)
> -		return NULL;
> -
> -	list = s->private;
> -	if (list_empty(list))
> -		return NULL;
> -
> -	if (s->index == 0)
> -		return list->next;
> -	return list;
> +	return NULL;
>  }
>  
>  static void *smk_seq_next(struct seq_file *s, void *v, loff_t *pos,
> @@ -590,17 +584,15 @@ static void *smk_seq_next(struct seq_file *s, void *v, loff_t *pos,
>  {
>  	struct list_head *list = v;
>  
> -	if (list_is_last(list, head)) {
> -		s->private = NULL;
> -		return NULL;
> -	}
> -	s->private = list->next;
> -	return list->next;
> +	++*pos;
> +	list = rcu_dereference(list_next_rcu(list));
> +
> +	return (list == head) ? NULL : list;
>  }
>  
>  static void smk_seq_stop(struct seq_file *s, void *v)
>  {
> -	/* No-op */
> +	rcu_read_unlock();
>  }
>  
>  static void smk_rule_show(struct seq_file *s, struct smack_rule *srp, int max)
> @@ -660,7 +652,7 @@ static int load_seq_show(struct seq_file *s, void *v)
>  {
>  	struct list_head *list = v;
>  	struct smack_master_list *smlp =
> -		 list_entry(list, struct smack_master_list, list);
> +		list_entry_rcu(list, struct smack_master_list, list);
>  
>  	smk_rule_show(s, smlp->smk_rule, SMK_LABELLEN);
>  
> @@ -808,7 +800,7 @@ static int cipso_seq_show(struct seq_file *s, void *v)
>  {
>  	struct list_head  *list = v;
>  	struct smack_known *skp =
> -		 list_entry(list, struct smack_known, list);
> +		list_entry_rcu(list, struct smack_known, list);
>  	struct netlbl_lsm_catmap *cmp = skp->smk_netlabel.attr.mls.cat;
>  	char sep = '/';
>  	int i;
> @@ -999,7 +991,7 @@ static int cipso2_seq_show(struct seq_file *s, void *v)
>  {
>  	struct list_head  *list = v;
>  	struct smack_known *skp =
> -		 list_entry(list, struct smack_known, list);
> +		list_entry_rcu(list, struct smack_known, list);
>  	struct netlbl_lsm_catmap *cmp = skp->smk_netlabel.attr.mls.cat;
>  	char sep = '/';
>  	int i;
> @@ -1083,7 +1075,7 @@ static int netlbladdr_seq_show(struct seq_file *s, void *v)
>  {
>  	struct list_head *list = v;
>  	struct smk_netlbladdr *skp =
> -			 list_entry(list, struct smk_netlbladdr, list);
> +			list_entry_rcu(list, struct smk_netlbladdr, list);
>  	unsigned char *hp = (char *) &skp->smk_host.sin_addr.s_addr;
>  	int maskn;
>  	u32 temp_mask = be32_to_cpu(skp->smk_mask.s_addr);
> @@ -1917,7 +1909,7 @@ static int load_self_seq_show(struct seq_file *s, void *v)
>  {
>  	struct list_head *list = v;
>  	struct smack_rule *srp =
> -		 list_entry(list, struct smack_rule, list);
> +		list_entry_rcu(list, struct smack_rule, list);
>  
>  	smk_rule_show(s, srp, SMK_LABELLEN);
>  
> @@ -2046,7 +2038,7 @@ static int load2_seq_show(struct seq_file *s, void *v)
>  {
>  	struct list_head *list = v;
>  	struct smack_master_list *smlp =
> -		 list_entry(list, struct smack_master_list, list);
> +		list_entry_rcu(list, struct smack_master_list, list);
>  
>  	smk_rule_show(s, smlp->smk_rule, SMK_LONGLABEL);
>  
> @@ -2123,7 +2115,7 @@ static int load_self2_seq_show(struct seq_file *s, void *v)
>  {
>  	struct list_head *list = v;
>  	struct smack_rule *srp =
> -		 list_entry(list, struct smack_rule, list);
> +		list_entry_rcu(list, struct smack_rule, list);
>  
>  	smk_rule_show(s, srp, SMK_LONGLABEL);
>  

Re: [PATCH 2/2 v2] Smack: allow multiple labels in onlycap

[Casey Schaufler]

On 6/2/2015 2:23 AM, Rafal Krypa wrote:
> Smack onlycap allows limiting of CAP_MAC_ADMIN and CAP_MAC_OVERRIDE to
> processes running with the configured label. But having single privileged
> label is not enough in some real use cases. On a complex system like Tizen,
> there maybe few programs that need to configure Smack policy in run-time
> and running them all with a single label is not always practical.
> This patch extends onlycap feature for multiple labels. They are configured
> in the same smackfs "onlycap" interface, separated by spaces.
>
> Signed-off-by: Rafal Krypa <r.krypa@samsung.com>

Applied to https://github.com/cschaufler/smack-next.git#smack-for-4.2-stacked

> ---
>  Documentation/security/Smack.txt |   6 +-
>  security/smack/smack.h           |  25 +++---
>  security/smack/smack_access.c    |  41 ++++++++++
>  security/smack/smackfs.c         | 163 ++++++++++++++++++++++++++-------------
>  4 files changed, 163 insertions(+), 72 deletions(-)
>
> diff --git a/Documentation/security/Smack.txt b/Documentation/security/Smack.txt
> index abc82f8..de5e1ae 100644
> --- a/Documentation/security/Smack.txt
> +++ b/Documentation/security/Smack.txt
> @@ -206,11 +206,11 @@ netlabel
>  	label. The format accepted on write is:
>  		"%d.%d.%d.%d label" or "%d.%d.%d.%d/%d label".
>  onlycap
> -	This contains the label processes must have for CAP_MAC_ADMIN
> +	This contains labels processes must have for CAP_MAC_ADMIN
>  	and CAP_MAC_OVERRIDE to be effective. If this file is empty
>  	these capabilities are effective at for processes with any
> -	label. The value is set by writing the desired label to the
> -	file or cleared by writing "-" to the file.
> +	label. The values are set by writing the desired labels, separated
> +	by spaces, to the file or cleared by writing "-" to the file.
>  ptrace
>  	This is used to define the current ptrace policy
>  	0 - default: this is the policy that relies on Smack access rules.
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index b8c1a86..244e035 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -138,6 +138,11 @@ struct smk_port_label {
>  	struct smack_known	*smk_out;	/* outgoing label */
>  };
>  
> +struct smack_onlycap {
> +	struct list_head	list;
> +	struct smack_known	*smk_label;
> +};
> +
>  /*
>   * Mount options
>   */
> @@ -249,6 +254,7 @@ int smk_netlbl_mls(int, char *, struct netlbl_lsm_secattr *, int);
>  struct smack_known *smk_import_entry(const char *, int);
>  void smk_insert_entry(struct smack_known *skp);
>  struct smack_known *smk_find_entry(const char *);
> +int smack_privileged(int cap);
>  
>  /*
>   * Shared data.
> @@ -257,7 +263,6 @@ extern int smack_enabled;
>  extern int smack_cipso_direct;
>  extern int smack_cipso_mapped;
>  extern struct smack_known *smack_net_ambient;
> -extern struct smack_known *smack_onlycap;
>  extern struct smack_known *smack_syslog_label;
>  #ifdef CONFIG_SECURITY_SMACK_BRINGUP
>  extern struct smack_known *smack_unconfined;
> @@ -276,6 +281,9 @@ extern struct mutex	smack_known_lock;
>  extern struct list_head smack_known_list;
>  extern struct list_head smk_netlbladdr_list;
>  
> +extern struct mutex     smack_onlycap_lock;
> +extern struct list_head smack_onlycap_list;
> +
>  #define SMACK_HASH_SLOTS 16
>  extern struct hlist_head smack_known_hash[SMACK_HASH_SLOTS];
>  
> @@ -332,21 +340,6 @@ static inline struct smack_known *smk_of_current(void)
>  }
>  
>  /*
> - * Is the task privileged and allowed to be privileged
> - * by the onlycap rule.
> - */
> -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 == skp)
> -		return 1;
> -	return 0;
> -}
> -
> -/*
>   * logging functions
>   */
>  #define SMACK_AUDIT_DENIED 0x1
> diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c
> index 408e20b..00f6b38 100644
> --- a/security/smack/smack_access.c
> +++ b/security/smack/smack_access.c
> @@ -617,3 +617,44 @@ struct smack_known *smack_from_secid(const u32 secid)
>  	rcu_read_unlock();
>  	return &smack_known_invalid;
>  }
> +
> +/*
> + * Unless a process is running with one of these labels
> + * even having CAP_MAC_OVERRIDE isn't enough to grant
> + * privilege to violate MAC policy. If no labels are
> + * designated (the empty list case) capabilities apply to
> + * everyone.
> + */
> +LIST_HEAD(smack_onlycap_list);
> +DEFINE_MUTEX(smack_onlycap_lock);
> +
> +/*
> + * Is the task privileged and allowed to be privileged
> + * by the onlycap rule.
> + *
> + * Returns 1 if the task is allowed to be privileged, 0 if it's not.
> + */
> +int smack_privileged(int cap)
> +{
> +	struct smack_known *skp = smk_of_current();
> +	struct smack_onlycap *sop;
> +
> +	if (!capable(cap))
> +		return 0;
> +
> +	rcu_read_lock();
> +	if (list_empty(&smack_onlycap_list)) {
> +		rcu_read_unlock();
> +		return 1;
> +	}
> +
> +	list_for_each_entry_rcu(sop, &smack_onlycap_list, list) {
> +		if (sop->smk_label == skp) {
> +			rcu_read_unlock();
> +			return 1;
> +		}
> +	}
> +	rcu_read_unlock();
> +
> +	return 0;
> +}
> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index e40dc48..f1c22a8 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c
> @@ -87,16 +87,6 @@ int smack_cipso_direct = SMACK_CIPSO_DIRECT_DEFAULT;
>   */
>  int smack_cipso_mapped = SMACK_CIPSO_MAPPED_DEFAULT;
>  
> -/*
> - * Unless a process is running with this label even
> - * having CAP_MAC_OVERRIDE isn't enough to grant
> - * privilege to violate MAC policy. If no label is
> - * designated (the NULL case) capabilities apply to
> - * everyone. It is expected that the hat (^) label
> - * will be used if any label is used.
> - */
> -struct smack_known *smack_onlycap;
> -
>  #ifdef CONFIG_SECURITY_SMACK_BRINGUP
>  /*
>   * Allow one label to be unconfined. This is for
> @@ -1636,34 +1626,79 @@ static const struct file_operations smk_ambient_ops = {
>  	.llseek		= default_llseek,
>  };
>  
> -/**
> - * smk_read_onlycap - read() for smackfs/onlycap
> - * @filp: file pointer, not actually used
> - * @buf: where to put the result
> - * @cn: maximum to send along
> - * @ppos: where to start
> - *
> - * Returns number of bytes read or error code, as appropriate
> +/*
> + * Seq_file operations for /smack/onlycap
>   */
> -static ssize_t smk_read_onlycap(struct file *filp, char __user *buf,
> -				size_t cn, loff_t *ppos)
> +static void *onlycap_seq_start(struct seq_file *s, loff_t *pos)
>  {
> -	char *smack = "";
> -	ssize_t rc = -EINVAL;
> -	int asize;
> +	return smk_seq_start(s, pos, &smack_onlycap_list);
> +}
>  
> -	if (*ppos != 0)
> -		return 0;
> +static void *onlycap_seq_next(struct seq_file *s, void *v, loff_t *pos)
> +{
> +	return smk_seq_next(s, v, pos, &smack_onlycap_list);
> +}
>  
> -	if (smack_onlycap != NULL)
> -		smack = smack_onlycap->smk_known;
> +static int onlycap_seq_show(struct seq_file *s, void *v)
> +{
> +	struct list_head *list = v;
> +	struct smack_onlycap *sop =
> +		list_entry_rcu(list, struct smack_onlycap, list);
>  
> -	asize = strlen(smack) + 1;
> +	seq_puts(s, sop->smk_label->smk_known);
> +	seq_putc(s, ' ');
>  
> -	if (cn >= asize)
> -		rc = simple_read_from_buffer(buf, cn, ppos, smack, asize);
> +	return 0;
> +}
>  
> -	return rc;
> +static const struct seq_operations onlycap_seq_ops = {
> +	.start = onlycap_seq_start,
> +	.next  = onlycap_seq_next,
> +	.show  = onlycap_seq_show,
> +	.stop  = smk_seq_stop,
> +};
> +
> +static int smk_open_onlycap(struct inode *inode, struct file *file)
> +{
> +	return seq_open(file, &onlycap_seq_ops);
> +}
> +
> +/**
> + * smk_list_swap_rcu - swap public list with a private one in RCU-safe way
> + * The caller must hold appropriate mutex to prevent concurrent modifications
> + * to the public list.
> + * Private list is assumed to be not accessible to other threads yet.
> + *
> + * @public: public list
> + * @private: private list
> + */
> +static void smk_list_swap_rcu(struct list_head *public,
> +			      struct list_head *private)
> +{
> +	struct list_head *first, *last;
> +
> +	if (list_empty(public)) {
> +		list_splice_init_rcu(private, public, synchronize_rcu);
> +	} else {
> +		/* Remember public list before replacing it */
> +		first = public->next;
> +		last = public->prev;
> +
> +		/* Publish private list in place of public in RCU-safe way */
> +		private->prev->next = public;
> +		private->next->prev = public;
> +		rcu_assign_pointer(public->next, private->next);
> +		public->prev = private->prev;
> +
> +		synchronize_rcu();
> +
> +		/* When all readers are done with the old public list,
> +		 * attach it in place of private */
> +		private->next = first;
> +		private->prev = last;
> +		first->prev = private;
> +		last->next = private;
> +	}
>  }
>  
>  /**
> @@ -1679,28 +1714,47 @@ static ssize_t smk_write_onlycap(struct file *file, const char __user *buf,
>  				 size_t count, loff_t *ppos)
>  {
>  	char *data;
> -	struct smack_known *skp = smk_of_task(current->cred->security);
> +	char *data_parse;
> +	char *tok;
> +	struct smack_known *skp;
> +	struct smack_onlycap *sop;
> +	struct smack_onlycap *sop2;
> +	LIST_HEAD(list_tmp);
>  	int rc = count;
>  
>  	if (!smack_privileged(CAP_MAC_ADMIN))
>  		return -EPERM;
>  
> -	/*
> -	 * This can be done using smk_access() but is done
> -	 * explicitly for clarity. The smk_access() implementation
> -	 * would use smk_access(smack_onlycap, MAY_WRITE)
> -	 */
> -	if (smack_onlycap != NULL && smack_onlycap != skp)
> -		return -EPERM;
> -
>  	data = kzalloc(count + 1, GFP_KERNEL);
>  	if (data == NULL)
>  		return -ENOMEM;
>  
>  	if (copy_from_user(data, buf, count) != 0) {
> -		rc = -EFAULT;
> -		goto freeout;
> +		kfree(data);
> +		return -EFAULT;
> +	}
> +
> +	data_parse = data;
> +	while ((tok = strsep(&data_parse, " ")) != NULL) {
> +		if (!*tok)
> +			continue;
> +
> +		skp = smk_import_entry(tok, 0);
> +		if (IS_ERR(skp)) {
> +			rc = PTR_ERR(skp);
> +			break;
> +		}
> +
> +		sop = kzalloc(sizeof(*sop), GFP_KERNEL);
> +		if (sop == NULL) {
> +			rc = -ENOMEM;
> +			break;
> +		}
> +
> +		sop->smk_label = skp;
> +		list_add_rcu(&sop->list, &list_tmp);
>  	}
> +	kfree(data);
>  
>  	/*
>  	 * Clear the smack_onlycap on invalid label errors. This means
> @@ -1710,26 +1764,29 @@ static ssize_t smk_write_onlycap(struct file *file, const char __user *buf,
>  	 * so "-usecapabilities" will also work.
>  	 *
>  	 * But do so only on invalid label, not on system errors.
> +	 * The invalid label must be first to count as clearing attempt.
>  	 */
> -	skp = smk_import_entry(data, count);
> -	if (PTR_ERR(skp) == -EINVAL)
> -		skp = NULL;
> -	else if (IS_ERR(skp)) {
> -		rc = PTR_ERR(skp);
> -		goto freeout;
> +	if (rc == -EINVAL && list_empty(&list_tmp))
> +		rc = count;
> +
> +	if (rc >= 0) {
> +		mutex_lock(&smack_onlycap_lock);
> +		smk_list_swap_rcu(&smack_onlycap_list, &list_tmp);
> +		mutex_unlock(&smack_onlycap_lock);
>  	}
>  
> -	smack_onlycap = skp;
> +	list_for_each_entry_safe(sop, sop2, &list_tmp, list)
> +		kfree(sop);
>  
> -freeout:
> -	kfree(data);
>  	return rc;
>  }
>  
>  static const struct file_operations smk_onlycap_ops = {
> -	.read		= smk_read_onlycap,
> +	.open		= smk_open_onlycap,
> +	.read		= seq_read,
>  	.write		= smk_write_onlycap,
> -	.llseek		= default_llseek,
> +	.llseek		= seq_lseek,
> +	.release	= seq_release,
>  };
>  
>  #ifdef CONFIG_SECURITY_SMACK_BRINGUP