From e774ad683f425a51f87711164ea166d9dcc41477 Mon Sep 17 00:00:00 2001 From: Lukasz Pawelczyk Date: Mon, 20 Apr 2015 17:12:54 +0200 Subject: smack: pass error code through pointers This patch makes the following functions to use ERR_PTR() and related macros to pass the appropriate error code through returned pointers: smk_parse_smack() smk_import_entry() smk_fetch() It also makes all the other functions that use them to handle the error cases properly. This ways correct error codes from places where they happened can be propagated to the user space if necessary. Doing this it fixes a bug in onlycap and unconfined files handling. Previously their content was cleared on any error from smk_import_entry/smk_parse_smack, be it EINVAL (as originally intended) or ENOMEM. Right now it only reacts on EINVAL passing other codes properly to userspace. Comments have been updated accordingly. Signed-off-by: Lukasz Pawelczyk --- security/smack/smack_access.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) (limited to 'security/smack/smack_access.c') diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c index 0f410fc56e33..408e20be1ad7 100644 --- a/security/smack/smack_access.c +++ b/security/smack/smack_access.c @@ -425,7 +425,7 @@ void smk_insert_entry(struct smack_known *skp) * @string: a text string that might be a Smack label * * Returns a pointer to the entry in the label list that - * matches the passed string. + * matches the passed string or NULL if not found. */ struct smack_known *smk_find_entry(const char *string) { @@ -448,7 +448,7 @@ struct smack_known *smk_find_entry(const char *string) * @string: a text string that might contain a Smack label * @len: the maximum size, or zero if it is NULL terminated. * - * Returns a pointer to the clean label, or NULL + * Returns a pointer to the clean label or an error code. */ char *smk_parse_smack(const char *string, int len) { @@ -464,7 +464,7 @@ char *smk_parse_smack(const char *string, int len) * including /smack/cipso and /smack/cipso2 */ if (string[0] == '-') - return NULL; + return ERR_PTR(-EINVAL); for (i = 0; i < len; i++) if (string[i] > '~' || string[i] <= ' ' || string[i] == '/' || @@ -472,11 +472,13 @@ char *smk_parse_smack(const char *string, int len) break; if (i == 0 || i >= SMK_LONGLABEL) - return NULL; + return ERR_PTR(-EINVAL); smack = kzalloc(i + 1, GFP_KERNEL); - if (smack != NULL) - strncpy(smack, string, i); + if (smack == NULL) + return ERR_PTR(-ENOMEM); + + strncpy(smack, string, i); return smack; } @@ -523,7 +525,8 @@ int smk_netlbl_mls(int level, char *catset, struct netlbl_lsm_secattr *sap, * @len: the maximum size, or zero if it is NULL terminated. * * Returns a pointer to the entry in the label list that - * matches the passed string, adding it if necessary. + * matches the passed string, adding it if necessary, + * or an error code. */ struct smack_known *smk_import_entry(const char *string, int len) { @@ -533,8 +536,8 @@ struct smack_known *smk_import_entry(const char *string, int len) int rc; smack = smk_parse_smack(string, len); - if (smack == NULL) - return NULL; + if (IS_ERR(smack)) + return ERR_CAST(smack); mutex_lock(&smack_known_lock); @@ -543,8 +546,10 @@ struct smack_known *smk_import_entry(const char *string, int len) goto freeout; skp = kzalloc(sizeof(*skp), GFP_KERNEL); - if (skp == NULL) + if (skp == NULL) { + skp = ERR_PTR(-ENOMEM); goto freeout; + } skp->smk_known = smack; skp->smk_secid = smack_next_secid++; @@ -577,7 +582,7 @@ struct smack_known *smk_import_entry(const char *string, int len) * smk_netlbl_mls failed. */ kfree(skp); - skp = NULL; + skp = ERR_PTR(rc); freeout: kfree(smack); unlockout: -- cgit v1.2.3 From c0d77c884461fc0dec0411e49797dc3f3651c31b Mon Sep 17 00:00:00 2001 From: Rafal Krypa Date: Tue, 2 Jun 2015 11:23:48 +0200 Subject: Smack: allow multiple labels in onlycap 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 --- 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(-) (limited to 'security/smack/smack_access.c') diff --git a/Documentation/security/Smack.txt b/Documentation/security/Smack.txt index abc82f85215b..de5e1aeca7fb 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 b8c1a869d85e..244e035e5a99 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]; @@ -331,21 +339,6 @@ static inline struct smack_known *smk_of_current(void) return smk_of_task(current_security()); } -/* - * 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 */ diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c index 408e20be1ad7..00f6b38bffbd 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 e40dc48737ff..f1c22a891b1a 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 -- cgit v1.2.3