From 5f2bfe2f1de8b745dc294acaf2ca2ad68e09b374 Mon Sep 17 00:00:00 2001 From: Lukasz Pawelczyk Date: Tue, 25 Aug 2015 12:39:46 +0200 Subject: Smack: fix a NULL dereference in wrong smack_import_entry() usage 'commit e774ad683f42 ("smack: pass error code through pointers")' made this function return proper error codes instead of NULL. Reflect that. This is a fix for a NULL dereference introduced in 'commit 21abb1ec414c ("Smack: IPv6 host labeling")' echo "$SOME_IPV6_ADDR \"test" > /smack/ipv6host (this should return EINVAL, it doesn't) cat /smack/ipv6host (derefences 0x000a) Signed-off-by: Lukasz Pawelczyk Acked-by: Casey Schaufler --- security/smack/smackfs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'security/smack') diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c index c20b154a33f2..103a619b1360 100644 --- a/security/smack/smackfs.c +++ b/security/smack/smackfs.c @@ -1501,8 +1501,8 @@ static ssize_t smk_write_net6addr(struct file *file, const char __user *buf, */ if (smack[0] != '-') { skp = smk_import_entry(smack, 0); - if (skp == NULL) { - rc = -EINVAL; + if (IS_ERR(skp)) { + rc = PTR_ERR(skp); goto free_out; } } else { -- cgit v1.2.3-59-g8ed1b From 8b549ef42a26f4ef604a9ede84f0260103942727 Mon Sep 17 00:00:00 2001 From: Geliang Tang Date: Sun, 27 Sep 2015 23:10:25 +0800 Subject: smack: smk_ipv6_port_list should be static Fixes the following sparse warning: security/smack/smack_lsm.c:55:1: warning: symbol 'smk_ipv6_port_list' was not declared. Should it be static? Signed-off-by: Geliang Tang Acked-by: Casey Schaufler --- security/smack/smack_lsm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security/smack') diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 996c88956438..f02438cc6d29 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -52,7 +52,7 @@ #define SMK_SENDING 2 #ifdef SMACK_IPV6_PORT_LABELING -LIST_HEAD(smk_ipv6_port_list); +static LIST_HEAD(smk_ipv6_port_list); #endif static struct kmem_cache *smack_inode_cache; int smack_enabled; -- cgit v1.2.3-59-g8ed1b From d21b7b049c0c1753a10c1d01606f42bad0e0f733 Mon Sep 17 00:00:00 2001 From: José Bollo Date: Fri, 2 Oct 2015 15:15:56 +0200 Subject: Smack: Minor initialisation improvement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change has two goals: - delay the setting of 'smack_enabled' until it will be really effective - ensure that smackfs is valid only if 'smack_enabled' is set (it is already the case in smack_netfilter.c) Signed-off-by: José Bollo Acked-by: Casey Schaufler --- security/smack/smack_lsm.c | 4 ++-- security/smack/smackfs.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'security/smack') diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index f02438cc6d29..2c0579d0da20 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -4708,8 +4708,6 @@ static __init int smack_init(void) if (!security_module_enable("smack")) return 0; - smack_enabled = 1; - smack_inode_cache = KMEM_CACHE(inode_smack, 0); if (!smack_inode_cache) return -ENOMEM; @@ -4721,6 +4719,8 @@ static __init int smack_init(void) return -ENOMEM; } + smack_enabled = 1; + pr_info("Smack: Initializing.\n"); #ifdef CONFIG_SECURITY_SMACK_NETFILTER pr_info("Smack: Netfilter enabled.\n"); diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c index 103a619b1360..ce8d503a18a0 100644 --- a/security/smack/smackfs.c +++ b/security/smack/smackfs.c @@ -2892,7 +2892,7 @@ static int __init init_smk_fs(void) int err; int rc; - if (!security_module_enable("smack")) + if (smack_enabled == 0) return 0; err = smk_init_sysfs(); -- cgit v1.2.3-59-g8ed1b From 8da4aba5bf9869f58d2a6bb30daaf54b2fa72569 Mon Sep 17 00:00:00 2001 From: Roman Kubiak Date: Mon, 5 Oct 2015 12:27:16 +0200 Subject: Smack: pipefs fix in smack_d_instantiate This fix writes the task label when smack_d_instantiate is called, before the label of the superblock was written on the pipe's inode. Signed-off-by: Roman Kubiak Acked-by: Casey Schaufler --- security/smack/smack_lsm.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'security/smack') diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 2c0579d0da20..c2d66ca1127a 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -3354,6 +3354,9 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode) */ isp->smk_inode = smk_of_current(); break; + case PIPEFS_MAGIC: + isp->smk_inode = smk_of_current(); + break; default: isp->smk_inode = sbsp->smk_root; break; -- cgit v1.2.3-59-g8ed1b From 38416e53936ecf896948fdeffc36b76979117952 Mon Sep 17 00:00:00 2001 From: Zbigniew Jasinski Date: Mon, 19 Oct 2015 18:23:53 +0200 Subject: Smack: limited capability for changing process label This feature introduces new kernel interface: - /relabel-self - for setting transition labels list This list is used to control smack label transition mechanism. List is set by, and per process. Process can transit to new label only if label is on the list. Only process with CAP_MAC_ADMIN capability can add labels to this list. With this list, process can change it's label without CAP_MAC_ADMIN but only once. After label changing, list is unset. Changes in v2: * use list_for_each_entry instead of _rcu during label write * added missing description in security/Smack.txt Changes in v3: * squashed into one commit Changes in v4: * switch from global list to per-task list * since the per-task list is accessed only by the task itself there is no need to use synchronization mechanisms on it Changes in v5: * change smackfs interface of relabel-self to the one used for onlycap multiple labels are accepted, separated by space, which replace the previous list upon write Signed-off-by: Zbigniew Jasinski Signed-off-by: Rafal Krypa Acked-by: Casey Schaufler --- Documentation/security/Smack.txt | 10 ++ security/smack/smack.h | 4 +- security/smack/smack_access.c | 6 +- security/smack/smack_lsm.c | 58 ++++++++++- security/smack/smackfs.c | 202 ++++++++++++++++++++++++++++++++------- 5 files changed, 239 insertions(+), 41 deletions(-) (limited to 'security/smack') diff --git a/Documentation/security/Smack.txt b/Documentation/security/Smack.txt index 5e6d07fbed07..945cc633d883 100644 --- a/Documentation/security/Smack.txt +++ b/Documentation/security/Smack.txt @@ -255,6 +255,16 @@ unconfined the access permitted if it wouldn't be otherwise. Note that this is dangerous and can ruin the proper labeling of your system. It should never be used in production. +relabel-self + This interface contains a list of labels to which the process can + transition to, by writing to /proc/self/attr/current. + Normally a process can change its own label to any legal value, but only + if it has CAP_MAC_ADMIN. This interface allows a process without + CAP_MAC_ADMIN to relabel itself to one of labels from predefined list. + A process without CAP_MAC_ADMIN can change its label only once. When it + does, this list will be cleared. + The values are set by writing the desired labels, separated + by spaces, to the file or cleared by writing "-" to the file. If you are using the smackload utility you can add access rules in /etc/smack/accesses. They take the form: diff --git a/security/smack/smack.h b/security/smack/smack.h index fff0c612bbb7..6c91156ae225 100644 --- a/security/smack/smack.h +++ b/security/smack/smack.h @@ -115,6 +115,7 @@ struct task_smack { struct smack_known *smk_forked; /* label when forked */ struct list_head smk_rules; /* per task access rules */ struct mutex smk_rules_lock; /* lock for the rules */ + struct list_head smk_relabel; /* transit allowed labels */ }; #define SMK_INODE_INSTANT 0x01 /* inode is instantiated */ @@ -169,7 +170,7 @@ struct smk_port_label { }; #endif /* SMACK_IPV6_PORT_LABELING */ -struct smack_onlycap { +struct smack_known_list_elem { struct list_head list; struct smack_known *smk_label; }; @@ -301,6 +302,7 @@ 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); +void smk_destroy_label_list(struct list_head *list); /* * Shared data. diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c index bc1053fb5d1d..a283f9e796c1 100644 --- a/security/smack/smack_access.c +++ b/security/smack/smack_access.c @@ -637,7 +637,7 @@ DEFINE_MUTEX(smack_onlycap_lock); int smack_privileged(int cap) { struct smack_known *skp = smk_of_current(); - struct smack_onlycap *sop; + struct smack_known_list_elem *sklep; /* * All kernel tasks are privileged @@ -654,8 +654,8 @@ int smack_privileged(int cap) return 1; } - list_for_each_entry_rcu(sop, &smack_onlycap_list, list) { - if (sop->smk_label == skp) { + list_for_each_entry_rcu(sklep, &smack_onlycap_list, list) { + if (sklep->smk_label == skp) { rcu_read_unlock(); return 1; } diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index c2d66ca1127a..ff81026f6ddb 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -326,6 +326,7 @@ static struct task_smack *new_task_smack(struct smack_known *task, tsp->smk_task = task; tsp->smk_forked = forked; INIT_LIST_HEAD(&tsp->smk_rules); + INIT_LIST_HEAD(&tsp->smk_relabel); mutex_init(&tsp->smk_rules_lock); return tsp; @@ -360,6 +361,35 @@ static int smk_copy_rules(struct list_head *nhead, struct list_head *ohead, return rc; } +/** + * smk_copy_relabel - copy smk_relabel labels list + * @nhead: new rules header pointer + * @ohead: old rules header pointer + * @gfp: type of the memory for the allocation + * + * Returns 0 on success, -ENOMEM on error + */ +static int smk_copy_relabel(struct list_head *nhead, struct list_head *ohead, + gfp_t gfp) +{ + struct smack_known_list_elem *nklep; + struct smack_known_list_elem *oklep; + + INIT_LIST_HEAD(nhead); + + list_for_each_entry(oklep, ohead, list) { + nklep = kzalloc(sizeof(struct smack_known_list_elem), gfp); + if (nklep == NULL) { + smk_destroy_label_list(nhead); + return -ENOMEM; + } + nklep->smk_label = oklep->smk_label; + list_add(&nklep->list, nhead); + } + + return 0; +} + /** * smk_ptrace_mode - helper function for converting PTRACE_MODE_* into MAY_* * @mode - input mode in form of PTRACE_MODE_* @@ -1922,6 +1952,8 @@ static void smack_cred_free(struct cred *cred) return; cred->security = NULL; + smk_destroy_label_list(&tsp->smk_relabel); + list_for_each_safe(l, n, &tsp->smk_rules) { rp = list_entry(l, struct smack_rule, list); list_del(&rp->list); @@ -1953,6 +1985,11 @@ static int smack_cred_prepare(struct cred *new, const struct cred *old, if (rc != 0) return rc; + rc = smk_copy_relabel(&new_tsp->smk_relabel, &old_tsp->smk_relabel, + gfp); + if (rc != 0) + return rc; + new->security = new_tsp; return 0; } @@ -3552,9 +3589,11 @@ static int smack_getprocattr(struct task_struct *p, char *name, char **value) static int smack_setprocattr(struct task_struct *p, char *name, void *value, size_t size) { - struct task_smack *tsp; + struct task_smack *tsp = current_security(); struct cred *new; struct smack_known *skp; + struct smack_known_list_elem *sklep; + int rc; /* * Changing another process' Smack value is too dangerous @@ -3563,7 +3602,7 @@ static int smack_setprocattr(struct task_struct *p, char *name, if (p != current) return -EPERM; - if (!smack_privileged(CAP_MAC_ADMIN)) + if (!smack_privileged(CAP_MAC_ADMIN) && list_empty(&tsp->smk_relabel)) return -EPERM; if (value == NULL || size == 0 || size >= SMK_LONGLABEL) @@ -3582,12 +3621,27 @@ static int smack_setprocattr(struct task_struct *p, char *name, if (skp == &smack_known_web) return -EPERM; + if (!smack_privileged(CAP_MAC_ADMIN)) { + rc = -EPERM; + list_for_each_entry(sklep, &tsp->smk_relabel, list) + if (sklep->smk_label == skp) { + rc = 0; + break; + } + if (rc) + return rc; + } + new = prepare_creds(); if (new == NULL) return -ENOMEM; tsp = new->security; tsp->smk_task = skp; + /* + * process can change its label only once + */ + smk_destroy_label_list(&tsp->smk_relabel); commit_creds(new); return size; diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c index ce8d503a18a0..94bd9e41c9ec 100644 --- a/security/smack/smackfs.c +++ b/security/smack/smackfs.c @@ -61,6 +61,7 @@ enum smk_inos { #if IS_ENABLED(CONFIG_IPV6) SMK_NET6ADDR = 23, /* single label IPv6 hosts */ #endif /* CONFIG_IPV6 */ + SMK_RELABEL_SELF = 24, /* relabel possible without CAP_MAC_ADMIN */ }; /* @@ -1914,10 +1915,10 @@ static void *onlycap_seq_next(struct seq_file *s, void *v, loff_t *pos) 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); + struct smack_known_list_elem *sklep = + list_entry_rcu(list, struct smack_known_list_elem, list); - seq_puts(s, sop->smk_label->smk_known); + seq_puts(s, sklep->smk_label->smk_known); seq_putc(s, ' '); return 0; @@ -1973,6 +1974,54 @@ static void smk_list_swap_rcu(struct list_head *public, } } +/** + * smk_parse_label_list - parse list of Smack labels, separated by spaces + * + * @data: the string to parse + * @private: destination list + * + * Returns zero on success or error code, as appropriate + */ +static int smk_parse_label_list(char *data, struct list_head *list) +{ + char *tok; + struct smack_known *skp; + struct smack_known_list_elem *sklep; + + while ((tok = strsep(&data, " ")) != NULL) { + if (!*tok) + continue; + + skp = smk_import_entry(tok, 0); + if (IS_ERR(skp)) + return PTR_ERR(skp); + + sklep = kzalloc(sizeof(*sklep), GFP_KERNEL); + if (sklep == NULL) + return -ENOMEM; + + sklep->smk_label = skp; + list_add(&sklep->list, list); + } + + return 0; +} + +/** + * smk_destroy_label_list - destroy a list of smack_known_list_elem + * @head: header pointer of the list to destroy + */ +void smk_destroy_label_list(struct list_head *list) +{ + struct smack_known_list_elem *sklep; + struct smack_known_list_elem *sklep2; + + list_for_each_entry_safe(sklep, sklep2, list, list) + kfree(sklep); + + INIT_LIST_HEAD(list); +} + /** * smk_write_onlycap - write() for smackfs/onlycap * @file: file pointer, not actually used @@ -1986,13 +2035,8 @@ static ssize_t smk_write_onlycap(struct file *file, const char __user *buf, size_t count, loff_t *ppos) { char *data; - char *data_parse; - char *tok; - struct smack_known *skp; - struct smack_onlycap *sop; - struct smack_onlycap *sop2; LIST_HEAD(list_tmp); - int rc = count; + int rc; if (!smack_privileged(CAP_MAC_ADMIN)) return -EPERM; @@ -2006,26 +2050,7 @@ static ssize_t smk_write_onlycap(struct file *file, const char __user *buf, 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); - } + rc = smk_parse_label_list(data, &list_tmp); kfree(data); /* @@ -2038,17 +2063,14 @@ static ssize_t smk_write_onlycap(struct file *file, const char __user *buf, * But do so only on invalid label, not on system errors. * The invalid label must be first to count as clearing attempt. */ - if (rc == -EINVAL && list_empty(&list_tmp)) - rc = count; - - if (rc >= 0) { + if (!rc || (rc == -EINVAL && list_empty(&list_tmp))) { mutex_lock(&smack_onlycap_lock); smk_list_swap_rcu(&smack_onlycap_list, &list_tmp); mutex_unlock(&smack_onlycap_lock); + rc = count; } - list_for_each_entry_safe(sop, sop2, &list_tmp, list) - kfree(sop); + smk_destroy_label_list(&list_tmp); return rc; } @@ -2698,6 +2720,113 @@ static const struct file_operations smk_syslog_ops = { .llseek = default_llseek, }; +/* + * Seq_file read operations for /smack/relabel-self + */ + +static void *relabel_self_seq_start(struct seq_file *s, loff_t *pos) +{ + struct task_smack *tsp = current_security(); + + return smk_seq_start(s, pos, &tsp->smk_relabel); +} + +static void *relabel_self_seq_next(struct seq_file *s, void *v, loff_t *pos) +{ + struct task_smack *tsp = current_security(); + + return smk_seq_next(s, v, pos, &tsp->smk_relabel); +} + +static int relabel_self_seq_show(struct seq_file *s, void *v) +{ + struct list_head *list = v; + struct smack_known_list_elem *sklep = + list_entry(list, struct smack_known_list_elem, list); + + seq_puts(s, sklep->smk_label->smk_known); + seq_putc(s, ' '); + + return 0; +} + +static const struct seq_operations relabel_self_seq_ops = { + .start = relabel_self_seq_start, + .next = relabel_self_seq_next, + .show = relabel_self_seq_show, + .stop = smk_seq_stop, +}; + +/** + * smk_open_relabel_self - open() for /smack/relabel-self + * @inode: inode structure representing file + * @file: "relabel-self" file pointer + * + * Connect our relabel_self_seq_* operations with /smack/relabel-self + * file_operations + */ +static int smk_open_relabel_self(struct inode *inode, struct file *file) +{ + return seq_open(file, &relabel_self_seq_ops); +} + +/** + * smk_write_relabel_self - write() for /smack/relabel-self + * @file: file pointer, not actually used + * @buf: where to get the data from + * @count: bytes sent + * @ppos: where to start - must be 0 + * + */ +static ssize_t smk_write_relabel_self(struct file *file, const char __user *buf, + size_t count, loff_t *ppos) +{ + struct task_smack *tsp = current_security(); + char *data; + int rc; + LIST_HEAD(list_tmp); + + /* + * Must have privilege. + */ + if (!smack_privileged(CAP_MAC_ADMIN)) + return -EPERM; + + /* + * Enough data must be present. + */ + if (*ppos != 0) + return -EINVAL; + + data = kzalloc(count + 1, GFP_KERNEL); + if (data == NULL) + return -ENOMEM; + + if (copy_from_user(data, buf, count) != 0) { + kfree(data); + return -EFAULT; + } + + rc = smk_parse_label_list(data, &list_tmp); + kfree(data); + + if (!rc || (rc == -EINVAL && list_empty(&list_tmp))) { + smk_destroy_label_list(&tsp->smk_relabel); + list_splice(&list_tmp, &tsp->smk_relabel); + return count; + } + + smk_destroy_label_list(&list_tmp); + return rc; +} + +static const struct file_operations smk_relabel_self_ops = { + .open = smk_open_relabel_self, + .read = seq_read, + .llseek = seq_lseek, + .write = smk_write_relabel_self, + .release = seq_release, +}; /** * smk_read_ptrace - read() for /smack/ptrace @@ -2824,6 +2953,9 @@ static int smk_fill_super(struct super_block *sb, void *data, int silent) [SMK_NET6ADDR] = { "ipv6host", &smk_net6addr_ops, S_IRUGO|S_IWUSR}, #endif /* CONFIG_IPV6 */ + [SMK_RELABEL_SELF] = { + "relabel-self", &smk_relabel_self_ops, + S_IRUGO|S_IWUGO}, /* last one */ {""} }; -- cgit v1.2.3-59-g8ed1b