From c593642c8be046915ca3a4a300243a68077cd207 Mon Sep 17 00:00:00 2001 From: Pankaj Bharadiya Date: Mon, 9 Dec 2019 10:31:43 -0800 Subject: treewide: Use sizeof_field() macro Replace all the occurrences of FIELD_SIZEOF() with sizeof_field() except at places where these are defined. Later patches will remove the unused definition of FIELD_SIZEOF(). This patch is generated using following script: EXCLUDE_FILES="include/linux/stddef.h|include/linux/kernel.h" git grep -l -e "\bFIELD_SIZEOF\b" | while read file; do if [[ "$file" =~ $EXCLUDE_FILES ]]; then continue fi sed -i -e 's/\bFIELD_SIZEOF\b/sizeof_field/g' $file; done Signed-off-by: Pankaj Bharadiya Link: https://lore.kernel.org/r/20190924105839.110713-3-pankaj.laxminarayan.bharadiya@intel.com Co-developed-by: Kees Cook Signed-off-by: Kees Cook Acked-by: David Miller # for net --- security/integrity/ima/ima_policy.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index f19a895ad7cd..ef8dfd47c7e3 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -45,7 +45,7 @@ #define DONT_HASH 0x0200 #define INVALID_PCR(a) (((a) < 0) || \ - (a) >= (FIELD_SIZEOF(struct integrity_iint_cache, measured_pcrs) * 8)) + (a) >= (sizeof_field(struct integrity_iint_cache, measured_pcrs) * 8)) int ima_policy_flag; static int temp_ima_appraise; @@ -274,7 +274,7 @@ static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry) * lsm rules can change */ memcpy(nentry, entry, sizeof(*nentry)); - memset(nentry->lsm, 0, FIELD_SIZEOF(struct ima_rule_entry, lsm)); + memset(nentry->lsm, 0, sizeof_field(struct ima_rule_entry, lsm)); for (i = 0; i < MAX_LSM_RULES; i++) { if (!entry->lsm[i].rule) -- cgit v1.2.3 From 6f7c41374b62fd80bbd8aae3536c43688c54d95e Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Mon, 25 Nov 2019 10:46:51 +0900 Subject: tomoyo: Don't use nifty names on sockets. syzbot is reporting that use of SOCKET_I()->sk from open() can result in use after free problem [1], for socket's inode is still reachable via /proc/pid/fd/n despite destruction of SOCKET_I()->sk already completed. At first I thought that this race condition applies to only open/getattr permission checks. But James Morris has pointed out that there are more permission checks where this race condition applies to. Thus, get rid of tomoyo_get_socket_name() instead of conditionally bypassing permission checks on sockets. As a side effect of this patch, "socket:[family=\$:type=\$:protocol=\$]" in the policy files has to be rewritten to "socket:[\$]". [1] https://syzkaller.appspot.com/bug?id=73d590010454403d55164cca23bd0565b1eb3b74 Signed-off-by: Tetsuo Handa Reported-by: syzbot Reported-by: James Morris --- security/tomoyo/realpath.c | 32 +------------------------------- 1 file changed, 1 insertion(+), 31 deletions(-) (limited to 'security') diff --git a/security/tomoyo/realpath.c b/security/tomoyo/realpath.c index e7832448d721..bf38fc1b59b2 100644 --- a/security/tomoyo/realpath.c +++ b/security/tomoyo/realpath.c @@ -217,31 +217,6 @@ out: return ERR_PTR(-ENOMEM); } -/** - * tomoyo_get_socket_name - Get the name of a socket. - * - * @path: Pointer to "struct path". - * @buffer: Pointer to buffer to return value in. - * @buflen: Sizeof @buffer. - * - * Returns the buffer. - */ -static char *tomoyo_get_socket_name(const struct path *path, char * const buffer, - const int buflen) -{ - struct inode *inode = d_backing_inode(path->dentry); - struct socket *sock = inode ? SOCKET_I(inode) : NULL; - struct sock *sk = sock ? sock->sk : NULL; - - if (sk) { - snprintf(buffer, buflen, "socket:[family=%u:type=%u:protocol=%u]", - sk->sk_family, sk->sk_type, sk->sk_protocol); - } else { - snprintf(buffer, buflen, "socket:[unknown]"); - } - return buffer; -} - /** * tomoyo_realpath_from_path - Returns realpath(3) of the given pathname but ignores chroot'ed root. * @@ -279,12 +254,7 @@ char *tomoyo_realpath_from_path(const struct path *path) break; /* To make sure that pos is '\0' terminated. */ buf[buf_len - 1] = '\0'; - /* Get better name for socket. */ - if (sb->s_magic == SOCKFS_MAGIC) { - pos = tomoyo_get_socket_name(path, buf, buf_len - 1); - goto encode; - } - /* For "pipe:[\$]". */ + /* For "pipe:[\$]" and "socket:[\$]". */ if (dentry->d_op && dentry->d_op->d_dname) { pos = dentry->d_op->d_dname(dentry, buf, buf_len - 1); goto encode; -- cgit v1.2.3 From 601f0093f2647db67be40b62e13cd0660990a7c8 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Wed, 9 Oct 2019 16:04:43 -0700 Subject: KEYS: remove CONFIG_KEYS_COMPAT KEYS_COMPAT now always takes the value of COMPAT && KEYS. But the security/keys/ directory is only compiled if KEYS is enabled, so in practice KEYS_COMPAT is the same as COMPAT. Therefore, remove the unnecessary KEYS_COMPAT and just use COMPAT directly. (Also remove an outdated comment from compat.c.) Reviewed-by: James Morris Signed-off-by: Eric Biggers Reviewed-by: Jarkko Sakkinen Tested-by: Jarkko Sakkinen Signed-off-by: Jarkko Sakkinen --- security/keys/Kconfig | 4 ---- security/keys/Makefile | 2 +- security/keys/compat.c | 5 ----- security/keys/internal.h | 4 ++-- 4 files changed, 3 insertions(+), 12 deletions(-) (limited to 'security') diff --git a/security/keys/Kconfig b/security/keys/Kconfig index dd313438fecf..47c041563d41 100644 --- a/security/keys/Kconfig +++ b/security/keys/Kconfig @@ -21,10 +21,6 @@ config KEYS If you are unsure as to whether this is required, answer N. -config KEYS_COMPAT - def_bool y - depends on COMPAT && KEYS - config KEYS_REQUEST_CACHE bool "Enable temporary caching of the last request_key() result" depends on KEYS diff --git a/security/keys/Makefile b/security/keys/Makefile index 074f27538f55..5f40807f05b3 100644 --- a/security/keys/Makefile +++ b/security/keys/Makefile @@ -17,7 +17,7 @@ obj-y := \ request_key_auth.o \ user_defined.o compat-obj-$(CONFIG_KEY_DH_OPERATIONS) += compat_dh.o -obj-$(CONFIG_KEYS_COMPAT) += compat.o $(compat-obj-y) +obj-$(CONFIG_COMPAT) += compat.o $(compat-obj-y) obj-$(CONFIG_PROC_FS) += proc.o obj-$(CONFIG_SYSCTL) += sysctl.o obj-$(CONFIG_PERSISTENT_KEYRINGS) += persistent.o diff --git a/security/keys/compat.c b/security/keys/compat.c index 9bcc404131aa..b975f8f11124 100644 --- a/security/keys/compat.c +++ b/security/keys/compat.c @@ -46,11 +46,6 @@ static long compat_keyctl_instantiate_key_iov( /* * The key control system call, 32-bit compatibility version for 64-bit archs - * - * This should only be called if the 64-bit arch uses weird pointers in 32-bit - * mode or doesn't guarantee that the top 32-bits of the argument registers on - * taking a 32-bit syscall are zero. If you can, you should call sys_keyctl() - * directly. */ COMPAT_SYSCALL_DEFINE5(keyctl, u32, option, u32, arg2, u32, arg3, u32, arg4, u32, arg5) diff --git a/security/keys/internal.h b/security/keys/internal.h index c039373488bd..ba3e2da14cef 100644 --- a/security/keys/internal.h +++ b/security/keys/internal.h @@ -264,7 +264,7 @@ extern long keyctl_dh_compute(struct keyctl_dh_params __user *, char __user *, size_t, struct keyctl_kdf_params __user *); extern long __keyctl_dh_compute(struct keyctl_dh_params __user *, char __user *, size_t, struct keyctl_kdf_params *); -#ifdef CONFIG_KEYS_COMPAT +#ifdef CONFIG_COMPAT extern long compat_keyctl_dh_compute(struct keyctl_dh_params __user *params, char __user *buffer, size_t buflen, struct compat_keyctl_kdf_params __user *kdf); @@ -279,7 +279,7 @@ static inline long keyctl_dh_compute(struct keyctl_dh_params __user *params, return -EOPNOTSUPP; } -#ifdef CONFIG_KEYS_COMPAT +#ifdef CONFIG_COMPAT static inline long compat_keyctl_dh_compute( struct keyctl_dh_params __user *params, char __user *buffer, size_t buflen, -- cgit v1.2.3 From 6bd5ce6089b561f5392460bfb654dea89356ab1b Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Mon, 16 Dec 2019 19:16:48 +0900 Subject: tomoyo: Suppress RCU warning at list_for_each_entry_rcu(). John Garry has reported that allmodconfig kernel on arm64 causes flood of "RCU-list traversed in non-reader section!!" warning. I don't know what change caused this warning, but this warning is safe because TOMOYO uses SRCU lock instead. Let's suppress this warning by explicitly telling that the caller is holding SRCU lock. Reported-and-tested-by: John Garry Signed-off-by: Tetsuo Handa --- security/tomoyo/common.c | 9 ++++++--- security/tomoyo/domain.c | 15 ++++++++++----- security/tomoyo/group.c | 9 ++++++--- security/tomoyo/util.c | 6 ++++-- 4 files changed, 26 insertions(+), 13 deletions(-) (limited to 'security') diff --git a/security/tomoyo/common.c b/security/tomoyo/common.c index dd3d5942e669..c36bafbcd77e 100644 --- a/security/tomoyo/common.c +++ b/security/tomoyo/common.c @@ -951,7 +951,8 @@ static bool tomoyo_manager(void) exe = tomoyo_get_exe(); if (!exe) return false; - list_for_each_entry_rcu(ptr, &tomoyo_kernel_namespace.policy_list[TOMOYO_ID_MANAGER], head.list) { + list_for_each_entry_rcu(ptr, &tomoyo_kernel_namespace.policy_list[TOMOYO_ID_MANAGER], head.list, + srcu_read_lock_held(&tomoyo_ss)) { if (!ptr->head.is_deleted && (!tomoyo_pathcmp(domainname, ptr->manager) || !strcmp(exe, ptr->manager->name))) { @@ -1095,7 +1096,8 @@ static int tomoyo_delete_domain(char *domainname) if (mutex_lock_interruptible(&tomoyo_policy_lock)) return -EINTR; /* Is there an active domain? */ - list_for_each_entry_rcu(domain, &tomoyo_domain_list, list) { + list_for_each_entry_rcu(domain, &tomoyo_domain_list, list, + srcu_read_lock_held(&tomoyo_ss)) { /* Never delete tomoyo_kernel_domain */ if (domain == &tomoyo_kernel_domain) continue; @@ -2778,7 +2780,8 @@ void tomoyo_check_profile(void) tomoyo_policy_loaded = true; pr_info("TOMOYO: 2.6.0\n"); - list_for_each_entry_rcu(domain, &tomoyo_domain_list, list) { + list_for_each_entry_rcu(domain, &tomoyo_domain_list, list, + srcu_read_lock_held(&tomoyo_ss)) { const u8 profile = domain->profile; struct tomoyo_policy_namespace *ns = domain->ns; diff --git a/security/tomoyo/domain.c b/security/tomoyo/domain.c index 8526a0a74023..7869d6a9980b 100644 --- a/security/tomoyo/domain.c +++ b/security/tomoyo/domain.c @@ -41,7 +41,8 @@ int tomoyo_update_policy(struct tomoyo_acl_head *new_entry, const int size, if (mutex_lock_interruptible(&tomoyo_policy_lock)) return -ENOMEM; - list_for_each_entry_rcu(entry, list, list) { + list_for_each_entry_rcu(entry, list, list, + srcu_read_lock_held(&tomoyo_ss)) { if (entry->is_deleted == TOMOYO_GC_IN_PROGRESS) continue; if (!check_duplicate(entry, new_entry)) @@ -119,7 +120,8 @@ int tomoyo_update_domain(struct tomoyo_acl_info *new_entry, const int size, } if (mutex_lock_interruptible(&tomoyo_policy_lock)) goto out; - list_for_each_entry_rcu(entry, list, list) { + list_for_each_entry_rcu(entry, list, list, + srcu_read_lock_held(&tomoyo_ss)) { if (entry->is_deleted == TOMOYO_GC_IN_PROGRESS) continue; if (!tomoyo_same_acl_head(entry, new_entry) || @@ -166,7 +168,8 @@ void tomoyo_check_acl(struct tomoyo_request_info *r, u16 i = 0; retry: - list_for_each_entry_rcu(ptr, list, list) { + list_for_each_entry_rcu(ptr, list, list, + srcu_read_lock_held(&tomoyo_ss)) { if (ptr->is_deleted || ptr->type != r->param_type) continue; if (!check_entry(r, ptr)) @@ -298,7 +301,8 @@ static inline bool tomoyo_scan_transition { const struct tomoyo_transition_control *ptr; - list_for_each_entry_rcu(ptr, list, head.list) { + list_for_each_entry_rcu(ptr, list, head.list, + srcu_read_lock_held(&tomoyo_ss)) { if (ptr->head.is_deleted || ptr->type != type) continue; if (ptr->domainname) { @@ -735,7 +739,8 @@ retry: /* Check 'aggregator' directive. */ candidate = &exename; - list_for_each_entry_rcu(ptr, list, head.list) { + list_for_each_entry_rcu(ptr, list, head.list, + srcu_read_lock_held(&tomoyo_ss)) { if (ptr->head.is_deleted || !tomoyo_path_matches_pattern(&exename, ptr->original_name)) diff --git a/security/tomoyo/group.c b/security/tomoyo/group.c index a37c7dc66e44..1cecdd797597 100644 --- a/security/tomoyo/group.c +++ b/security/tomoyo/group.c @@ -133,7 +133,8 @@ tomoyo_path_matches_group(const struct tomoyo_path_info *pathname, { struct tomoyo_path_group *member; - list_for_each_entry_rcu(member, &group->member_list, head.list) { + list_for_each_entry_rcu(member, &group->member_list, head.list, + srcu_read_lock_held(&tomoyo_ss)) { if (member->head.is_deleted) continue; if (!tomoyo_path_matches_pattern(pathname, member->member_name)) @@ -161,7 +162,8 @@ bool tomoyo_number_matches_group(const unsigned long min, struct tomoyo_number_group *member; bool matched = false; - list_for_each_entry_rcu(member, &group->member_list, head.list) { + list_for_each_entry_rcu(member, &group->member_list, head.list, + srcu_read_lock_held(&tomoyo_ss)) { if (member->head.is_deleted) continue; if (min > member->number.values[1] || @@ -191,7 +193,8 @@ bool tomoyo_address_matches_group(const bool is_ipv6, const __be32 *address, bool matched = false; const u8 size = is_ipv6 ? 16 : 4; - list_for_each_entry_rcu(member, &group->member_list, head.list) { + list_for_each_entry_rcu(member, &group->member_list, head.list, + srcu_read_lock_held(&tomoyo_ss)) { if (member->head.is_deleted) continue; if (member->address.is_ipv6 != is_ipv6) diff --git a/security/tomoyo/util.c b/security/tomoyo/util.c index 52752e1a84ed..eba0b3395851 100644 --- a/security/tomoyo/util.c +++ b/security/tomoyo/util.c @@ -594,7 +594,8 @@ struct tomoyo_domain_info *tomoyo_find_domain(const char *domainname) name.name = domainname; tomoyo_fill_path_info(&name); - list_for_each_entry_rcu(domain, &tomoyo_domain_list, list) { + list_for_each_entry_rcu(domain, &tomoyo_domain_list, list, + srcu_read_lock_held(&tomoyo_ss)) { if (!domain->is_deleted && !tomoyo_pathcmp(&name, domain->domainname)) return domain; @@ -1028,7 +1029,8 @@ bool tomoyo_domain_quota_is_ok(struct tomoyo_request_info *r) return false; if (!domain) return true; - list_for_each_entry_rcu(ptr, &domain->acl_info_list, list) { + list_for_each_entry_rcu(ptr, &domain->acl_info_list, list, + srcu_read_lock_held(&tomoyo_ss)) { u16 perm; u8 i; -- cgit v1.2.3 From 45477b3fe3d10376b649b1b85fce72b2f9f1da84 Mon Sep 17 00:00:00 2001 From: James Bottomley Date: Thu, 12 Dec 2019 12:58:35 -0500 Subject: security: keys: trusted: fix lost handle flush The original code, before it was moved into security/keys/trusted-keys had a flush after the blob unseal. Without that flush, the volatile handles increase in the TPM until it becomes unusable and the system either has to be rebooted or the TPM volatile area manually flushed. Fix by adding back the lost flush, which we now have to export because of the relocation of the trusted key code may cause the consumer to be modular. Signed-off-by: James Bottomley Fixes: 2e19e10131a0 ("KEYS: trusted: Move TPM2 trusted keys code") Reviewed-by: Jerry Snitselaar Reviewed-by: Jarkko Sakkinen Signed-off-by: Jarkko Sakkinen --- drivers/char/tpm/tpm.h | 1 - drivers/char/tpm/tpm2-cmd.c | 1 + include/linux/tpm.h | 1 + security/keys/trusted-keys/trusted_tpm2.c | 1 + 4 files changed, 3 insertions(+), 1 deletion(-) (limited to 'security') diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h index b9e1547be6b5..5620747da0cf 100644 --- a/drivers/char/tpm/tpm.h +++ b/drivers/char/tpm/tpm.h @@ -218,7 +218,6 @@ int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx, int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, struct tpm_digest *digests); int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max); -void tpm2_flush_context(struct tpm_chip *chip, u32 handle); ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id, u32 *value, const char *desc); diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c index fdb457704aa7..13696deceae8 100644 --- a/drivers/char/tpm/tpm2-cmd.c +++ b/drivers/char/tpm/tpm2-cmd.c @@ -362,6 +362,7 @@ void tpm2_flush_context(struct tpm_chip *chip, u32 handle) tpm_transmit_cmd(chip, &buf, 0, "flushing context"); tpm_buf_destroy(&buf); } +EXPORT_SYMBOL_GPL(tpm2_flush_context); struct tpm2_get_cap_out { u8 more_data; diff --git a/include/linux/tpm.h b/include/linux/tpm.h index 0d6e949ba315..03e9b184411b 100644 --- a/include/linux/tpm.h +++ b/include/linux/tpm.h @@ -403,6 +403,7 @@ extern int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, extern int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen); extern int tpm_get_random(struct tpm_chip *chip, u8 *data, size_t max); extern struct tpm_chip *tpm_default_chip(void); +void tpm2_flush_context(struct tpm_chip *chip, u32 handle); #else static inline int tpm_is_tpm2(struct tpm_chip *chip) { diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c index a9810ac2776f..08ec7f48f01d 100644 --- a/security/keys/trusted-keys/trusted_tpm2.c +++ b/security/keys/trusted-keys/trusted_tpm2.c @@ -309,6 +309,7 @@ int tpm2_unseal_trusted(struct tpm_chip *chip, return rc; rc = tpm2_unseal_cmd(chip, payload, options, blob_handle); + tpm2_flush_context(chip, blob_handle); return rc; } -- cgit v1.2.3