From 08382c9f6efe08b8bb30645c2744480cbd8f161a Mon Sep 17 00:00:00 2001 From: jooseong lee Date: Thu, 3 Nov 2016 11:54:39 +0100 Subject: Smack: Assign smack_known_web label for kernel thread's Assign smack_known_web label for kernel thread's socket Creating struct sock by sk_alloc function in various kernel subsystems like bluetooth doesn't call smack_socket_post_create(). In such case, received sock label is the floor('_') label and makes access deny. Signed-off-by: jooseong lee Acked-by: Casey Schaufler --- security/smack/smack_lsm.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 1cb060293505..e73460d5acc6 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -2337,8 +2337,16 @@ static int smack_sk_alloc_security(struct sock *sk, int family, gfp_t gfp_flags) if (ssp == NULL) return -ENOMEM; - ssp->smk_in = skp; - ssp->smk_out = skp; + /* + * Sockets created by kernel threads receive web label. + */ + if (unlikely(current->flags & PF_KTHREAD)) { + ssp->smk_in = &smack_known_web; + ssp->smk_out = &smack_known_web; + } else { + ssp->smk_in = skp; + ssp->smk_out = skp; + } ssp->smk_packet = NULL; sk->sk_security = ssp; -- cgit v1.2.3-59-g8ed1b From 2097f59920ea81516d7783396683cad22c26d140 Mon Sep 17 00:00:00 2001 From: Himanshu Shukla Date: Thu, 10 Nov 2016 16:19:52 +0530 Subject: smack: parse mnt opts after privileges check In smack_set_mnt_opts()first the SMACK mount options are being parsed and later it is being checked whether the user calling mount has CAP_MAC_ADMIN capability. This sequence of operationis will allow unauthorized user to add SMACK labels in label list and may cause denial of security attack by adding many labels by allocating kernel memory by unauthorized user. Superblock smack flag is also being set as initialized though function may return with EPERM error. First check the capability of calling user then set the SMACK attributes and smk_flags. Signed-off-by: Himanshu Shukla Acked-by: Casey Schaufler --- security/smack/smack_lsm.c | 50 +++++++++++++++++++++++----------------------- 1 file changed, 25 insertions(+), 25 deletions(-) (limited to 'security') diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index e73460d5acc6..788a5faf3774 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -769,6 +769,31 @@ static int smack_set_mnt_opts(struct super_block *sb, if (sp->smk_flags & SMK_SB_INITIALIZED) return 0; + if (!smack_privileged(CAP_MAC_ADMIN)) { + /* + * Unprivileged mounts don't get to specify Smack values. + */ + if (num_opts) + return -EPERM; + /* + * Unprivileged mounts get root and default from the caller. + */ + skp = smk_of_current(); + sp->smk_root = skp; + sp->smk_default = skp; + /* + * For a handful of fs types with no user-controlled + * backing store it's okay to trust security labels + * in the filesystem. The rest are untrusted. + */ + if (sb->s_user_ns != &init_user_ns && + sb->s_magic != SYSFS_MAGIC && sb->s_magic != TMPFS_MAGIC && + sb->s_magic != RAMFS_MAGIC) { + transmute = 1; + sp->smk_flags |= SMK_SB_UNTRUSTED; + } + } + sp->smk_flags |= SMK_SB_INITIALIZED; for (i = 0; i < num_opts; i++) { @@ -809,31 +834,6 @@ static int smack_set_mnt_opts(struct super_block *sb, } } - if (!smack_privileged(CAP_MAC_ADMIN)) { - /* - * Unprivileged mounts don't get to specify Smack values. - */ - if (num_opts) - return -EPERM; - /* - * Unprivileged mounts get root and default from the caller. - */ - skp = smk_of_current(); - sp->smk_root = skp; - sp->smk_default = skp; - /* - * For a handful of fs types with no user-controlled - * backing store it's okay to trust security labels - * in the filesystem. The rest are untrusted. - */ - if (sb->s_user_ns != &init_user_ns && - sb->s_magic != SYSFS_MAGIC && sb->s_magic != TMPFS_MAGIC && - sb->s_magic != RAMFS_MAGIC) { - transmute = 1; - sp->smk_flags |= SMK_SB_UNTRUSTED; - } - } - /* * Initialize the root inode. */ -- cgit v1.2.3-59-g8ed1b From 7128ea159d60a91b3f0a7d10a1ea7d62b53cda93 Mon Sep 17 00:00:00 2001 From: Himanshu Shukla Date: Thu, 10 Nov 2016 16:17:49 +0530 Subject: SMACK: Do not apply star label in smack_setprocattr hook Smack prohibits processes from using the star ("*") and web ("@") labels. Checks have been added in other functions. In smack_setprocattr() hook, only check for web ("@") label has been added and restricted from applying web ("@") label. Check for star ("*") label should also be added in smack_setprocattr() hook. Return error should be "-EINVAL" not "-EPERM" as permission is there for setting label but not the label value as star ("*") or web ("@"). Signed-off-by: Himanshu Shukla Acked-by: Casey Schaufler --- security/smack/smack_lsm.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'security') diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 788a5faf3774..3a5684b47354 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -3669,10 +3669,11 @@ static int smack_setprocattr(struct task_struct *p, char *name, return PTR_ERR(skp); /* - * No process is ever allowed the web ("@") label. + * No process is ever allowed the web ("@") label + * and the star ("*") label. */ - if (skp == &smack_known_web) - return -EPERM; + if (skp == &smack_known_web || skp == &smack_known_star) + return -EINVAL; if (!smack_privileged(CAP_MAC_ADMIN)) { rc = -EPERM; -- cgit v1.2.3-59-g8ed1b From b437aba85b5c4689543409d8407c016749231aae Mon Sep 17 00:00:00 2001 From: Himanshu Shukla Date: Thu, 10 Nov 2016 16:17:02 +0530 Subject: SMACK: Fix the memory leak in smack_cred_prepare() hook Memory leak in smack_cred_prepare()function. smack_cred_prepare() hook returns error if there is error in allocating memory in smk_copy_rules() or smk_copy_relabel() function. If smack_cred_prepare() function returns error then the calling function should call smack_cred_free() function for cleanup. In smack_cred_free() function first credential is extracted and then all rules are deleted. In smack_cred_prepare() function security field is assigned in the end when all function return success. But this function may return before and memory will not be freed. Signed-off-by: Himanshu Shukla Acked-by: Casey Schaufler --- security/smack/smack_lsm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'security') diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 3a5684b47354..f7ee77c3db55 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -2023,6 +2023,8 @@ static int smack_cred_prepare(struct cred *new, const struct cred *old, if (new_tsp == NULL) return -ENOMEM; + new->security = new_tsp; + rc = smk_copy_rules(&new_tsp->smk_rules, &old_tsp->smk_rules, gfp); if (rc != 0) return rc; @@ -2032,7 +2034,6 @@ static int smack_cred_prepare(struct cred *new, const struct cred *old, if (rc != 0) return rc; - new->security = new_tsp; return 0; } -- cgit v1.2.3-59-g8ed1b From 2e4939f7026f938a1dc81533d020801198562804 Mon Sep 17 00:00:00 2001 From: Casey Schaufler Date: Mon, 7 Nov 2016 19:01:09 -0800 Subject: Smack: ipv6 label match fix The check for a deleted entry in the list of IPv6 host addresses was being performed in the wrong place, leading to most peculiar results in some cases. This puts the check into the right place. Signed-off-by: Casey Schaufler --- security/smack/smack_lsm.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'security') diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index f7ee77c3db55..b2c839f735c0 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -2443,18 +2443,18 @@ static struct smack_known *smack_ipv6host_label(struct sockaddr_in6 *sip) return NULL; list_for_each_entry_rcu(snp, &smk_net6addr_list, list) { + /* + * If the label is NULL the entry has + * been renounced. Ignore it. + */ + if (snp->smk_label == NULL) + continue; /* * we break after finding the first match because * the list is sorted from longest to shortest mask * so we have found the most specific match */ for (found = 1, i = 0; i < 8; i++) { - /* - * If the label is NULL the entry has - * been renounced. Ignore it. - */ - if (snp->smk_label == NULL) - continue; if ((sap->s6_addr16[i] & snp->smk_mask.s6_addr16[i]) != snp->smk_host.s6_addr16[i]) { found = 0; -- cgit v1.2.3-59-g8ed1b From 9a11a18902bc3b904353063763d06480620245a6 Mon Sep 17 00:00:00 2001 From: Eric Richter Date: Thu, 13 Oct 2016 17:47:36 -0500 Subject: ima: fix memory leak in ima_release_policy When the "policy" securityfs file is opened for read, it is opened as a sequential file. However, when it is eventually released, there is no cleanup for the sequential file, therefore some memory is leaked. This patch adds a call to seq_release() in ima_release_policy() to clean up the memory when the file is opened for read. Fixes: 80eae209d63a IMA: allow reading back the current policy Reported-by: Colin Ian King Signed-off-by: Eric Richter Tested-by: Colin Ian King Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_fs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index c07a3844ea0a..3df46906492d 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -401,7 +401,7 @@ static int ima_release_policy(struct inode *inode, struct file *file) const char *cause = valid_policy ? "completed" : "failed"; if ((file->f_flags & O_ACCMODE) == O_RDONLY) - return 0; + return seq_release(inode, file); if (valid_policy && ima_check_policy() < 0) { cause = "failed"; -- cgit v1.2.3-59-g8ed1b From f5acb3dcba1ffb7f0b8cbb9dba61500eea5d610b Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Wed, 2 Nov 2016 09:14:16 -0400 Subject: Revert "ima: limit file hash setting by user to fix and log modes" Userspace applications have been modified to write security xattrs, but they are not context aware. In the case of security.ima, the security xattr can be either a file hash or a file signature. Permitting writing one, but not the other requires the application to be context aware. In addition, userspace applications might write files to a staging area, which might not be in policy, and then change some file metadata (eg. owner) making it in policy. As a result, these files are not labeled properly. This reverts commit c68ed80c97d9720f51ef31fe91560fdd1e121533, which prevents writing file hashes as security.ima xattrs. Requested-by: Patrick Ohly Cc: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_appraise.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 389325ac6067..a705598ced5f 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -384,14 +384,10 @@ int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name, result = ima_protect_xattr(dentry, xattr_name, xattr_value, xattr_value_len); if (result == 1) { - bool digsig; - if (!xattr_value_len || (xvalue->type >= IMA_XATTR_LAST)) return -EINVAL; - digsig = (xvalue->type == EVM_IMA_XATTR_DIGSIG); - if (!digsig && (ima_appraise & IMA_APPRAISE_ENFORCE)) - return -EPERM; - ima_reset_appraise_flags(d_backing_inode(dentry), digsig); + ima_reset_appraise_flags(d_backing_inode(dentry), + (xvalue->type == EVM_IMA_XATTR_DIGSIG) ? 1 : 0); result = 0; } return result; -- cgit v1.2.3-59-g8ed1b From 064be15c525d02e46251fd529d84e5835b0b1339 Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Mon, 31 Oct 2016 13:22:15 -0400 Subject: ima: include the reason for TPM-bypass mode This patch includes the reason for going into TPM-bypass mode and not using the TPM. Signed-off-by: Mimi Zohar (zohar@linux.vnet.ibm> --- security/integrity/ima/ima_init.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'security') diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c index 32912bd54ead..2ac1f41db5c0 100644 --- a/security/integrity/ima/ima_init.c +++ b/security/integrity/ima/ima_init.c @@ -115,7 +115,8 @@ int __init ima_init(void) ima_used_chip = 1; if (!ima_used_chip) - pr_info("No TPM chip found, activating TPM-bypass!\n"); + pr_info("No TPM chip found, activating TPM-bypass! (rc=%d)\n", + rc); rc = integrity_init_keyring(INTEGRITY_KEYRING_IMA); if (rc) -- cgit v1.2.3-59-g8ed1b From b4bfec7f4a86424b114f94f41c4e1841ec102df3 Mon Sep 17 00:00:00 2001 From: Seth Forshee Date: Mon, 1 Aug 2016 08:19:10 -0500 Subject: security/integrity: Harden against malformed xattrs In general the handling of IMA/EVM xattrs is good, but I found a few locations where either the xattr size or the value of the type field in the xattr are not checked. Add a few simple checks to these locations to prevent malformed or malicious xattrs from causing problems. Signed-off-by: Seth Forshee Signed-off-by: Mimi Zohar --- security/integrity/digsig.c | 2 +- security/integrity/evm/evm_main.c | 4 ++++ security/integrity/ima/ima_appraise.c | 5 ++++- 3 files changed, 9 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c index 4304372b323f..106e855e2d9d 100644 --- a/security/integrity/digsig.c +++ b/security/integrity/digsig.c @@ -51,7 +51,7 @@ static bool init_keyring __initdata; int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen, const char *digest, int digestlen) { - if (id >= INTEGRITY_KEYRING_MAX) + if (id >= INTEGRITY_KEYRING_MAX || siglen < 2) return -EINVAL; if (!keyring[id]) { diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index ba8615576d4d..e2ed498c0f5f 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -145,6 +145,10 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry, /* check value type */ switch (xattr_data->type) { case EVM_XATTR_HMAC: + if (xattr_len != sizeof(struct evm_ima_xattr_data)) { + evm_status = INTEGRITY_FAIL; + goto out; + } rc = evm_calc_hmac(dentry, xattr_name, xattr_value, xattr_value_len, calc.digest); if (rc) diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index a705598ced5f..1fd9539a969d 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -130,6 +130,7 @@ enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, int xattr_len) { struct signature_v2_hdr *sig; + enum hash_algo ret; if (!xattr_value || xattr_len < 2) /* return default hash algo */ @@ -143,7 +144,9 @@ enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, return sig->hash_algo; break; case IMA_XATTR_DIGEST_NG: - return xattr_value->digest[0]; + ret = xattr_value->digest[0]; + if (ret < HASH_ALGO__LAST) + return ret; break; case IMA_XATTR_DIGEST: /* this is for backward compatibility */ -- cgit v1.2.3-59-g8ed1b From 8931c3bdb3bfbdfb3c86f590f517ed9acfed27d7 Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Mon, 14 Nov 2016 20:16:12 +0900 Subject: SELinux: Use GFP_KERNEL for selinux_parse_opts_str(). Since selinux_parse_opts_str() is calling match_strdup() which uses GFP_KERNEL, it is safe to use GFP_KERNEL from kcalloc() which is called by selinux_parse_opts_str(). Signed-off-by: Tetsuo Handa Signed-off-by: Paul Moore --- security/selinux/hooks.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 09fd6108e421..d1b141e33ecb 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1100,11 +1100,12 @@ static int selinux_parse_opts_str(char *options, } rc = -ENOMEM; - opts->mnt_opts = kcalloc(NUM_SEL_MNT_OPTS, sizeof(char *), GFP_ATOMIC); + opts->mnt_opts = kcalloc(NUM_SEL_MNT_OPTS, sizeof(char *), GFP_KERNEL); if (!opts->mnt_opts) goto out_err; - opts->mnt_opts_flags = kcalloc(NUM_SEL_MNT_OPTS, sizeof(int), GFP_ATOMIC); + opts->mnt_opts_flags = kcalloc(NUM_SEL_MNT_OPTS, sizeof(int), + GFP_KERNEL); if (!opts->mnt_opts_flags) { kfree(opts->mnt_opts); goto out_err; -- cgit v1.2.3-59-g8ed1b From 420591128cb206201dc444c2d42fb6f299b2ecd0 Mon Sep 17 00:00:00 2001 From: Andreas Gruenbacher Date: Thu, 10 Nov 2016 22:18:27 +0100 Subject: selinux: Minor cleanups Fix the comment for function __inode_security_revalidate, which returns an integer. Use the LABEL_* constants consistently for isec->initialized. Signed-off-by: Andreas Gruenbacher Signed-off-by: Paul Moore --- security/selinux/hooks.c | 3 ++- security/selinux/selinuxfs.c | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index d1b141e33ecb..1c0721708ccc 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -237,6 +237,7 @@ static int inode_alloc_security(struct inode *inode) isec->sid = SECINITSID_UNLABELED; isec->sclass = SECCLASS_FILE; isec->task_sid = sid; + isec->initialized = LABEL_INVALID; inode->i_security = isec; return 0; @@ -247,7 +248,7 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent /* * Try reloading inode security labels that have been marked as invalid. The * @may_sleep parameter indicates when sleeping and thus reloading labels is - * allowed; when set to false, returns ERR_PTR(-ECHILD) when the label is + * allowed; when set to false, returns -ECHILD when the label is * invalid. The @opt_dentry parameter should be set to a dentry of the inode; * when no dentry is available, set it to NULL instead. */ diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index 72c145dd799f..50fca204d3f1 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -1301,7 +1301,7 @@ static int sel_make_bools(void) goto out; isec->sid = sid; - isec->initialized = 1; + isec->initialized = LABEL_INITIALIZED; inode->i_fop = &sel_bool_ops; inode->i_ino = i|SEL_BOOL_INO_OFFSET; d_add(dentry, inode); @@ -1834,7 +1834,7 @@ static int sel_fill_super(struct super_block *sb, void *data, int silent) isec = (struct inode_security_struct *)inode->i_security; isec->sid = SECINITSID_DEVNULL; isec->sclass = SECCLASS_CHR_FILE; - isec->initialized = 1; + isec->initialized = LABEL_INITIALIZED; init_special_inode(inode, S_IFCHR | S_IRUGO | S_IWUGO, MKDEV(MEM_MAJOR, 3)); d_add(dentry, inode); -- cgit v1.2.3-59-g8ed1b From db978da8fa1d0819b210c137d31a339149b88875 Mon Sep 17 00:00:00 2001 From: Andreas Gruenbacher Date: Thu, 10 Nov 2016 22:18:28 +0100 Subject: proc: Pass file mode to proc_pid_make_inode Pass the file mode of the proc inode to be created to proc_pid_make_inode. In proc_pid_make_inode, initialize inode->i_mode before calling security_task_to_inode. This allows selinux to set isec->sclass right away without introducing "half-initialized" inode security structs. Signed-off-by: Andreas Gruenbacher Signed-off-by: Paul Moore --- fs/proc/base.c | 23 +++++++++-------------- fs/proc/fd.c | 6 ++---- fs/proc/internal.h | 2 +- fs/proc/namespaces.c | 3 +-- security/selinux/hooks.c | 1 + 5 files changed, 14 insertions(+), 21 deletions(-) (limited to 'security') diff --git a/fs/proc/base.c b/fs/proc/base.c index 8e654468ab67..9de05e5854ef 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -1663,7 +1663,8 @@ const struct inode_operations proc_pid_link_inode_operations = { /* building an inode */ -struct inode *proc_pid_make_inode(struct super_block * sb, struct task_struct *task) +struct inode *proc_pid_make_inode(struct super_block * sb, + struct task_struct *task, umode_t mode) { struct inode * inode; struct proc_inode *ei; @@ -1677,6 +1678,7 @@ struct inode *proc_pid_make_inode(struct super_block * sb, struct task_struct *t /* Common stuff */ ei = PROC_I(inode); + inode->i_mode = mode; inode->i_ino = get_next_ino(); inode->i_mtime = inode->i_atime = inode->i_ctime = current_time(inode); inode->i_op = &proc_def_inode_operations; @@ -2003,7 +2005,9 @@ proc_map_files_instantiate(struct inode *dir, struct dentry *dentry, struct proc_inode *ei; struct inode *inode; - inode = proc_pid_make_inode(dir->i_sb, task); + inode = proc_pid_make_inode(dir->i_sb, task, S_IFLNK | + ((mode & FMODE_READ ) ? S_IRUSR : 0) | + ((mode & FMODE_WRITE) ? S_IWUSR : 0)); if (!inode) return -ENOENT; @@ -2012,12 +2016,6 @@ proc_map_files_instantiate(struct inode *dir, struct dentry *dentry, inode->i_op = &proc_map_files_link_inode_operations; inode->i_size = 64; - inode->i_mode = S_IFLNK; - - if (mode & FMODE_READ) - inode->i_mode |= S_IRUSR; - if (mode & FMODE_WRITE) - inode->i_mode |= S_IWUSR; d_set_d_op(dentry, &tid_map_files_dentry_operations); d_add(dentry, inode); @@ -2371,12 +2369,11 @@ static int proc_pident_instantiate(struct inode *dir, struct inode *inode; struct proc_inode *ei; - inode = proc_pid_make_inode(dir->i_sb, task); + inode = proc_pid_make_inode(dir->i_sb, task, p->mode); if (!inode) goto out; ei = PROC_I(inode); - inode->i_mode = p->mode; if (S_ISDIR(inode->i_mode)) set_nlink(inode, 2); /* Use getattr to fix if necessary */ if (p->iop) @@ -3058,11 +3055,10 @@ static int proc_pid_instantiate(struct inode *dir, { struct inode *inode; - inode = proc_pid_make_inode(dir->i_sb, task); + inode = proc_pid_make_inode(dir->i_sb, task, S_IFDIR | S_IRUGO | S_IXUGO); if (!inode) goto out; - inode->i_mode = S_IFDIR|S_IRUGO|S_IXUGO; inode->i_op = &proc_tgid_base_inode_operations; inode->i_fop = &proc_tgid_base_operations; inode->i_flags|=S_IMMUTABLE; @@ -3351,11 +3347,10 @@ static int proc_task_instantiate(struct inode *dir, struct dentry *dentry, struct task_struct *task, const void *ptr) { struct inode *inode; - inode = proc_pid_make_inode(dir->i_sb, task); + inode = proc_pid_make_inode(dir->i_sb, task, S_IFDIR | S_IRUGO | S_IXUGO); if (!inode) goto out; - inode->i_mode = S_IFDIR|S_IRUGO|S_IXUGO; inode->i_op = &proc_tid_base_inode_operations; inode->i_fop = &proc_tid_base_operations; inode->i_flags|=S_IMMUTABLE; diff --git a/fs/proc/fd.c b/fs/proc/fd.c index d21dafef3102..4274f83bf100 100644 --- a/fs/proc/fd.c +++ b/fs/proc/fd.c @@ -183,14 +183,13 @@ proc_fd_instantiate(struct inode *dir, struct dentry *dentry, struct proc_inode *ei; struct inode *inode; - inode = proc_pid_make_inode(dir->i_sb, task); + inode = proc_pid_make_inode(dir->i_sb, task, S_IFLNK); if (!inode) goto out; ei = PROC_I(inode); ei->fd = fd; - inode->i_mode = S_IFLNK; inode->i_op = &proc_pid_link_inode_operations; inode->i_size = 64; @@ -322,14 +321,13 @@ proc_fdinfo_instantiate(struct inode *dir, struct dentry *dentry, struct proc_inode *ei; struct inode *inode; - inode = proc_pid_make_inode(dir->i_sb, task); + inode = proc_pid_make_inode(dir->i_sb, task, S_IFREG | S_IRUSR); if (!inode) goto out; ei = PROC_I(inode); ei->fd = fd; - inode->i_mode = S_IFREG | S_IRUSR; inode->i_fop = &proc_fdinfo_file_operations; d_set_d_op(dentry, &tid_fd_dentry_operations); diff --git a/fs/proc/internal.h b/fs/proc/internal.h index 5378441ec1b7..f4494dcbdc8b 100644 --- a/fs/proc/internal.h +++ b/fs/proc/internal.h @@ -162,7 +162,7 @@ extern int proc_pid_statm(struct seq_file *, struct pid_namespace *, extern const struct dentry_operations pid_dentry_operations; extern int pid_getattr(struct vfsmount *, struct dentry *, struct kstat *); extern int proc_setattr(struct dentry *, struct iattr *); -extern struct inode *proc_pid_make_inode(struct super_block *, struct task_struct *); +extern struct inode *proc_pid_make_inode(struct super_block *, struct task_struct *, umode_t); extern int pid_revalidate(struct dentry *, unsigned int); extern int pid_delete_dentry(const struct dentry *); extern int proc_pid_readdir(struct file *, struct dir_context *); diff --git a/fs/proc/namespaces.c b/fs/proc/namespaces.c index 51b8b0a8ad91..766f0c637ad1 100644 --- a/fs/proc/namespaces.c +++ b/fs/proc/namespaces.c @@ -92,12 +92,11 @@ static int proc_ns_instantiate(struct inode *dir, struct inode *inode; struct proc_inode *ei; - inode = proc_pid_make_inode(dir->i_sb, task); + inode = proc_pid_make_inode(dir->i_sb, task, S_IFLNK | S_IRWXUGO); if (!inode) goto out; ei = PROC_I(inode); - inode->i_mode = S_IFLNK|S_IRWXUGO; inode->i_op = &proc_ns_link_inode_operations; ei->ns_ops = ns_ops; diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 1c0721708ccc..32beac817bf5 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3955,6 +3955,7 @@ static void selinux_task_to_inode(struct task_struct *p, struct inode_security_struct *isec = inode->i_security; u32 sid = task_sid(p); + isec->sclass = inode_mode_to_security_class(inode->i_mode); isec->sid = sid; isec->initialized = LABEL_INITIALIZED; } -- cgit v1.2.3-59-g8ed1b From 13457d073c29da92001f6ee809075eaa8757fb96 Mon Sep 17 00:00:00 2001 From: Andreas Gruenbacher Date: Thu, 10 Nov 2016 22:18:29 +0100 Subject: selinux: Clean up initialization of isec->sclass Now that isec->initialized == LABEL_INITIALIZED implies that isec->sclass is valid, skip such inodes immediately in inode_doinit_with_dentry. For the remaining inodes, initialize isec->sclass at the beginning of inode_doinit_with_dentry to simplify the code. Signed-off-by: Andreas Gruenbacher Signed-off-by: Paul Moore --- security/selinux/hooks.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 32beac817bf5..2a506861a772 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1390,12 +1390,15 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent int rc = 0; if (isec->initialized == LABEL_INITIALIZED) - goto out; + return 0; mutex_lock(&isec->lock); if (isec->initialized == LABEL_INITIALIZED) goto out_unlock; + if (isec->sclass == SECCLASS_FILE) + isec->sclass = inode_mode_to_security_class(inode->i_mode); + sbsec = inode->i_sb->s_security; if (!(sbsec->flags & SE_SBINITIALIZED)) { /* Defer initialization until selinux_complete_init, @@ -1513,7 +1516,6 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent isec->sid = sbsec->sid; /* Try to obtain a transition SID. */ - isec->sclass = inode_mode_to_security_class(inode->i_mode); rc = security_transition_sid(isec->task_sid, sbsec->sid, isec->sclass, NULL, &sid); if (rc) @@ -1549,7 +1551,6 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent */ if (!dentry) goto out_unlock; - isec->sclass = inode_mode_to_security_class(inode->i_mode); rc = selinux_genfs_get_sid(dentry, isec->sclass, sbsec->flags, &sid); dput(dentry); @@ -1564,9 +1565,6 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent out_unlock: mutex_unlock(&isec->lock); -out: - if (isec->sclass == SECCLASS_FILE) - isec->sclass = inode_mode_to_security_class(inode->i_mode); return rc; } -- cgit v1.2.3-59-g8ed1b From 8c15d66e429afd099b66f05393527c23f85ca41c Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Mon, 14 Nov 2016 20:12:55 +0900 Subject: Smack: Use GFP_KERNEL for smack_parse_opts_str(). Since smack_parse_opts_str() is calling match_strdup() which uses GFP_KERNEL, it is safe to use GFP_KERNEL from kcalloc() which is called by smack_parse_opts_str(). Signed-off-by: Tetsuo Handa Acked-by: Casey Schaufler --- security/smack/smack_lsm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index b2c839f735c0..46d8be434466 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -692,12 +692,12 @@ static int smack_parse_opts_str(char *options, } } - opts->mnt_opts = kcalloc(NUM_SMK_MNT_OPTS, sizeof(char *), GFP_ATOMIC); + opts->mnt_opts = kcalloc(NUM_SMK_MNT_OPTS, sizeof(char *), GFP_KERNEL); if (!opts->mnt_opts) goto out_err; opts->mnt_opts_flags = kcalloc(NUM_SMK_MNT_OPTS, sizeof(int), - GFP_ATOMIC); + GFP_KERNEL); if (!opts->mnt_opts_flags) { kfree(opts->mnt_opts); goto out_err; -- cgit v1.2.3-59-g8ed1b From 152f91d4d11a30106b9cc0b27b47e0e80b633ee8 Mon Sep 17 00:00:00 2001 From: Casey Schaufler Date: Mon, 14 Nov 2016 09:38:15 -0800 Subject: Smack: Remove unnecessary smack_known_invalid The invalid Smack label ("") and the Huh ("?") Smack label serve the same purpose and having both is unnecessary. While pulling out the invalid label it became clear that the use of smack_from_secid() was inconsistent, so that is repaired. The setting of inode labels to the invalid label could never happen in a functional system, has never been observed in the wild and is not what you'd really want for a failure behavior in any case. That is removed. Signed-off-by: Casey Schaufler --- security/smack/smack.h | 1 - security/smack/smack_access.c | 7 +------ security/smack/smack_lsm.c | 29 +++-------------------------- security/smack/smackfs.c | 3 --- 4 files changed, 4 insertions(+), 36 deletions(-) (limited to 'security') diff --git a/security/smack/smack.h b/security/smack/smack.h index 51fd30192c08..77abe2efacae 100644 --- a/security/smack/smack.h +++ b/security/smack/smack.h @@ -336,7 +336,6 @@ extern int smack_ptrace_rule; extern struct smack_known smack_known_floor; extern struct smack_known smack_known_hat; extern struct smack_known smack_known_huh; -extern struct smack_known smack_known_invalid; extern struct smack_known smack_known_star; extern struct smack_known smack_known_web; diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c index 23e5808a0970..356e3764cad9 100644 --- a/security/smack/smack_access.c +++ b/security/smack/smack_access.c @@ -36,11 +36,6 @@ struct smack_known smack_known_floor = { .smk_secid = 5, }; -struct smack_known smack_known_invalid = { - .smk_known = "", - .smk_secid = 6, -}; - struct smack_known smack_known_web = { .smk_known = "@", .smk_secid = 7, @@ -615,7 +610,7 @@ struct smack_known *smack_from_secid(const u32 secid) * of a secid that is not on the list. */ rcu_read_unlock(); - return &smack_known_invalid; + return &smack_known_huh; } /* diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 46d8be434466..4d90257d03ad 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -1384,20 +1384,14 @@ static void smack_inode_post_setxattr(struct dentry *dentry, const char *name, skp = smk_import_entry(value, size); if (!IS_ERR(skp)) isp->smk_inode = skp; - else - isp->smk_inode = &smack_known_invalid; } else if (strcmp(name, XATTR_NAME_SMACKEXEC) == 0) { skp = smk_import_entry(value, size); if (!IS_ERR(skp)) isp->smk_task = skp; - else - isp->smk_task = &smack_known_invalid; } else if (strcmp(name, XATTR_NAME_SMACKMMAP) == 0) { skp = smk_import_entry(value, size); if (!IS_ERR(skp)) isp->smk_mmap = skp; - else - isp->smk_mmap = &smack_known_invalid; } return; @@ -2068,12 +2062,8 @@ static void smack_cred_transfer(struct cred *new, const struct cred *old) static int smack_kernel_act_as(struct cred *new, u32 secid) { struct task_smack *new_tsp = new->security; - struct smack_known *skp = smack_from_secid(secid); - - if (skp == NULL) - return -EINVAL; - new_tsp->smk_task = skp; + new_tsp->smk_task = smack_from_secid(secid); return 0; } @@ -3894,21 +3884,11 @@ static struct smack_known *smack_from_secattr(struct netlbl_lsm_secattr *sap, return &smack_known_web; return &smack_known_star; } - if ((sap->flags & NETLBL_SECATTR_SECID) != 0) { + if ((sap->flags & NETLBL_SECATTR_SECID) != 0) /* * Looks like a fallback, which gives us a secid. */ - skp = smack_from_secid(sap->attr.secid); - /* - * This has got to be a bug because it is - * impossible to specify a fallback without - * specifying the label, which will ensure - * it has a secid, and the only way to get a - * secid is from a fallback. - */ - BUG_ON(skp == NULL); - return skp; - } + return smack_from_secid(sap->attr.secid); /* * Without guidance regarding the smack value * for the packet fall back on the network @@ -4771,7 +4751,6 @@ static __init void init_smack_known_list(void) mutex_init(&smack_known_hat.smk_rules_lock); mutex_init(&smack_known_floor.smk_rules_lock); mutex_init(&smack_known_star.smk_rules_lock); - mutex_init(&smack_known_invalid.smk_rules_lock); mutex_init(&smack_known_web.smk_rules_lock); /* * Initialize rule lists @@ -4780,7 +4759,6 @@ static __init void init_smack_known_list(void) INIT_LIST_HEAD(&smack_known_hat.smk_rules); INIT_LIST_HEAD(&smack_known_star.smk_rules); INIT_LIST_HEAD(&smack_known_floor.smk_rules); - INIT_LIST_HEAD(&smack_known_invalid.smk_rules); INIT_LIST_HEAD(&smack_known_web.smk_rules); /* * Create the known labels list @@ -4789,7 +4767,6 @@ static __init void init_smack_known_list(void) smk_insert_entry(&smack_known_hat); smk_insert_entry(&smack_known_star); smk_insert_entry(&smack_known_floor); - smk_insert_entry(&smack_known_invalid); smk_insert_entry(&smack_known_web); } diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c index 6492fe96cae4..13743a01b35b 100644 --- a/security/smack/smackfs.c +++ b/security/smack/smackfs.c @@ -2996,9 +2996,6 @@ static int __init init_smk_fs(void) if (err == 0 && rc < 0) err = rc; rc = smk_preset_netlabel(&smack_known_huh); - if (err == 0 && rc < 0) - err = rc; - rc = smk_preset_netlabel(&smack_known_invalid); if (err == 0 && rc < 0) err = rc; rc = smk_preset_netlabel(&smack_known_star); -- cgit v1.2.3-59-g8ed1b From ea49d10eee5a220b717dbf2ee429c9e3d59c978c Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Fri, 18 Nov 2016 09:30:38 -0500 Subject: selinux: normalize input to /sys/fs/selinux/enforce At present, one can write any signed integer value to /sys/fs/selinux/enforce and it will be stored, e.g. echo -1 > /sys/fs/selinux/enforce or echo 2 > /sys/fs/selinux/enforce. This makes no real difference to the kernel, since it only ever cares if it is zero or non-zero, but some userspace code compares it with 1 to decide if SELinux is enforcing, and this could confuse it. Only a process that is already root and is allowed the setenforce permission in SELinux policy can write to /sys/fs/selinux/enforce, so this is not considered to be a security issue, but it should be fixed. Signed-off-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/selinuxfs.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'security') diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index 50fca204d3f1..cf9293e01fc1 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -163,6 +163,8 @@ static ssize_t sel_write_enforce(struct file *file, const char __user *buf, if (sscanf(page, "%d", &new_value) != 1) goto out; + new_value = !!new_value; + if (new_value != selinux_enforcing) { length = task_has_security(current, SECURITY__SETENFORCE); if (length) -- cgit v1.2.3-59-g8ed1b From 3322d0d64f4e942862a152f6f11137a1f5eac2e9 Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Fri, 18 Nov 2016 10:23:09 -0500 Subject: selinux: keep SELinux in sync with new capability definitions When a new capability is defined, SELinux needs to be updated. Trigger a build error if a new capability is defined without corresponding update to security/selinux/include/classmap.h's COMMON_CAP2_PERMS. This is similar to BUILD_BUG_ON() guards in the SELinux nlmsgtab code to ensure that SELinux tracks new netlink message types as needed. Note that there is already a similar build guard in security/selinux/hooks.c to detect when more than 64 capabilities are defined, since that will require adding a third capability class to SELinux. A nicer way to do this would be to extend scripts/selinux/genheaders or a similar tool to auto-generate the necessary definitions and code for SELinux capability checking from include/uapi/linux/capability.h. AppArmor does something similar in its Makefile, although it only needs to generate a single table of names. That is left as future work. Signed-off-by: Stephen Smalley [PM: reformat the description to keep checkpatch.pl happy] Signed-off-by: Paul Moore --- security/selinux/include/classmap.h | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'security') diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h index 1f1f4b2f6018..e2d4ad3a4b4c 100644 --- a/security/selinux/include/classmap.h +++ b/security/selinux/include/classmap.h @@ -24,6 +24,10 @@ #define COMMON_CAP2_PERMS "mac_override", "mac_admin", "syslog", \ "wake_alarm", "block_suspend", "audit_read" +#if CAP_LAST_CAP > CAP_AUDIT_READ +#error New capability defined, please update COMMON_CAP2_PERMS. +#endif + /* * Note: The name for any socket class should be suffixed by "socket", * and doesn't contain more than one substr of "socket". -- cgit v1.2.3-59-g8ed1b From 9287aed2ad1ff1bde5eb190bcd6dccd5f1cf47d3 Mon Sep 17 00:00:00 2001 From: Andreas Gruenbacher Date: Tue, 15 Nov 2016 11:06:40 +0100 Subject: selinux: Convert isec->lock into a spinlock Convert isec->lock from a mutex into a spinlock. Instead of holding the lock while sleeping in inode_doinit_with_dentry, set isec->initialized to LABEL_PENDING and release the lock. Then, when the sid has been determined, re-acquire the lock. If isec->initialized is still set to LABEL_PENDING, set isec->sid; otherwise, the sid has been set by another task (LABEL_INITIALIZED) or invalidated (LABEL_INVALID) in the meantime. This fixes a deadlock on gfs2 where * one task is in inode_doinit_with_dentry -> gfs2_getxattr, holds isec->lock, and tries to acquire the inode's glock, and * another task is in do_xmote -> inode_go_inval -> selinux_inode_invalidate_secctx, holds the inode's glock, and tries to acquire isec->lock. Signed-off-by: Andreas Gruenbacher [PM: minor tweaks to keep checkpatch.pl happy] Signed-off-by: Paul Moore --- security/selinux/hooks.c | 101 ++++++++++++++++++++++++-------------- security/selinux/include/objsec.h | 5 +- 2 files changed, 66 insertions(+), 40 deletions(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 2a506861a772..98a2e92b3168 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -231,7 +231,7 @@ static int inode_alloc_security(struct inode *inode) if (!isec) return -ENOMEM; - mutex_init(&isec->lock); + spin_lock_init(&isec->lock); INIT_LIST_HEAD(&isec->list); isec->inode = inode; isec->sid = SECINITSID_UNLABELED; @@ -1382,7 +1382,8 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent { struct superblock_security_struct *sbsec = NULL; struct inode_security_struct *isec = inode->i_security; - u32 sid; + u32 task_sid, sid = 0; + u16 sclass; struct dentry *dentry; #define INITCONTEXTLEN 255 char *context = NULL; @@ -1392,7 +1393,7 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent if (isec->initialized == LABEL_INITIALIZED) return 0; - mutex_lock(&isec->lock); + spin_lock(&isec->lock); if (isec->initialized == LABEL_INITIALIZED) goto out_unlock; @@ -1411,12 +1412,18 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent goto out_unlock; } + sclass = isec->sclass; + task_sid = isec->task_sid; + sid = isec->sid; + isec->initialized = LABEL_PENDING; + spin_unlock(&isec->lock); + switch (sbsec->behavior) { case SECURITY_FS_USE_NATIVE: break; case SECURITY_FS_USE_XATTR: if (!(inode->i_opflags & IOP_XATTR)) { - isec->sid = sbsec->def_sid; + sid = sbsec->def_sid; break; } /* Need a dentry, since the xattr API requires one. @@ -1438,7 +1445,7 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent * inode_doinit with a dentry, before these inodes could * be used again by userspace. */ - goto out_unlock; + goto out; } len = INITCONTEXTLEN; @@ -1446,7 +1453,7 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent if (!context) { rc = -ENOMEM; dput(dentry); - goto out_unlock; + goto out; } context[len] = '\0'; rc = __vfs_getxattr(dentry, inode, XATTR_NAME_SELINUX, context, len); @@ -1457,14 +1464,14 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent rc = __vfs_getxattr(dentry, inode, XATTR_NAME_SELINUX, NULL, 0); if (rc < 0) { dput(dentry); - goto out_unlock; + goto out; } len = rc; context = kmalloc(len+1, GFP_NOFS); if (!context) { rc = -ENOMEM; dput(dentry); - goto out_unlock; + goto out; } context[len] = '\0'; rc = __vfs_getxattr(dentry, inode, XATTR_NAME_SELINUX, context, len); @@ -1476,7 +1483,7 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent "%d for dev=%s ino=%ld\n", __func__, -rc, inode->i_sb->s_id, inode->i_ino); kfree(context); - goto out_unlock; + goto out; } /* Map ENODATA to the default file SID */ sid = sbsec->def_sid; @@ -1506,28 +1513,25 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent } } kfree(context); - isec->sid = sid; break; case SECURITY_FS_USE_TASK: - isec->sid = isec->task_sid; + sid = task_sid; break; case SECURITY_FS_USE_TRANS: /* Default to the fs SID. */ - isec->sid = sbsec->sid; + sid = sbsec->sid; /* Try to obtain a transition SID. */ - rc = security_transition_sid(isec->task_sid, sbsec->sid, - isec->sclass, NULL, &sid); + rc = security_transition_sid(task_sid, sid, sclass, NULL, &sid); if (rc) - goto out_unlock; - isec->sid = sid; + goto out; break; case SECURITY_FS_USE_MNTPOINT: - isec->sid = sbsec->mntpoint_sid; + sid = sbsec->mntpoint_sid; break; default: /* Default to the fs superblock SID. */ - isec->sid = sbsec->sid; + sid = sbsec->sid; if ((sbsec->flags & SE_SBGENFS) && !S_ISLNK(inode->i_mode)) { /* We must have a dentry to determine the label on @@ -1550,21 +1554,30 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent * could be used again by userspace. */ if (!dentry) - goto out_unlock; - rc = selinux_genfs_get_sid(dentry, isec->sclass, + goto out; + rc = selinux_genfs_get_sid(dentry, sclass, sbsec->flags, &sid); dput(dentry); if (rc) - goto out_unlock; - isec->sid = sid; + goto out; } break; } - isec->initialized = LABEL_INITIALIZED; +out: + spin_lock(&isec->lock); + if (isec->initialized == LABEL_PENDING) { + if (!sid || rc) { + isec->initialized = LABEL_INVALID; + goto out_unlock; + } + + isec->initialized = LABEL_INITIALIZED; + isec->sid = sid; + } out_unlock: - mutex_unlock(&isec->lock); + spin_unlock(&isec->lock); return rc; } @@ -3195,9 +3208,11 @@ static void selinux_inode_post_setxattr(struct dentry *dentry, const char *name, } isec = backing_inode_security(dentry); + spin_lock(&isec->lock); isec->sclass = inode_mode_to_security_class(inode->i_mode); isec->sid = newsid; isec->initialized = LABEL_INITIALIZED; + spin_unlock(&isec->lock); return; } @@ -3290,9 +3305,11 @@ static int selinux_inode_setsecurity(struct inode *inode, const char *name, if (rc) return rc; + spin_lock(&isec->lock); isec->sclass = inode_mode_to_security_class(inode->i_mode); isec->sid = newsid; isec->initialized = LABEL_INITIALIZED; + spin_unlock(&isec->lock); return 0; } @@ -3953,9 +3970,11 @@ static void selinux_task_to_inode(struct task_struct *p, struct inode_security_struct *isec = inode->i_security; u32 sid = task_sid(p); + spin_lock(&isec->lock); isec->sclass = inode_mode_to_security_class(inode->i_mode); isec->sid = sid; isec->initialized = LABEL_INITIALIZED; + spin_unlock(&isec->lock); } /* Returns error only if unable to parse addresses */ @@ -4274,24 +4293,24 @@ static int selinux_socket_post_create(struct socket *sock, int family, const struct task_security_struct *tsec = current_security(); struct inode_security_struct *isec = inode_security_novalidate(SOCK_INODE(sock)); struct sk_security_struct *sksec; + u16 sclass = socket_type_to_security_class(family, type, protocol); + u32 sid = SECINITSID_KERNEL; int err = 0; - isec->sclass = socket_type_to_security_class(family, type, protocol); - - if (kern) - isec->sid = SECINITSID_KERNEL; - else { - err = socket_sockcreate_sid(tsec, isec->sclass, &(isec->sid)); + if (!kern) { + err = socket_sockcreate_sid(tsec, sclass, &sid); if (err) return err; } + isec->sclass = sclass; + isec->sid = sid; isec->initialized = LABEL_INITIALIZED; if (sock->sk) { sksec = sock->sk->sk_security; - sksec->sid = isec->sid; - sksec->sclass = isec->sclass; + sksec->sclass = sclass; + sksec->sid = sid; err = selinux_netlbl_socket_post_create(sock->sk, family); } @@ -4467,16 +4486,22 @@ static int selinux_socket_accept(struct socket *sock, struct socket *newsock) int err; struct inode_security_struct *isec; struct inode_security_struct *newisec; + u16 sclass; + u32 sid; err = sock_has_perm(current, sock->sk, SOCKET__ACCEPT); if (err) return err; - newisec = inode_security_novalidate(SOCK_INODE(newsock)); - isec = inode_security_novalidate(SOCK_INODE(sock)); - newisec->sclass = isec->sclass; - newisec->sid = isec->sid; + spin_lock(&isec->lock); + sclass = isec->sclass; + sid = isec->sid; + spin_unlock(&isec->lock); + + newisec = inode_security_novalidate(SOCK_INODE(newsock)); + newisec->sclass = sclass; + newisec->sid = sid; newisec->initialized = LABEL_INITIALIZED; return 0; @@ -5979,9 +6004,9 @@ static void selinux_inode_invalidate_secctx(struct inode *inode) { struct inode_security_struct *isec = inode->i_security; - mutex_lock(&isec->lock); + spin_lock(&isec->lock); isec->initialized = LABEL_INVALID; - mutex_unlock(&isec->lock); + spin_unlock(&isec->lock); } /* diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h index c21e135460a5..e8dab0f02c72 100644 --- a/security/selinux/include/objsec.h +++ b/security/selinux/include/objsec.h @@ -39,7 +39,8 @@ struct task_security_struct { enum label_initialized { LABEL_INVALID, /* invalid or not initialized */ - LABEL_INITIALIZED /* initialized */ + LABEL_INITIALIZED, /* initialized */ + LABEL_PENDING }; struct inode_security_struct { @@ -52,7 +53,7 @@ struct inode_security_struct { u32 sid; /* SID of this object */ u16 sclass; /* security class of this object */ unsigned char initialized; /* initialization flag */ - struct mutex lock; + spinlock_t lock; }; struct file_security_struct { -- cgit v1.2.3-59-g8ed1b From 50523a29d900d5a403e0352d3d7aeda6a33df25c Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Fri, 2 Dec 2016 15:49:43 -0800 Subject: Yama: allow access for the current ptrace parent Under ptrace_scope=1, it's possible to have a tracee that is already ptrace-attached, but is no longer a direct descendant. For instance, a forking daemon will be re-parented to init, losing its ancestry to the tracer that launched it. The tracer can continue using ptrace in that state, but it will be denied other accesses that check PTRACE_MODE_ATTACH, like process_vm_rw and various procfs files. There's no reason to prevent such access for a tracer that already has ptrace control anyway. This patch adds a case to ptracer_exception_found to allow access for any task in the same thread group as the current ptrace parent. Signed-off-by: Josh Stone Cc: Kees Cook Cc: James Morris Cc: "Serge E. Hallyn" Cc: linux-security-module@vger.kernel.org Signed-off-by: Kees Cook Signed-off-by: James Morris --- security/yama/yama_lsm.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) (limited to 'security') diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c index 0309f2111c70..968e5e0a3f81 100644 --- a/security/yama/yama_lsm.c +++ b/security/yama/yama_lsm.c @@ -309,7 +309,7 @@ static int task_is_descendant(struct task_struct *parent, * @tracer: the task_struct of the process attempting ptrace * @tracee: the task_struct of the process to be ptraced * - * Returns 1 if tracer has is ptracer exception ancestor for tracee. + * Returns 1 if tracer has a ptracer exception ancestor for tracee. */ static int ptracer_exception_found(struct task_struct *tracer, struct task_struct *tracee) @@ -320,6 +320,18 @@ static int ptracer_exception_found(struct task_struct *tracer, bool found = false; rcu_read_lock(); + + /* + * If there's already an active tracing relationship, then make an + * exception for the sake of other accesses, like process_vm_rw(). + */ + parent = ptrace_parent(tracee); + if (parent != NULL && same_thread_group(parent, tracer)) { + rc = 1; + goto unlock; + } + + /* Look for a PR_SET_PTRACER relationship. */ if (!thread_group_leader(tracee)) tracee = rcu_dereference(tracee->group_leader); list_for_each_entry_rcu(relation, &ptracer_relations, node) { @@ -334,6 +346,8 @@ static int ptracer_exception_found(struct task_struct *tracer, if (found && (parent == NULL || task_is_descendant(parent, tracer))) rc = 1; + +unlock: rcu_read_unlock(); return rc; -- cgit v1.2.3-59-g8ed1b