From 29707b206c5171ac6583a4d1e9ec3af937e8c2e4 Mon Sep 17 00:00:00 2001 From: Jingoo Han Date: Wed, 5 Feb 2014 15:13:14 +0900 Subject: security: replace strict_strto*() with kstrto*() The usage of strict_strto*() is not preferred, because strict_strto*() is obsolete. Thus, kstrto*() should be used. Signed-off-by: Jingoo Han Signed-off-by: James Morris --- security/apparmor/lsm.c | 2 +- security/integrity/ima/ima_policy.c | 6 +++--- security/integrity/integrity_audit.c | 2 +- security/keys/encrypted-keys/encrypted.c | 2 +- security/keys/trusted.c | 6 +++--- security/selinux/hooks.c | 4 ++-- security/selinux/selinuxfs.c | 2 +- 7 files changed, 12 insertions(+), 12 deletions(-) (limited to 'security') diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 4257b7e..9981000 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -751,7 +751,7 @@ module_param_named(enabled, apparmor_enabled, bool, S_IRUGO); static int __init apparmor_enabled_setup(char *str) { unsigned long enabled; - int error = strict_strtoul(str, 0, &enabled); + int error = kstrtoul(str, 0, &enabled); if (!error) apparmor_enabled = enabled ? 1 : 0; return 1; diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index a9c3d3c..354b125 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -520,7 +520,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) break; } - result = strict_strtoul(args[0].from, 16, + result = kstrtoul(args[0].from, 16, &entry->fsmagic); if (!result) entry->flags |= IMA_FSMAGIC; @@ -547,7 +547,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) break; } - result = strict_strtoul(args[0].from, 10, &lnum); + result = kstrtoul(args[0].from, 10, &lnum); if (!result) { entry->uid = make_kuid(current_user_ns(), (uid_t)lnum); if (!uid_valid(entry->uid) || (((uid_t)lnum) != lnum)) @@ -564,7 +564,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) break; } - result = strict_strtoul(args[0].from, 10, &lnum); + result = kstrtoul(args[0].from, 10, &lnum); if (!result) { entry->fowner = make_kuid(current_user_ns(), (uid_t)lnum); if (!uid_valid(entry->fowner) || (((uid_t)lnum) != lnum)) diff --git a/security/integrity/integrity_audit.c b/security/integrity/integrity_audit.c index d7efb30..809ec842 100644 --- a/security/integrity/integrity_audit.c +++ b/security/integrity/integrity_audit.c @@ -22,7 +22,7 @@ static int __init integrity_audit_setup(char *str) { unsigned long audit; - if (!strict_strtoul(str, 0, &audit)) + if (!kstrtoul(str, 0, &audit)) integrity_audit_info = audit ? 1 : 0; return 1; } diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c index 9e1e005..5fe443d 100644 --- a/security/keys/encrypted-keys/encrypted.c +++ b/security/keys/encrypted-keys/encrypted.c @@ -609,7 +609,7 @@ static struct encrypted_key_payload *encrypted_key_alloc(struct key *key, long dlen; int ret; - ret = strict_strtol(datalen, 10, &dlen); + ret = kstrtol(datalen, 10, &dlen); if (ret < 0 || dlen < MIN_DATA_SIZE || dlen > MAX_DATA_SIZE) return ERR_PTR(-EINVAL); diff --git a/security/keys/trusted.c b/security/keys/trusted.c index e13fcf7..6b804aa 100644 --- a/security/keys/trusted.c +++ b/security/keys/trusted.c @@ -753,7 +753,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay, return -EINVAL; break; case Opt_keyhandle: - res = strict_strtoul(args[0].from, 16, &handle); + res = kstrtoul(args[0].from, 16, &handle); if (res < 0) return -EINVAL; opt->keytype = SEAL_keytype; @@ -782,7 +782,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay, return -EINVAL; break; case Opt_pcrlock: - res = strict_strtoul(args[0].from, 10, &lock); + res = kstrtoul(args[0].from, 10, &lock); if (res < 0) return -EINVAL; opt->pcrlock = lock; @@ -820,7 +820,7 @@ static int datablob_parse(char *datablob, struct trusted_key_payload *p, c = strsep(&datablob, " \t"); if (!c) return -EINVAL; - ret = strict_strtol(c, 10, &keylen); + ret = kstrtol(c, 10, &keylen); if (ret < 0 || keylen < MIN_KEY_SIZE || keylen > MAX_KEY_SIZE) return -EINVAL; p->key_len = keylen; diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 3219560..0e68bdb 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -106,7 +106,7 @@ int selinux_enforcing; static int __init enforcing_setup(char *str) { unsigned long enforcing; - if (!strict_strtoul(str, 0, &enforcing)) + if (!kstrtoul(str, 0, &enforcing)) selinux_enforcing = enforcing ? 1 : 0; return 1; } @@ -119,7 +119,7 @@ int selinux_enabled = CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE; static int __init selinux_enabled_setup(char *str) { unsigned long enabled; - if (!strict_strtoul(str, 0, &enabled)) + if (!kstrtoul(str, 0, &enabled)) selinux_enabled = enabled ? 1 : 0; return 1; } diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index 5122aff..170b4b1 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -54,7 +54,7 @@ unsigned int selinux_checkreqprot = CONFIG_SECURITY_SELINUX_CHECKREQPROT_VALUE; static int __init checkreqprot_setup(char *str) { unsigned long checkreqprot; - if (!strict_strtoul(str, 0, &checkreqprot)) + if (!kstrtoul(str, 0, &checkreqprot)) selinux_checkreqprot = checkreqprot ? 1 : 0; return 1; } -- cgit v1.1 From 073219e995b4a3f8cf1ce8228b7ef440b6994ac0 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sat, 8 Feb 2014 10:36:58 -0500 Subject: cgroup: clean up cgroup_subsys names and initialization cgroup_subsys is a bit messier than it needs to be. * The name of a subsys can be different from its internal identifier defined in cgroup_subsys.h. Most subsystems use the matching name but three - cpu, memory and perf_event - use different ones. * cgroup_subsys_id enums are postfixed with _subsys_id and each cgroup_subsys is postfixed with _subsys. cgroup.h is widely included throughout various subsystems, it doesn't and shouldn't have claim on such generic names which don't have any qualifier indicating that they belong to cgroup. * cgroup_subsys->subsys_id should always equal the matching cgroup_subsys_id enum; however, we require each controller to initialize it and then BUG if they don't match, which is a bit silly. This patch cleans up cgroup_subsys names and initialization by doing the followings. * cgroup_subsys_id enums are now postfixed with _cgrp_id, and each cgroup_subsys with _cgrp_subsys. * With the above, renaming subsys identifiers to match the userland visible names doesn't cause any naming conflicts. All non-matching identifiers are renamed to match the official names. cpu_cgroup -> cpu mem_cgroup -> memory perf -> perf_event * controllers no longer need to initialize ->subsys_id and ->name. They're generated in cgroup core and set automatically during boot. * Redundant cgroup_subsys declarations removed. * While updating BUG_ON()s in cgroup_init_early(), convert them to WARN()s. BUGging that early during boot is stupid - the kernel can't print anything, even through serial console and the trap handler doesn't even link stack frame properly for back-tracing. This patch doesn't introduce any behavior changes. v2: Rebased on top of fe1217c4f3f7 ("net: net_cls: move cgroupfs classid handling into core"). Signed-off-by: Tejun Heo Acked-by: Neil Horman Acked-by: "David S. Miller" Acked-by: "Rafael J. Wysocki" Acked-by: Michal Hocko Acked-by: Peter Zijlstra Acked-by: Aristeu Rozanski Acked-by: Ingo Molnar Acked-by: Li Zefan Cc: Johannes Weiner Cc: Balbir Singh Cc: KAMEZAWA Hiroyuki Cc: Serge E. Hallyn Cc: Vivek Goyal Cc: Thomas Graf --- security/device_cgroup.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'security') diff --git a/security/device_cgroup.c b/security/device_cgroup.c index d3b6d2c..7f88bcd 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c @@ -58,11 +58,9 @@ static inline struct dev_cgroup *css_to_devcgroup(struct cgroup_subsys_state *s) static inline struct dev_cgroup *task_devcgroup(struct task_struct *task) { - return css_to_devcgroup(task_css(task, devices_subsys_id)); + return css_to_devcgroup(task_css(task, devices_cgrp_id)); } -struct cgroup_subsys devices_subsys; - /* * called under devcgroup_mutex */ @@ -684,13 +682,11 @@ static struct cftype dev_cgroup_files[] = { { } /* terminate */ }; -struct cgroup_subsys devices_subsys = { - .name = "devices", +struct cgroup_subsys devices_cgrp_subsys = { .css_alloc = devcgroup_css_alloc, .css_free = devcgroup_css_free, .css_online = devcgroup_online, .css_offline = devcgroup_offline, - .subsys_id = devices_subsys_id, .base_cftypes = dev_cgroup_files, }; -- cgit v1.1 From ca925cf1534ebcec332c08719a7dee6ee1782ce4 Mon Sep 17 00:00:00 2001 From: Fan Du Date: Sat, 18 Jan 2014 09:55:27 +0800 Subject: flowcache: Make flow cache name space aware Inserting a entry into flowcache, or flushing flowcache should be based on per net scope. The reason to do so is flushing operation from fat netns crammed with flow entries will also making the slim netns with only a few flow cache entries go away in original implementation. Since flowcache is tightly coupled with IPsec, so it would be easier to put flow cache global parameters into xfrm namespace part. And one last thing needs to do is bumping flow cache genid, and flush flow cache should also be made in per net style. Signed-off-by: Fan Du Signed-off-by: Steffen Klassert --- security/selinux/include/xfrm.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/selinux/include/xfrm.h b/security/selinux/include/xfrm.h index 48c3cc9..dfe3fda 100644 --- a/security/selinux/include/xfrm.h +++ b/security/selinux/include/xfrm.h @@ -45,10 +45,11 @@ static inline void selinux_xfrm_notify_policyload(void) { struct net *net; - atomic_inc(&flow_cache_genid); rtnl_lock(); - for_each_net(net) + for_each_net(net) { + atomic_inc(&net->xfrm.flow_cache_genid); rt_genid_bump_all(net); + } rtnl_unlock(); } #else -- cgit v1.1 From e0c2de2b15da83ef39d56c326f5a7e681c6c4e7f Mon Sep 17 00:00:00 2001 From: Sam Ravnborg Date: Sat, 15 Feb 2014 22:49:30 +0100 Subject: security: cleanup Makefiles to use standard syntax for specifying sub-directories The Makefiles in security/ uses a non-standard way to specify sub-directories for building. Fix it up so the normal (and documented) approach is used. Signed-off-by: Sam Ravnborg Cc: Michal Marek Signed-off-by: James Morris --- security/Makefile | 12 ++++++------ security/integrity/Makefile | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) (limited to 'security') diff --git a/security/Makefile b/security/Makefile index a5918e0..05f1c93 100644 --- a/security/Makefile +++ b/security/Makefile @@ -16,14 +16,14 @@ obj-$(CONFIG_MMU) += min_addr.o # Object file lists obj-$(CONFIG_SECURITY) += security.o capability.o obj-$(CONFIG_SECURITYFS) += inode.o -obj-$(CONFIG_SECURITY_SELINUX) += selinux/built-in.o -obj-$(CONFIG_SECURITY_SMACK) += smack/built-in.o +obj-$(CONFIG_SECURITY_SELINUX) += selinux/ +obj-$(CONFIG_SECURITY_SMACK) += smack/ obj-$(CONFIG_AUDIT) += lsm_audit.o -obj-$(CONFIG_SECURITY_TOMOYO) += tomoyo/built-in.o -obj-$(CONFIG_SECURITY_APPARMOR) += apparmor/built-in.o -obj-$(CONFIG_SECURITY_YAMA) += yama/built-in.o +obj-$(CONFIG_SECURITY_TOMOYO) += tomoyo/ +obj-$(CONFIG_SECURITY_APPARMOR) += apparmor/ +obj-$(CONFIG_SECURITY_YAMA) += yama/ obj-$(CONFIG_CGROUP_DEVICE) += device_cgroup.o # Object integrity file lists subdir-$(CONFIG_INTEGRITY) += integrity -obj-$(CONFIG_INTEGRITY) += integrity/built-in.o +obj-$(CONFIG_INTEGRITY) += integrity/ diff --git a/security/integrity/Makefile b/security/integrity/Makefile index 0f9cffb..0793f48 100644 --- a/security/integrity/Makefile +++ b/security/integrity/Makefile @@ -10,6 +10,6 @@ obj-$(CONFIG_INTEGRITY_ASYMMETRIC_KEYS) += digsig_asymmetric.o integrity-y := iint.o subdir-$(CONFIG_IMA) += ima -obj-$(CONFIG_IMA) += ima/built-in.o +obj-$(CONFIG_IMA) += ima/ subdir-$(CONFIG_EVM) += evm -obj-$(CONFIG_EVM) += evm/built-in.o +obj-$(CONFIG_EVM) += evm/ -- cgit v1.1 From 31d4b761896101660b3efe01816b63d9814755eb Mon Sep 17 00:00:00 2001 From: Libo Chen Date: Wed, 11 Dec 2013 14:11:28 +0800 Subject: ima: new helper: file_inode(file) Replace "file->f_dentry->d_inode" with the new file_inode() helper function. Signed-off-by: Libo Chen Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_api.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index c38bbce..6d76d4a 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -132,7 +132,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename, const char *op, const char *cause) { struct ima_template_entry *entry; - struct inode *inode = file->f_dentry->d_inode; + struct inode *inode = file_inode(file); int violation = 1; int result; -- cgit v1.1 From 875ec3da4c50571906dcd23952f1dfcdd92ae3f1 Mon Sep 17 00:00:00 2001 From: Heiko Carstens Date: Mon, 3 Mar 2014 16:34:41 +0100 Subject: security/compat: convert to COMPAT_SYSCALL_DEFINE Convert all compat system call functions where all parameter types have a size of four or less than four bytes, or are pointer types to COMPAT_SYSCALL_DEFINE. The implicit casts within COMPAT_SYSCALL_DEFINE will perform proper zero and sign extension to 64 bit of all parameters if needed. Signed-off-by: Heiko Carstens --- security/keys/compat.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/keys/compat.c b/security/keys/compat.c index bbd32c7..3478965 100644 --- a/security/keys/compat.c +++ b/security/keys/compat.c @@ -65,8 +65,8 @@ no_payload: * taking a 32-bit syscall are zero. If you can, you should call sys_keyctl() * directly. */ -asmlinkage long compat_sys_keyctl(u32 option, - u32 arg2, u32 arg3, u32 arg4, u32 arg5) +COMPAT_SYSCALL_DEFINE5(keyctl, u32, option, + u32, arg2, u32, arg3, u32, arg4, u32, arg5) { switch (option) { case KEYCTL_GET_KEYRING_ID: -- cgit v1.1 From d4a141c8e77043bd674dd6aa0b40bc3675cb7b1d Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Wed, 5 Mar 2014 12:47:37 -0500 Subject: security: have cap_dentry_init_security return error Currently, cap_dentry_init_security returns 0 without actually initializing the security label. This confuses its only caller (nfs4_label_init_security) which expects an error in that situation, and causes it to end up sending out junk onto the wire instead of simply suppressing the label in the attributes sent. When CONFIG_SECURITY is disabled, security_dentry_init_security returns -EOPNOTSUPP. Have cap_dentry_init_security do the same. Signed-off-by: Jeff Layton Acked-by: Serge E. Hallyn Signed-off-by: James Morris --- security/capability.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/capability.c b/security/capability.c index 8b4f24a..9323bbe 100644 --- a/security/capability.c +++ b/security/capability.c @@ -116,7 +116,7 @@ static int cap_dentry_init_security(struct dentry *dentry, int mode, struct qstr *name, void **ctx, u32 *ctxlen) { - return 0; + return -EOPNOTSUPP; } static int cap_inode_alloc_security(struct inode *inode) -- cgit v1.1 From 52a13284844b354c7a37533f5366cb5b653a76b3 Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Wed, 11 Dec 2013 14:44:04 -0500 Subject: ima: use static const char array definitions A const char pointer allocates memory for a pointer as well as for a string, This patch replaces a number of the const char pointers throughout IMA, with a static const char array. Suggested-by: David Howells Signed-off-by: Mimi Zohar Acked-by: David Howells --- security/integrity/ima/ima_api.c | 8 ++++---- security/integrity/ima/ima_appraise.c | 4 ++-- security/integrity/ima/ima_init.c | 4 ++-- security/integrity/ima/ima_policy.c | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index 6d76d4a..393b9d4 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -92,8 +92,8 @@ int ima_store_template(struct ima_template_entry *entry, int violation, struct inode *inode, const unsigned char *filename) { - const char *op = "add_template_measure"; - const char *audit_cause = "hashing_error"; + static const char op[] = "add_template_measure"; + static const char audit_cause[] = "hashing_error"; char *template_name = entry->template_desc->name; int result; struct { @@ -260,8 +260,8 @@ void ima_store_measurement(struct integrity_iint_cache *iint, struct evm_ima_xattr_data *xattr_value, int xattr_len) { - const char *op = "add_template_measure"; - const char *audit_cause = "ENOMEM"; + static const char op[] = "add_template_measure"; + static const char audit_cause[] = "ENOMEM"; int result = -ENOMEM; struct inode *inode = file_inode(file); struct ima_template_entry *entry; diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 734e946..291bf0f 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -177,11 +177,11 @@ int ima_appraise_measurement(int func, struct integrity_iint_cache *iint, struct evm_ima_xattr_data *xattr_value, int xattr_len) { + static const char op[] = "appraise_data"; + char *cause = "unknown"; struct dentry *dentry = file->f_dentry; struct inode *inode = dentry->d_inode; enum integrity_status status = INTEGRITY_UNKNOWN; - const char *op = "appraise_data"; - char *cause = "unknown"; int rc = xattr_len, hash_start = 0; if (!ima_appraise) diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c index 3712276..315f2b9 100644 --- a/security/integrity/ima/ima_init.c +++ b/security/integrity/ima/ima_init.c @@ -42,10 +42,10 @@ int ima_used_chip; */ static void __init ima_add_boot_aggregate(void) { + static const char op[] = "add_boot_aggregate"; + const char *audit_cause = "ENOMEM"; struct ima_template_entry *entry; struct integrity_iint_cache tmp_iint, *iint = &tmp_iint; - const char *op = "add_boot_aggregate"; - const char *audit_cause = "ENOMEM"; int result = -ENOMEM; int violation = 0; struct { diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 354b125..3f6b8a4 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -329,7 +329,7 @@ void __init ima_init_policy(void) */ void ima_update_policy(void) { - const char *op = "policy_update"; + static const char op[] = "policy_update"; const char *cause = "already exists"; int result = 1; int audit_info = 0; @@ -645,7 +645,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) */ ssize_t ima_parse_add_rule(char *rule) { - const char *op = "update_policy"; + static const char op[] = "update_policy"; char *p; struct ima_rule_entry *entry; ssize_t result, len; -- cgit v1.1 From 73a6b44a003ad5dd1af9a8d05f01589dce7cd47a Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Tue, 24 Dec 2013 20:49:01 +0900 Subject: Integrity: Pass commname via get_task_comm() When we pass task->comm to audit_log_untrustedstring(), we need to pass it via get_task_comm() because task->comm can be changed to contain untrusted string by other threads after audit_log_untrustedstring() confirmed that task->comm does not contain untrusted string. Signed-off-by: Tetsuo Handa Signed-off-by: Mimi Zohar --- security/integrity/integrity_audit.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'security') diff --git a/security/integrity/integrity_audit.c b/security/integrity/integrity_audit.c index 809ec842..4b996ba 100644 --- a/security/integrity/integrity_audit.c +++ b/security/integrity/integrity_audit.c @@ -33,6 +33,7 @@ void integrity_audit_msg(int audit_msgno, struct inode *inode, const char *cause, int result, int audit_info) { struct audit_buffer *ab; + char name[TASK_COMM_LEN]; if (!integrity_audit_info && audit_info == 1) /* Skip info messages */ return; @@ -49,7 +50,7 @@ void integrity_audit_msg(int audit_msgno, struct inode *inode, audit_log_format(ab, " cause="); audit_log_string(ab, cause); audit_log_format(ab, " comm="); - audit_log_untrustedstring(ab, current->comm); + audit_log_untrustedstring(ab, get_task_comm(name, current)); if (fname) { audit_log_format(ab, " name="); audit_log_untrustedstring(ab, fname); -- cgit v1.1 From c019e307ad82a8ee652b8ccbacf69ae94263b07b Mon Sep 17 00:00:00 2001 From: Roberto Sassu Date: Mon, 3 Feb 2014 13:56:04 +0100 Subject: ima: restore the original behavior for sending data with ima template With the new template mechanism introduced in IMA since kernel 3.13, the format of data sent through the binary_runtime_measurements interface is slightly changed. Now, for a generic measurement, the format of template data (after the template name) is: template_len | field1_len | field1 | ... | fieldN_len | fieldN In addition, fields containing a string now include the '\0' termination character. Instead, the format for the 'ima' template should be: SHA1 digest | event name length | event name It must be noted that while in the IMA 3.13 code 'event name length' is 'IMA_EVENT_NAME_LEN_MAX + 1' (256 bytes), so that the template digest is calculated correctly, and 'event name' contains '\0', in the pre 3.13 code 'event name length' is exactly the string length and 'event name' does not contain the termination character. The patch restores the behavior of the IMA code pre 3.13 for the 'ima' template so that legacy userspace tools obtain a consistent behavior when receiving data from the binary_runtime_measurements interface regardless of which kernel version is used. Signed-off-by: Roberto Sassu Cc: # 3.3.13: 3ce1217 ima: define template fields library Signed-off-by: Mimi Zohar --- security/integrity/ima/ima.h | 2 +- security/integrity/ima/ima_fs.c | 2 ++ security/integrity/ima/ima_template_lib.c | 10 +++++++--- 3 files changed, 10 insertions(+), 4 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 0356e1d..f79fa8b 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -27,7 +27,7 @@ #include "../integrity.h" enum ima_show_type { IMA_SHOW_BINARY, IMA_SHOW_BINARY_NO_FIELD_LEN, - IMA_SHOW_ASCII }; + IMA_SHOW_BINARY_OLD_STRING_FMT, IMA_SHOW_ASCII }; enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8 }; /* digest size for IMA, fits SHA1 or MD5 */ diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index db01125..468a3ba 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -160,6 +160,8 @@ static int ima_measurements_show(struct seq_file *m, void *v) if (is_ima_template && strcmp(field->field_id, "d") == 0) show = IMA_SHOW_BINARY_NO_FIELD_LEN; + if (is_ima_template && strcmp(field->field_id, "n") == 0) + show = IMA_SHOW_BINARY_OLD_STRING_FMT; field->field_show(m, show, &e->template_data[i]); } return 0; diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c index 1683bbf..e8592e7 100644 --- a/security/integrity/ima/ima_template_lib.c +++ b/security/integrity/ima/ima_template_lib.c @@ -109,13 +109,16 @@ static void ima_show_template_data_binary(struct seq_file *m, enum data_formats datafmt, struct ima_field_data *field_data) { + u32 len = (show == IMA_SHOW_BINARY_OLD_STRING_FMT) ? + strlen(field_data->data) : field_data->len; + if (show != IMA_SHOW_BINARY_NO_FIELD_LEN) - ima_putc(m, &field_data->len, sizeof(u32)); + ima_putc(m, &len, sizeof(len)); - if (!field_data->len) + if (!len) return; - ima_putc(m, field_data->data, field_data->len); + ima_putc(m, field_data->data, len); } static void ima_show_template_field_data(struct seq_file *m, @@ -129,6 +132,7 @@ static void ima_show_template_field_data(struct seq_file *m, break; case IMA_SHOW_BINARY: case IMA_SHOW_BINARY_NO_FIELD_LEN: + case IMA_SHOW_BINARY_OLD_STRING_FMT: ima_show_template_data_binary(m, show, datafmt, field_data); break; default: -- cgit v1.1 From e3b64c268b485f578a498c2f6d5704ef54ab4432 Mon Sep 17 00:00:00 2001 From: Roberto Sassu Date: Mon, 3 Feb 2014 13:56:05 +0100 Subject: ima: reduce memory usage when a template containing the n field is used Before this change, to correctly calculate the template digest for the 'ima' template, the event name field (id: 'n') length was set to the fixed size of 256 bytes. This patch reduces the length of the event name field to the string length incremented of one (to make room for the termination character '\0') and handles the specific case of the digest calculation for the 'ima' template directly in ima_calc_field_array_hash_tfm(). Signed-off-by: Roberto Sassu Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_crypto.c | 11 +++++++++-- security/integrity/ima/ima_template_lib.c | 19 ++++--------------- 2 files changed, 13 insertions(+), 17 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c index fdf60de..d8b55c9 100644 --- a/security/integrity/ima/ima_crypto.c +++ b/security/integrity/ima/ima_crypto.c @@ -161,15 +161,22 @@ static int ima_calc_field_array_hash_tfm(struct ima_field_data *field_data, return rc; for (i = 0; i < num_fields; i++) { + u8 buffer[IMA_EVENT_NAME_LEN_MAX + 1] = { 0 }; + u8 *data_to_hash = field_data[i].data; + u32 datalen = field_data[i].len; + if (strcmp(td->name, IMA_TEMPLATE_IMA_NAME) != 0) { rc = crypto_shash_update(&desc.shash, (const u8 *) &field_data[i].len, sizeof(field_data[i].len)); if (rc) break; + } else if (strcmp(td->fields[i]->field_id, "n") == 0) { + memcpy(buffer, data_to_hash, datalen); + data_to_hash = buffer; + datalen = IMA_EVENT_NAME_LEN_MAX + 1; } - rc = crypto_shash_update(&desc.shash, field_data[i].data, - field_data[i].len); + rc = crypto_shash_update(&desc.shash, data_to_hash, datalen); if (rc) break; } diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c index e8592e7..1506f02 100644 --- a/security/integrity/ima/ima_template_lib.c +++ b/security/integrity/ima/ima_template_lib.c @@ -27,7 +27,6 @@ static bool ima_template_hash_algo_allowed(u8 algo) enum data_formats { DATA_FMT_DIGEST = 0, DATA_FMT_DIGEST_WITH_ALGO, - DATA_FMT_EVENT_NAME, DATA_FMT_STRING, DATA_FMT_HEX }; @@ -37,18 +36,10 @@ static int ima_write_template_field_data(const void *data, const u32 datalen, struct ima_field_data *field_data) { u8 *buf, *buf_ptr; - u32 buflen; + u32 buflen = datalen; - switch (datafmt) { - case DATA_FMT_EVENT_NAME: - buflen = IMA_EVENT_NAME_LEN_MAX + 1; - break; - case DATA_FMT_STRING: + if (datafmt == DATA_FMT_STRING) buflen = datalen + 1; - break; - default: - buflen = datalen; - } buf = kzalloc(buflen, GFP_KERNEL); if (!buf) @@ -63,7 +54,7 @@ static int ima_write_template_field_data(const void *data, const u32 datalen, * split into multiple template fields (the space is the delimitator * character for measurements lists in ASCII format). */ - if (datafmt == DATA_FMT_EVENT_NAME || datafmt == DATA_FMT_STRING) { + if (datafmt == DATA_FMT_STRING) { for (buf_ptr = buf; buf_ptr - buf < datalen; buf_ptr++) if (*buf_ptr == ' ') *buf_ptr = '_'; @@ -281,8 +272,6 @@ static int ima_eventname_init_common(struct integrity_iint_cache *iint, { const char *cur_filename = NULL; u32 cur_filename_len = 0; - enum data_formats fmt = size_limit ? - DATA_FMT_EVENT_NAME : DATA_FMT_STRING; BUG_ON(filename == NULL && file == NULL); @@ -305,7 +294,7 @@ static int ima_eventname_init_common(struct integrity_iint_cache *iint, cur_filename_len = IMA_EVENT_NAME_LEN_MAX; out: return ima_write_template_field_data(cur_filename, cur_filename_len, - fmt, field_data); + DATA_FMT_STRING, field_data); } /* -- cgit v1.1 From 20ee451f5a7cd43edda56ba36cbec4d881d3329f Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Mon, 24 Feb 2014 13:59:56 -0800 Subject: security: integrity: Use a more current logging style Convert printks to pr_. Add pr_fmt. Remove embedded prefixes. Signed-off-by: Joe Perches Signed-off-by: Mimi Zohar --- security/integrity/evm/evm_crypto.c | 4 +++- security/integrity/evm/evm_main.c | 6 ++++-- security/integrity/evm/evm_secfs.c | 6 ++++-- security/integrity/ima/ima_crypto.c | 4 +++- security/integrity/ima/ima_init.c | 5 ++++- security/integrity/ima/ima_queue.c | 8 +++++--- security/integrity/ima/ima_template.c | 5 ++++- 7 files changed, 27 insertions(+), 11 deletions(-) (limited to 'security') diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c index 3bab89e..9bd329f 100644 --- a/security/integrity/evm/evm_crypto.c +++ b/security/integrity/evm/evm_crypto.c @@ -13,6 +13,8 @@ * Using root's kernel master key (kmk), calculate the HMAC */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include #include #include @@ -221,7 +223,7 @@ int evm_init_hmac(struct inode *inode, const struct xattr *lsm_xattr, desc = init_desc(EVM_XATTR_HMAC); if (IS_ERR(desc)) { - printk(KERN_INFO "init_desc failed\n"); + pr_info("init_desc failed\n"); return PTR_ERR(desc); } diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 336b3dd..996092f 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -14,6 +14,8 @@ * evm_inode_removexattr, and evm_verifyxattr */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include #include #include @@ -432,7 +434,7 @@ static int __init init_evm(void) error = evm_init_secfs(); if (error < 0) { - printk(KERN_INFO "EVM: Error registering secfs\n"); + pr_info("Error registering secfs\n"); goto err; } @@ -449,7 +451,7 @@ static int __init evm_display_config(void) char **xattrname; for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) - printk(KERN_INFO "EVM: %s\n", *xattrname); + pr_info("%s\n", *xattrname); return 0; } diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c index 30f670a..cf12a04 100644 --- a/security/integrity/evm/evm_secfs.c +++ b/security/integrity/evm/evm_secfs.c @@ -13,6 +13,8 @@ * - Get the key and enable EVM */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include #include #include "evm.h" @@ -79,9 +81,9 @@ static ssize_t evm_write_key(struct file *file, const char __user *buf, error = evm_init_key(); if (!error) { evm_initialized = 1; - pr_info("EVM: initialized\n"); + pr_info("initialized\n"); } else - pr_err("EVM: initialization failed\n"); + pr_err("initialization failed\n"); return count; } diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c index d8b55c9..9999057 100644 --- a/security/integrity/ima/ima_crypto.c +++ b/security/integrity/ima/ima_crypto.c @@ -13,6 +13,8 @@ * Calculates md5/sha1 file hash, template hash, boot-aggreate hash */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include #include #include @@ -212,7 +214,7 @@ static void __init ima_pcrread(int idx, u8 *pcr) return; if (tpm_pcr_read(TPM_ANY_NUM, idx, pcr) != 0) - pr_err("IMA: Error Communicating to TPM chip\n"); + pr_err("Error Communicating to TPM chip\n"); } /* diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c index 315f2b9..e8f9d70 100644 --- a/security/integrity/ima/ima_init.c +++ b/security/integrity/ima/ima_init.c @@ -14,6 +14,9 @@ * File: ima_init.c * initialization and cleanup functions */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include #include #include @@ -93,7 +96,7 @@ int __init ima_init(void) ima_used_chip = 1; if (!ima_used_chip) - pr_info("IMA: No TPM chip found, activating TPM-bypass!\n"); + pr_info("No TPM chip found, activating TPM-bypass!\n"); rc = ima_init_crypto(); if (rc) diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c index d85e997..91128b4 100644 --- a/security/integrity/ima/ima_queue.c +++ b/security/integrity/ima/ima_queue.c @@ -18,6 +18,9 @@ * The measurement list is append-only. No entry is * ever removed or changed during the boot-cycle. */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include #include #include @@ -72,7 +75,7 @@ static int ima_add_digest_entry(struct ima_template_entry *entry) qe = kmalloc(sizeof(*qe), GFP_KERNEL); if (qe == NULL) { - pr_err("IMA: OUT OF MEMORY ERROR creating queue entry.\n"); + pr_err("OUT OF MEMORY ERROR creating queue entry\n"); return -ENOMEM; } qe->entry = entry; @@ -95,8 +98,7 @@ static int ima_pcr_extend(const u8 *hash) result = tpm_pcr_extend(TPM_ANY_NUM, CONFIG_IMA_MEASURE_PCR_IDX, hash); if (result != 0) - pr_err("IMA: Error Communicating to TPM chip, result: %d\n", - result); + pr_err("Error Communicating to TPM chip, result: %d\n", result); return result; } diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c index 635695f..9a4a0d1 100644 --- a/security/integrity/ima/ima_template.c +++ b/security/integrity/ima/ima_template.c @@ -12,6 +12,9 @@ * File: ima_template.c * Helpers to manage template descriptors. */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include #include "ima.h" @@ -58,7 +61,7 @@ static int __init ima_template_setup(char *str) */ if (template_len == 3 && strcmp(str, IMA_TEMPLATE_IMA_NAME) == 0 && ima_hash_algo != HASH_ALGO_SHA1 && ima_hash_algo != HASH_ALGO_MD5) { - pr_err("IMA: template does not support hash alg\n"); + pr_err("template does not support hash alg\n"); return 1; } -- cgit v1.1 From 09b1148ef59c93d292a3355c00e9b5779b2ecad0 Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Wed, 13 Nov 2013 23:42:39 +0200 Subject: ima: fix erroneous removal of security.ima xattr ima_inode_post_setattr() calls ima_must_appraise() to check if the file needs to be appraised. If it does not then it removes security.ima xattr. With original policy matching code it might happen that even file needs to be appraised with FILE_CHECK hook, it might not be for POST_SETATTR hook. 'security.ima' might be erronously removed. This patch treats POST_SETATTR as special wildcard function and will cause ima_must_appraise() to be true if any of the hooks rules matches. security.ima will not be removed if any of the hooks would require appraisal. Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_policy.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 3f6b8a4..a556d5b9 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -167,9 +167,11 @@ static bool ima_match_rules(struct ima_rule_entry *rule, const struct cred *cred = current_cred(); int i; - if ((rule->flags & IMA_FUNC) && rule->func != func) + if ((rule->flags & IMA_FUNC) && + (rule->func != func && func != POST_SETATTR)) return false; - if ((rule->flags & IMA_MASK) && rule->mask != mask) + if ((rule->flags & IMA_MASK) && + (rule->mask != mask && func != POST_SETATTR)) return false; if ((rule->flags & IMA_FSMAGIC) && rule->fsmagic != inode->i_sb->s_magic) -- cgit v1.1 From 2bb930abcf39d8be243ddb4583cf013ea2a750d6 Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Tue, 4 Mar 2014 18:04:20 +0200 Subject: integrity: fix checkpatch errors Between checkpatch changes (eg. sizeof) and inconsistencies between Lindent and checkpatch, unfixed checkpatch errors make it difficult to see new errors. This patch fixes them. Some lines with over 80 chars remained unchanged to improve code readability. The "extern" keyword is removed from internal evm.h to make it consistent with internal ima.h. Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/evm/evm.h | 28 +++++++-------- security/integrity/evm/evm_crypto.c | 4 +-- security/integrity/iint.c | 2 +- security/integrity/ima/ima_api.c | 8 ++--- security/integrity/ima/ima_crypto.c | 2 +- security/integrity/ima/ima_fs.c | 6 ++-- security/integrity/ima/ima_main.c | 4 +-- security/integrity/ima/ima_policy.c | 65 +++++++++++++++++------------------ security/integrity/ima/ima_queue.c | 4 +-- security/integrity/ima/ima_template.c | 14 ++++---- security/integrity/integrity_audit.c | 2 +- 11 files changed, 69 insertions(+), 70 deletions(-) (limited to 'security') diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h index 30bd1ec..37c88dd 100644 --- a/security/integrity/evm/evm.h +++ b/security/integrity/evm/evm.h @@ -32,19 +32,19 @@ extern struct crypto_shash *hash_tfm; /* List of EVM protected security xattrs */ extern char *evm_config_xattrnames[]; -extern int evm_init_key(void); -extern int evm_update_evmxattr(struct dentry *dentry, - const char *req_xattr_name, - const char *req_xattr_value, - size_t req_xattr_value_len); -extern int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name, - const char *req_xattr_value, - size_t req_xattr_value_len, char *digest); -extern int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name, - const char *req_xattr_value, - size_t req_xattr_value_len, char *digest); -extern int evm_init_hmac(struct inode *inode, const struct xattr *xattr, - char *hmac_val); -extern int evm_init_secfs(void); +int evm_init_key(void); +int evm_update_evmxattr(struct dentry *dentry, + const char *req_xattr_name, + const char *req_xattr_value, + size_t req_xattr_value_len); +int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name, + const char *req_xattr_value, + size_t req_xattr_value_len, char *digest); +int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name, + const char *req_xattr_value, + size_t req_xattr_value_len, char *digest); +int evm_init_hmac(struct inode *inode, const struct xattr *xattr, + char *hmac_val); +int evm_init_secfs(void); #endif diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c index 9bd329f..babd862 100644 --- a/security/integrity/evm/evm_crypto.c +++ b/security/integrity/evm/evm_crypto.c @@ -105,13 +105,13 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode, umode_t mode; } hmac_misc; - memset(&hmac_misc, 0, sizeof hmac_misc); + memset(&hmac_misc, 0, sizeof(hmac_misc)); hmac_misc.ino = inode->i_ino; hmac_misc.generation = inode->i_generation; hmac_misc.uid = from_kuid(&init_user_ns, inode->i_uid); hmac_misc.gid = from_kgid(&init_user_ns, inode->i_gid); hmac_misc.mode = inode->i_mode; - crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof hmac_misc); + crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc)); if (evm_hmac_version > 1) crypto_shash_update(desc, inode->i_sb->s_uuid, sizeof(inode->i_sb->s_uuid)); diff --git a/security/integrity/iint.c b/security/integrity/iint.c index c49d3f1..a521edf 100644 --- a/security/integrity/iint.c +++ b/security/integrity/iint.c @@ -151,7 +151,7 @@ static void init_once(void *foo) { struct integrity_iint_cache *iint = foo; - memset(iint, 0, sizeof *iint); + memset(iint, 0, sizeof(*iint)); iint->version = 0; iint->flags = 0UL; iint->ima_file_status = INTEGRITY_UNKNOWN; diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index 393b9d4..c6b4a73 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -160,10 +160,10 @@ err_out: * @function: calling function (FILE_CHECK, BPRM_CHECK, MMAP_CHECK, MODULE_CHECK) * * The policy is defined in terms of keypairs: - * subj=, obj=, type=, func=, mask=, fsmagic= + * subj=, obj=, type=, func=, mask=, fsmagic= * subj,obj, and type: are LSM specific. - * func: FILE_CHECK | BPRM_CHECK | MMAP_CHECK | MODULE_CHECK - * mask: contains the permission mask + * func: FILE_CHECK | BPRM_CHECK | MMAP_CHECK | MODULE_CHECK + * mask: contains the permission mask * fsmagic: hex value * * Returns IMA_MEASURE, IMA_APPRAISE mask. @@ -248,7 +248,7 @@ int ima_collect_measurement(struct integrity_iint_cache *iint, * * We only get here if the inode has not already been measured, * but the measurement could already exist: - * - multiple copies of the same file on either the same or + * - multiple copies of the same file on either the same or * different filesystems. * - the inode was previously flushed as well as the iint info, * containing the hashing info. diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c index 9999057..d257e36 100644 --- a/security/integrity/ima/ima_crypto.c +++ b/security/integrity/ima/ima_crypto.c @@ -10,7 +10,7 @@ * the Free Software Foundation, version 2 of the License. * * File: ima_crypto.c - * Calculates md5/sha1 file hash, template hash, boot-aggreate hash + * Calculates md5/sha1 file hash, template hash, boot-aggreate hash */ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index 468a3ba..da92fcc 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -133,14 +133,14 @@ static int ima_measurements_show(struct seq_file *m, void *v) * PCR used is always the same (config option) in * little-endian format */ - ima_putc(m, &pcr, sizeof pcr); + ima_putc(m, &pcr, sizeof(pcr)); /* 2nd: template digest */ ima_putc(m, e->digest, TPM_DIGEST_SIZE); /* 3rd: template name size */ namelen = strlen(e->template_desc->name); - ima_putc(m, &namelen, sizeof namelen); + ima_putc(m, &namelen, sizeof(namelen)); /* 4th: template name */ ima_putc(m, e->template_desc->name, namelen); @@ -292,7 +292,7 @@ static atomic_t policy_opencount = ATOMIC_INIT(1); /* * ima_open_policy: sequentialize access to the policy file */ -static int ima_open_policy(struct inode * inode, struct file * filp) +static int ima_open_policy(struct inode *inode, struct file *filp) { /* No point in being allowed to open it if you aren't going to write */ if (!(filp->f_flags & O_WRONLY)) diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 149ee11..50413d0 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -71,10 +71,10 @@ __setup("ima_hash=", hash_setup); * ima_rdwr_violation_check * * Only invalidate the PCR for measured files: - * - Opening a file for write when already open for read, + * - Opening a file for write when already open for read, * results in a time of measure, time of use (ToMToU) error. * - Opening a file for read when already open for write, - * could result in a file measurement error. + * could result in a file measurement error. * */ static void ima_rdwr_violation_check(struct file *file) diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index a556d5b9..93873a4 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -7,7 +7,7 @@ * the Free Software Foundation, version 2 of the License. * * ima_policy.c - * - initialize default measure policy rules + * - initialize default measure policy rules * */ #include @@ -21,8 +21,8 @@ #include "ima.h" /* flags definitions */ -#define IMA_FUNC 0x0001 -#define IMA_MASK 0x0002 +#define IMA_FUNC 0x0001 +#define IMA_MASK 0x0002 #define IMA_FSMAGIC 0x0004 #define IMA_UID 0x0008 #define IMA_FOWNER 0x0010 @@ -69,35 +69,35 @@ struct ima_rule_entry { * and running executables. */ static struct ima_rule_entry default_rules[] = { - {.action = DONT_MEASURE,.fsmagic = PROC_SUPER_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_MEASURE,.fsmagic = SYSFS_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_MEASURE,.fsmagic = DEBUGFS_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_MEASURE,.fsmagic = TMPFS_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_MEASURE,.fsmagic = DEVPTS_SUPER_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_MEASURE,.fsmagic = BINFMTFS_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_MEASURE,.fsmagic = SECURITYFS_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_MEASURE,.fsmagic = SELINUX_MAGIC,.flags = IMA_FSMAGIC}, - {.action = MEASURE,.func = MMAP_CHECK,.mask = MAY_EXEC, + {.action = DONT_MEASURE, .fsmagic = PROC_SUPER_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_MEASURE, .fsmagic = SYSFS_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_MEASURE, .fsmagic = DEBUGFS_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_MEASURE, .fsmagic = TMPFS_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_MEASURE, .fsmagic = DEVPTS_SUPER_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_MEASURE, .fsmagic = BINFMTFS_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_MEASURE, .fsmagic = SECURITYFS_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_MEASURE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC}, + {.action = MEASURE, .func = MMAP_CHECK, .mask = MAY_EXEC, .flags = IMA_FUNC | IMA_MASK}, - {.action = MEASURE,.func = BPRM_CHECK,.mask = MAY_EXEC, + {.action = MEASURE, .func = BPRM_CHECK, .mask = MAY_EXEC, .flags = IMA_FUNC | IMA_MASK}, - {.action = MEASURE,.func = FILE_CHECK,.mask = MAY_READ,.uid = GLOBAL_ROOT_UID, + {.action = MEASURE, .func = FILE_CHECK, .mask = MAY_READ, .uid = GLOBAL_ROOT_UID, .flags = IMA_FUNC | IMA_MASK | IMA_UID}, - {.action = MEASURE,.func = MODULE_CHECK, .flags = IMA_FUNC}, + {.action = MEASURE, .func = MODULE_CHECK, .flags = IMA_FUNC}, }; static struct ima_rule_entry default_appraise_rules[] = { - {.action = DONT_APPRAISE,.fsmagic = PROC_SUPER_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_APPRAISE,.fsmagic = SYSFS_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_APPRAISE,.fsmagic = DEBUGFS_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_APPRAISE,.fsmagic = TMPFS_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_APPRAISE,.fsmagic = RAMFS_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_APPRAISE,.fsmagic = DEVPTS_SUPER_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_APPRAISE,.fsmagic = BINFMTFS_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_APPRAISE,.fsmagic = SECURITYFS_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_APPRAISE,.fsmagic = SELINUX_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_APPRAISE,.fsmagic = CGROUP_SUPER_MAGIC,.flags = IMA_FSMAGIC}, - {.action = APPRAISE,.fowner = GLOBAL_ROOT_UID,.flags = IMA_FOWNER}, + {.action = DONT_APPRAISE, .fsmagic = PROC_SUPER_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_APPRAISE, .fsmagic = SYSFS_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_APPRAISE, .fsmagic = DEBUGFS_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_APPRAISE, .fsmagic = TMPFS_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_APPRAISE, .fsmagic = RAMFS_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_APPRAISE, .fsmagic = DEVPTS_SUPER_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_APPRAISE, .fsmagic = BINFMTFS_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_APPRAISE, .fsmagic = SECURITYFS_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_APPRAISE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_APPRAISE, .fsmagic = CGROUP_SUPER_MAGIC, .flags = IMA_FSMAGIC}, + {.action = APPRAISE, .fowner = GLOBAL_ROOT_UID, .flags = IMA_FOWNER}, }; static LIST_HEAD(ima_default_rules); @@ -122,12 +122,12 @@ static int __init default_appraise_policy_setup(char *str) } __setup("ima_appraise_tcb", default_appraise_policy_setup); -/* +/* * Although the IMA policy does not change, the LSM policy can be * reloaded, leaving the IMA LSM based rules referring to the old, * stale LSM policy. * - * Update the IMA LSM based rules to reflect the reloaded LSM policy. + * Update the IMA LSM based rules to reflect the reloaded LSM policy. * We assume the rules still exist; and BUG_ON() if they don't. */ static void ima_lsm_update_rules(void) @@ -218,7 +218,7 @@ retry: retried = 1; ima_lsm_update_rules(); goto retry; - } + } if (!rc) return false; } @@ -234,7 +234,7 @@ static int get_subaction(struct ima_rule_entry *rule, int func) if (!(rule->flags & IMA_FUNC)) return IMA_FILE_APPRAISE; - switch(func) { + switch (func) { case MMAP_CHECK: return IMA_MMAP_APPRAISE; case BPRM_CHECK: @@ -306,7 +306,7 @@ void __init ima_init_policy(void) measure_entries = ima_use_tcb ? ARRAY_SIZE(default_rules) : 0; appraise_entries = ima_use_appraise_tcb ? ARRAY_SIZE(default_appraise_rules) : 0; - + for (i = 0; i < measure_entries + appraise_entries; i++) { if (i < measure_entries) list_add_tail(&default_rules[i].list, @@ -522,8 +522,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) break; } - result = kstrtoul(args[0].from, 16, - &entry->fsmagic); + result = kstrtoul(args[0].from, 16, &entry->fsmagic); if (!result) entry->flags |= IMA_FSMAGIC; break; diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c index 91128b4..552705d 100644 --- a/security/integrity/ima/ima_queue.c +++ b/security/integrity/ima/ima_queue.c @@ -117,7 +117,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation, mutex_lock(&ima_extend_list_mutex); if (!violation) { - memcpy(digest, entry->digest, sizeof digest); + memcpy(digest, entry->digest, sizeof(digest)); if (ima_lookup_digest_entry(digest)) { audit_cause = "hash_exists"; result = -EEXIST; @@ -133,7 +133,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation, } if (violation) /* invalidate pcr */ - memset(digest, 0xff, sizeof digest); + memset(digest, 0xff, sizeof(digest)); tpmresult = ima_pcr_extend(digest); if (tpmresult != 0) { diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c index 9a4a0d1..a076a96 100644 --- a/security/integrity/ima/ima_template.c +++ b/security/integrity/ima/ima_template.c @@ -22,20 +22,20 @@ static struct ima_template_desc defined_templates[] = { {.name = IMA_TEMPLATE_IMA_NAME, .fmt = IMA_TEMPLATE_IMA_FMT}, - {.name = "ima-ng",.fmt = "d-ng|n-ng"}, - {.name = "ima-sig",.fmt = "d-ng|n-ng|sig"}, + {.name = "ima-ng", .fmt = "d-ng|n-ng"}, + {.name = "ima-sig", .fmt = "d-ng|n-ng|sig"}, }; static struct ima_template_field supported_fields[] = { - {.field_id = "d",.field_init = ima_eventdigest_init, + {.field_id = "d", .field_init = ima_eventdigest_init, .field_show = ima_show_template_digest}, - {.field_id = "n",.field_init = ima_eventname_init, + {.field_id = "n", .field_init = ima_eventname_init, .field_show = ima_show_template_string}, - {.field_id = "d-ng",.field_init = ima_eventdigest_ng_init, + {.field_id = "d-ng", .field_init = ima_eventdigest_ng_init, .field_show = ima_show_template_digest_ng}, - {.field_id = "n-ng",.field_init = ima_eventname_ng_init, + {.field_id = "n-ng", .field_init = ima_eventname_ng_init, .field_show = ima_show_template_string}, - {.field_id = "sig",.field_init = ima_eventsig_init, + {.field_id = "sig", .field_init = ima_eventsig_init, .field_show = ima_show_template_sig}, }; diff --git a/security/integrity/integrity_audit.c b/security/integrity/integrity_audit.c index 4b996ba..aab9fa5 100644 --- a/security/integrity/integrity_audit.c +++ b/security/integrity/integrity_audit.c @@ -7,7 +7,7 @@ * the Free Software Foundation, version 2 of the License. * * File: integrity_audit.c - * Audit calls for the integrity subsystem + * Audit calls for the integrity subsystem */ #include -- cgit v1.1 From 61997c4383c28fe93fb053295562ff6482ef5c07 Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Wed, 13 Nov 2013 22:23:20 +0200 Subject: ima: return d_name.name if d_path fails This is a small refactoring so ima_d_path() returns dentry name if path reconstruction fails. It simplifies callers actions and removes code duplication. Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_api.c | 2 +- security/integrity/ima/ima_main.c | 7 +------ 2 files changed, 2 insertions(+), 7 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index c6b4a73..ba9e4d7 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -332,5 +332,5 @@ const char *ima_d_path(struct path *path, char **pathbuf) pathname = NULL; } } - return pathname; + return pathname ?: (const char *)path->dentry->d_name.name; } diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 50413d0..52ac6cf 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -79,7 +79,6 @@ __setup("ima_hash=", hash_setup); */ static void ima_rdwr_violation_check(struct file *file) { - struct dentry *dentry = file->f_path.dentry; struct inode *inode = file_inode(file); fmode_t mode = file->f_mode; int must_measure; @@ -111,8 +110,6 @@ out: return; pathname = ima_d_path(&file->f_path, &pathbuf); - if (!pathname || strlen(pathname) > IMA_EVENT_NAME_LEN_MAX) - pathname = dentry->d_name.name; if (send_tomtou) ima_add_violation(file, pathname, "invalid_pcr", "ToMToU"); @@ -220,9 +217,7 @@ static int process_measurement(struct file *file, const char *filename, if (rc != 0) goto out_digsig; - pathname = !filename ? ima_d_path(&file->f_path, &pathbuf) : filename; - if (!pathname) - pathname = (const char *)file->f_dentry->d_name.name; + pathname = filename ?: ima_d_path(&file->f_path, &pathbuf); if (action & IMA_MEASURE) ima_store_measurement(iint, file, pathname, -- cgit v1.1 From e0420039b643a832231028000a5c0d7358b14f3b Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Wed, 26 Feb 2014 17:47:46 +0200 Subject: evm: EVM does not use MD5 EVM does not use MD5 HMAC. Selection of CRYPTO_MD5 can be safely removed. Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/evm/Kconfig | 1 - 1 file changed, 1 deletion(-) (limited to 'security') diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig index fea9749..5aa9103 100644 --- a/security/integrity/evm/Kconfig +++ b/security/integrity/evm/Kconfig @@ -2,7 +2,6 @@ config EVM boolean "EVM support" depends on SECURITY && KEYS && (TRUSTED_KEYS=y || TRUSTED_KEYS=n) select CRYPTO_HMAC - select CRYPTO_MD5 select CRYPTO_SHA1 select ENCRYPTED_KEYS default n -- cgit v1.1 From 1d91ac6213003f525ac34da5e39cbb6612d19deb Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Thu, 27 Feb 2014 20:16:47 +0200 Subject: ima: skip memory allocation for empty files Memory allocation is unnecessary for empty files. This patch calculates the hash without memory allocation. Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_crypto.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c index d257e36..1bde8e6 100644 --- a/security/integrity/ima/ima_crypto.c +++ b/security/integrity/ima/ima_crypto.c @@ -87,16 +87,20 @@ static int ima_calc_file_hash_tfm(struct file *file, if (rc != 0) return rc; - rbuf = kzalloc(PAGE_SIZE, GFP_KERNEL); - if (!rbuf) { - rc = -ENOMEM; + i_size = i_size_read(file_inode(file)); + + if (i_size == 0) goto out; - } + + rbuf = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!rbuf) + return -ENOMEM; + if (!(file->f_mode & FMODE_READ)) { file->f_mode |= FMODE_READ; read = 1; } - i_size = i_size_read(file_inode(file)); + while (offset < i_size) { int rbuf_len; @@ -113,12 +117,12 @@ static int ima_calc_file_hash_tfm(struct file *file, if (rc) break; } - kfree(rbuf); - if (!rc) - rc = crypto_shash_final(&desc.shash, hash->digest); if (read) file->f_mode &= ~FMODE_READ; + kfree(rbuf); out: + if (!rc) + rc = crypto_shash_final(&desc.shash, hash->digest); return rc; } -- cgit v1.1 From a3aef94b312ec51b5dfc199ef884924e60ad1b75 Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Fri, 28 Feb 2014 14:18:09 +0200 Subject: evm: enable key retention service automatically If keys are not enabled, EVM is not visible in the configuration menu. It may be difficult to figure out what to do unless you really know. Other subsystems as NFS, CIFS select keys automatically. This patch does the same. This patch also removes '(TRUSTED_KEYS=y || TRUSTED_KEYS=n)' dependency, which is unnecessary. EVM does not depend on trusted keys, but on encrypted keys. evm.h provides compile time dependency. Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/evm/Kconfig | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig index 5aa9103..d35b491 100644 --- a/security/integrity/evm/Kconfig +++ b/security/integrity/evm/Kconfig @@ -1,9 +1,10 @@ config EVM boolean "EVM support" - depends on SECURITY && KEYS && (TRUSTED_KEYS=y || TRUSTED_KEYS=n) + depends on SECURITY + select KEYS + select ENCRYPTED_KEYS select CRYPTO_HMAC select CRYPTO_SHA1 - select ENCRYPTED_KEYS default n help EVM protects a file's security extended attributes against -- cgit v1.1 From 4d3bb511b5f9980fc3e9ae5939ebc475b231d3fc Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Wed, 19 Mar 2014 10:23:54 -0400 Subject: cgroup: drop const from @buffer of cftype->write_string() cftype->write_string() just passes on the writeable buffer from kernfs and there's no reason to add const restriction on the buffer. The only thing const achieves is unnecessarily complicating parsing of the buffer. Drop const from @buffer. Signed-off-by: Tejun Heo Acked-by: Li Zefan Cc: Peter Zijlstra Cc: Paul Mackerras Cc: Ingo Molnar Cc: Arnaldo Carvalho de Melo Cc: Daniel Borkmann Cc: Michal Hocko Cc: Johannes Weiner Cc: Balbir Singh Cc: KAMEZAWA Hiroyuki --- security/device_cgroup.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/device_cgroup.c b/security/device_cgroup.c index 7f88bcd..8365909 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c @@ -496,7 +496,7 @@ static inline bool has_children(struct dev_cgroup *devcgroup) * parent cgroup has the access you're asking for. */ static int devcgroup_update_access(struct dev_cgroup *devcgroup, - int filetype, const char *buffer) + int filetype, char *buffer) { const char *b; char temp[12]; /* 11 + 1 characters needed for a u32 */ @@ -652,7 +652,7 @@ static int devcgroup_update_access(struct dev_cgroup *devcgroup, } static int devcgroup_access_write(struct cgroup_subsys_state *css, - struct cftype *cft, const char *buffer) + struct cftype *cft, char *buffer) { int retval; -- cgit v1.1 From 98883bfd9d603a2760f6d53eccfaa3ae2c053e72 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Wed, 19 Mar 2014 16:46:11 -0400 Subject: selinux: put the mmap() DAC controls before the MAC controls It turns out that doing the SELinux MAC checks for mmap() before the DAC checks was causing users and the SELinux policy folks headaches as users were seeing a lot of SELinux AVC denials for the memprotect:mmap_zero permission that would have also been denied by the normal DAC capability checks (CAP_SYS_RAWIO). Example: # cat mmap_test.c #include #include #include #include int main(int argc, char *argv[]) { int rc; void *mem; mem = mmap(0x0, 4096, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0); if (mem == MAP_FAILED) return errno; printf("mem = %p\n", mem); munmap(mem, 4096); return 0; } # gcc -g -O0 -o mmap_test mmap_test.c # ./mmap_test mem = (nil) # ausearch -m AVC | grep mmap_zero type=AVC msg=audit(...): avc: denied { mmap_zero } for pid=1025 comm="mmap_test" scontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 tcontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 tclass=memprotect This patch corrects things so that when the above example is run by a user without CAP_SYS_RAWIO the SELinux AVC is no longer generated as the DAC capability check fails before the SELinux permission check. Signed-off-by: Paul Moore Acked-by: Stephen Smalley --- security/selinux/hooks.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 0e68bdb..1dd9484 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3188,24 +3188,20 @@ error: static int selinux_mmap_addr(unsigned long addr) { - int rc = 0; - u32 sid = current_sid(); + int rc; + + /* do DAC check on address space usage */ + rc = cap_mmap_addr(addr); + if (rc) + return rc; - /* - * notice that we are intentionally putting the SELinux check before - * the secondary cap_file_mmap check. This is such a likely attempt - * at bad behaviour/exploit that we always want to get the AVC, even - * if DAC would have also denied the operation. - */ if (addr < CONFIG_LSM_MMAP_MIN_ADDR) { + u32 sid = current_sid(); rc = avc_has_perm(sid, sid, SECCLASS_MEMPROTECT, MEMPROTECT__MMAP_ZERO, NULL); - if (rc) - return rc; } - /* do DAC check on address space usage */ - return cap_mmap_addr(addr); + return rc; } static int selinux_mmap_file(struct file *file, unsigned long reqprot, -- cgit v1.1 From f64410ec665479d7b4b77b7519e814253ed0f686 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Wed, 19 Mar 2014 16:46:18 -0400 Subject: selinux: correctly label /proc inodes in use before the policy is loaded This patch is based on an earlier patch by Eric Paris, he describes the problem below: "If an inode is accessed before policy load it will get placed on a list of inodes to be initialized after policy load. After policy load we call inode_doinit() which calls inode_doinit_with_dentry() on all inodes accessed before policy load. In the case of inodes in procfs that means we'll end up at the bottom where it does: /* Default to the fs superblock SID. */ isec->sid = sbsec->sid; if ((sbsec->flags & SE_SBPROC) && !S_ISLNK(inode->i_mode)) { if (opt_dentry) { isec->sclass = inode_mode_to_security_class(...) rc = selinux_proc_get_sid(opt_dentry, isec->sclass, &sid); if (rc) goto out_unlock; isec->sid = sid; } } Since opt_dentry is null, we'll never call selinux_proc_get_sid() and will leave the inode labeled with the label on the superblock. I believe a fix would be to mimic the behavior of xattrs. Look for an alias of the inode. If it can't be found, just leave the inode uninitialized (and pick it up later) if it can be found, we should be able to call selinux_proc_get_sid() ..." On a system exhibiting this problem, you will notice a lot of files in /proc with the generic "proc_t" type (at least the ones that were accessed early in the boot), for example: # ls -Z /proc/sys/kernel/shmmax | awk '{ print $4 " " $5 }' system_u:object_r:proc_t:s0 /proc/sys/kernel/shmmax However, with this patch in place we see the expected result: # ls -Z /proc/sys/kernel/shmmax | awk '{ print $4 " " $5 }' system_u:object_r:sysctl_kernel_t:s0 /proc/sys/kernel/shmmax Cc: Eric Paris Signed-off-by: Paul Moore Acked-by: Eric Paris --- security/selinux/hooks.c | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 1dd9484..d5d67c9 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1402,15 +1402,33 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent isec->sid = sbsec->sid; if ((sbsec->flags & SE_SBPROC) && !S_ISLNK(inode->i_mode)) { - if (opt_dentry) { - isec->sclass = inode_mode_to_security_class(inode->i_mode); - rc = selinux_proc_get_sid(opt_dentry, - isec->sclass, - &sid); - if (rc) - goto out_unlock; - isec->sid = sid; - } + /* We must have a dentry to determine the label on + * procfs inodes */ + if (opt_dentry) + /* Called from d_instantiate or + * d_splice_alias. */ + dentry = dget(opt_dentry); + else + /* Called from selinux_complete_init, try to + * find a dentry. */ + dentry = d_find_alias(inode); + /* + * This can be hit on boot when a file is accessed + * before the policy is loaded. When we load policy we + * may find inodes that have no dentry on the + * sbsec->isec_head list. No reason to complain as + * these will get fixed up the next time we go through + * inode_doinit() with a dentry, before these inodes + * could be used again by userspace. + */ + if (!dentry) + goto out_unlock; + isec->sclass = inode_mode_to_security_class(inode->i_mode); + rc = selinux_proc_get_sid(dentry, isec->sclass, &sid); + dput(dentry); + if (rc) + goto out_unlock; + isec->sid = sid; } break; } -- cgit v1.1 From f1dc4867ff41b7bcca57fa19449d1fe7ad517ac1 Mon Sep 17 00:00:00 2001 From: Richard Guy Briggs Date: Wed, 11 Dec 2013 13:52:26 -0500 Subject: audit: anchor all pid references in the initial pid namespace Store and log all PIDs with reference to the initial PID namespace and use the access functions task_pid_nr() and task_tgid_nr() for task->pid and task->tgid. Cc: "Eric W. Biederman" (informed by ebiederman's c776b5d2) Signed-off-by: Richard Guy Briggs --- security/integrity/integrity_audit.c | 2 +- security/lsm_audit.c | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) (limited to 'security') diff --git a/security/integrity/integrity_audit.c b/security/integrity/integrity_audit.c index d7efb30..85253b5 100644 --- a/security/integrity/integrity_audit.c +++ b/security/integrity/integrity_audit.c @@ -39,7 +39,7 @@ void integrity_audit_msg(int audit_msgno, struct inode *inode, ab = audit_log_start(current->audit_context, GFP_KERNEL, audit_msgno); audit_log_format(ab, "pid=%d uid=%u auid=%u ses=%u", - current->pid, + task_pid_nr(current), from_kuid(&init_user_ns, current_cred()->uid), from_kuid(&init_user_ns, audit_get_loginuid(current)), audit_get_sessionid(current)); diff --git a/security/lsm_audit.c b/security/lsm_audit.c index 9a62045..69fdf3b 100644 --- a/security/lsm_audit.c +++ b/security/lsm_audit.c @@ -220,7 +220,7 @@ 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=", tsk->pid); + audit_log_format(ab, " pid=%d comm=", task_pid_nr(tsk)); audit_log_untrustedstring(ab, tsk->comm); switch (a->type) { @@ -278,9 +278,12 @@ static void dump_common_audit_data(struct audit_buffer *ab, } case LSM_AUDIT_DATA_TASK: tsk = a->u.tsk; - if (tsk && tsk->pid) { - audit_log_format(ab, " pid=%d comm=", tsk->pid); - audit_log_untrustedstring(ab, tsk->comm); + if (tsk) { + pid_t pid = task_pid_nr(tsk); + if (pid) { + audit_log_format(ab, " pid=%d comm=", pid); + audit_log_untrustedstring(ab, tsk->comm); + } } break; case LSM_AUDIT_DATA_NET: -- cgit v1.1 From 5d50ffd7c31dab47c6b828841ca1ec70a1b40169 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Mon, 3 Feb 2014 12:13:10 -0500 Subject: locks: add new fcntl cmd values for handling file private locks Due to some unfortunate history, POSIX locks have very strange and unhelpful semantics. The thing that usually catches people by surprise is that they are dropped whenever the process closes any file descriptor associated with the inode. This is extremely problematic for people developing file servers that need to implement byte-range locks. Developers often need a "lock management" facility to ensure that file descriptors are not closed until all of the locks associated with the inode are finished. Additionally, "classic" POSIX locks are owned by the process. Locks taken between threads within the same process won't conflict with one another, which renders them useless for synchronization between threads. This patchset adds a new type of lock that attempts to address these issues. These locks conflict with classic POSIX read/write locks, but have semantics that are more like BSD locks with respect to inheritance and behavior on close. This is implemented primarily by changing how fl_owner field is set for these locks. Instead of having them owned by the files_struct of the process, they are instead owned by the filp on which they were acquired. Thus, they are inherited across fork() and are only released when the last reference to a filp is put. These new semantics prevent them from being merged with classic POSIX locks, even if they are acquired by the same process. These locks will also conflict with classic POSIX locks even if they are acquired by the same process or on the same file descriptor. The new locks are managed using a new set of cmd values to the fcntl() syscall. The initial implementation of this converts these values to "classic" cmd values at a fairly high level, and the details are not exposed to the underlying filesystem. We may eventually want to push this handing out to the lower filesystem code but for now I don't see any need for it. Also, note that with this implementation the new cmd values are only available via fcntl64() on 32-bit arches. There's little need to add support for legacy apps on a new interface like this. Signed-off-by: Jeff Layton --- security/selinux/hooks.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 4b34847..3aa8763 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3302,6 +3302,9 @@ static int selinux_file_fcntl(struct file *file, unsigned int cmd, case F_GETLK: case F_SETLK: case F_SETLKW: + case F_GETLKP: + case F_SETLKP: + case F_SETLKPW: #if BITS_PER_LONG == 32 case F_GETLK64: case F_SETLK64: -- cgit v1.1 From 0b3974eb04c4874e85fa1d4fc70450d12f28611d Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Tue, 1 Apr 2014 17:08:43 +0200 Subject: security: add flags to rename hooks Add flags to security_path_rename() and security_inode_rename() hooks. Signed-off-by: Miklos Szeredi Reviewed-by: J. Bruce Fields --- security/security.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/security.c b/security/security.c index 919cad9..284fbc9 100644 --- a/security/security.c +++ b/security/security.c @@ -433,7 +433,8 @@ int security_path_link(struct dentry *old_dentry, struct path *new_dir, } int security_path_rename(struct path *old_dir, struct dentry *old_dentry, - struct path *new_dir, struct dentry *new_dentry) + struct path *new_dir, struct dentry *new_dentry, + unsigned int flags) { if (unlikely(IS_PRIVATE(old_dentry->d_inode) || (new_dentry->d_inode && IS_PRIVATE(new_dentry->d_inode)))) @@ -524,7 +525,8 @@ int security_inode_mknod(struct inode *dir, struct dentry *dentry, umode_t mode, } int security_inode_rename(struct inode *old_dir, struct dentry *old_dentry, - struct inode *new_dir, struct dentry *new_dentry) + struct inode *new_dir, struct dentry *new_dentry, + unsigned int flags) { if (unlikely(IS_PRIVATE(old_dentry->d_inode) || (new_dentry->d_inode && IS_PRIVATE(new_dentry->d_inode)))) -- cgit v1.1 From da1ce0670c14d8380e423a3239e562a1dc15fa9e Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Tue, 1 Apr 2014 17:08:43 +0200 Subject: vfs: add cross-rename If flags contain RENAME_EXCHANGE then exchange source and destination files. There's no restriction on the type of the files; e.g. a directory can be exchanged with a symlink. Signed-off-by: Miklos Szeredi Reviewed-by: Jan Kara Reviewed-by: J. Bruce Fields --- security/security.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'security') diff --git a/security/security.c b/security/security.c index 284fbc9..8b774f3 100644 --- a/security/security.c +++ b/security/security.c @@ -439,6 +439,14 @@ int security_path_rename(struct path *old_dir, struct dentry *old_dentry, if (unlikely(IS_PRIVATE(old_dentry->d_inode) || (new_dentry->d_inode && IS_PRIVATE(new_dentry->d_inode)))) return 0; + + if (flags & RENAME_EXCHANGE) { + int err = security_ops->path_rename(new_dir, new_dentry, + old_dir, old_dentry); + if (err) + return err; + } + return security_ops->path_rename(old_dir, old_dentry, new_dir, new_dentry); } @@ -531,6 +539,14 @@ int security_inode_rename(struct inode *old_dir, struct dentry *old_dentry, if (unlikely(IS_PRIVATE(old_dentry->d_inode) || (new_dentry->d_inode && IS_PRIVATE(new_dentry->d_inode)))) return 0; + + if (flags & RENAME_EXCHANGE) { + int err = security_ops->inode_rename(new_dir, new_dentry, + old_dir, old_dentry); + if (err) + return err; + } + return security_ops->inode_rename(old_dir, old_dentry, new_dir, new_dentry); } -- cgit v1.1 From 627bf81ac625f05060db033a0f3791521ad7bd79 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 1 Feb 2014 04:43:32 -0500 Subject: get rid of pointless checks for NULL ->i_op Signed-off-by: Al Viro --- security/integrity/evm/evm_crypto.c | 2 +- security/integrity/evm/evm_main.c | 2 +- security/tomoyo/realpath.c | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) (limited to 'security') diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c index 3bab89e..e90ab0e 100644 --- a/security/integrity/evm/evm_crypto.c +++ b/security/integrity/evm/evm_crypto.c @@ -137,7 +137,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry, int error; int size; - if (!inode->i_op || !inode->i_op->getxattr) + if (!inode->i_op->getxattr) return -EOPNOTSUPP; desc = init_desc(type); if (IS_ERR(desc)) diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 336b3dd..bab1c39 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -62,7 +62,7 @@ static int evm_find_protected_xattrs(struct dentry *dentry) int error; int count = 0; - if (!inode->i_op || !inode->i_op->getxattr) + if (!inode->i_op->getxattr) return -EOPNOTSUPP; for (xattr = evm_config_xattrnames; *xattr != NULL; xattr++) { diff --git a/security/tomoyo/realpath.c b/security/tomoyo/realpath.c index 80a09c3..a3386d1 100644 --- a/security/tomoyo/realpath.c +++ b/security/tomoyo/realpath.c @@ -173,7 +173,7 @@ static char *tomoyo_get_local_path(struct dentry *dentry, char * const buffer, * Use filesystem name if filesystem does not support rename() * operation. */ - if (inode->i_op && !inode->i_op->rename) + if (!inode->i_op->rename) goto prepend_filesystem_name; } /* Prepend device name. */ @@ -282,7 +282,7 @@ char *tomoyo_realpath_from_path(struct path *path) * Get local name for filesystems without rename() operation * or dentry without vfsmount. */ - if (!path->mnt || (inode->i_op && !inode->i_op->rename)) + if (!path->mnt || !inode->i_op->rename) pos = tomoyo_get_local_path(path->dentry, buf, buf_len - 1); /* Get absolute name for the rest. */ -- cgit v1.1 From 79d719749d23234e9b725098aa49133f3ef7299d Mon Sep 17 00:00:00 2001 From: Aristeu Rozanski Date: Mon, 21 Apr 2014 12:13:03 -0400 Subject: device_cgroup: rework device access check and exception checking Whenever a device file is opened and checked against current device cgroup rules, it uses the same function (may_access()) as when a new exception rule is added by writing devices.{allow,deny}. And in both cases, the algorithm is the same, doesn't matter the behavior. First problem is having device access to be considered the same as rule checking. Consider the following structure: A (default behavior: allow, exceptions disallow access) \ B (default behavior: allow, exceptions disallow access) A new exception is added to B by writing devices.deny: c 12:34 rw When checking if that exception is allowed in may_access(): if (dev_cgroup->behavior == DEVCG_DEFAULT_ALLOW) { if (behavior == DEVCG_DEFAULT_ALLOW) { /* the exception will deny access to certain devices */ return true; Which is ok, since B is not getting more privileges than A, it doesn't matter and the rule is accepted Now, consider it's a device file open check and the process belongs to cgroup B. The access will be generated as: behavior: allow exception: c 12:34 rw The very same chunk of code will allow it, even if there's an explicit exception telling to do otherwise. A simple test case: # mkdir new_group # cd new_group # echo $$ >tasks # echo "c 1:3 w" >devices.deny # echo >/dev/null # echo $? 0 This is a serious bug and was introduced on c39a2a3018f8 devcg: prepare may_access() for hierarchy support To solve this problem, the device file open function was split from the new exception check. Second problem is how exceptions are processed by may_access(). The first part of the said function tries to match fully with an existing exception: list_for_each_entry_rcu(ex, &dev_cgroup->exceptions, list) { if ((refex->type & DEV_BLOCK) && !(ex->type & DEV_BLOCK)) continue; if ((refex->type & DEV_CHAR) && !(ex->type & DEV_CHAR)) continue; if (ex->major != ~0 && ex->major != refex->major) continue; if (ex->minor != ~0 && ex->minor != refex->minor) continue; if (refex->access & (~ex->access)) continue; match = true; break; } That means the new exception should be contained into an existing one to be considered a match: New exception Existing match? notes b 12:34 rwm b 12:34 rwm yes b 12:34 r b *:34 rw yes b 12:34 rw b 12:34 w no extra "r" b *:34 rw b 12:34 rw no too broad "*" b *:34 rw b *:34 rwm yes Which is fine in some cases. Consider: A (default behavior: deny, exceptions allow access) \ B (default behavior: deny, exceptions allow access) In this case the full match makes sense, the new exception cannot add more access than the parent allows But this doesn't always work, consider: A (default behavior: allow, exceptions disallow access) \ B (default behavior: deny, exceptions allow access) In this case, a new exception in B shouldn't match any of the exceptions in A, after all you can't allow something that was forbidden by A. But consider this scenario: New exception Existing in A match? outcome b 12:34 rw b 12:34 r no exception is accepted Because the new exception has "w" as extra, it doesn't match, so it'll be added to B's exception list. The same problem can happen during a file access check. Consider a cgroup with allow as default behavior: Access Exception match? b 12:34 rw b 12:34 r no In this case, the access didn't match any of the exceptions in the cgroup, which is required since exceptions will disallow access. To solve this problem, two new functions were created to match an exception either fully or partially. In the example above, a partial check will be performed and it'll produce a match since at least "b 12:34 r" from "b 12:34 rw" access matches. Cc: cgroups@vger.kernel.org Cc: Tejun Heo Cc: Serge Hallyn Cc: Li Zefan Cc: stable@vger.kernel.org Signed-off-by: Aristeu Rozanski Signed-off-by: Tejun Heo --- security/device_cgroup.c | 162 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 122 insertions(+), 40 deletions(-) (limited to 'security') diff --git a/security/device_cgroup.c b/security/device_cgroup.c index 8365909..b9048dc 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c @@ -306,57 +306,139 @@ static int devcgroup_seq_show(struct seq_file *m, void *v) } /** - * may_access - verifies if a new exception is part of what is allowed - * by a dev cgroup based on the default policy + - * exceptions. This is used to make sure a child cgroup - * won't have more privileges than its parent or to - * verify if a certain access is allowed. - * @dev_cgroup: dev cgroup to be tested against - * @refex: new exception - * @behavior: behavior of the exception + * match_exception - iterates the exception list trying to match a rule + * based on type, major, minor and access type. It is + * considered a match if an exception is found that + * will contain the entire range of provided parameters. + * @exceptions: list of exceptions + * @type: device type (DEV_BLOCK or DEV_CHAR) + * @major: device file major number, ~0 to match all + * @minor: device file minor number, ~0 to match all + * @access: permission mask (ACC_READ, ACC_WRITE, ACC_MKNOD) + * + * returns: true in case it matches an exception completely */ -static bool may_access(struct dev_cgroup *dev_cgroup, - struct dev_exception_item *refex, - enum devcg_behavior behavior) +static bool match_exception(struct list_head *exceptions, short type, + u32 major, u32 minor, short access) { struct dev_exception_item *ex; - bool match = false; - rcu_lockdep_assert(rcu_read_lock_held() || - lockdep_is_held(&devcgroup_mutex), - "device_cgroup::may_access() called without proper synchronization"); + list_for_each_entry_rcu(ex, exceptions, list) { + if ((type & DEV_BLOCK) && !(ex->type & DEV_BLOCK)) + continue; + if ((type & DEV_CHAR) && !(ex->type & DEV_CHAR)) + continue; + if (ex->major != ~0 && ex->major != major) + continue; + if (ex->minor != ~0 && ex->minor != minor) + continue; + /* provided access cannot have more than the exception rule */ + if (access & (~ex->access)) + continue; + return true; + } + return false; +} + +/** + * match_exception_partial - iterates the exception list trying to match a rule + * based on type, major, minor and access type. It is + * considered a match if an exception's range is + * found to contain *any* of the devices specified by + * provided parameters. This is used to make sure no + * extra access is being granted that is forbidden by + * any of the exception list. + * @exceptions: list of exceptions + * @type: device type (DEV_BLOCK or DEV_CHAR) + * @major: device file major number, ~0 to match all + * @minor: device file minor number, ~0 to match all + * @access: permission mask (ACC_READ, ACC_WRITE, ACC_MKNOD) + * + * returns: true in case the provided range mat matches an exception completely + */ +static bool match_exception_partial(struct list_head *exceptions, short type, + u32 major, u32 minor, short access) +{ + struct dev_exception_item *ex; - list_for_each_entry_rcu(ex, &dev_cgroup->exceptions, list) { - if ((refex->type & DEV_BLOCK) && !(ex->type & DEV_BLOCK)) + list_for_each_entry_rcu(ex, exceptions, list) { + if ((type & DEV_BLOCK) && !(ex->type & DEV_BLOCK)) continue; - if ((refex->type & DEV_CHAR) && !(ex->type & DEV_CHAR)) + if ((type & DEV_CHAR) && !(ex->type & DEV_CHAR)) continue; - if (ex->major != ~0 && ex->major != refex->major) + /* + * We must be sure that both the exception and the provided + * range aren't masking all devices + */ + if (ex->major != ~0 && major != ~0 && ex->major != major) continue; - if (ex->minor != ~0 && ex->minor != refex->minor) + if (ex->minor != ~0 && minor != ~0 && ex->minor != minor) continue; - if (refex->access & (~ex->access)) + /* + * In order to make sure the provided range isn't matching + * an exception, all its access bits shouldn't match the + * exception's access bits + */ + if (!(access & ex->access)) continue; - match = true; - break; + return true; } + return false; +} + +/** + * verify_new_ex - verifies if a new exception is part of what is allowed + * by a dev cgroup based on the default policy + + * exceptions. This is used to make sure a child cgroup + * won't have more privileges than its parent + * @dev_cgroup: dev cgroup to be tested against + * @refex: new exception + * @behavior: behavior of the exception's dev_cgroup + */ +static bool verify_new_ex(struct dev_cgroup *dev_cgroup, + struct dev_exception_item *refex, + enum devcg_behavior behavior) +{ + bool match = false; + + rcu_lockdep_assert(rcu_read_lock_held() || + lockdep_is_held(&devcgroup_mutex), + "device_cgroup:verify_new_ex called without proper synchronization"); if (dev_cgroup->behavior == DEVCG_DEFAULT_ALLOW) { if (behavior == DEVCG_DEFAULT_ALLOW) { - /* the exception will deny access to certain devices */ + /* + * new exception in the child doesn't matter, only + * adding extra restrictions + */ return true; } else { - /* the exception will allow access to certain devices */ + /* + * new exception in the child will add more devices + * that can be acessed, so it can't match any of + * parent's exceptions, even slightly + */ + match = match_exception_partial(&dev_cgroup->exceptions, + refex->type, + refex->major, + refex->minor, + refex->access); + if (match) - /* - * a new exception allowing access shouldn't - * match an parent's exception - */ return false; return true; } } else { - /* only behavior == DEVCG_DEFAULT_DENY allowed here */ + /* + * Only behavior == DEVCG_DEFAULT_DENY allowed here, therefore + * the new exception will add access to more devices and must + * be contained completely in an parent's exception to be + * allowed + */ + match = match_exception(&dev_cgroup->exceptions, refex->type, + refex->major, refex->minor, + refex->access); + if (match) /* parent has an exception that matches the proposed */ return true; @@ -378,7 +460,7 @@ static int parent_has_perm(struct dev_cgroup *childcg, if (!parent) return 1; - return may_access(parent, ex, childcg->behavior); + return verify_new_ex(parent, ex, childcg->behavior); } /** @@ -704,18 +786,18 @@ static int __devcgroup_check_permission(short type, u32 major, u32 minor, short access) { struct dev_cgroup *dev_cgroup; - struct dev_exception_item ex; - int rc; - - memset(&ex, 0, sizeof(ex)); - ex.type = type; - ex.major = major; - ex.minor = minor; - ex.access = access; + bool rc; rcu_read_lock(); dev_cgroup = task_devcgroup(current); - rc = may_access(dev_cgroup, &ex, dev_cgroup->behavior); + if (dev_cgroup->behavior == DEVCG_DEFAULT_ALLOW) + /* Can't match any of the exceptions, even partially */ + rc = !match_exception_partial(&dev_cgroup->exceptions, + type, major, minor, access); + else + /* Need to match completely one exception to be allowed */ + rc = match_exception(&dev_cgroup->exceptions, type, major, + minor, access); rcu_read_unlock(); if (!rc) -- cgit v1.1 From 0d3f7a2dd2f5cf9642982515e020c1aee2cf7af6 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Tue, 22 Apr 2014 08:23:58 -0400 Subject: locks: rename file-private locks to "open file description locks" File-private locks have been merged into Linux for v3.15, and *now* people are commenting that the name and macro definitions for the new file-private locks suck. ...and I can't even disagree. The names and command macros do suck. We're going to have to live with these for a long time, so it's important that we be happy with the names before we're stuck with them. The consensus on the lists so far is that they should be rechristened as "open file description locks". The name isn't a big deal for the kernel, but the command macros are not visually distinct enough from the traditional POSIX lock macros. The glibc and documentation folks are recommending that we change them to look like F_OFD_{GETLK|SETLK|SETLKW}. That lessens the chance that a programmer will typo one of the commands wrong, and also makes it easier to spot this difference when reading code. This patch makes the following changes that I think are necessary before v3.15 ships: 1) rename the command macros to their new names. These end up in the uapi headers and so are part of the external-facing API. It turns out that glibc doesn't actually use the fcntl.h uapi header, but it's hard to be sure that something else won't. Changing it now is safest. 2) make the the /proc/locks output display these as type "OFDLCK" Cc: Michael Kerrisk Cc: Christoph Hellwig Cc: Carlos O'Donell Cc: Stefan Metzmacher Cc: Andy Lutomirski Cc: Frank Filz Cc: Theodore Ts'o Signed-off-by: Jeff Layton --- security/selinux/hooks.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index b4beb77..2c7341d 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3317,9 +3317,9 @@ static int selinux_file_fcntl(struct file *file, unsigned int cmd, case F_GETLK: case F_SETLK: case F_SETLKW: - case F_GETLKP: - case F_SETLKP: - case F_SETLKPW: + case F_OFD_GETLK: + case F_OFD_SETLK: + case F_OFD_SETLKW: #if BITS_PER_LONG == 32 case F_GETLK64: case F_SETLK64: -- cgit v1.1 From f5f3cf6f7e49b9529fc00a2c4629fa92cf2755fe Mon Sep 17 00:00:00 2001 From: Aristeu Rozanski Date: Thu, 24 Apr 2014 15:33:21 -0400 Subject: device_cgroup: fix the comment format for recently added functions Moving more extensive explanations to the end of the comment. Cc: Li Zefan Signed-off-by: Aristeu Rozanski Acked-by: Serge Hallyn Signed-off-by: Tejun Heo --- security/device_cgroup.c | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) (limited to 'security') diff --git a/security/device_cgroup.c b/security/device_cgroup.c index b9048dc..6b1266d 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c @@ -306,17 +306,17 @@ static int devcgroup_seq_show(struct seq_file *m, void *v) } /** - * match_exception - iterates the exception list trying to match a rule - * based on type, major, minor and access type. It is - * considered a match if an exception is found that - * will contain the entire range of provided parameters. + * match_exception - iterates the exception list trying to find a complete match * @exceptions: list of exceptions * @type: device type (DEV_BLOCK or DEV_CHAR) * @major: device file major number, ~0 to match all * @minor: device file minor number, ~0 to match all * @access: permission mask (ACC_READ, ACC_WRITE, ACC_MKNOD) * - * returns: true in case it matches an exception completely + * It is considered a complete match if an exception is found that will + * contain the entire range of provided parameters. + * + * Return: true in case it matches an exception completely */ static bool match_exception(struct list_head *exceptions, short type, u32 major, u32 minor, short access) @@ -341,20 +341,19 @@ static bool match_exception(struct list_head *exceptions, short type, } /** - * match_exception_partial - iterates the exception list trying to match a rule - * based on type, major, minor and access type. It is - * considered a match if an exception's range is - * found to contain *any* of the devices specified by - * provided parameters. This is used to make sure no - * extra access is being granted that is forbidden by - * any of the exception list. + * match_exception_partial - iterates the exception list trying to find a partial match * @exceptions: list of exceptions * @type: device type (DEV_BLOCK or DEV_CHAR) * @major: device file major number, ~0 to match all * @minor: device file minor number, ~0 to match all * @access: permission mask (ACC_READ, ACC_WRITE, ACC_MKNOD) * - * returns: true in case the provided range mat matches an exception completely + * It is considered a partial match if an exception's range is found to + * contain *any* of the devices specified by provided parameters. This is + * used to make sure no extra access is being granted that is forbidden by + * any of the exception list. + * + * Return: true in case the provided range mat matches an exception completely */ static bool match_exception_partial(struct list_head *exceptions, short type, u32 major, u32 minor, short access) @@ -387,13 +386,13 @@ static bool match_exception_partial(struct list_head *exceptions, short type, } /** - * verify_new_ex - verifies if a new exception is part of what is allowed - * by a dev cgroup based on the default policy + - * exceptions. This is used to make sure a child cgroup - * won't have more privileges than its parent + * verify_new_ex - verifies if a new exception is allowed by parent cgroup's permissions * @dev_cgroup: dev cgroup to be tested against * @refex: new exception * @behavior: behavior of the exception's dev_cgroup + * + * This is used to make sure a child cgroup won't have more privileges + * than its parent */ static bool verify_new_ex(struct dev_cgroup *dev_cgroup, struct dev_exception_item *refex, -- cgit v1.1 From d2c2b11cfa134f4fbdcc34088824da26a084d8de Mon Sep 17 00:00:00 2001 From: Aristeu Rozanski Date: Mon, 5 May 2014 11:18:59 -0400 Subject: device_cgroup: check if exception removal is allowed [PATCH v3 1/2] device_cgroup: check if exception removal is allowed When the device cgroup hierarchy was introduced in bd2953ebbb53 - devcg: propagate local changes down the hierarchy a specific case was overlooked. Consider the hierarchy bellow: A default policy: ALLOW, exceptions will deny access \ B default policy: ALLOW, exceptions will deny access There's no need to verify when an new exception is added to B because in this case exceptions will deny access to further devices, which is always fine. Hierarchy in device cgroup only makes sure B won't have more access than A. But when an exception is removed (by writing devices.allow), it isn't checked if the user is in fact removing an inherited exception from A, thus giving more access to B. Example: # echo 'a' >A/devices.allow # echo 'c 1:3 rw' >A/devices.deny # echo $$ >A/B/tasks # echo >/dev/null -bash: /dev/null: Operation not permitted # echo 'c 1:3 w' >A/B/devices.allow # echo >/dev/null # This shouldn't be allowed and this patch fixes it by making sure to never allow exceptions in this case to be removed if the exception is partially or fully present on the parent. v3: missing '*' in function description v2: improved log message and formatting fixes Cc: cgroups@vger.kernel.org Cc: Li Zefan Cc: stable@vger.kernel.org Signed-off-by: Aristeu Rozanski Acked-by: Serge Hallyn Signed-off-by: Tejun Heo --- security/device_cgroup.c | 41 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 38 insertions(+), 3 deletions(-) (limited to 'security') diff --git a/security/device_cgroup.c b/security/device_cgroup.c index 6b1266d..9134dbf 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c @@ -463,6 +463,37 @@ static int parent_has_perm(struct dev_cgroup *childcg, } /** + * parent_allows_removal - verify if it's ok to remove an exception + * @childcg: child cgroup from where the exception will be removed + * @ex: exception being removed + * + * When removing an exception in cgroups with default ALLOW policy, it must + * be checked if removing it will give the child cgroup more access than the + * parent. + * + * Return: true if it's ok to remove exception, false otherwise + */ +static bool parent_allows_removal(struct dev_cgroup *childcg, + struct dev_exception_item *ex) +{ + struct dev_cgroup *parent = css_to_devcgroup(css_parent(&childcg->css)); + + if (!parent) + return true; + + /* It's always allowed to remove access to devices */ + if (childcg->behavior == DEVCG_DEFAULT_DENY) + return true; + + /* + * Make sure you're not removing part or a whole exception existing in + * the parent cgroup + */ + return !match_exception_partial(&parent->exceptions, ex->type, + ex->major, ex->minor, ex->access); +} + +/** * may_allow_all - checks if it's possible to change the behavior to * allow based on parent's rules. * @parent: device cgroup's parent @@ -697,17 +728,21 @@ static int devcgroup_update_access(struct dev_cgroup *devcgroup, switch (filetype) { case DEVCG_ALLOW: - if (!parent_has_perm(devcgroup, &ex)) - return -EPERM; /* * If the default policy is to allow by default, try to remove * an matching exception instead. And be silent about it: we * don't want to break compatibility */ if (devcgroup->behavior == DEVCG_DEFAULT_ALLOW) { + /* Check if the parent allows removing it first */ + if (!parent_allows_removal(devcgroup, &ex)) + return -EPERM; dev_exception_rm(devcgroup, &ex); - return 0; + break; } + + if (!parent_has_perm(devcgroup, &ex)) + return -EPERM; rc = dev_exception_add(devcgroup, &ex); break; case DEVCG_DENY: -- cgit v1.1 From 39f1f78d53b9bcbca91967380c5f0f2305a5c55f Mon Sep 17 00:00:00 2001 From: Al Viro Date: Tue, 6 May 2014 14:02:53 -0400 Subject: nick kvfree() from apparmor too many places open-code it Signed-off-by: Al Viro --- security/apparmor/include/apparmor.h | 1 - security/apparmor/lib.c | 14 -------------- 2 files changed, 15 deletions(-) (limited to 'security') diff --git a/security/apparmor/include/apparmor.h b/security/apparmor/include/apparmor.h index 8fb1488..97130f8 100644 --- a/security/apparmor/include/apparmor.h +++ b/security/apparmor/include/apparmor.h @@ -66,7 +66,6 @@ extern int apparmor_initialized __initdata; char *aa_split_fqname(char *args, char **ns_name); void aa_info_message(const char *str); void *__aa_kvmalloc(size_t size, gfp_t flags); -void kvfree(void *buffer); static inline void *kvmalloc(size_t size) { diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c index 6968992..c1827e0 100644 --- a/security/apparmor/lib.c +++ b/security/apparmor/lib.c @@ -104,17 +104,3 @@ void *__aa_kvmalloc(size_t size, gfp_t flags) } return buffer; } - -/** - * kvfree - free an allocation do by kvmalloc - * @buffer: buffer to free (MAYBE_NULL) - * - * Free a buffer allocated by kvmalloc - */ -void kvfree(void *buffer) -{ - if (is_vmalloc_addr(buffer)) - vfree(buffer); - else - kfree(buffer); -} -- cgit v1.1