From 41a4695ca46d8798f89b477855973eb2ad3f4f69 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Wed, 27 Feb 2013 08:37:56 -0800 Subject: Yama: do not modify global sysctl table entry When the sysctl table is constified, we won't be able to directly modify it. Instead, use a table copy that carries any needed changes. Suggested-by: PaX Team Signed-off-by: Kees Cook --- security/yama/yama_lsm.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) (limited to 'security') diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c index 13c88fbcf037..24aae2ae2b30 100644 --- a/security/yama/yama_lsm.c +++ b/security/yama/yama_lsm.c @@ -379,20 +379,17 @@ static struct security_operations yama_ops = { static int yama_dointvec_minmax(struct ctl_table *table, int write, void __user *buffer, size_t *lenp, loff_t *ppos) { - int rc; + struct ctl_table table_copy; if (write && !capable(CAP_SYS_PTRACE)) return -EPERM; - rc = proc_dointvec_minmax(table, write, buffer, lenp, ppos); - if (rc) - return rc; - /* Lock the max value if it ever gets set. */ - if (write && *(int *)table->data == *(int *)table->extra2) - table->extra1 = table->extra2; + table_copy = *table; + if (*(int *)table_copy.data == *(int *)table_copy.extra2) + table_copy.extra1 = table_copy.extra2; - return rc; + return proc_dointvec_minmax(&table_copy, write, buffer, lenp, ppos); } static int zero; -- cgit v1.2.3 From 44aa1d4413876cca0962debc9483ba009d71737f Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Fri, 27 Feb 2015 16:23:59 -0500 Subject: security/yama: Remove unnecessary selects from Kconfig. Yama selects SECURITYFS and SECURITY_PATH, but requires neither. Remove them. Signed-off-by: Stephen Smalley Signed-off-by: Kees Cook --- security/yama/Kconfig | 2 -- 1 file changed, 2 deletions(-) (limited to 'security') diff --git a/security/yama/Kconfig b/security/yama/Kconfig index 20ef5143c0c0..3123e1da2fed 100644 --- a/security/yama/Kconfig +++ b/security/yama/Kconfig @@ -1,8 +1,6 @@ config SECURITY_YAMA bool "Yama support" depends on SECURITY - select SECURITYFS - select SECURITY_PATH default n help This selects Yama, which extends DAC support with additional -- cgit v1.2.3 From 7412301b76bd53ee53b860f611fc3b5b1c2245b5 Mon Sep 17 00:00:00 2001 From: Marcin Lis Date: Thu, 22 Jan 2015 15:40:33 +0100 Subject: Smack: Assign smack_known_web as default smk_in label for kernel thread's socket This change fixes the bug associated with sockets owned by kernel threads. These sockets, created usually by network devices' drivers tasks, received smk_in label from the task that created them - the "floor" label in the most cases. The result was that they were not able to receive data packets because of missing smack rules. The main reason of the access deny is that the socket smk_in label is placed as the object during smk check, kernel thread's capabilities are omitted. Signed-off-by: Marcin Lis --- security/smack/smack_lsm.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) (limited to 'security') diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index c934311812f1..a097dc7d4669 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -2452,7 +2452,21 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name, static int smack_socket_post_create(struct socket *sock, int family, int type, int protocol, int kern) { - if (family != PF_INET || sock->sk == NULL) + struct socket_smack *ssp; + + if (sock->sk == NULL) + return 0; + + /* + * Sockets created by kernel threads receive web label. + */ + if (unlikely(current->flags & PF_KTHREAD)) { + ssp = sock->sk->sk_security; + ssp->smk_in = &smack_known_web; + ssp->smk_out = &smack_known_web; + } + + if (family != PF_INET) return 0; /* * Set the outbound netlbl. -- cgit v1.2.3 From 7fc5f36e980a8f4830efdae3858f6e64eee538b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Bollo?= Date: Tue, 17 Feb 2015 15:41:22 +0100 Subject: Smack: getting the Smack security context of keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With this commit, the LSM Smack implements the LSM side part of the system call keyctl with the action code KEYCTL_GET_SECURITY. It is now possible to get the context of, for example, the user session key using the command "keyctl security @s". The original patch has been modified for merge. Signed-off-by: José Bollo Signed-off-by: Casey Schaufler --- security/smack/smack_lsm.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) (limited to 'security') diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index a097dc7d4669..e2d1a7b073c0 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -4000,6 +4000,36 @@ static int smack_key_permission(key_ref_t key_ref, rc = smk_bu_note("key access", tkp, keyp->security, request, rc); return rc; } + +/* + * smack_key_getsecurity - Smack label tagging the key + * @key points to the key to be queried + * @_buffer points to a pointer that should be set to point to the + * resulting string (if no label or an error occurs). + * Return the length of the string (including terminating NUL) or -ve if + * an error. + * May also return 0 (and a NULL buffer pointer) if there is no label. + */ +static int smack_key_getsecurity(struct key *key, char **_buffer) +{ + struct smack_known *skp = key->security; + size_t length; + char *copy; + + if (key->security == NULL) { + *_buffer = NULL; + return 0; + } + + copy = kstrdup(skp->smk_known, GFP_KERNEL); + if (copy == NULL) + return -ENOMEM; + length = strlen(copy) + 1; + + *_buffer = copy; + return length; +} + #endif /* CONFIG_KEYS */ /* @@ -4324,6 +4354,7 @@ struct security_operations smack_ops = { .key_alloc = smack_key_alloc, .key_free = smack_key_free, .key_permission = smack_key_permission, + .key_getsecurity = smack_key_getsecurity, #endif /* CONFIG_KEYS */ /* Audit hooks */ -- cgit v1.2.3 From bf4b2fee99799780ea3dbb6d79d1909b3e32be13 Mon Sep 17 00:00:00 2001 From: Casey Schaufler Date: Sat, 21 Mar 2015 18:26:40 -0700 Subject: Smack: Allow an unconfined label in bringup mode I have vehemently opposed adding a "permissive" mode to Smack for the simple reasons that it would be subject to massive abuse and that developers refuse to turn it off come product release. I still believe that this is true, and still refuse to add a general "permissive mode". So don't ask again. Bumjin Im suggested an approach that addresses most of the concerns, and I have implemented it here. I still believe that we'd be better off without this sort of thing, but it looks like this minimizes the abuse potential. Firstly, you have to configure Smack Bringup Mode. That allows for "release" software to be ammune from abuse. Second, only one label gets to be "permissive" at a time. You can use it for debugging, but that's about it. A label written to smackfs/unconfined is treated specially. If either the subject or object label of an access check matches the "unconfined" label, and the access would not have been allowed otherwise an audit record and a console message are generated. The audit record "request" string is marked with either "(US)" or "(UO)", to indicate that the request was granted because of an unconfined label. The fact that an inode was accessed by an unconfined label is remembered, and subsequent accesses to that "impure" object are noted in the log. The impurity is not stored in the filesystem, so a file mislabled as a side effect of using an unconfined label may still cause concern after a reboot. So, it's there, it's dangerous, but so many application developers seem incapable of living without it I have given in. I've tried to make it as safe as I can, but in the end it's still a chain saw. Signed-off-by: Casey Schaufler --- security/smack/smack.h | 8 ++++ security/smack/smack_access.c | 43 ++++++++++++++----- security/smack/smack_lsm.c | 52 +++++++++++++++++++---- security/smack/smackfs.c | 96 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 182 insertions(+), 17 deletions(-) (limited to 'security') diff --git a/security/smack/smack.h b/security/smack/smack.h index 67ccb7b2b89b..49eada6266ec 100644 --- a/security/smack/smack.h +++ b/security/smack/smack.h @@ -105,6 +105,7 @@ struct task_smack { #define SMK_INODE_INSTANT 0x01 /* inode is instantiated */ #define SMK_INODE_TRANSMUTE 0x02 /* directory is transmuting */ #define SMK_INODE_CHANGED 0x04 /* smack was transmuted */ +#define SMK_INODE_IMPURE 0x08 /* involved in an impure transaction */ /* * A label access rule. @@ -193,6 +194,10 @@ struct smk_port_label { #define MAY_LOCK 0x00002000 /* Locks should be writes, but ... */ #define MAY_BRINGUP 0x00004000 /* Report use of this rule */ +#define SMACK_BRINGUP_ALLOW 1 /* Allow bringup mode */ +#define SMACK_UNCONFINED_SUBJECT 2 /* Allow unconfined label */ +#define SMACK_UNCONFINED_OBJECT 3 /* Allow unconfined label */ + /* * Just to make the common cases easier to deal with */ @@ -254,6 +259,9 @@ 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; +#endif extern struct smack_known smack_cipso_option; extern int smack_ptrace_rule; diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c index 1158430f5bb9..0f410fc56e33 100644 --- a/security/smack/smack_access.c +++ b/security/smack/smack_access.c @@ -130,7 +130,8 @@ int smk_access(struct smack_known *subject, struct smack_known *object, /* * Hardcoded comparisons. - * + */ + /* * A star subject can't access any object. */ if (subject == &smack_known_star) { @@ -189,10 +190,20 @@ int smk_access(struct smack_known *subject, struct smack_known *object, * succeed because of "b" rules. */ if (may & MAY_BRINGUP) - rc = MAY_BRINGUP; + rc = SMACK_BRINGUP_ALLOW; #endif out_audit: + +#ifdef CONFIG_SECURITY_SMACK_BRINGUP + if (rc < 0) { + if (object == smack_unconfined) + rc = SMACK_UNCONFINED_OBJECT; + if (subject == smack_unconfined) + rc = SMACK_UNCONFINED_SUBJECT; + } +#endif + #ifdef CONFIG_AUDIT if (a) smack_log(subject->smk_known, object->smk_known, @@ -338,19 +349,16 @@ static void smack_log_callback(struct audit_buffer *ab, void *a) void smack_log(char *subject_label, char *object_label, int request, int result, struct smk_audit_info *ad) { +#ifdef CONFIG_SECURITY_SMACK_BRINGUP + char request_buffer[SMK_NUM_ACCESS_TYPE + 5]; +#else char request_buffer[SMK_NUM_ACCESS_TYPE + 1]; +#endif struct smack_audit_data *sad; struct common_audit_data *a = &ad->a; -#ifdef CONFIG_SECURITY_SMACK_BRINGUP - /* - * The result may be positive in bringup mode. - */ - if (result > 0) - result = 0; -#endif /* check if we have to log the current event */ - if (result != 0 && (log_policy & SMACK_AUDIT_DENIED) == 0) + if (result < 0 && (log_policy & SMACK_AUDIT_DENIED) == 0) return; if (result == 0 && (log_policy & SMACK_AUDIT_ACCEPT) == 0) return; @@ -364,6 +372,21 @@ void smack_log(char *subject_label, char *object_label, int request, smack_str_from_perm(request_buffer, request); sad->subject = subject_label; sad->object = object_label; +#ifdef CONFIG_SECURITY_SMACK_BRINGUP + /* + * The result may be positive in bringup mode. + * A positive result is an allow, but not for normal reasons. + * Mark it as successful, but don't filter it out even if + * the logging policy says to do so. + */ + if (result == SMACK_UNCONFINED_SUBJECT) + strcat(request_buffer, "(US)"); + else if (result == SMACK_UNCONFINED_OBJECT) + strcat(request_buffer, "(UO)"); + + if (result > 0) + result = 0; +#endif sad->request = request_buffer; sad->result = result; diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index e2d1a7b073c0..6f3c7d866d04 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -57,6 +57,13 @@ static struct kmem_cache *smack_inode_cache; int smack_enabled; #ifdef CONFIG_SECURITY_SMACK_BRINGUP +static char *smk_bu_mess[] = { + "Bringup Error", /* Unused */ + "Bringup", /* SMACK_BRINGUP_ALLOW */ + "Unconfined Subject", /* SMACK_UNCONFINED_SUBJECT */ + "Unconfined Object", /* SMACK_UNCONFINED_OBJECT */ +}; + static void smk_bu_mode(int mode, char *s) { int i = 0; @@ -87,9 +94,11 @@ static int smk_bu_note(char *note, struct smack_known *sskp, if (rc <= 0) return rc; + if (rc > SMACK_UNCONFINED_OBJECT) + rc = 0; smk_bu_mode(mode, acc); - pr_info("Smack Bringup: (%s %s %s) %s\n", + pr_info("Smack %s: (%s %s %s) %s\n", smk_bu_mess[rc], sskp->smk_known, oskp->smk_known, acc, note); return 0; } @@ -106,9 +115,11 @@ static int smk_bu_current(char *note, struct smack_known *oskp, if (rc <= 0) return rc; + if (rc > SMACK_UNCONFINED_OBJECT) + rc = 0; smk_bu_mode(mode, acc); - pr_info("Smack Bringup: (%s %s %s) %s %s\n", + pr_info("Smack %s: (%s %s %s) %s %s\n", smk_bu_mess[rc], tsp->smk_task->smk_known, oskp->smk_known, acc, current->comm, note); return 0; @@ -126,9 +137,11 @@ static int smk_bu_task(struct task_struct *otp, int mode, int rc) if (rc <= 0) return rc; + if (rc > SMACK_UNCONFINED_OBJECT) + rc = 0; smk_bu_mode(mode, acc); - pr_info("Smack Bringup: (%s %s %s) %s to %s\n", + pr_info("Smack %s: (%s %s %s) %s to %s\n", smk_bu_mess[rc], tsp->smk_task->smk_known, smk_task->smk_known, acc, current->comm, otp->comm); return 0; @@ -141,14 +154,25 @@ static int smk_bu_task(struct task_struct *otp, int mode, int rc) static int smk_bu_inode(struct inode *inode, int mode, int rc) { struct task_smack *tsp = current_security(); + struct inode_smack *isp = inode->i_security; char acc[SMK_NUM_ACCESS_TYPE + 1]; + if (isp->smk_flags & SMK_INODE_IMPURE) + pr_info("Smack Unconfined Corruption: inode=(%s %ld) %s\n", + inode->i_sb->s_id, inode->i_ino, current->comm); + if (rc <= 0) return rc; + if (rc > SMACK_UNCONFINED_OBJECT) + rc = 0; + if (rc == SMACK_UNCONFINED_SUBJECT && + (mode & (MAY_WRITE | MAY_APPEND))) + isp->smk_flags |= SMK_INODE_IMPURE; smk_bu_mode(mode, acc); - pr_info("Smack Bringup: (%s %s %s) inode=(%s %ld) %s\n", - tsp->smk_task->smk_known, smk_of_inode(inode)->smk_known, acc, + + pr_info("Smack %s: (%s %s %s) inode=(%s %ld) %s\n", smk_bu_mess[rc], + tsp->smk_task->smk_known, isp->smk_inode->smk_known, acc, inode->i_sb->s_id, inode->i_ino, current->comm); return 0; } @@ -162,13 +186,20 @@ static int smk_bu_file(struct file *file, int mode, int rc) struct task_smack *tsp = current_security(); struct smack_known *sskp = tsp->smk_task; struct inode *inode = file_inode(file); + struct inode_smack *isp = inode->i_security; char acc[SMK_NUM_ACCESS_TYPE + 1]; + if (isp->smk_flags & SMK_INODE_IMPURE) + pr_info("Smack Unconfined Corruption: inode=(%s %ld) %s\n", + inode->i_sb->s_id, inode->i_ino, current->comm); + if (rc <= 0) return rc; + if (rc > SMACK_UNCONFINED_OBJECT) + rc = 0; smk_bu_mode(mode, acc); - pr_info("Smack Bringup: (%s %s %s) file=(%s %ld %pD) %s\n", + pr_info("Smack %s: (%s %s %s) file=(%s %ld %pD) %s\n", smk_bu_mess[rc], sskp->smk_known, smk_of_inode(inode)->smk_known, acc, inode->i_sb->s_id, inode->i_ino, file, current->comm); @@ -185,13 +216,20 @@ static int smk_bu_credfile(const struct cred *cred, struct file *file, struct task_smack *tsp = cred->security; struct smack_known *sskp = tsp->smk_task; struct inode *inode = file->f_inode; + struct inode_smack *isp = inode->i_security; char acc[SMK_NUM_ACCESS_TYPE + 1]; + if (isp->smk_flags & SMK_INODE_IMPURE) + pr_info("Smack Unconfined Corruption: inode=(%s %ld) %s\n", + inode->i_sb->s_id, inode->i_ino, current->comm); + if (rc <= 0) return rc; + if (rc > SMACK_UNCONFINED_OBJECT) + rc = 0; smk_bu_mode(mode, acc); - pr_info("Smack Bringup: (%s %s %s) file=(%s %ld %pD) %s\n", + pr_info("Smack %s: (%s %s %s) file=(%s %ld %pD) %s\n", smk_bu_mess[rc], sskp->smk_known, smk_of_inode(inode)->smk_known, acc, inode->i_sb->s_id, inode->i_ino, file, current->comm); diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c index bce4e8f1b267..deb3d3bfbbf3 100644 --- a/security/smack/smackfs.c +++ b/security/smack/smackfs.c @@ -54,6 +54,9 @@ enum smk_inos { SMK_CHANGE_RULE = 19, /* change or add rules (long labels) */ SMK_SYSLOG = 20, /* change syslog label) */ SMK_PTRACE = 21, /* set ptrace rule */ +#ifdef CONFIG_SECURITY_SMACK_BRINGUP + SMK_UNCONFINED = 22, /* define an unconfined label */ +#endif }; /* @@ -95,6 +98,16 @@ int smack_cipso_mapped = SMACK_CIPSO_MAPPED_DEFAULT; */ struct smack_known *smack_onlycap; +#ifdef CONFIG_SECURITY_SMACK_BRINGUP +/* + * Allow one label to be unconfined. This is for + * debugging and application bring-up purposes only. + * It is bad and wrong, but everyone seems to expect + * to have it. + */ +struct smack_known *smack_unconfined; +#endif + /* * If this value is set restrict syslog use to the label specified. * It can be reset via smackfs/syslog @@ -1717,6 +1730,85 @@ static const struct file_operations smk_onlycap_ops = { .llseek = default_llseek, }; +#ifdef CONFIG_SECURITY_SMACK_BRINGUP +/** + * smk_read_unconfined - read() for smackfs/unconfined + * @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 + */ +static ssize_t smk_read_unconfined(struct file *filp, char __user *buf, + size_t cn, loff_t *ppos) +{ + char *smack = ""; + ssize_t rc = -EINVAL; + int asize; + + if (*ppos != 0) + return 0; + + if (smack_unconfined != NULL) + smack = smack_unconfined->smk_known; + + asize = strlen(smack) + 1; + + if (cn >= asize) + rc = simple_read_from_buffer(buf, cn, ppos, smack, asize); + + return rc; +} + +/** + * smk_write_unconfined - write() for smackfs/unconfined + * @file: file pointer, not actually used + * @buf: where to get the data from + * @count: bytes sent + * @ppos: where to start + * + * Returns number of bytes written or error code, as appropriate + */ +static ssize_t smk_write_unconfined(struct file *file, const char __user *buf, + size_t count, loff_t *ppos) +{ + char *data; + int rc = count; + + if (!smack_privileged(CAP_MAC_ADMIN)) + return -EPERM; + + data = kzalloc(count + 1, GFP_KERNEL); + if (data == NULL) + return -ENOMEM; + + /* + * Should the null string be passed in unset the unconfined value. + * This seems like something to be careful with as usually + * smk_import only expects to return NULL for errors. It + * is usually the case that a nullstring or "\n" would be + * bad to pass to smk_import but in fact this is useful here. + * + * smk_import will also reject a label beginning with '-', + * so "-confine" will also work. + */ + if (copy_from_user(data, buf, count) != 0) + rc = -EFAULT; + else + smack_unconfined = smk_import_entry(data, count); + + kfree(data); + return rc; +} + +static const struct file_operations smk_unconfined_ops = { + .read = smk_read_unconfined, + .write = smk_write_unconfined, + .llseek = default_llseek, +}; +#endif /* CONFIG_SECURITY_SMACK_BRINGUP */ + /** * smk_read_logging - read() for /smack/logging * @filp: file pointer, not actually used @@ -2384,6 +2476,10 @@ static int smk_fill_super(struct super_block *sb, void *data, int silent) "syslog", &smk_syslog_ops, S_IRUGO|S_IWUSR}, [SMK_PTRACE] = { "ptrace", &smk_ptrace_ops, S_IRUGO|S_IWUSR}, +#ifdef CONFIG_SECURITY_SMACK_BRINGUP + [SMK_UNCONFINED] = { + "unconfined", &smk_unconfined_ops, S_IRUGO|S_IWUSR}, +#endif /* last one */ {""} }; -- cgit v1.2.3 From f43b65bad6d54df7562c522a13d30efddae91234 Mon Sep 17 00:00:00 2001 From: Paul Gortmaker Date: Mon, 23 Mar 2015 14:03:17 -0400 Subject: smack: Fix gcc warning from unused smack_syslog_lock mutex in smackfs.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In commit 00f84f3f2e9d088f06722f4351d67f5f577abe22 ("Smack: Make the syslog control configurable") this mutex was added, but the rest of the final commit never actually made use of it, resulting in: In file included from include/linux/mutex.h:29:0, from include/linux/notifier.h:13, from include/linux/memory_hotplug.h:6, from include/linux/mmzone.h:821, from include/linux/gfp.h:5, from include/linux/slab.h:14, from include/linux/security.h:27, from security/smack/smackfs.c:21: security/smack/smackfs.c:63:21: warning: ‘smack_syslog_lock’ defined but not used [-Wunused-variable] static DEFINE_MUTEX(smack_syslog_lock); ^ A git grep shows no other instances/references to smack_syslog_lock. Delete it, assuming that the mutex addition was just a leftover from an earlier work in progress version of the change. Signed-off-by: Paul Gortmaker --- security/smack/smackfs.c | 1 - 1 file changed, 1 deletion(-) (limited to 'security') diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c index deb3d3bfbbf3..06f719ed63c9 100644 --- a/security/smack/smackfs.c +++ b/security/smack/smackfs.c @@ -64,7 +64,6 @@ enum smk_inos { */ static DEFINE_MUTEX(smack_cipso_lock); static DEFINE_MUTEX(smack_ambient_lock); -static DEFINE_MUTEX(smack_syslog_lock); static DEFINE_MUTEX(smk_netlbladdr_lock); /* -- cgit v1.2.3 From 83d4a806ae46397f606de7376b831524bd3a21e5 Mon Sep 17 00:00:00 2001 From: Jeff Vander Stoep Date: Thu, 26 Feb 2015 13:54:17 -0800 Subject: selinux: remove unnecessary pointer reassignment Commit f01e1af445fa ("selinux: don't pass in NULL avd to avc_has_perm_noaudit") made this pointer reassignment unnecessary. Avd should continue to reference the stack-based copy. Signed-off-by: Jeff Vander Stoep Acked-by: Stephen Smalley [PM: tweaked subject line] Signed-off-by: Paul Moore --- security/selinux/avc.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'security') diff --git a/security/selinux/avc.c b/security/selinux/avc.c index afcc0aed9393..3c17dda9571d 100644 --- a/security/selinux/avc.c +++ b/security/selinux/avc.c @@ -724,12 +724,10 @@ inline int avc_has_perm_noaudit(u32 ssid, u32 tsid, rcu_read_lock(); node = avc_lookup(ssid, tsid, tclass); - if (unlikely(!node)) { + if (unlikely(!node)) node = avc_compute_av(ssid, tsid, tclass, avd); - } else { + else memcpy(avd, &node->ae.avd, sizeof(*avd)); - avd = &node->ae.avd; - } denied = requested & ~(avd->allowed); if (unlikely(denied)) -- cgit v1.2.3 From da8026fa0f9154b1c571c4d160dd51a7b8c34495 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 17 Feb 2015 15:30:23 -0500 Subject: selinux: reconcile security_netlbl_secattr_to_sid() and mls_import_netlbl_cat() Move the NetLabel secattr MLS category import logic into mls_import_netlbl_cat() where it belongs, and use the mls_import_netlbl_cat() function in security_netlbl_secattr_to_sid(). Reported-by: Rickard Strandqvist Signed-off-by: Paul Moore --- security/selinux/ss/mls.c | 10 +++------- security/selinux/ss/services.c | 6 +----- 2 files changed, 4 insertions(+), 12 deletions(-) (limited to 'security') diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c index d307b37ddc2b..e1088842232c 100644 --- a/security/selinux/ss/mls.c +++ b/security/selinux/ss/mls.c @@ -654,19 +654,15 @@ int mls_import_netlbl_cat(struct context *context, rc = ebitmap_netlbl_import(&context->range.level[0].cat, secattr->attr.mls.cat); - if (rc != 0) - goto import_netlbl_cat_failure; - - rc = ebitmap_cpy(&context->range.level[1].cat, - &context->range.level[0].cat); - if (rc != 0) + if (rc) goto import_netlbl_cat_failure; + memcpy(&context->range.level[1].cat, &context->range.level[0].cat, + sizeof(context->range.level[0].cat)); return 0; import_netlbl_cat_failure: ebitmap_destroy(&context->range.level[0].cat); - ebitmap_destroy(&context->range.level[1].cat); return rc; } #endif /* CONFIG_NETLABEL */ diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index a1d3944751b9..9e2d82070915 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -3179,13 +3179,9 @@ int security_netlbl_secattr_to_sid(struct netlbl_lsm_secattr *secattr, ctx_new.type = ctx->type; mls_import_netlbl_lvl(&ctx_new, secattr); if (secattr->flags & NETLBL_SECATTR_MLS_CAT) { - rc = ebitmap_netlbl_import(&ctx_new.range.level[0].cat, - secattr->attr.mls.cat); + rc = mls_import_netlbl_cat(&ctx_new, secattr); if (rc) goto out; - memcpy(&ctx_new.range.level[1].cat, - &ctx_new.range.level[0].cat, - sizeof(ctx_new.range.level[0].cat)); } rc = -EIDRM; if (!mls_context_isvalid(&policydb, &ctx_new)) -- cgit v1.2.3 From ba39db6e0519aa8362dbda6523ceb69349a18dc3 Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Tue, 24 Mar 2015 16:54:16 -0400 Subject: selinux: convert avtab hash table to flex_array Previously we shrank the avtab max hash buckets to avoid high order memory allocations, but this causes avtab lookups to degenerate to very long linear searches for the Fedora policy. Convert to using a flex_array instead so that we can increase the buckets without such limitations. This change does not alter the max hash buckets; that is left to a separate follow-on change. Signed-off-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/ss/avtab.c | 31 +++++++++++++++++++------------ security/selinux/ss/avtab.h | 4 +++- 2 files changed, 22 insertions(+), 13 deletions(-) (limited to 'security') diff --git a/security/selinux/ss/avtab.c b/security/selinux/ss/avtab.c index a3dd9faa19c0..3ea0198fc964 100644 --- a/security/selinux/ss/avtab.c +++ b/security/selinux/ss/avtab.c @@ -46,8 +46,12 @@ avtab_insert_node(struct avtab *h, int hvalue, newnode->next = prev->next; prev->next = newnode; } else { - newnode->next = h->htable[hvalue]; - h->htable[hvalue] = newnode; + newnode->next = flex_array_get_ptr(h->htable, hvalue); + if (flex_array_put_ptr(h->htable, hvalue, newnode, + GFP_KERNEL|__GFP_ZERO)) { + kmem_cache_free(avtab_node_cachep, newnode); + return NULL; + } } h->nel++; @@ -64,7 +68,7 @@ static int avtab_insert(struct avtab *h, struct avtab_key *key, struct avtab_dat return -EINVAL; hvalue = avtab_hash(key, h->mask); - for (prev = NULL, cur = h->htable[hvalue]; + for (prev = NULL, cur = flex_array_get_ptr(h->htable, hvalue); cur; prev = cur, cur = cur->next) { if (key->source_type == cur->key.source_type && @@ -104,7 +108,7 @@ avtab_insert_nonunique(struct avtab *h, struct avtab_key *key, struct avtab_datu if (!h || !h->htable) return NULL; hvalue = avtab_hash(key, h->mask); - for (prev = NULL, cur = h->htable[hvalue]; + for (prev = NULL, cur = flex_array_get_ptr(h->htable, hvalue); cur; prev = cur, cur = cur->next) { if (key->source_type == cur->key.source_type && @@ -135,7 +139,8 @@ struct avtab_datum *avtab_search(struct avtab *h, struct avtab_key *key) return NULL; hvalue = avtab_hash(key, h->mask); - for (cur = h->htable[hvalue]; cur; cur = cur->next) { + for (cur = flex_array_get_ptr(h->htable, hvalue); cur; + cur = cur->next) { if (key->source_type == cur->key.source_type && key->target_type == cur->key.target_type && key->target_class == cur->key.target_class && @@ -170,7 +175,8 @@ avtab_search_node(struct avtab *h, struct avtab_key *key) return NULL; hvalue = avtab_hash(key, h->mask); - for (cur = h->htable[hvalue]; cur; cur = cur->next) { + for (cur = flex_array_get_ptr(h->htable, hvalue); cur; + cur = cur->next) { if (key->source_type == cur->key.source_type && key->target_type == cur->key.target_type && key->target_class == cur->key.target_class && @@ -228,15 +234,14 @@ void avtab_destroy(struct avtab *h) return; for (i = 0; i < h->nslot; i++) { - cur = h->htable[i]; + cur = flex_array_get_ptr(h->htable, i); while (cur) { temp = cur; cur = cur->next; kmem_cache_free(avtab_node_cachep, temp); } - h->htable[i] = NULL; } - kfree(h->htable); + flex_array_free(h->htable); h->htable = NULL; h->nslot = 0; h->mask = 0; @@ -270,7 +275,8 @@ int avtab_alloc(struct avtab *h, u32 nrules) nslot = MAX_AVTAB_HASH_BUCKETS; mask = nslot - 1; - h->htable = kcalloc(nslot, sizeof(*(h->htable)), GFP_KERNEL); + h->htable = flex_array_alloc(sizeof(struct avtab_node *), nslot, + GFP_KERNEL | __GFP_ZERO); if (!h->htable) return -ENOMEM; @@ -293,7 +299,7 @@ void avtab_hash_eval(struct avtab *h, char *tag) max_chain_len = 0; chain2_len_sum = 0; for (i = 0; i < h->nslot; i++) { - cur = h->htable[i]; + cur = flex_array_get_ptr(h->htable, i); if (cur) { slots_used++; chain_len = 0; @@ -534,7 +540,8 @@ int avtab_write(struct policydb *p, struct avtab *a, void *fp) return rc; for (i = 0; i < a->nslot; i++) { - for (cur = a->htable[i]; cur; cur = cur->next) { + for (cur = flex_array_get_ptr(a->htable, i); cur; + cur = cur->next) { rc = avtab_write_item(p, cur, fp); if (rc) return rc; diff --git a/security/selinux/ss/avtab.h b/security/selinux/ss/avtab.h index 63ce2f9e441d..9318b2b8f6c9 100644 --- a/security/selinux/ss/avtab.h +++ b/security/selinux/ss/avtab.h @@ -23,6 +23,8 @@ #ifndef _SS_AVTAB_H_ #define _SS_AVTAB_H_ +#include + struct avtab_key { u16 source_type; /* source type */ u16 target_type; /* target type */ @@ -51,7 +53,7 @@ struct avtab_node { }; struct avtab { - struct avtab_node **htable; + struct flex_array *htable; u32 nel; /* number of elements */ u32 nslot; /* number of hash slots */ u16 mask; /* mask to compute hash func */ -- cgit v1.2.3 From 33ebc1932a07efd8728975750409741940334489 Mon Sep 17 00:00:00 2001 From: John Brooks Date: Tue, 24 Mar 2015 16:54:17 -0400 Subject: selinux: Use a better hash function for avtab This function, based on murmurhash3, has much better distribution than the original. Using the current default of 2048 buckets, there are many fewer collisions: Before: 101421 entries and 2048/2048 buckets used, longest chain length 374 After: 101421 entries and 2048/2048 buckets used, longest chain length 81 The difference becomes much more significant when buckets are increased. A naive attempt to expand the current function to larger outputs doesn't yield any significant improvement; so this function is a prerequisite for increasing the bucket size. sds: Adapted from the original patches for libsepol to the kernel. Signed-off-by: John Brooks Signed-off-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/ss/avtab.c | 41 +++++++++++++++++++++++++++++++++++++---- security/selinux/ss/avtab.h | 2 +- 2 files changed, 38 insertions(+), 5 deletions(-) (limited to 'security') diff --git a/security/selinux/ss/avtab.c b/security/selinux/ss/avtab.c index 3ea0198fc964..b64f2772b030 100644 --- a/security/selinux/ss/avtab.c +++ b/security/selinux/ss/avtab.c @@ -25,10 +25,43 @@ static struct kmem_cache *avtab_node_cachep; -static inline int avtab_hash(struct avtab_key *keyp, u16 mask) +/* Based on MurmurHash3, written by Austin Appleby and placed in the + * public domain. + */ +static inline int avtab_hash(struct avtab_key *keyp, u32 mask) { - return ((keyp->target_class + (keyp->target_type << 2) + - (keyp->source_type << 9)) & mask); + static const u32 c1 = 0xcc9e2d51; + static const u32 c2 = 0x1b873593; + static const u32 r1 = 15; + static const u32 r2 = 13; + static const u32 m = 5; + static const u32 n = 0xe6546b64; + + u32 hash = 0; + +#define mix(input) { \ + u32 v = input; \ + v *= c1; \ + v = (v << r1) | (v >> (32 - r1)); \ + v *= c2; \ + hash ^= v; \ + hash = (hash << r2) | (hash >> (32 - r2)); \ + hash = hash * m + n; \ +} + + mix(keyp->target_class); + mix(keyp->target_type); + mix(keyp->source_type); + +#undef mix + + hash ^= hash >> 16; + hash *= 0x85ebca6b; + hash ^= hash >> 13; + hash *= 0xc2b2ae35; + hash ^= hash >> 16; + + return hash & mask; } static struct avtab_node* @@ -256,7 +289,7 @@ int avtab_init(struct avtab *h) int avtab_alloc(struct avtab *h, u32 nrules) { - u16 mask = 0; + u32 mask = 0; u32 shift = 0; u32 work = nrules; u32 nslot = 0; diff --git a/security/selinux/ss/avtab.h b/security/selinux/ss/avtab.h index 9318b2b8f6c9..6d794a2eee57 100644 --- a/security/selinux/ss/avtab.h +++ b/security/selinux/ss/avtab.h @@ -56,7 +56,7 @@ struct avtab { struct flex_array *htable; u32 nel; /* number of elements */ u32 nslot; /* number of hash slots */ - u16 mask; /* mask to compute hash func */ + u32 mask; /* mask to compute hash func */ }; -- cgit v1.2.3 From cf7b6c0205f11cdb015384244c0b423b00e35c69 Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Tue, 24 Mar 2015 16:54:18 -0400 Subject: selinux: increase avtab max buckets Now that we can safely increase the avtab max buckets without triggering high order allocations and have a hash function that will make better use of the larger number of buckets, increase the max buckets to 2^16. Original: 101421 entries and 2048/2048 buckets used, longest chain length 374 With new hash function: 101421 entries and 2048/2048 buckets used, longest chain length 81 With increased max buckets: 101421 entries and 31078/32768 buckets used, longest chain length 12 Signed-off-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/ss/avtab.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/selinux/ss/avtab.h b/security/selinux/ss/avtab.h index 6d794a2eee57..adb451cd44f9 100644 --- a/security/selinux/ss/avtab.h +++ b/security/selinux/ss/avtab.h @@ -86,7 +86,7 @@ struct avtab_node *avtab_search_node_next(struct avtab_node *node, int specified void avtab_cache_init(void); void avtab_cache_destroy(void); -#define MAX_AVTAB_HASH_BITS 11 +#define MAX_AVTAB_HASH_BITS 16 #define MAX_AVTAB_HASH_BUCKETS (1 << MAX_AVTAB_HASH_BITS) #endif /* _SS_AVTAB_H_ */ -- cgit v1.2.3 From 7e114bbf51fbb015dc25d8123e090afcce5b5048 Mon Sep 17 00:00:00 2001 From: Michal Marek Date: Fri, 9 Jan 2015 14:08:26 +0100 Subject: tomoyo: Use bin2c to generate builtin-policy.h Simplify the Makefile by using a readily available tool instead of a custom sed script. The downside is that builtin-policy.h becomes unreadable for humans, but it is only a generated file. Acked-by: Tetsuo Handa Signed-off-by: Michal Marek --- security/tomoyo/Kconfig | 1 + security/tomoyo/Makefile | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 10 deletions(-) (limited to 'security') diff --git a/security/tomoyo/Kconfig b/security/tomoyo/Kconfig index 604e718d68d3..404dce66952a 100644 --- a/security/tomoyo/Kconfig +++ b/security/tomoyo/Kconfig @@ -6,6 +6,7 @@ config SECURITY_TOMOYO select SECURITY_PATH select SECURITY_NETWORK select SRCU + select BUILD_BIN2C default n help This selects TOMOYO Linux, pathname-based access control. diff --git a/security/tomoyo/Makefile b/security/tomoyo/Makefile index 56a0c7be409e..a6c02a5948b6 100644 --- a/security/tomoyo/Makefile +++ b/security/tomoyo/Makefile @@ -29,20 +29,20 @@ $(obj)/policy/stat.conf: $(obj)/builtin-policy.h: $(obj)/policy/profile.conf $(obj)/policy/exception_policy.conf $(obj)/policy/domain_policy.conf $(obj)/policy/manager.conf $(obj)/policy/stat.conf @echo Generating built-in policy for TOMOYO 2.5.x. @echo "static char tomoyo_builtin_profile[] __initdata =" > $@.tmp - @sed -e 's/\\/\\\\/g' -e 's/\"/\\"/g' -e 's/\(.*\)/"\1\\n"/' < $(obj)/policy/profile.conf >> $@.tmp - @echo "\"\";" >> $@.tmp + @$(objtree)/scripts/basic/bin2c < $(obj)/policy/profile.conf >> $@.tmp + @echo ";" >> $@.tmp @echo "static char tomoyo_builtin_exception_policy[] __initdata =" >> $@.tmp - @sed -e 's/\\/\\\\/g' -e 's/\"/\\"/g' -e 's/\(.*\)/"\1\\n"/' < $(obj)/policy/exception_policy.conf >> $@.tmp - @echo "\"\";" >> $@.tmp + @$(objtree)/scripts/basic/bin2c < $(obj)/policy/exception_policy.conf >> $@.tmp + @echo ";" >> $@.tmp @echo "static char tomoyo_builtin_domain_policy[] __initdata =" >> $@.tmp - @sed -e 's/\\/\\\\/g' -e 's/\"/\\"/g' -e 's/\(.*\)/"\1\\n"/' < $(obj)/policy/domain_policy.conf >> $@.tmp - @echo "\"\";" >> $@.tmp + @$(objtree)/scripts/basic/bin2c < $(obj)/policy/domain_policy.conf >> $@.tmp + @echo ";" >> $@.tmp @echo "static char tomoyo_builtin_manager[] __initdata =" >> $@.tmp - @sed -e 's/\\/\\\\/g' -e 's/\"/\\"/g' -e 's/\(.*\)/"\1\\n"/' < $(obj)/policy/manager.conf >> $@.tmp - @echo "\"\";" >> $@.tmp + @$(objtree)/scripts/basic/bin2c < $(obj)/policy/manager.conf >> $@.tmp + @echo ";" >> $@.tmp @echo "static char tomoyo_builtin_stat[] __initdata =" >> $@.tmp - @sed -e 's/\\/\\\\/g' -e 's/\"/\\"/g' -e 's/\(.*\)/"\1\\n"/' < $(obj)/policy/stat.conf >> $@.tmp - @echo "\"\";" >> $@.tmp + @$(objtree)/scripts/basic/bin2c < $(obj)/policy/stat.conf >> $@.tmp + @echo ";" >> $@.tmp @mv $@.tmp $@ $(obj)/common.o: $(obj)/builtin-policy.h -- cgit v1.2.3 From bf7a9ab43c2f692bce4ee3ed1456f42c77eb1346 Mon Sep 17 00:00:00 2001 From: Michal Marek Date: Fri, 9 Jan 2015 14:36:27 +0100 Subject: tomoyo: Use if_changed when generating builtin-policy.h Combine the generation of builtin-policy.h into a single command and use if_changed, so that the file is regenerated each time the command changes. The next patch will make use of this. Acked-by: Tetsuo Handa Signed-off-by: Michal Marek --- security/tomoyo/Makefile | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) (limited to 'security') diff --git a/security/tomoyo/Makefile b/security/tomoyo/Makefile index a6c02a5948b6..ecdefb583fcf 100644 --- a/security/tomoyo/Makefile +++ b/security/tomoyo/Makefile @@ -26,23 +26,16 @@ $(obj)/policy/stat.conf: @echo Creating an empty policy/stat.conf @touch $@ -$(obj)/builtin-policy.h: $(obj)/policy/profile.conf $(obj)/policy/exception_policy.conf $(obj)/policy/domain_policy.conf $(obj)/policy/manager.conf $(obj)/policy/stat.conf - @echo Generating built-in policy for TOMOYO 2.5.x. - @echo "static char tomoyo_builtin_profile[] __initdata =" > $@.tmp - @$(objtree)/scripts/basic/bin2c < $(obj)/policy/profile.conf >> $@.tmp - @echo ";" >> $@.tmp - @echo "static char tomoyo_builtin_exception_policy[] __initdata =" >> $@.tmp - @$(objtree)/scripts/basic/bin2c < $(obj)/policy/exception_policy.conf >> $@.tmp - @echo ";" >> $@.tmp - @echo "static char tomoyo_builtin_domain_policy[] __initdata =" >> $@.tmp - @$(objtree)/scripts/basic/bin2c < $(obj)/policy/domain_policy.conf >> $@.tmp - @echo ";" >> $@.tmp - @echo "static char tomoyo_builtin_manager[] __initdata =" >> $@.tmp - @$(objtree)/scripts/basic/bin2c < $(obj)/policy/manager.conf >> $@.tmp - @echo ";" >> $@.tmp - @echo "static char tomoyo_builtin_stat[] __initdata =" >> $@.tmp - @$(objtree)/scripts/basic/bin2c < $(obj)/policy/stat.conf >> $@.tmp - @echo ";" >> $@.tmp - @mv $@.tmp $@ +targets += builtin-policy.h +define do_policy +echo "static char tomoyo_builtin_$(1)[] __initdata ="; \ +$(objtree)/scripts/basic/bin2c <$(obj)/policy/$(1).conf; \ +echo ";" +endef +quiet_cmd_policy = POLICY $@ + cmd_policy = ($(call do_policy,profile); $(call do_policy,exception_policy); $(call do_policy,domain_policy); $(call do_policy,manager); $(call do_policy,stat)) >$@ + +$(obj)/builtin-policy.h: $(obj)/policy/profile.conf $(obj)/policy/exception_policy.conf $(obj)/policy/domain_policy.conf $(obj)/policy/manager.conf $(obj)/policy/stat.conf FORCE + $(call if_changed,policy) $(obj)/common.o: $(obj)/builtin-policy.h -- cgit v1.2.3 From f02dee2d148ba854464e7dbf09f1241ee159173a Mon Sep 17 00:00:00 2001 From: Michal Marek Date: Thu, 15 Jan 2015 10:39:22 +0100 Subject: tomoyo: Do not generate empty policy files The Makefile automatically generates the tomoyo policy files, which are not removed by make clean (because they could have been provided by the user). Instead of generating the missing files, use /dev/null if a given file is not provided. Store the default exception_policy in exception_policy.conf.default. Acked-by: Tetsuo Handa Signed-off-by: Michal Marek --- security/tomoyo/.gitignore | 2 +- security/tomoyo/Makefile | 30 ++-------------------- .../tomoyo/policy/exception_policy.conf.default | 2 ++ 3 files changed, 5 insertions(+), 29 deletions(-) create mode 100644 security/tomoyo/policy/exception_policy.conf.default (limited to 'security') diff --git a/security/tomoyo/.gitignore b/security/tomoyo/.gitignore index 5caf1a6f5907..dc0f220a210b 100644 --- a/security/tomoyo/.gitignore +++ b/security/tomoyo/.gitignore @@ -1,2 +1,2 @@ builtin-policy.h -policy/ +policy/*.conf diff --git a/security/tomoyo/Makefile b/security/tomoyo/Makefile index ecdefb583fcf..65dbcb2fd850 100644 --- a/security/tomoyo/Makefile +++ b/security/tomoyo/Makefile @@ -1,41 +1,15 @@ obj-y = audit.o common.o condition.o domain.o environ.o file.o gc.o group.o load_policy.o memory.o mount.o network.o realpath.o securityfs_if.o tomoyo.o util.o -$(obj)/policy/profile.conf: - @mkdir -p $(obj)/policy/ - @echo Creating an empty policy/profile.conf - @touch $@ - -$(obj)/policy/exception_policy.conf: - @mkdir -p $(obj)/policy/ - @echo Creating a default policy/exception_policy.conf - @echo initialize_domain /sbin/modprobe from any >> $@ - @echo initialize_domain /sbin/hotplug from any >> $@ - -$(obj)/policy/domain_policy.conf: - @mkdir -p $(obj)/policy/ - @echo Creating an empty policy/domain_policy.conf - @touch $@ - -$(obj)/policy/manager.conf: - @mkdir -p $(obj)/policy/ - @echo Creating an empty policy/manager.conf - @touch $@ - -$(obj)/policy/stat.conf: - @mkdir -p $(obj)/policy/ - @echo Creating an empty policy/stat.conf - @touch $@ - targets += builtin-policy.h define do_policy echo "static char tomoyo_builtin_$(1)[] __initdata ="; \ -$(objtree)/scripts/basic/bin2c <$(obj)/policy/$(1).conf; \ +$(objtree)/scripts/basic/bin2c <$(firstword $(wildcard $(obj)/policy/$(1).conf $(srctree)/$(src)/policy/$(1).conf.default) /dev/null); \ echo ";" endef quiet_cmd_policy = POLICY $@ cmd_policy = ($(call do_policy,profile); $(call do_policy,exception_policy); $(call do_policy,domain_policy); $(call do_policy,manager); $(call do_policy,stat)) >$@ -$(obj)/builtin-policy.h: $(obj)/policy/profile.conf $(obj)/policy/exception_policy.conf $(obj)/policy/domain_policy.conf $(obj)/policy/manager.conf $(obj)/policy/stat.conf FORCE +$(obj)/builtin-policy.h: $(wildcard $(obj)/policy/*.conf $(src)/policy/*.conf.default) FORCE $(call if_changed,policy) $(obj)/common.o: $(obj)/builtin-policy.h diff --git a/security/tomoyo/policy/exception_policy.conf.default b/security/tomoyo/policy/exception_policy.conf.default new file mode 100644 index 000000000000..2678df4964ee --- /dev/null +++ b/security/tomoyo/policy/exception_policy.conf.default @@ -0,0 +1,2 @@ +initialize_domain /sbin/modprobe from any +initialize_domain /sbin/hotplug from any -- cgit v1.2.3 From 5deeb5cece3f9b30c8129786726b9d02c412c8ca Mon Sep 17 00:00:00 2001 From: Richard Guy Briggs Date: Tue, 14 Apr 2015 11:01:02 -0400 Subject: lsm: copy comm before calling audit_log to avoid race in string printing When task->comm is passed directly to audit_log_untrustedstring() without getting a copy or using the task_lock, there is a race that could happen that would output a NULL (\0) in the middle of the output string that would effectively truncate the rest of the report text after the comm= field in the audit log message, losing fields. Using get_task_comm() to get a copy while acquiring the task_lock to prevent this and to prevent the result from being a mixture of old and new values of comm would incur potentially unacceptable overhead, considering that the value can be influenced by userspace and therefore untrusted anyways. Copy the value before passing it to audit_log_untrustedstring() ensures that a local copy is used to calculate the length *and* subsequently printed. Even if this value contains a mix of old and new values, it will only calculate and copy up to the first NULL, preventing the rest of the audit log message being truncated. Use a second local copy of comm to avoid a race between the first and second calls to audit_log_untrustedstring() with comm. Reported-by: Tetsuo Handa Signed-off-by: Richard Guy Briggs Signed-off-by: James Morris --- security/lsm_audit.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) (limited to 'security') diff --git a/security/lsm_audit.c b/security/lsm_audit.c index 69fdf3bc765b..b526ddc3add5 100644 --- a/security/lsm_audit.c +++ b/security/lsm_audit.c @@ -211,7 +211,7 @@ static inline void print_ipv4_addr(struct audit_buffer *ab, __be32 addr, static void dump_common_audit_data(struct audit_buffer *ab, struct common_audit_data *a) { - struct task_struct *tsk = current; + char comm[sizeof(current->comm)]; /* * To keep stack sizes in check force programers to notice if they @@ -220,8 +220,8 @@ static void dump_common_audit_data(struct audit_buffer *ab, */ BUILD_BUG_ON(sizeof(a->u) > sizeof(void *)*2); - audit_log_format(ab, " pid=%d comm=", task_pid_nr(tsk)); - audit_log_untrustedstring(ab, tsk->comm); + audit_log_format(ab, " pid=%d comm=", task_pid_nr(current)); + audit_log_untrustedstring(ab, memcpy(comm, current->comm, sizeof(comm))); switch (a->type) { case LSM_AUDIT_DATA_NONE: @@ -276,16 +276,19 @@ static void dump_common_audit_data(struct audit_buffer *ab, audit_log_format(ab, " ino=%lu", inode->i_ino); break; } - case LSM_AUDIT_DATA_TASK: - tsk = a->u.tsk; + case LSM_AUDIT_DATA_TASK: { + struct task_struct *tsk = a->u.tsk; if (tsk) { pid_t pid = task_pid_nr(tsk); if (pid) { + char comm[sizeof(tsk->comm)]; audit_log_format(ab, " pid=%d comm=", pid); - audit_log_untrustedstring(ab, tsk->comm); + audit_log_untrustedstring(ab, + memcpy(comm, tsk->comm, sizeof(comm))); } } break; + } case LSM_AUDIT_DATA_NET: if (a->u.net->sk) { struct sock *sk = a->u.net->sk; -- cgit v1.2.3