From 0909c0ae999c325b9d34c6f4710f40730ae3bc24 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Fri, 28 Feb 2014 07:23:24 -0500 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 4b34847208cc..a3230de656e4 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3204,24 +3204,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.2.3 From eee3094683fbc7fe6bcdaef58c1ef31f8460cdca Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Wed, 5 Mar 2014 15:54:57 -0500 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 a3230de656e4..8b1656f053f8 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1418,15 +1418,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.2.3 From 626b9740fa73cad043e136bfb3b6fca68a4f8a7c Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Tue, 29 Apr 2014 11:29:04 -0700 Subject: selinux: Report permissive mode in avc: denied messages. We cannot presently tell from an avc: denied message whether access was in fact denied or was allowed due to global or per-domain permissive mode. Add a permissive= field to the avc message to reflect this information. Signed-off-by: Stephen Smalley Acked-by: Eric Paris Signed-off-by: Paul Moore --- security/selinux/avc.c | 7 ++++++- security/selinux/hooks.c | 5 +++-- security/selinux/include/avc.h | 4 ++-- 3 files changed, 11 insertions(+), 5 deletions(-) (limited to 'security') diff --git a/security/selinux/avc.c b/security/selinux/avc.c index fc3e6628a864..a18f1fa6440b 100644 --- a/security/selinux/avc.c +++ b/security/selinux/avc.c @@ -444,11 +444,15 @@ static void avc_audit_post_callback(struct audit_buffer *ab, void *a) avc_dump_query(ab, ad->selinux_audit_data->ssid, ad->selinux_audit_data->tsid, ad->selinux_audit_data->tclass); + if (ad->selinux_audit_data->denied) { + audit_log_format(ab, " permissive=%u", + ad->selinux_audit_data->result ? 0 : 1); + } } /* This is the slow part of avc audit with big stack footprint */ noinline int slow_avc_audit(u32 ssid, u32 tsid, u16 tclass, - u32 requested, u32 audited, u32 denied, + u32 requested, u32 audited, u32 denied, int result, struct common_audit_data *a, unsigned flags) { @@ -477,6 +481,7 @@ noinline int slow_avc_audit(u32 ssid, u32 tsid, u16 tclass, sad.tsid = tsid; sad.audited = audited; sad.denied = denied; + sad.result = result; a->selinux_audit_data = &sad; diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index d58946dca8c9..889cf4c3c3fa 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2770,6 +2770,7 @@ static int selinux_inode_follow_link(struct dentry *dentry, struct nameidata *na static noinline int audit_inode_permission(struct inode *inode, u32 perms, u32 audited, u32 denied, + int result, unsigned flags) { struct common_audit_data ad; @@ -2780,7 +2781,7 @@ static noinline int audit_inode_permission(struct inode *inode, ad.u.inode = inode; rc = slow_avc_audit(current_sid(), isec->sid, isec->sclass, perms, - audited, denied, &ad, flags); + audited, denied, result, &ad, flags); if (rc) return rc; return 0; @@ -2822,7 +2823,7 @@ static int selinux_inode_permission(struct inode *inode, int mask) if (likely(!audited)) return rc; - rc2 = audit_inode_permission(inode, perms, audited, denied, flags); + rc2 = audit_inode_permission(inode, perms, audited, denied, rc, flags); if (rc2) return rc2; return rc; diff --git a/security/selinux/include/avc.h b/security/selinux/include/avc.h index f53ee3c58d0f..ddf8eec03f21 100644 --- a/security/selinux/include/avc.h +++ b/security/selinux/include/avc.h @@ -102,7 +102,7 @@ static inline u32 avc_audit_required(u32 requested, } int slow_avc_audit(u32 ssid, u32 tsid, u16 tclass, - u32 requested, u32 audited, u32 denied, + u32 requested, u32 audited, u32 denied, int result, struct common_audit_data *a, unsigned flags); @@ -137,7 +137,7 @@ static inline int avc_audit(u32 ssid, u32 tsid, if (likely(!audited)) return 0; return slow_avc_audit(ssid, tsid, tclass, - requested, audited, denied, + requested, audited, denied, result, a, 0); } -- cgit v1.2.3 From 4f189988a0a5890db597ec48fc0e8b09922f290a Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Thu, 15 May 2014 11:16:06 -0400 Subject: selinux: reject setexeccon() on MNT_NOSUID applications with -EACCES We presently prevent processes from using setexecon() to set the security label of exec()'d processes when NO_NEW_PRIVS is enabled by returning an error; however, we silently ignore setexeccon() when exec()'ing from a nosuid mounted filesystem. This patch makes things a bit more consistent by returning an error in the setexeccon()/nosuid case. Signed-off-by: Paul Moore Acked-by: Andy Lutomirski Acked-by: Stephen Smalley --- security/selinux/hooks.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 889cf4c3c3fa..b03b0776955a 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2123,11 +2123,13 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm) new_tsec->exec_sid = 0; /* - * Minimize confusion: if no_new_privs and a transition is - * explicitly requested, then fail the exec. + * Minimize confusion: if no_new_privs or nosuid and a + * transition is explicitly requested, then fail the exec. */ if (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) return -EPERM; + if (bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID) + return -EACCES; } else { /* Check for a default transition on this program. */ rc = security_transition_sid(old_tsec->sid, isec->sid, -- cgit v1.2.3 From 612c353178c45250fbec271e7e8e75596d5cbbea Mon Sep 17 00:00:00 2001 From: Dave Jones Date: Thu, 15 May 2014 15:02:53 -0400 Subject: selinux: conditionally reschedule in mls_convert_context while loading selinux policy On a slow machine (with debugging enabled), upgrading selinux policy may take a considerable amount of time. Long enough that the softlockup detector gets triggered. The backtrace looks like this.. > BUG: soft lockup - CPU#2 stuck for 23s! [load_policy:19045] > Call Trace: > [] symcmp+0xf/0x20 > [] hashtab_search+0x47/0x80 > [] mls_convert_context+0xdc/0x1c0 > [] convert_context+0x378/0x460 > [] ? security_context_to_sid_core+0x240/0x240 > [] sidtab_map+0x45/0x80 > [] security_load_policy+0x3ff/0x580 > [] ? sched_clock_cpu+0xa8/0x100 > [] ? sched_clock_local+0x1d/0x80 > [] ? sched_clock_cpu+0xa8/0x100 > [] ? __change_page_attr_set_clr+0x82a/0xa50 > [] ? sched_clock_local+0x1d/0x80 > [] ? sched_clock_cpu+0xa8/0x100 > [] ? __change_page_attr_set_clr+0x82a/0xa50 > [] ? sched_clock_cpu+0xa8/0x100 > [] ? retint_restore_args+0xe/0xe > [] ? trace_hardirqs_on_caller+0xfd/0x1c0 > [] ? trace_hardirqs_on_thunk+0x3a/0x3f > [] ? rcu_irq_exit+0x68/0xb0 > [] ? retint_restore_args+0xe/0xe > [] sel_write_load+0xa7/0x770 > [] ? vfs_write+0x1c3/0x200 > [] ? security_file_permission+0x1e/0xa0 > [] vfs_write+0xbb/0x200 > [] ? fget_light+0x397/0x4b0 > [] SyS_write+0x47/0xa0 > [] tracesys+0xdd/0xe2 Stephen Smalley suggested: > Maybe put a cond_resched() within the ebitmap_for_each_positive_bit() > loop in mls_convert_context()? That seems to do the trick. Tested by downgrading and re-upgrading selinux-policy-targeted. Signed-off-by: Dave Jones Acked-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/ss/mls.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'security') diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c index c85bc1ec040c..d307b37ddc2b 100644 --- a/security/selinux/ss/mls.c +++ b/security/selinux/ss/mls.c @@ -492,6 +492,8 @@ int mls_convert_context(struct policydb *oldp, rc = ebitmap_set_bit(&bitmap, catdatum->value - 1, 1); if (rc) return rc; + + cond_resched(); } ebitmap_destroy(&c->range.level[l].cat); c->range.level[l].cat = bitmap; -- cgit v1.2.3 From 47dd0b76ace953bd2c0479076db0d3e3b9594003 Mon Sep 17 00:00:00 2001 From: Dave Jones Date: Thu, 15 May 2014 15:03:53 -0400 Subject: selinux: conditionally reschedule in hashtab_insert while loading selinux policy After silencing the sleeping warning in mls_convert_context() I started seeing similar traces from hashtab_insert. Do a cond_resched there too. Signed-off-by: Dave Jones Acked-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/ss/hashtab.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'security') diff --git a/security/selinux/ss/hashtab.c b/security/selinux/ss/hashtab.c index 933e735bb185..2cc496149842 100644 --- a/security/selinux/ss/hashtab.c +++ b/security/selinux/ss/hashtab.c @@ -6,6 +6,7 @@ #include #include #include +#include #include "hashtab.h" struct hashtab *hashtab_create(u32 (*hash_value)(struct hashtab *h, const void *key), @@ -40,6 +41,8 @@ int hashtab_insert(struct hashtab *h, void *key, void *datum) u32 hvalue; struct hashtab_node *prev, *cur, *newnode; + cond_resched(); + if (!h || h->nel == HASHTAB_MAX_NODES) return -EINVAL; -- cgit v1.2.3 From 5c7001b84be55917a99c35ce96df7e67b3c40cea Mon Sep 17 00:00:00 2001 From: Himangi Saraogi Date: Tue, 17 Jun 2014 01:41:05 +0530 Subject: SELinux: use ARRAY_SIZE ARRAY_SIZE is more concise to use when the size of an array is divided by the size of its type or the size of its first element. The Coccinelle semantic patch that makes this change is as follows: // @@ type T; T[] E; @@ - (sizeof(E)/sizeof(E[...])) + ARRAY_SIZE(E) // Signed-off-by: Himangi Saraogi Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'security') diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 9c5cdc2caaef..56eb65f67cb1 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -2608,7 +2608,7 @@ static int mls_write_range_helper(struct mls_range *r, void *fp) if (!eq) buf[2] = cpu_to_le32(r->level[1].sens); - BUG_ON(items > (sizeof(buf)/sizeof(buf[0]))); + BUG_ON(items > ARRAY_SIZE(buf)); rc = put_entry(buf, sizeof(u32), items, fp); if (rc) @@ -2990,7 +2990,7 @@ static int role_write(void *vkey, void *datum, void *ptr) if (p->policyvers >= POLICYDB_VERSION_BOUNDARY) buf[items++] = cpu_to_le32(role->bounds); - BUG_ON(items > (sizeof(buf)/sizeof(buf[0]))); + BUG_ON(items > ARRAY_SIZE(buf)); rc = put_entry(buf, sizeof(u32), items, fp); if (rc) @@ -3040,7 +3040,7 @@ static int type_write(void *vkey, void *datum, void *ptr) } else { buf[items++] = cpu_to_le32(typdatum->primary); } - BUG_ON(items > (sizeof(buf) / sizeof(buf[0]))); + BUG_ON(items > ARRAY_SIZE(buf)); rc = put_entry(buf, sizeof(u32), items, fp); if (rc) return rc; @@ -3069,7 +3069,7 @@ static int user_write(void *vkey, void *datum, void *ptr) buf[items++] = cpu_to_le32(usrdatum->value); if (p->policyvers >= POLICYDB_VERSION_BOUNDARY) buf[items++] = cpu_to_le32(usrdatum->bounds); - BUG_ON(items > (sizeof(buf) / sizeof(buf[0]))); + BUG_ON(items > ARRAY_SIZE(buf)); rc = put_entry(buf, sizeof(u32), items, fp); if (rc) return rc; -- cgit v1.2.3 From 4b6f405f72112175a5e044b015c4119b3a5b6f52 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Sun, 15 Jun 2014 23:02:51 +0900 Subject: selinux: introduce str_read() helper There're some code duplication for reading a string value during policydb_read(). Add str_read() helper to fix it. Signed-off-by: Namhyung Kim Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 133 ++++++++++++----------------------------- 1 file changed, 37 insertions(+), 96 deletions(-) (limited to 'security') diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 56eb65f67cb1..bc2a586f095c 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -1080,6 +1080,26 @@ out: * binary representation file. */ +static int str_read(char **strp, gfp_t flags, void *fp, u32 len) +{ + int rc; + char *str; + + str = kmalloc(len + 1, flags); + if (!str) + return -ENOMEM; + + /* it's expected the caller should free the str */ + *strp = str; + + rc = next_entry(str, fp, len); + if (rc) + return rc; + + str[len] = '\0'; + return 0; +} + static int perm_read(struct policydb *p, struct hashtab *h, void *fp) { char *key = NULL; @@ -1100,15 +1120,9 @@ static int perm_read(struct policydb *p, struct hashtab *h, void *fp) len = le32_to_cpu(buf[0]); perdatum->value = le32_to_cpu(buf[1]); - rc = -ENOMEM; - key = kmalloc(len + 1, GFP_KERNEL); - if (!key) - goto bad; - - rc = next_entry(key, fp, len); + rc = str_read(&key, GFP_KERNEL, fp, len); if (rc) goto bad; - key[len] = '\0'; rc = hashtab_insert(h, key, perdatum); if (rc) @@ -1146,15 +1160,9 @@ static int common_read(struct policydb *p, struct hashtab *h, void *fp) comdatum->permissions.nprim = le32_to_cpu(buf[2]); nel = le32_to_cpu(buf[3]); - rc = -ENOMEM; - key = kmalloc(len + 1, GFP_KERNEL); - if (!key) - goto bad; - - rc = next_entry(key, fp, len); + rc = str_read(&key, GFP_KERNEL, fp, len); if (rc) goto bad; - key[len] = '\0'; for (i = 0; i < nel; i++) { rc = perm_read(p, comdatum->permissions.table, fp); @@ -1321,25 +1329,14 @@ static int class_read(struct policydb *p, struct hashtab *h, void *fp) ncons = le32_to_cpu(buf[5]); - rc = -ENOMEM; - key = kmalloc(len + 1, GFP_KERNEL); - if (!key) - goto bad; - - rc = next_entry(key, fp, len); + rc = str_read(&key, GFP_KERNEL, fp, len); if (rc) goto bad; - key[len] = '\0'; if (len2) { - rc = -ENOMEM; - cladatum->comkey = kmalloc(len2 + 1, GFP_KERNEL); - if (!cladatum->comkey) - goto bad; - rc = next_entry(cladatum->comkey, fp, len2); + rc = str_read(&cladatum->comkey, GFP_KERNEL, fp, len2); if (rc) goto bad; - cladatum->comkey[len2] = '\0'; rc = -EINVAL; cladatum->comdatum = hashtab_search(p->p_commons.table, cladatum->comkey); @@ -1422,15 +1419,9 @@ static int role_read(struct policydb *p, struct hashtab *h, void *fp) if (p->policyvers >= POLICYDB_VERSION_BOUNDARY) role->bounds = le32_to_cpu(buf[2]); - rc = -ENOMEM; - key = kmalloc(len + 1, GFP_KERNEL); - if (!key) - goto bad; - - rc = next_entry(key, fp, len); + rc = str_read(&key, GFP_KERNEL, fp, len); if (rc) goto bad; - key[len] = '\0'; rc = ebitmap_read(&role->dominates, fp); if (rc) @@ -1495,14 +1486,9 @@ static int type_read(struct policydb *p, struct hashtab *h, void *fp) typdatum->primary = le32_to_cpu(buf[2]); } - rc = -ENOMEM; - key = kmalloc(len + 1, GFP_KERNEL); - if (!key) - goto bad; - rc = next_entry(key, fp, len); + rc = str_read(&key, GFP_KERNEL, fp, len); if (rc) goto bad; - key[len] = '\0'; rc = hashtab_insert(h, key, typdatum); if (rc) @@ -1565,14 +1551,9 @@ static int user_read(struct policydb *p, struct hashtab *h, void *fp) if (p->policyvers >= POLICYDB_VERSION_BOUNDARY) usrdatum->bounds = le32_to_cpu(buf[2]); - rc = -ENOMEM; - key = kmalloc(len + 1, GFP_KERNEL); - if (!key) - goto bad; - rc = next_entry(key, fp, len); + rc = str_read(&key, GFP_KERNEL, fp, len); if (rc) goto bad; - key[len] = '\0'; rc = ebitmap_read(&usrdatum->roles, fp); if (rc) @@ -1616,14 +1597,9 @@ static int sens_read(struct policydb *p, struct hashtab *h, void *fp) len = le32_to_cpu(buf[0]); levdatum->isalias = le32_to_cpu(buf[1]); - rc = -ENOMEM; - key = kmalloc(len + 1, GFP_ATOMIC); - if (!key) - goto bad; - rc = next_entry(key, fp, len); + rc = str_read(&key, GFP_ATOMIC, fp, len); if (rc) goto bad; - key[len] = '\0'; rc = -ENOMEM; levdatum->level = kmalloc(sizeof(struct mls_level), GFP_ATOMIC); @@ -1664,14 +1640,9 @@ static int cat_read(struct policydb *p, struct hashtab *h, void *fp) catdatum->value = le32_to_cpu(buf[1]); catdatum->isalias = le32_to_cpu(buf[2]); - rc = -ENOMEM; - key = kmalloc(len + 1, GFP_ATOMIC); - if (!key) - goto bad; - rc = next_entry(key, fp, len); + rc = str_read(&key, GFP_ATOMIC, fp, len); if (rc) goto bad; - key[len] = '\0'; rc = hashtab_insert(h, key, catdatum); if (rc) @@ -1968,18 +1939,12 @@ static int filename_trans_read(struct policydb *p, void *fp) goto out; len = le32_to_cpu(buf[0]); - rc = -ENOMEM; - name = kmalloc(len + 1, GFP_KERNEL); - if (!name) - goto out; - - ft->name = name; - /* path component string */ - rc = next_entry(name, fp, len); + rc = str_read(&name, GFP_KERNEL, fp, len); if (rc) goto out; - name[len] = 0; + + ft->name = name; rc = next_entry(buf, fp, sizeof(u32) * 4); if (rc) @@ -2045,17 +2010,10 @@ static int genfs_read(struct policydb *p, void *fp) if (!newgenfs) goto out; - rc = -ENOMEM; - newgenfs->fstype = kmalloc(len + 1, GFP_KERNEL); - if (!newgenfs->fstype) - goto out; - - rc = next_entry(newgenfs->fstype, fp, len); + rc = str_read(&newgenfs->fstype, GFP_KERNEL, fp, len); if (rc) goto out; - newgenfs->fstype[len] = 0; - for (genfs_p = NULL, genfs = p->genfs; genfs; genfs_p = genfs, genfs = genfs->next) { rc = -EINVAL; @@ -2091,15 +2049,9 @@ static int genfs_read(struct policydb *p, void *fp) if (!newc) goto out; - rc = -ENOMEM; - newc->u.name = kmalloc(len + 1, GFP_KERNEL); - if (!newc->u.name) - goto out; - - rc = next_entry(newc->u.name, fp, len); + rc = str_read(&newc->u.name, GFP_KERNEL, fp, len); if (rc) goto out; - newc->u.name[len] = 0; rc = next_entry(buf, fp, sizeof(u32)); if (rc) @@ -2189,16 +2141,10 @@ static int ocontext_read(struct policydb *p, struct policydb_compat_info *info, goto out; len = le32_to_cpu(buf[0]); - rc = -ENOMEM; - c->u.name = kmalloc(len + 1, GFP_KERNEL); - if (!c->u.name) - goto out; - - rc = next_entry(c->u.name, fp, len); + rc = str_read(&c->u.name, GFP_KERNEL, fp, len); if (rc) goto out; - c->u.name[len] = 0; rc = context_read_and_validate(&c->context[0], p, fp); if (rc) goto out; @@ -2240,16 +2186,11 @@ static int ocontext_read(struct policydb *p, struct policydb_compat_info *info, if (c->v.behavior > SECURITY_FS_USE_MAX) goto out; - rc = -ENOMEM; len = le32_to_cpu(buf[1]); - c->u.name = kmalloc(len + 1, GFP_KERNEL); - if (!c->u.name) - goto out; - - rc = next_entry(c->u.name, fp, len); + rc = str_read(&c->u.name, GFP_KERNEL, fp, len); if (rc) goto out; - c->u.name[len] = 0; + rc = context_read_and_validate(&c->context[0], p, fp); if (rc) goto out; -- cgit v1.2.3 From 4bb9398300a3a2c691e5c0ad6b9cfa78775e767e Mon Sep 17 00:00:00 2001 From: Gideon Israel Dsouza Date: Wed, 11 Jun 2014 21:25:30 +0530 Subject: security: Used macros from compiler.h instead of __attribute__((...)) To increase compiler portability there is which provides convenience macros for various gcc constructs. Eg: __packed for __attribute__((packed)). This patch is part of a large task I've taken to clean the gcc specific attributes and use the the macros instead. Signed-off-by: Gideon Israel Dsouza Signed-off-by: Paul Moore --- security/selinux/include/security.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'security') diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index ce7852cf526b..d1e0b239b602 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -8,6 +8,7 @@ #ifndef _SELINUX_SECURITY_H_ #define _SELINUX_SECURITY_H_ +#include #include #include #include @@ -220,7 +221,7 @@ struct selinux_kernel_status { /* * The version > 0 supports above members. */ -} __attribute__((packed)); +} __packed; extern void selinux_status_update_setenforce(int enforcing); extern void selinux_status_update_policyload(int seqno); -- cgit v1.2.3 From f004afe60db5b98f2b981978fde8a0d4c6298c5d Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Sun, 15 Jun 2014 01:19:01 +0900 Subject: selinux: simple cleanup for cond_read_node() The node->cur_state and len can be read in a single call of next_entry(). And setting len before reading is a dead write so can be eliminated. Signed-off-by: Namhyung Kim (Minor tweak to the length parameter in the call to next_entry()) Signed-off-by: Paul Moore --- security/selinux/ss/conditional.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) (limited to 'security') diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c index 377d148e7157..f09cc7268b65 100644 --- a/security/selinux/ss/conditional.c +++ b/security/selinux/ss/conditional.c @@ -402,19 +402,14 @@ static int cond_read_node(struct policydb *p, struct cond_node *node, void *fp) int rc; struct cond_expr *expr = NULL, *last = NULL; - rc = next_entry(buf, fp, sizeof(u32)); + rc = next_entry(buf, fp, sizeof(u32) * 2); if (rc) return rc; node->cur_state = le32_to_cpu(buf[0]); - len = 0; - rc = next_entry(buf, fp, sizeof(u32)); - if (rc) - return rc; - /* expr */ - len = le32_to_cpu(buf[0]); + len = le32_to_cpu(buf[1]); for (i = 0; i < len; i++) { rc = next_entry(buf, fp, sizeof(u32) * 2); -- cgit v1.2.3 From 6e51f9cbfa04a92b40e7f9c1e76c8ecbff534a22 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Sun, 15 Jun 2014 01:19:02 +0900 Subject: selinux: fix a possible memory leak in cond_read_node() The cond_read_node() should free the given node on error path as it's not linked to p->cond_list yet. This is done via cond_node_destroy() but it's not called when next_entry() fails before the expr loop. Signed-off-by: Namhyung Kim Signed-off-by: Paul Moore --- security/selinux/ss/conditional.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c index f09cc7268b65..62c6773be0b7 100644 --- a/security/selinux/ss/conditional.c +++ b/security/selinux/ss/conditional.c @@ -404,7 +404,7 @@ static int cond_read_node(struct policydb *p, struct cond_node *node, void *fp) rc = next_entry(buf, fp, sizeof(u32) * 2); if (rc) - return rc; + goto err; node->cur_state = le32_to_cpu(buf[0]); -- cgit v1.2.3 From f31e799459659ae88c341aeac16a8a5efb1271d4 Mon Sep 17 00:00:00 2001 From: Waiman Long Date: Mon, 23 Jun 2014 11:28:51 -0400 Subject: selinux: no recursive read_lock of policy_rwlock in security_genfs_sid() With the introduction of fair queued rwlock, recursive read_lock() may hang the offending process if there is a write_lock() somewhere in between. With recursive read_lock checking enabled, the following error was reported: ============================================= [ INFO: possible recursive locking detected ] 3.16.0-rc1 #2 Tainted: G E --------------------------------------------- load_policy/708 is trying to acquire lock: (policy_rwlock){.+.+..}, at: [] security_genfs_sid+0x3a/0x170 but task is already holding lock: (policy_rwlock){.+.+..}, at: [] security_fs_use+0x2c/0x110 other info that might help us debug this: Possible unsafe locking scenario: CPU0 ---- lock(policy_rwlock); lock(policy_rwlock); This patch fixes the occurrence of recursive read_lock() of policy_rwlock by adding a helper function __security_genfs_sid() which requires caller to take the lock before calling it. The security_fs_use() was then modified to call the new helper function. Signed-off-by: Waiman Long Acked-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/ss/services.c | 41 ++++++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 9 deletions(-) (limited to 'security') diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 4bca49414a40..2aa9d172dc7e 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -2277,7 +2277,7 @@ out: } /** - * security_genfs_sid - Obtain a SID for a file in a filesystem + * __security_genfs_sid - Helper to obtain a SID for a file in a filesystem * @fstype: filesystem type * @path: path from root of mount * @sclass: file security class @@ -2286,11 +2286,13 @@ out: * Obtain a SID to use for a file in a filesystem that * cannot support xattr or use a fixed labeling behavior like * transition SIDs or task SIDs. + * + * The caller must acquire the policy_rwlock before calling this function. */ -int security_genfs_sid(const char *fstype, - char *path, - u16 orig_sclass, - u32 *sid) +static inline int __security_genfs_sid(const char *fstype, + char *path, + u16 orig_sclass, + u32 *sid) { int len; u16 sclass; @@ -2301,8 +2303,6 @@ int security_genfs_sid(const char *fstype, while (path[0] == '/' && path[1] == '/') path++; - read_lock(&policy_rwlock); - sclass = unmap_class(orig_sclass); *sid = SECINITSID_UNLABELED; @@ -2336,10 +2336,32 @@ int security_genfs_sid(const char *fstype, *sid = c->sid[0]; rc = 0; out: - read_unlock(&policy_rwlock); return rc; } +/** + * security_genfs_sid - Obtain a SID for a file in a filesystem + * @fstype: filesystem type + * @path: path from root of mount + * @sclass: file security class + * @sid: SID for path + * + * Acquire policy_rwlock before calling __security_genfs_sid() and release + * it afterward. + */ +int security_genfs_sid(const char *fstype, + char *path, + u16 orig_sclass, + u32 *sid) +{ + int retval; + + read_lock(&policy_rwlock); + retval = __security_genfs_sid(fstype, path, orig_sclass, sid); + read_unlock(&policy_rwlock); + return retval; +} + /** * security_fs_use - Determine how to handle labeling for a filesystem. * @sb: superblock in question @@ -2370,7 +2392,8 @@ int security_fs_use(struct super_block *sb) } sbsec->sid = c->sid[0]; } else { - rc = security_genfs_sid(fstype, "/", SECCLASS_DIR, &sbsec->sid); + rc = __security_genfs_sid(fstype, "/", SECCLASS_DIR, + &sbsec->sid); if (rc) { sbsec->behavior = SECURITY_FS_USE_NONE; rc = 0; -- cgit v1.2.3 From 615e51fdda6f274e94b1e905fcaf6111e0d9aa20 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Thu, 26 Jun 2014 14:33:56 -0400 Subject: selinux: reduce the number of calls to synchronize_net() when flushing caches When flushing the AVC, such as during a policy load, the various network caches are also flushed, with each making a call to synchronize_net() which has shown to be expensive in some cases. This patch consolidates the network cache flushes into a single AVC callback which only calls synchronize_net() once for each AVC cache flush. Reported-by: Jaejyn Shin Signed-off-by: Paul Moore --- security/selinux/hooks.c | 14 ++++++++++++++ security/selinux/include/netif.h | 2 ++ security/selinux/include/netnode.h | 2 ++ security/selinux/include/netport.h | 2 ++ security/selinux/netif.c | 15 +-------------- security/selinux/netnode.c | 15 +-------------- security/selinux/netport.c | 15 +-------------- 7 files changed, 23 insertions(+), 42 deletions(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 336f0a04450e..39bc8c94b969 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -161,6 +161,17 @@ static int selinux_peerlbl_enabled(void) return (selinux_policycap_alwaysnetwork || netlbl_enabled() || selinux_xfrm_enabled()); } +static int selinux_netcache_avc_callback(u32 event) +{ + if (event == AVC_CALLBACK_RESET) { + sel_netif_flush(); + sel_netnode_flush(); + sel_netport_flush(); + synchronize_net(); + } + return 0; +} + /* * initialise the security for the init task */ @@ -5993,6 +6004,9 @@ static __init int selinux_init(void) if (register_security(&selinux_ops)) panic("SELinux: Unable to register with kernel.\n"); + if (avc_add_callback(selinux_netcache_avc_callback, AVC_CALLBACK_RESET)) + panic("SELinux: Unable to register AVC netcache callback\n"); + if (selinux_enforcing) printk(KERN_DEBUG "SELinux: Starting in enforcing mode\n"); else diff --git a/security/selinux/include/netif.h b/security/selinux/include/netif.h index 43d507242b42..57c6eae81eac 100644 --- a/security/selinux/include/netif.h +++ b/security/selinux/include/netif.h @@ -17,6 +17,8 @@ #ifndef _SELINUX_NETIF_H_ #define _SELINUX_NETIF_H_ +void sel_netif_flush(void); + int sel_netif_sid(int ifindex, u32 *sid); #endif /* _SELINUX_NETIF_H_ */ diff --git a/security/selinux/include/netnode.h b/security/selinux/include/netnode.h index df7a5ed6c694..937668dd3024 100644 --- a/security/selinux/include/netnode.h +++ b/security/selinux/include/netnode.h @@ -27,6 +27,8 @@ #ifndef _SELINUX_NETNODE_H #define _SELINUX_NETNODE_H +void sel_netnode_flush(void); + int sel_netnode_sid(void *addr, u16 family, u32 *sid); #endif diff --git a/security/selinux/include/netport.h b/security/selinux/include/netport.h index 4d965b83d735..d1ce896b2cb0 100644 --- a/security/selinux/include/netport.h +++ b/security/selinux/include/netport.h @@ -26,6 +26,8 @@ #ifndef _SELINUX_NETPORT_H #define _SELINUX_NETPORT_H +void sel_netport_flush(void); + int sel_netport_sid(u8 protocol, u16 pnum, u32 *sid); #endif diff --git a/security/selinux/netif.c b/security/selinux/netif.c index 694e9e43855f..3c3de4ca0ebc 100644 --- a/security/selinux/netif.c +++ b/security/selinux/netif.c @@ -240,7 +240,7 @@ static void sel_netif_kill(int ifindex) * Remove all entries from the network interface table. * */ -static void sel_netif_flush(void) +void sel_netif_flush(void) { int idx; struct sel_netif *netif; @@ -252,15 +252,6 @@ static void sel_netif_flush(void) spin_unlock_bh(&sel_netif_lock); } -static int sel_netif_avc_callback(u32 event) -{ - if (event == AVC_CALLBACK_RESET) { - sel_netif_flush(); - synchronize_net(); - } - return 0; -} - static int sel_netif_netdev_notifier_handler(struct notifier_block *this, unsigned long event, void *ptr) { @@ -291,10 +282,6 @@ static __init int sel_netif_init(void) register_netdevice_notifier(&sel_netif_netdev_notifier); - err = avc_add_callback(sel_netif_avc_callback, AVC_CALLBACK_RESET); - if (err) - panic("avc_add_callback() failed, error %d\n", err); - return err; } diff --git a/security/selinux/netnode.c b/security/selinux/netnode.c index 03a72c32afd7..ddf315260839 100644 --- a/security/selinux/netnode.c +++ b/security/selinux/netnode.c @@ -283,7 +283,7 @@ int sel_netnode_sid(void *addr, u16 family, u32 *sid) * Remove all entries from the network address table. * */ -static void sel_netnode_flush(void) +void sel_netnode_flush(void) { unsigned int idx; struct sel_netnode *node, *node_tmp; @@ -300,15 +300,6 @@ static void sel_netnode_flush(void) spin_unlock_bh(&sel_netnode_lock); } -static int sel_netnode_avc_callback(u32 event) -{ - if (event == AVC_CALLBACK_RESET) { - sel_netnode_flush(); - synchronize_net(); - } - return 0; -} - static __init int sel_netnode_init(void) { int iter; @@ -322,10 +313,6 @@ static __init int sel_netnode_init(void) sel_netnode_hash[iter].size = 0; } - ret = avc_add_callback(sel_netnode_avc_callback, AVC_CALLBACK_RESET); - if (ret != 0) - panic("avc_add_callback() failed, error %d\n", ret); - return ret; } diff --git a/security/selinux/netport.c b/security/selinux/netport.c index d35379781c2c..73ac6784d091 100644 --- a/security/selinux/netport.c +++ b/security/selinux/netport.c @@ -217,7 +217,7 @@ int sel_netport_sid(u8 protocol, u16 pnum, u32 *sid) * Remove all entries from the network address table. * */ -static void sel_netport_flush(void) +void sel_netport_flush(void) { unsigned int idx; struct sel_netport *port, *port_tmp; @@ -234,15 +234,6 @@ static void sel_netport_flush(void) spin_unlock_bh(&sel_netport_lock); } -static int sel_netport_avc_callback(u32 event) -{ - if (event == AVC_CALLBACK_RESET) { - sel_netport_flush(); - synchronize_net(); - } - return 0; -} - static __init int sel_netport_init(void) { int iter; @@ -256,10 +247,6 @@ static __init int sel_netport_init(void) sel_netport_hash[iter].size = 0; } - ret = avc_add_callback(sel_netport_avc_callback, AVC_CALLBACK_RESET); - if (ret != 0) - panic("avc_add_callback() failed, error %d\n", ret); - return ret; } -- cgit v1.2.3 From 4da6daf4d3df5a977e4623963f141a627fd2efce Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Thu, 10 Jul 2014 10:17:48 -0400 Subject: selinux: fix the default socket labeling in sock_graft() The sock_graft() hook has special handling for AF_INET, AF_INET, and AF_UNIX sockets as those address families have special hooks which label the sock before it is attached its associated socket. Unfortunately, the sock_graft() hook was missing a default approach to labeling sockets which meant that any other address family which made use of connections or the accept() syscall would find the returned socket to be in an "unlabeled" state. This was recently demonstrated by the kcrypto/AF_ALG subsystem and the newly released cryptsetup package (cryptsetup v1.6.5 and later). This patch preserves the special handling in selinux_sock_graft(), but adds a default behavior - setting the sock's label equal to the associated socket - which resolves the problem with AF_ALG and presumably any other address family which makes use of accept(). Cc: stable@vger.kernel.org Signed-off-by: Paul Moore Tested-by: Milan Broz --- security/selinux/hooks.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 336f0a04450e..b3a6754e932b 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -4499,9 +4499,18 @@ static void selinux_sock_graft(struct sock *sk, struct socket *parent) struct inode_security_struct *isec = SOCK_INODE(parent)->i_security; struct sk_security_struct *sksec = sk->sk_security; - if (sk->sk_family == PF_INET || sk->sk_family == PF_INET6 || - sk->sk_family == PF_UNIX) + switch (sk->sk_family) { + case PF_INET: + case PF_INET6: + case PF_UNIX: isec->sid = sksec->sid; + break; + default: + /* by default there is no special labeling mechanism for the + * sksec label so inherit the label from the parent socket */ + BUG_ON(sksec->sid != SECINITSID_UNLABELED); + sksec->sid = isec->sid; + } sksec->sclass = isec->sclass; } -- cgit v1.2.3 From 2c50b964823ebb7f0a098878c399ce859cd38e9e Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Fri, 13 Jun 2014 18:55:47 +0300 Subject: ima: remove unnecessary i_mutex locking from ima_rdwr_violation_check() Before 2.6.39 inode->i_readcount was maintained by IMA. It was not atomic and protected using spinlock. For 2.6.39, i_readcount was converted to atomic and maintaining was moved VFS layer. Spinlock for some unclear reason was replaced by i_mutex. After analyzing the code, we came to conclusion that i_mutex locking is unnecessary, especially when an IMA policy has not been defined. This patch removes i_mutex locking from ima_rdwr_violation_check(). Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_main.c | 4 ---- 1 file changed, 4 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 09baa335ebc7..cf1c3696c72e 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -88,8 +88,6 @@ static void ima_rdwr_violation_check(struct file *file) if (!S_ISREG(inode->i_mode) || !ima_initialized) return; - mutex_lock(&inode->i_mutex); /* file metadata: permissions, xattr */ - if (mode & FMODE_WRITE) { if (atomic_read(&inode->i_readcount) && IS_IMA(inode)) { struct integrity_iint_cache *iint; @@ -104,8 +102,6 @@ static void ima_rdwr_violation_check(struct file *file) send_writers = true; } - mutex_unlock(&inode->i_mutex); - if (!send_tomtou && !send_writers) return; -- cgit v1.2.3 From 209b43ca64a6f2b0c7ac66b457f530c52d608c3e Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Fri, 13 Jun 2014 18:55:48 +0300 Subject: ima: delay template descriptor lookup until use process_measurement() always calls ima_template_desc_current(), including when an IMA policy has not been defined. This patch delays template descriptor lookup until action is determined. Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_main.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'security') diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index cf1c3696c72e..f474c608fa11 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -159,7 +159,7 @@ static int process_measurement(struct file *file, const char *filename, { struct inode *inode = file_inode(file); struct integrity_iint_cache *iint; - struct ima_template_desc *template_desc = ima_template_desc_current(); + struct ima_template_desc *template_desc; char *pathbuf = NULL; const char *pathname = NULL; int rc = -ENOMEM, action, must_appraise, _func; @@ -203,6 +203,7 @@ static int process_measurement(struct file *file, const char *filename, goto out_digsig; } + template_desc = ima_template_desc_current(); if (strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) == 0) { if (action & IMA_APPRAISE_SUBMASK) xattr_ptr = &xattr_value; -- cgit v1.2.3 From 7e9001f663636116fdc2ea7978f0350849ced624 Mon Sep 17 00:00:00 2001 From: Richard Guy Briggs Date: Mon, 16 Jun 2014 15:52:07 -0400 Subject: audit: fix dangling keywords in integrity ima message output Replace spaces in op keyword labels in log output since userspace audit tools can't parse orphaned keywords. Reported-by: Steve Grubb Signed-off-by: Richard Guy Briggs Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_appraise.c | 2 +- security/integrity/ima/ima_policy.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index d3113d4aaa3c..59ac90275070 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -214,7 +214,7 @@ int ima_appraise_measurement(int func, struct integrity_iint_cache *iint, hash_start = 1; case IMA_XATTR_DIGEST: if (iint->flags & IMA_DIGSIG_REQUIRED) { - cause = "IMA signature required"; + cause = "IMA-signature-required"; status = INTEGRITY_FAIL; break; } diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 40a7488f6721..cea84d8bd7be 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -332,7 +332,7 @@ void __init ima_init_policy(void) void ima_update_policy(void) { static const char op[] = "policy_update"; - const char *cause = "already exists"; + const char *cause = "already-exists"; int result = 1; int audit_info = 0; @@ -659,7 +659,7 @@ ssize_t ima_parse_add_rule(char *rule) /* Prevent installed policy from changing */ if (ima_rules != &ima_default_rules) { integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, - NULL, op, "already exists", + NULL, op, "already-exists", -EACCES, audit_info); return -EACCES; } @@ -685,7 +685,7 @@ ssize_t ima_parse_add_rule(char *rule) if (result) { kfree(entry); integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, - NULL, op, "invalid policy", result, + NULL, op, "invalid-policy", result, audit_info); return result; } -- cgit v1.2.3 From 3bcced39ea7d1b0da0a991e221f53de480c6b60b Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Wed, 26 Feb 2014 17:05:20 +0200 Subject: ima: use ahash API for file hash calculation Async hash API allows the use of HW acceleration for hash calculation. It may give significant performance gain and/or reduce power consumption, which might be very beneficial for battery powered devices. This patch introduces hash calculation using ahash API. ahash performance depends on the data size and the particular HW. Depending on the specific system, shash performance may be better. This patch defines 'ahash_minsize' module parameter, which is used to define the minimal file size to use with ahash. If this minimum file size is not set or the file is smaller than defined by the parameter, shash will be used. Changes in v3: - kernel parameter replaced with module parameter - pr_crit replaced with pr_crit_ratelimited - more comment changes - Mimi Changes in v2: - ima_ahash_size became as ima_ahash - ahash pre-allocation moved out from __init code to be able to use ahash crypto modules. Ahash allocated once on the first use. - hash calculation falls back to shash if ahash allocation/calculation fails - complex initialization separated from variable declaration - improved comments Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_crypto.c | 187 +++++++++++++++++++++++++++++++++++- 1 file changed, 183 insertions(+), 4 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c index ccd0ac8fa9a0..b9e5120559d4 100644 --- a/security/integrity/ima/ima_crypto.c +++ b/security/integrity/ima/ima_crypto.c @@ -16,6 +16,8 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt #include +#include +#include #include #include #include @@ -25,7 +27,18 @@ #include #include "ima.h" +struct ahash_completion { + struct completion completion; + int err; +}; + +/* minimum file size for ahash use */ +static unsigned long ima_ahash_minsize; +module_param_named(ahash_minsize, ima_ahash_minsize, ulong, 0644); +MODULE_PARM_DESC(ahash_minsize, "Minimum file size for ahash use"); + static struct crypto_shash *ima_shash_tfm; +static struct crypto_ahash *ima_ahash_tfm; /** * ima_kernel_read - read file content @@ -93,9 +106,146 @@ static void ima_free_tfm(struct crypto_shash *tfm) crypto_free_shash(tfm); } -/* - * Calculate the MD5/SHA1 file digest - */ +static struct crypto_ahash *ima_alloc_atfm(enum hash_algo algo) +{ + struct crypto_ahash *tfm = ima_ahash_tfm; + int rc; + + if ((algo != ima_hash_algo && algo < HASH_ALGO__LAST) || !tfm) { + tfm = crypto_alloc_ahash(hash_algo_name[algo], 0, 0); + if (!IS_ERR(tfm)) { + if (algo == ima_hash_algo) + ima_ahash_tfm = tfm; + } else { + rc = PTR_ERR(tfm); + pr_err("Can not allocate %s (reason: %d)\n", + hash_algo_name[algo], rc); + } + } + return tfm; +} + +static void ima_free_atfm(struct crypto_ahash *tfm) +{ + if (tfm != ima_ahash_tfm) + crypto_free_ahash(tfm); +} + +static void ahash_complete(struct crypto_async_request *req, int err) +{ + struct ahash_completion *res = req->data; + + if (err == -EINPROGRESS) + return; + res->err = err; + complete(&res->completion); +} + +static int ahash_wait(int err, struct ahash_completion *res) +{ + switch (err) { + case 0: + break; + case -EINPROGRESS: + case -EBUSY: + wait_for_completion(&res->completion); + reinit_completion(&res->completion); + err = res->err; + /* fall through */ + default: + pr_crit_ratelimited("ahash calculation failed: err: %d\n", err); + } + + return err; +} + +static int ima_calc_file_hash_atfm(struct file *file, + struct ima_digest_data *hash, + struct crypto_ahash *tfm) +{ + loff_t i_size, offset; + char *rbuf; + int rc, read = 0, rbuf_len; + struct ahash_request *req; + struct scatterlist sg[1]; + struct ahash_completion res; + + hash->length = crypto_ahash_digestsize(tfm); + + req = ahash_request_alloc(tfm, GFP_KERNEL); + if (!req) + return -ENOMEM; + + init_completion(&res.completion); + ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG | + CRYPTO_TFM_REQ_MAY_SLEEP, + ahash_complete, &res); + + rc = ahash_wait(crypto_ahash_init(req), &res); + if (rc) + goto out1; + + i_size = i_size_read(file_inode(file)); + + if (i_size == 0) + goto out2; + + rbuf = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!rbuf) { + rc = -ENOMEM; + goto out1; + } + + if (!(file->f_mode & FMODE_READ)) { + file->f_mode |= FMODE_READ; + read = 1; + } + + for (offset = 0; offset < i_size; offset += rbuf_len) { + rbuf_len = ima_kernel_read(file, offset, rbuf, PAGE_SIZE); + if (rbuf_len < 0) { + rc = rbuf_len; + break; + } + if (rbuf_len == 0) + break; + + sg_init_one(&sg[0], rbuf, rbuf_len); + ahash_request_set_crypt(req, sg, NULL, rbuf_len); + + rc = ahash_wait(crypto_ahash_update(req), &res); + if (rc) + break; + } + if (read) + file->f_mode &= ~FMODE_READ; + kfree(rbuf); +out2: + if (!rc) { + ahash_request_set_crypt(req, NULL, hash->digest, 0); + rc = ahash_wait(crypto_ahash_final(req), &res); + } +out1: + ahash_request_free(req); + return rc; +} + +static int ima_calc_file_ahash(struct file *file, struct ima_digest_data *hash) +{ + struct crypto_ahash *tfm; + int rc; + + tfm = ima_alloc_atfm(hash->algo); + if (IS_ERR(tfm)) + return PTR_ERR(tfm); + + rc = ima_calc_file_hash_atfm(file, hash, tfm); + + ima_free_atfm(tfm); + + return rc; +} + static int ima_calc_file_hash_tfm(struct file *file, struct ima_digest_data *hash, struct crypto_shash *tfm) @@ -156,7 +306,7 @@ out: return rc; } -int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash) +static int ima_calc_file_shash(struct file *file, struct ima_digest_data *hash) { struct crypto_shash *tfm; int rc; @@ -172,6 +322,35 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash) return rc; } +/* + * ima_calc_file_hash - calculate file hash + * + * Asynchronous hash (ahash) allows using HW acceleration for calculating + * a hash. ahash performance varies for different data sizes on different + * crypto accelerators. shash performance might be better for smaller files. + * The 'ima.ahash_minsize' module parameter allows specifying the best + * minimum file size for using ahash on the system. + * + * If the ima.ahash_minsize parameter is not specified, this function uses + * shash for the hash calculation. If ahash fails, it falls back to using + * shash. + */ +int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash) +{ + loff_t i_size; + int rc; + + i_size = i_size_read(file_inode(file)); + + if (ima_ahash_minsize && i_size >= ima_ahash_minsize) { + rc = ima_calc_file_ahash(file, hash); + if (!rc) + return 0; + } + + return ima_calc_file_shash(file, hash); +} + /* * Calculate the hash of template data */ -- cgit v1.2.3 From 6edf7a89260859c5e72861dc4e6e169495f076c8 Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Tue, 6 May 2014 14:47:13 +0300 Subject: ima: introduce multi-page collect buffers Use of multiple-page collect buffers reduces: 1) the number of block IO requests 2) the number of asynchronous hash update requests Second is important for HW accelerated hashing, because significant amount of time is spent for preparation of hash update operation, which includes configuring acceleration HW, DMA engine, etc... Thus, HW accelerators are more efficient when working on large chunks of data. This patch introduces usage of multi-page collect buffers. Buffer size can be specified using 'ahash_bufsize' module parameter. Default buffer size is 4096 bytes. Changes in v3: - kernel parameter replaced with module parameter Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_crypto.c | 98 ++++++++++++++++++++++++++++++++++++- 1 file changed, 96 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c index b9e5120559d4..84b938f86bea 100644 --- a/security/integrity/ima/ima_crypto.c +++ b/security/integrity/ima/ima_crypto.c @@ -37,6 +37,33 @@ static unsigned long ima_ahash_minsize; module_param_named(ahash_minsize, ima_ahash_minsize, ulong, 0644); MODULE_PARM_DESC(ahash_minsize, "Minimum file size for ahash use"); +/* default is 0 - 1 page. */ +static int ima_maxorder; +static unsigned int ima_bufsize = PAGE_SIZE; + +static int param_set_bufsize(const char *val, const struct kernel_param *kp) +{ + unsigned long long size; + int order; + + size = memparse(val, NULL); + order = get_order(size); + if (order >= MAX_ORDER) + return -EINVAL; + ima_maxorder = order; + ima_bufsize = PAGE_SIZE << order; + return 0; +} + +static struct kernel_param_ops param_ops_bufsize = { + .set = param_set_bufsize, + .get = param_get_uint, +}; +#define param_check_bufsize(name, p) __param_check(name, p, unsigned int) + +module_param_named(ahash_bufsize, ima_bufsize, bufsize, 0644); +MODULE_PARM_DESC(ahash_bufsize, "Maximum ahash buffer size"); + static struct crypto_shash *ima_shash_tfm; static struct crypto_ahash *ima_ahash_tfm; @@ -106,6 +133,68 @@ static void ima_free_tfm(struct crypto_shash *tfm) crypto_free_shash(tfm); } +/** + * ima_alloc_pages() - Allocate contiguous pages. + * @max_size: Maximum amount of memory to allocate. + * @allocated_size: Returned size of actual allocation. + * @last_warn: Should the min_size allocation warn or not. + * + * Tries to do opportunistic allocation for memory first trying to allocate + * max_size amount of memory and then splitting that until zero order is + * reached. Allocation is tried without generating allocation warnings unless + * last_warn is set. Last_warn set affects only last allocation of zero order. + * + * By default, ima_maxorder is 0 and it is equivalent to kmalloc(GFP_KERNEL) + * + * Return pointer to allocated memory, or NULL on failure. + */ +static void *ima_alloc_pages(loff_t max_size, size_t *allocated_size, + int last_warn) +{ + void *ptr; + int order = ima_maxorder; + gfp_t gfp_mask = __GFP_WAIT | __GFP_NOWARN | __GFP_NORETRY; + + if (order) + order = min(get_order(max_size), order); + + for (; order; order--) { + ptr = (void *)__get_free_pages(gfp_mask, order); + if (ptr) { + *allocated_size = PAGE_SIZE << order; + return ptr; + } + } + + /* order is zero - one page */ + + gfp_mask = GFP_KERNEL; + + if (!last_warn) + gfp_mask |= __GFP_NOWARN; + + ptr = (void *)__get_free_pages(gfp_mask, 0); + if (ptr) { + *allocated_size = PAGE_SIZE; + return ptr; + } + + *allocated_size = 0; + return NULL; +} + +/** + * ima_free_pages() - Free pages allocated by ima_alloc_pages(). + * @ptr: Pointer to allocated pages. + * @size: Size of allocated buffer. + */ +static void ima_free_pages(void *ptr, size_t size) +{ + if (!ptr) + return; + free_pages((unsigned long)ptr, get_order(size)); +} + static struct crypto_ahash *ima_alloc_atfm(enum hash_algo algo) { struct crypto_ahash *tfm = ima_ahash_tfm; @@ -169,6 +258,7 @@ static int ima_calc_file_hash_atfm(struct file *file, struct ahash_request *req; struct scatterlist sg[1]; struct ahash_completion res; + size_t rbuf_size; hash->length = crypto_ahash_digestsize(tfm); @@ -190,7 +280,11 @@ static int ima_calc_file_hash_atfm(struct file *file, if (i_size == 0) goto out2; - rbuf = kzalloc(PAGE_SIZE, GFP_KERNEL); + /* + * Try to allocate maximum size of memory. + * Fail if even a single page cannot be allocated. + */ + rbuf = ima_alloc_pages(i_size, &rbuf_size, 1); if (!rbuf) { rc = -ENOMEM; goto out1; @@ -219,7 +313,7 @@ static int ima_calc_file_hash_atfm(struct file *file, } if (read) file->f_mode &= ~FMODE_READ; - kfree(rbuf); + ima_free_pages(rbuf, rbuf_size); out2: if (!rc) { ahash_request_set_crypt(req, NULL, hash->digest, 0); -- cgit v1.2.3 From 32c2e6752ff0f48fe03b9e1c7c64bde580a840d2 Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Tue, 6 May 2014 14:54:27 +0300 Subject: ima: provide double buffering for hash calculation The asynchronous hash API allows initiating a hash calculation and then performing other tasks, while waiting for the hash calculation to complete. This patch introduces usage of double buffering for simultaneous hashing and reading of the next chunk of data from storage. Changes in v3: - better comments Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_crypto.c | 65 ++++++++++++++++++++++++++++--------- 1 file changed, 49 insertions(+), 16 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c index 84b938f86bea..0bd732843fe7 100644 --- a/security/integrity/ima/ima_crypto.c +++ b/security/integrity/ima/ima_crypto.c @@ -253,12 +253,12 @@ static int ima_calc_file_hash_atfm(struct file *file, struct crypto_ahash *tfm) { loff_t i_size, offset; - char *rbuf; - int rc, read = 0, rbuf_len; + char *rbuf[2] = { NULL, }; + int rc, read = 0, rbuf_len, active = 0, ahash_rc = 0; struct ahash_request *req; struct scatterlist sg[1]; struct ahash_completion res; - size_t rbuf_size; + size_t rbuf_size[2]; hash->length = crypto_ahash_digestsize(tfm); @@ -284,36 +284,69 @@ static int ima_calc_file_hash_atfm(struct file *file, * Try to allocate maximum size of memory. * Fail if even a single page cannot be allocated. */ - rbuf = ima_alloc_pages(i_size, &rbuf_size, 1); - if (!rbuf) { + rbuf[0] = ima_alloc_pages(i_size, &rbuf_size[0], 1); + if (!rbuf[0]) { rc = -ENOMEM; goto out1; } + /* Only allocate one buffer if that is enough. */ + if (i_size > rbuf_size[0]) { + /* + * Try to allocate secondary buffer. If that fails fallback to + * using single buffering. Use previous memory allocation size + * as baseline for possible allocation size. + */ + rbuf[1] = ima_alloc_pages(i_size - rbuf_size[0], + &rbuf_size[1], 0); + } + if (!(file->f_mode & FMODE_READ)) { file->f_mode |= FMODE_READ; read = 1; } for (offset = 0; offset < i_size; offset += rbuf_len) { - rbuf_len = ima_kernel_read(file, offset, rbuf, PAGE_SIZE); - if (rbuf_len < 0) { - rc = rbuf_len; - break; + if (!rbuf[1] && offset) { + /* Not using two buffers, and it is not the first + * read/request, wait for the completion of the + * previous ahash_update() request. + */ + rc = ahash_wait(ahash_rc, &res); + if (rc) + goto out3; + } + /* read buffer */ + rbuf_len = min_t(loff_t, i_size - offset, rbuf_size[active]); + rc = ima_kernel_read(file, offset, rbuf[active], rbuf_len); + if (rc != rbuf_len) + goto out3; + + if (rbuf[1] && offset) { + /* Using two buffers, and it is not the first + * read/request, wait for the completion of the + * previous ahash_update() request. + */ + rc = ahash_wait(ahash_rc, &res); + if (rc) + goto out3; } - if (rbuf_len == 0) - break; - sg_init_one(&sg[0], rbuf, rbuf_len); + sg_init_one(&sg[0], rbuf[active], rbuf_len); ahash_request_set_crypt(req, sg, NULL, rbuf_len); - rc = ahash_wait(crypto_ahash_update(req), &res); - if (rc) - break; + ahash_rc = crypto_ahash_update(req); + + if (rbuf[1]) + active = !active; /* swap buffers, if we use two */ } + /* wait for the last update request to complete */ + rc = ahash_wait(ahash_rc, &res); +out3: if (read) file->f_mode &= ~FMODE_READ; - ima_free_pages(rbuf, rbuf_size); + ima_free_pages(rbuf[0], rbuf_size[0]); + ima_free_pages(rbuf[1], rbuf_size[1]); out2: if (!rc) { ahash_request_set_crypt(req, NULL, hash->digest, 0); -- cgit v1.2.3 From a4e3b8d79a5c6d40f4a9703abf7fe3abcc6c3b8d Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Thu, 22 May 2014 14:02:23 -0400 Subject: KEYS: special dot prefixed keyring name bug fix Dot prefixed keyring names are supposed to be reserved for the kernel, but add_key() calls key_get_type_from_user(), which incorrectly verifies the 'type' field, not the 'description' field. This patch verifies the 'description' field isn't dot prefixed, when creating a new keyring, and removes the dot prefix test in key_get_type_from_user(). Changelog v6: - whitespace and other cleanup Changelog v5: - Only prevent userspace from creating a dot prefixed keyring, not regular keys - Dmitry Reported-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar Acked-by: David Howells --- security/keys/keyctl.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c index cd5bd0cef25d..8a8c23357291 100644 --- a/security/keys/keyctl.c +++ b/security/keys/keyctl.c @@ -37,8 +37,6 @@ static int key_get_type_from_user(char *type, return ret; if (ret == 0 || ret >= len) return -EINVAL; - if (type[0] == '.') - return -EPERM; type[len - 1] = '\0'; return 0; } @@ -86,6 +84,10 @@ SYSCALL_DEFINE5(add_key, const char __user *, _type, if (!*description) { kfree(description); description = NULL; + } else if ((description[0] == '.') && + (strncmp(type, "keyring", 7) == 0)) { + ret = -EPERM; + goto error2; } } -- cgit v1.2.3 From 7d2ce2320e8efdc4a6dcbae7b329ed3f0d1cd778 Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Tue, 13 Aug 2013 08:47:43 -0400 Subject: ima: define '.ima' as a builtin 'trusted' keyring Require all keys added to the IMA keyring be signed by an existing trusted key on the system trusted keyring. Changelog v6: - remove ifdef CONFIG_IMA_TRUSTED_KEYRING in C code - Dmitry - update Kconfig dependency and help - select KEYS_DEBUG_PROC_KEYS - Dmitry Changelog v5: - Move integrity_init_keyring() to init_ima() - Dmitry - reset keyring[id] on failure - Dmitry Changelog v1: - don't link IMA trusted keyring to user keyring Changelog: - define stub integrity_init_keyring() function (reported-by Fengguang Wu) - differentiate between regular and trusted keyring names. - replace printk with pr_info (D. Kasatkin) - only make the IMA keyring a trusted keyring (reported-by D. Kastatkin) - define stub integrity_init_keyring() definition based on CONFIG_INTEGRITY_SIGNATURE, not CONFIG_INTEGRITY_ASYMMETRIC_KEYS. (reported-by Jim Davis) Signed-off-by: Mimi Zohar Signed-off-by: Dmitry Kasatkin Acked-by: David Howells --- security/integrity/digsig.c | 28 ++++++++++++++++++++++++++++ security/integrity/ima/Kconfig | 10 ++++++++++ security/integrity/ima/ima.h | 12 ++++++++++++ security/integrity/ima/ima_main.c | 10 ++++++++-- security/integrity/integrity.h | 5 +++++ 5 files changed, 63 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c index b4af4ebc5be2..8d4fbff8b87c 100644 --- a/security/integrity/digsig.c +++ b/security/integrity/digsig.c @@ -13,7 +13,9 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt #include +#include #include +#include #include #include @@ -24,7 +26,11 @@ static struct key *keyring[INTEGRITY_KEYRING_MAX]; static const char *keyring_name[INTEGRITY_KEYRING_MAX] = { "_evm", "_module", +#ifndef CONFIG_IMA_TRUSTED_KEYRING "_ima", +#else + ".ima", +#endif }; int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen, @@ -56,3 +62,25 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen, return -EOPNOTSUPP; } + +int integrity_init_keyring(const unsigned int id) +{ + const struct cred *cred = current_cred(); + int err = 0; + + keyring[id] = keyring_alloc(keyring_name[id], KUIDT_INIT(0), + KGIDT_INIT(0), cred, + ((KEY_POS_ALL & ~KEY_POS_SETATTR) | + KEY_USR_VIEW | KEY_USR_READ | + KEY_USR_WRITE | KEY_USR_SEARCH), + KEY_ALLOC_NOT_IN_QUOTA, NULL); + if (!IS_ERR(keyring[id])) + set_bit(KEY_FLAG_TRUSTED_ONLY, &keyring[id]->flags); + else { + err = PTR_ERR(keyring[id]); + pr_info("Can't allocate %s keyring (%d)\n", + keyring_name[id], err); + keyring[id] = NULL; + } + return err; +} diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig index 81a27971d884..08758fbd496f 100644 --- a/security/integrity/ima/Kconfig +++ b/security/integrity/ima/Kconfig @@ -123,3 +123,13 @@ config IMA_APPRAISE For more information on integrity appraisal refer to: If unsure, say N. + +config IMA_TRUSTED_KEYRING + bool "Require all keys on the .ima keyring be signed" + depends on IMA_APPRAISE && SYSTEM_TRUSTED_KEYRING + depends on INTEGRITY_ASYMMETRIC_KEYS + select KEYS_DEBUG_PROC_KEYS + default y + help + This option requires that all keys added to the .ima + keyring be signed by a key on the system trusted keyring. diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index f79fa8be203c..c42056edfc97 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -249,4 +249,16 @@ static inline int security_filter_rule_match(u32 secid, u32 field, u32 op, return -EINVAL; } #endif /* CONFIG_IMA_LSM_RULES */ + +#ifdef CONFIG_IMA_TRUSTED_KEYRING +static inline int ima_init_keyring(const unsigned int id) +{ + return integrity_init_keyring(id); +} +#else +static inline int ima_init_keyring(const unsigned int id) +{ + return 0; +} +#endif /* CONFIG_IMA_TRUSTED_KEYRING */ #endif diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index f474c608fa11..0d696431209c 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -325,8 +325,14 @@ static int __init init_ima(void) hash_setup(CONFIG_IMA_DEFAULT_HASH); error = ima_init(); - if (!error) - ima_initialized = 1; + if (error) + goto out; + + error = ima_init_keyring(INTEGRITY_KEYRING_IMA); + if (error) + goto out; + ima_initialized = 1; +out: return error; } diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index 33c0a70f6b15..09c440d9aaee 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -124,6 +124,7 @@ struct integrity_iint_cache *integrity_iint_find(struct inode *inode); int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen, const char *digest, int digestlen); +int integrity_init_keyring(const unsigned int id); #else static inline int integrity_digsig_verify(const unsigned int id, @@ -133,6 +134,10 @@ static inline int integrity_digsig_verify(const unsigned int id, return -EOPNOTSUPP; } +static inline int integrity_init_keyring(const unsigned int id) +{ + return 0; +} #endif /* CONFIG_INTEGRITY_SIGNATURE */ #ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS -- cgit v1.2.3 From 0c7774abb41bd00d5836d9ba098825a40fa94133 Mon Sep 17 00:00:00 2001 From: David Howells Date: Thu, 17 Jul 2014 20:45:08 +0100 Subject: KEYS: Allow special keys (eg. DNS results) to be invalidated by CAP_SYS_ADMIN Special kernel keys, such as those used to hold DNS results for AFS, CIFS and NFS and those used to hold idmapper results for NFS, used to be 'invalidateable' with key_revoke(). However, since the default permissions for keys were reduced: Commit: 96b5c8fea6c0861621051290d705ec2e971963f1 KEYS: Reduce initial permissions on keys it has become impossible to do this. Add a key flag (KEY_FLAG_ROOT_CAN_INVAL) that will permit a key to be invalidated by root. This should not be used for system keyrings as the garbage collector will try and remove any invalidate key. For system keyrings, KEY_FLAG_ROOT_CAN_CLEAR can be used instead. After this, from userspace, keyctl_invalidate() and "keyctl invalidate" can be used by any possessor of CAP_SYS_ADMIN (typically root) to invalidate DNS and idmapper keys. Invalidated keys are immediately garbage collected and will be immediately rerequested if needed again. Signed-off-by: David Howells Tested-by: Steve Dickson --- security/keys/keyctl.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) (limited to 'security') diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c index cd5bd0cef25d..609f8d326ddc 100644 --- a/security/keys/keyctl.c +++ b/security/keys/keyctl.c @@ -404,12 +404,25 @@ long keyctl_invalidate_key(key_serial_t id) key_ref = lookup_user_key(id, 0, KEY_NEED_SEARCH); if (IS_ERR(key_ref)) { ret = PTR_ERR(key_ref); + + /* Root is permitted to invalidate certain special keys */ + if (capable(CAP_SYS_ADMIN)) { + key_ref = lookup_user_key(id, 0, 0); + if (IS_ERR(key_ref)) + goto error; + if (test_bit(KEY_FLAG_ROOT_CAN_INVAL, + &key_ref_to_ptr(key_ref)->flags)) + goto invalidate; + goto error_put; + } + goto error; } +invalidate: key_invalidate(key_ref_to_ptr(key_ref)); ret = 0; - +error_put: key_ref_put(key_ref); error: kleave(" = %ld", ret); -- cgit v1.2.3 From 6a09d17bb66a533c165be81e8a4c3557f68e1a3b Mon Sep 17 00:00:00 2001 From: David Howells Date: Fri, 18 Jul 2014 18:56:34 +0100 Subject: KEYS: Provide a generic instantiation function Provide a generic instantiation function for key types that use the preparse hook. This makes it easier to prereserve key quota before keyrings get locked to retain the new key. Signed-off-by: David Howells Acked-by: Steve Dickson Acked-by: Jeff Layton Reviewed-by: Sage Weil --- security/keys/key.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) (limited to 'security') diff --git a/security/keys/key.c b/security/keys/key.c index 2048a110e7f1..7c9acbf106b6 100644 --- a/security/keys/key.c +++ b/security/keys/key.c @@ -1023,6 +1023,36 @@ void key_invalidate(struct key *key) } EXPORT_SYMBOL(key_invalidate); +/** + * generic_key_instantiate - Simple instantiation of a key from preparsed data + * @key: The key to be instantiated + * @prep: The preparsed data to load. + * + * Instantiate a key from preparsed data. We assume we can just copy the data + * in directly and clear the old pointers. + * + * This can be pointed to directly by the key type instantiate op pointer. + */ +int generic_key_instantiate(struct key *key, struct key_preparsed_payload *prep) +{ + int ret; + + pr_devel("==>%s()\n", __func__); + + ret = key_payload_reserve(key, prep->quotalen); + if (ret == 0) { + key->type_data.p[0] = prep->type_data[0]; + key->type_data.p[1] = prep->type_data[1]; + rcu_assign_keypointer(key, prep->payload); + prep->type_data[0] = NULL; + prep->type_data[1] = NULL; + prep->payload = NULL; + } + pr_devel("<==%s() = %d\n", __func__, ret); + return ret; +} +EXPORT_SYMBOL(generic_key_instantiate); + /** * register_key_type - Register a type of key. * @ktype: The new key type. -- cgit v1.2.3 From 1d4457f99928a968767f6405b4a1f50845aa15fd Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Wed, 21 May 2014 15:23:46 -0700 Subject: sched: move no_new_privs into new atomic flags Since seccomp transitions between threads requires updates to the no_new_privs flag to be atomic, the flag must be part of an atomic flag set. This moves the nnp flag into a separate task field, and introduces accessors. Signed-off-by: Kees Cook Reviewed-by: Oleg Nesterov Reviewed-by: Andy Lutomirski --- security/apparmor/domain.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 452567d3a08e..d97cba3e3849 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -621,7 +621,7 @@ int aa_change_hat(const char *hats[], int count, u64 token, bool permtest) * There is no exception for unconfined as change_hat is not * available. */ - if (current->no_new_privs) + if (task_no_new_privs(current)) return -EPERM; /* released below */ @@ -776,7 +776,7 @@ int aa_change_profile(const char *ns_name, const char *hname, bool onexec, * no_new_privs is set because this aways results in a reduction * of permissions. */ - if (current->no_new_privs && !unconfined(profile)) { + if (task_no_new_privs(current) && !unconfined(profile)) { put_cred(cred); return -EPERM; } -- cgit v1.2.3 From fc7c70e0b6b637bbf6cf8b9cee547d5ae83899c9 Mon Sep 17 00:00:00 2001 From: David Howells Date: Fri, 18 Jul 2014 18:56:34 +0100 Subject: KEYS: struct key_preparsed_payload should have two payload pointers struct key_preparsed_payload should have two payload pointers to correspond with those in struct key. Signed-off-by: David Howells Acked-by: Steve Dickson Acked-by: Jeff Layton Reviewed-by: Sage Weil --- security/keys/encrypted-keys/encrypted.c | 2 +- security/keys/key.c | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) (limited to 'security') diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c index 5fe443d120af..d252c5704f8a 100644 --- a/security/keys/encrypted-keys/encrypted.c +++ b/security/keys/encrypted-keys/encrypted.c @@ -811,7 +811,7 @@ static int encrypted_instantiate(struct key *key, goto out; } - rcu_assign_keypointer(key, epayload); + prep->payload[0] = epayload; out: kfree(datablob); return ret; diff --git a/security/keys/key.c b/security/keys/key.c index 7c9acbf106b6..03620a35a4dc 100644 --- a/security/keys/key.c +++ b/security/keys/key.c @@ -1043,10 +1043,12 @@ int generic_key_instantiate(struct key *key, struct key_preparsed_payload *prep) if (ret == 0) { key->type_data.p[0] = prep->type_data[0]; key->type_data.p[1] = prep->type_data[1]; - rcu_assign_keypointer(key, prep->payload); + rcu_assign_keypointer(key, prep->payload[0]); + key->payload.data2[1] = prep->payload[1]; prep->type_data[0] = NULL; prep->type_data[1] = NULL; - prep->payload = NULL; + prep->payload[0] = NULL; + prep->payload[1] = NULL; } pr_devel("<==%s() = %d\n", __func__, ret); return ret; -- cgit v1.2.3 From 7dfa0ca6a95de65b7a7760630cdbd7d30f204bfa Mon Sep 17 00:00:00 2001 From: David Howells Date: Fri, 18 Jul 2014 18:56:34 +0100 Subject: KEYS: Allow expiry time to be set when preparsing a key Allow a key type's preparsing routine to set the expiry time for a key. Signed-off-by: David Howells Acked-by: Steve Dickson Acked-by: Jeff Layton Reviewed-by: Sage Weil --- security/keys/key.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'security') diff --git a/security/keys/key.c b/security/keys/key.c index 03620a35a4dc..755fb02df5af 100644 --- a/security/keys/key.c +++ b/security/keys/key.c @@ -437,6 +437,11 @@ static int __key_instantiate_and_link(struct key *key, /* disable the authorisation key */ if (authkey) key_revoke(authkey); + + if (prep->expiry != TIME_T_MAX) { + key->expiry = prep->expiry; + key_schedule_gc(prep->expiry + key_gc_delay); + } } } @@ -479,6 +484,7 @@ int key_instantiate_and_link(struct key *key, prep.data = data; prep.datalen = datalen; prep.quotalen = key->type->def_datalen; + prep.expiry = TIME_T_MAX; if (key->type->preparse) { ret = key->type->preparse(&prep); if (ret < 0) @@ -811,6 +817,7 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref, prep.datalen = plen; prep.quotalen = index_key.type->def_datalen; prep.trusted = flags & KEY_ALLOC_TRUSTED; + prep.expiry = TIME_T_MAX; if (index_key.type->preparse) { ret = index_key.type->preparse(&prep); if (ret < 0) { @@ -941,6 +948,7 @@ int key_update(key_ref_t key_ref, const void *payload, size_t plen) prep.data = payload; prep.datalen = plen; prep.quotalen = key->type->def_datalen; + prep.expiry = TIME_T_MAX; if (key->type->preparse) { ret = key->type->preparse(&prep); if (ret < 0) -- cgit v1.2.3 From 4d8c0250b841159b128785f7a7efbaff40cc8501 Mon Sep 17 00:00:00 2001 From: David Howells Date: Fri, 18 Jul 2014 18:56:34 +0100 Subject: KEYS: Call ->free_preparse() even after ->preparse() returns an error Call the ->free_preparse() key type op even after ->preparse() returns an error as it does cleaning up type stuff. Signed-off-by: David Howells Acked-by: Steve Dickson Acked-by: Jeff Layton Reviewed-by: Sage Weil --- security/keys/key.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'security') diff --git a/security/keys/key.c b/security/keys/key.c index 755fb02df5af..b90a68c4e2c4 100644 --- a/security/keys/key.c +++ b/security/keys/key.c @@ -494,7 +494,7 @@ int key_instantiate_and_link(struct key *key, if (keyring) { ret = __key_link_begin(keyring, &key->index_key, &edit); if (ret < 0) - goto error_free_preparse; + goto error; } ret = __key_instantiate_and_link(key, &prep, keyring, authkey, &edit); @@ -502,10 +502,9 @@ int key_instantiate_and_link(struct key *key, if (keyring) __key_link_end(keyring, &key->index_key, edit); -error_free_preparse: +error: if (key->type->preparse) key->type->free_preparse(&prep); -error: return ret; } @@ -822,7 +821,7 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref, ret = index_key.type->preparse(&prep); if (ret < 0) { key_ref = ERR_PTR(ret); - goto error_put_type; + goto error_free_prep; } if (!index_key.description) index_key.description = prep.description; @@ -964,9 +963,9 @@ int key_update(key_ref_t key_ref, const void *payload, size_t plen) up_write(&key->sem); +error: if (key->type->preparse) key->type->free_preparse(&prep); -error: return ret; } EXPORT_SYMBOL(key_update); -- cgit v1.2.3 From f9167789df53f22af771fb6690a3d36aa21d74c5 Mon Sep 17 00:00:00 2001 From: David Howells Date: Fri, 18 Jul 2014 18:56:35 +0100 Subject: KEYS: user: Use key preparsing Make use of key preparsing in user-defined and logon keys so that quota size determination can take place prior to keyring locking when a key is being added. Also the idmapper key types need to change to match as they use the user-defined key type routines. Signed-off-by: David Howells Acked-by: Steve Dickson Acked-by: Jeff Layton --- security/keys/user_defined.c | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-) (limited to 'security') diff --git a/security/keys/user_defined.c b/security/keys/user_defined.c index faa2caeb593f..eee340011f2b 100644 --- a/security/keys/user_defined.c +++ b/security/keys/user_defined.c @@ -27,7 +27,9 @@ static int logon_vet_description(const char *desc); struct key_type key_type_user = { .name = "user", .def_lookup_type = KEYRING_SEARCH_LOOKUP_DIRECT, - .instantiate = user_instantiate, + .preparse = user_preparse, + .free_preparse = user_free_preparse, + .instantiate = generic_key_instantiate, .update = user_update, .match = user_match, .revoke = user_revoke, @@ -47,7 +49,9 @@ EXPORT_SYMBOL_GPL(key_type_user); struct key_type key_type_logon = { .name = "logon", .def_lookup_type = KEYRING_SEARCH_LOOKUP_DIRECT, - .instantiate = user_instantiate, + .preparse = user_preparse, + .free_preparse = user_free_preparse, + .instantiate = generic_key_instantiate, .update = user_update, .match = user_match, .revoke = user_revoke, @@ -58,38 +62,37 @@ struct key_type key_type_logon = { EXPORT_SYMBOL_GPL(key_type_logon); /* - * instantiate a user defined key + * Preparse a user defined key payload */ -int user_instantiate(struct key *key, struct key_preparsed_payload *prep) +int user_preparse(struct key_preparsed_payload *prep) { struct user_key_payload *upayload; size_t datalen = prep->datalen; - int ret; - ret = -EINVAL; if (datalen <= 0 || datalen > 32767 || !prep->data) - goto error; - - ret = key_payload_reserve(key, datalen); - if (ret < 0) - goto error; + return -EINVAL; - ret = -ENOMEM; upayload = kmalloc(sizeof(*upayload) + datalen, GFP_KERNEL); if (!upayload) - goto error; + return -ENOMEM; /* attach the data */ + prep->quotalen = datalen; + prep->payload[0] = upayload; upayload->datalen = datalen; memcpy(upayload->data, prep->data, datalen); - rcu_assign_keypointer(key, upayload); - ret = 0; - -error: - return ret; + return 0; } +EXPORT_SYMBOL_GPL(user_preparse); -EXPORT_SYMBOL_GPL(user_instantiate); +/* + * Free a preparse of a user defined key payload + */ +void user_free_preparse(struct key_preparsed_payload *prep) +{ + kfree(prep->payload[0]); +} +EXPORT_SYMBOL_GPL(user_free_preparse); /* * update a user defined key -- cgit v1.2.3 From 002edaf76f09af658241029817f5ef66f6bef5e4 Mon Sep 17 00:00:00 2001 From: David Howells Date: Fri, 18 Jul 2014 18:56:36 +0100 Subject: KEYS: big_key: Use key preparsing Make use of key preparsing in the big key type so that quota size determination can take place prior to keyring locking when a key is being added. Signed-off-by: David Howells Acked-by: Steve Dickson --- security/keys/big_key.c | 41 +++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 16 deletions(-) (limited to 'security') diff --git a/security/keys/big_key.c b/security/keys/big_key.c index 8137b27d641d..c2f91a0cf889 100644 --- a/security/keys/big_key.c +++ b/security/keys/big_key.c @@ -34,7 +34,9 @@ MODULE_LICENSE("GPL"); struct key_type key_type_big_key = { .name = "big_key", .def_lookup_type = KEYRING_SEARCH_LOOKUP_DIRECT, - .instantiate = big_key_instantiate, + .preparse = big_key_preparse, + .free_preparse = big_key_free_preparse, + .instantiate = generic_key_instantiate, .match = user_match, .revoke = big_key_revoke, .destroy = big_key_destroy, @@ -43,11 +45,11 @@ struct key_type key_type_big_key = { }; /* - * Instantiate a big key + * Preparse a big key */ -int big_key_instantiate(struct key *key, struct key_preparsed_payload *prep) +int big_key_preparse(struct key_preparsed_payload *prep) { - struct path *path = (struct path *)&key->payload.data2; + struct path *path = (struct path *)&prep->payload; struct file *file; ssize_t written; size_t datalen = prep->datalen; @@ -58,11 +60,9 @@ int big_key_instantiate(struct key *key, struct key_preparsed_payload *prep) goto error; /* Set an arbitrary quota */ - ret = key_payload_reserve(key, 16); - if (ret < 0) - goto error; + prep->quotalen = 16; - key->type_data.x[1] = datalen; + prep->type_data[1] = (void *)(unsigned long)datalen; if (datalen > BIG_KEY_FILE_THRESHOLD) { /* Create a shmem file to store the data in. This will permit the data @@ -73,7 +73,7 @@ int big_key_instantiate(struct key *key, struct key_preparsed_payload *prep) file = shmem_kernel_file_setup("", datalen, 0); if (IS_ERR(file)) { ret = PTR_ERR(file); - goto err_quota; + goto error; } written = kernel_write(file, prep->data, prep->datalen, 0); @@ -93,23 +93,32 @@ int big_key_instantiate(struct key *key, struct key_preparsed_payload *prep) } else { /* Just store the data in a buffer */ void *data = kmalloc(datalen, GFP_KERNEL); - if (!data) { - ret = -ENOMEM; - goto err_quota; - } + if (!data) + return -ENOMEM; - key->payload.data = memcpy(data, prep->data, prep->datalen); + prep->payload[0] = memcpy(data, prep->data, prep->datalen); } return 0; err_fput: fput(file); -err_quota: - key_payload_reserve(key, 0); error: return ret; } +/* + * Clear preparsement. + */ +void big_key_free_preparse(struct key_preparsed_payload *prep) +{ + if (prep->datalen > BIG_KEY_FILE_THRESHOLD) { + struct path *path = (struct path *)&prep->payload; + path_put(path); + } else { + kfree(prep->payload[0]); + } +} + /* * dispose of the links from a revoked keyring * - called with the key sem write-locked -- cgit v1.2.3 From 5d19e20b534ff4c17dfba792f1f9e33e1378e3f9 Mon Sep 17 00:00:00 2001 From: David Howells Date: Fri, 18 Jul 2014 18:56:36 +0100 Subject: KEYS: keyring: Provide key preparsing Provide key preparsing in the keyring so that we can make preparsing mandatory. For keyrings, however, only an empty payload is permitted. Signed-off-by: David Howells Acked-by: Steve Dickson Acked-by: Jeff Layton --- security/keys/keyring.c | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) (limited to 'security') diff --git a/security/keys/keyring.c b/security/keys/keyring.c index 9cf2575f0d97..8314a7d2104d 100644 --- a/security/keys/keyring.c +++ b/security/keys/keyring.c @@ -73,6 +73,8 @@ static inline unsigned keyring_hash(const char *desc) * can be treated as ordinary keys in addition to having their own special * operations. */ +static int keyring_preparse(struct key_preparsed_payload *prep); +static void keyring_free_preparse(struct key_preparsed_payload *prep); static int keyring_instantiate(struct key *keyring, struct key_preparsed_payload *prep); static void keyring_revoke(struct key *keyring); @@ -84,6 +86,8 @@ static long keyring_read(const struct key *keyring, struct key_type key_type_keyring = { .name = "keyring", .def_datalen = 0, + .preparse = keyring_preparse, + .free_preparse = keyring_free_preparse, .instantiate = keyring_instantiate, .match = user_match, .revoke = keyring_revoke, @@ -122,6 +126,21 @@ static void keyring_publish_name(struct key *keyring) } } +/* + * Preparse a keyring payload + */ +static int keyring_preparse(struct key_preparsed_payload *prep) +{ + return prep->datalen != 0 ? -EINVAL : 0; +} + +/* + * Free a preparse of a user defined key payload + */ +static void keyring_free_preparse(struct key_preparsed_payload *prep) +{ +} + /* * Initialise a keyring. * @@ -130,17 +149,10 @@ static void keyring_publish_name(struct key *keyring) static int keyring_instantiate(struct key *keyring, struct key_preparsed_payload *prep) { - int ret; - - ret = -EINVAL; - if (prep->datalen == 0) { - assoc_array_init(&keyring->keys); - /* make the keyring available by name if it has one */ - keyring_publish_name(keyring); - ret = 0; - } - - return ret; + assoc_array_init(&keyring->keys); + /* make the keyring available by name if it has one */ + keyring_publish_name(keyring); + return 0; } /* -- cgit v1.2.3 From f1dcde91a3503f68ef209667a8798ead2b50b02a Mon Sep 17 00:00:00 2001 From: David Howells Date: Fri, 18 Jul 2014 18:56:36 +0100 Subject: KEYS: request_key_auth: Provide key preparsing Provide key preparsing for the request_key_auth key type so that we can make preparsing mandatory. This does nothing as this type can only be set up internally to the kernel. Signed-off-by: David Howells Acked-by: Steve Dickson Acked-by: Jeff Layton --- security/keys/request_key_auth.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'security') diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c index 7495a93b4b90..842e6f410d50 100644 --- a/security/keys/request_key_auth.c +++ b/security/keys/request_key_auth.c @@ -20,6 +20,8 @@ #include "internal.h" #include +static int request_key_auth_preparse(struct key_preparsed_payload *); +static void request_key_auth_free_preparse(struct key_preparsed_payload *); static int request_key_auth_instantiate(struct key *, struct key_preparsed_payload *); static void request_key_auth_describe(const struct key *, struct seq_file *); @@ -33,6 +35,8 @@ static long request_key_auth_read(const struct key *, char __user *, size_t); struct key_type key_type_request_key_auth = { .name = ".request_key_auth", .def_datalen = sizeof(struct request_key_auth), + .preparse = request_key_auth_preparse, + .free_preparse = request_key_auth_free_preparse, .instantiate = request_key_auth_instantiate, .describe = request_key_auth_describe, .revoke = request_key_auth_revoke, @@ -40,6 +44,15 @@ struct key_type key_type_request_key_auth = { .read = request_key_auth_read, }; +int request_key_auth_preparse(struct key_preparsed_payload *prep) +{ + return 0; +} + +void request_key_auth_free_preparse(struct key_preparsed_payload *prep) +{ +} + /* * Instantiate a request-key authorisation key. */ -- cgit v1.2.3 From 6d6f3328422a3bc56b0d8dd026a5de845d2abfa7 Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Tue, 22 Jul 2014 21:20:01 +0900 Subject: commoncap: don't alloc the credential unless needed in cap_task_prctl In function cap_task_prctl(), we would allocate a credential unconditionally and then check if we support the requested function. If not we would release this credential with abort_creds() by using RCU method. But on some archs such as powerpc, the sys_prctl is heavily used to get/set the floating point exception mode. So the unnecessary allocating/releasing of credential not only introduce runtime overhead but also do cause OOM due to the RCU implementation. This patch removes abort_creds() from cap_task_prctl() by calling prepare_creds() only when we need to modify it. Reported-by: Kevin Hao Signed-off-by: Tetsuo Handa Reviewed-by: Paul Moore Acked-by: Serge E. Hallyn Reviewed-by: Kees Cook Signed-off-by: James Morris --- security/commoncap.c | 72 ++++++++++++++++++++++------------------------------ 1 file changed, 30 insertions(+), 42 deletions(-) (limited to 'security') diff --git a/security/commoncap.c b/security/commoncap.c index b9d613e0ef14..9fe46e22c7f2 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -822,15 +822,20 @@ int cap_task_setnice(struct task_struct *p, int nice) * Implement PR_CAPBSET_DROP. Attempt to remove the specified capability from * the current task's bounding set. Returns 0 on success, -ve on error. */ -static long cap_prctl_drop(struct cred *new, unsigned long cap) +static int cap_prctl_drop(unsigned long cap) { + struct cred *new; + if (!ns_capable(current_user_ns(), CAP_SETPCAP)) return -EPERM; if (!cap_valid(cap)) return -EINVAL; + new = prepare_creds(); + if (!new) + return -ENOMEM; cap_lower(new->cap_bset, cap); - return 0; + return commit_creds(new); } /** @@ -848,26 +853,17 @@ static long cap_prctl_drop(struct cred *new, unsigned long cap) int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3, unsigned long arg4, unsigned long arg5) { + const struct cred *old = current_cred(); struct cred *new; - long error = 0; - - new = prepare_creds(); - if (!new) - return -ENOMEM; switch (option) { case PR_CAPBSET_READ: - error = -EINVAL; if (!cap_valid(arg2)) - goto error; - error = !!cap_raised(new->cap_bset, arg2); - goto no_change; + return -EINVAL; + return !!cap_raised(old->cap_bset, arg2); case PR_CAPBSET_DROP: - error = cap_prctl_drop(new, arg2); - if (error < 0) - goto error; - goto changed; + return cap_prctl_drop(arg2); /* * The next four prctl's remain to assist with transitioning a @@ -889,10 +885,9 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3, * capability-based-privilege environment. */ case PR_SET_SECUREBITS: - error = -EPERM; - if ((((new->securebits & SECURE_ALL_LOCKS) >> 1) - & (new->securebits ^ arg2)) /*[1]*/ - || ((new->securebits & SECURE_ALL_LOCKS & ~arg2)) /*[2]*/ + if ((((old->securebits & SECURE_ALL_LOCKS) >> 1) + & (old->securebits ^ arg2)) /*[1]*/ + || ((old->securebits & SECURE_ALL_LOCKS & ~arg2)) /*[2]*/ || (arg2 & ~(SECURE_ALL_LOCKS | SECURE_ALL_BITS)) /*[3]*/ || (cap_capable(current_cred(), current_cred()->user_ns, CAP_SETPCAP, @@ -906,46 +901,39 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3, */ ) /* cannot change a locked bit */ - goto error; + return -EPERM; + + new = prepare_creds(); + if (!new) + return -ENOMEM; new->securebits = arg2; - goto changed; + return commit_creds(new); case PR_GET_SECUREBITS: - error = new->securebits; - goto no_change; + return old->securebits; case PR_GET_KEEPCAPS: - if (issecure(SECURE_KEEP_CAPS)) - error = 1; - goto no_change; + return !!issecure(SECURE_KEEP_CAPS); case PR_SET_KEEPCAPS: - error = -EINVAL; if (arg2 > 1) /* Note, we rely on arg2 being unsigned here */ - goto error; - error = -EPERM; + return -EINVAL; if (issecure(SECURE_KEEP_CAPS_LOCKED)) - goto error; + return -EPERM; + + new = prepare_creds(); + if (!new) + return -ENOMEM; if (arg2) new->securebits |= issecure_mask(SECURE_KEEP_CAPS); else new->securebits &= ~issecure_mask(SECURE_KEEP_CAPS); - goto changed; + return commit_creds(new); default: /* No functionality available - continue with default */ - error = -ENOSYS; - goto error; + return -ENOSYS; } - - /* Functionality provided */ -changed: - return commit_creds(new); - -no_change: -error: - abort_creds(new); - return error; } /** -- cgit v1.2.3 From 7d8b6c63751cfbbe5eef81a48c22978b3407a3ad Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Wed, 23 Jul 2014 15:36:26 -0400 Subject: CAPABILITIES: remove undefined caps from all processes This is effectively a revert of 7b9a7ec565505699f503b4fcf61500dceb36e744 plus fixing it a different way... We found, when trying to run an application from an application which had dropped privs that the kernel does security checks on undefined capability bits. This was ESPECIALLY difficult to debug as those undefined bits are hidden from /proc/$PID/status. Consider a root application which drops all capabilities from ALL 4 capability sets. We assume, since the application is going to set eff/perm/inh from an array that it will clear not only the defined caps less than CAP_LAST_CAP, but also the higher 28ish bits which are undefined future capabilities. The BSET gets cleared differently. Instead it is cleared one bit at a time. The problem here is that in security/commoncap.c::cap_task_prctl() we actually check the validity of a capability being read. So any task which attempts to 'read all things set in bset' followed by 'unset all things set in bset' will not even attempt to unset the undefined bits higher than CAP_LAST_CAP. So the 'parent' will look something like: CapInh: 0000000000000000 CapPrm: 0000000000000000 CapEff: 0000000000000000 CapBnd: ffffffc000000000 All of this 'should' be fine. Given that these are undefined bits that aren't supposed to have anything to do with permissions. But they do... So lets now consider a task which cleared the eff/perm/inh completely and cleared all of the valid caps in the bset (but not the invalid caps it couldn't read out of the kernel). We know that this is exactly what the libcap-ng library does and what the go capabilities library does. They both leave you in that above situation if you try to clear all of you capapabilities from all 4 sets. If that root task calls execve() the child task will pick up all caps not blocked by the bset. The bset however does not block bits higher than CAP_LAST_CAP. So now the child task has bits in eff which are not in the parent. These are 'meaningless' undefined bits, but still bits which the parent doesn't have. The problem is now in cred_cap_issubset() (or any operation which does a subset test) as the child, while a subset for valid cap bits, is not a subset for invalid cap bits! So now we set durring commit creds that the child is not dumpable. Given it is 'more priv' than its parent. It also means the parent cannot ptrace the child and other stupidity. The solution here: 1) stop hiding capability bits in status This makes debugging easier! 2) stop giving any task undefined capability bits. it's simple, it you don't put those invalid bits in CAP_FULL_SET you won't get them in init and you won't get them in any other task either. This fixes the cap_issubset() tests and resulting fallout (which made the init task in a docker container untraceable among other things) 3) mask out undefined bits when sys_capset() is called as it might use ~0, ~0 to denote 'all capabilities' for backward/forward compatibility. This lets 'capsh --caps="all=eip" -- -c /bin/bash' run. 4) mask out undefined bit when we read a file capability off of disk as again likely all bits are set in the xattr for forward/backward compatibility. This lets 'setcap all+pe /bin/bash; /bin/bash' run Signed-off-by: Eric Paris Reviewed-by: Kees Cook Cc: Andrew Vagin Cc: Andrew G. Morgan Cc: Serge E. Hallyn Cc: Kees Cook Cc: Steve Grubb Cc: Dan Walsh Cc: stable@vger.kernel.org Signed-off-by: James Morris --- security/commoncap.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'security') diff --git a/security/commoncap.c b/security/commoncap.c index 9fe46e22c7f2..bab0611afc1e 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -421,6 +421,9 @@ int get_vfs_caps_from_disk(const struct dentry *dentry, struct cpu_vfs_cap_data cpu_caps->inheritable.cap[i] = le32_to_cpu(caps.data[i].inheritable); } + cpu_caps->permitted.cap[CAP_LAST_U32] &= CAP_LAST_U32_VALID_MASK; + cpu_caps->inheritable.cap[CAP_LAST_U32] &= CAP_LAST_U32_VALID_MASK; + return 0; } -- cgit v1.2.3 From 13752fe2d7f2d41c2fd92a5d1b1c6e38c4de0c05 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Tue, 25 Feb 2014 10:28:04 -0800 Subject: security: introduce kernel_fw_from_file hook In order to validate the contents of firmware being loaded, there must be a hook to evaluate any loaded firmware that wasn't built into the kernel itself. Without this, there is a risk that a root user could load malicious firmware designed to mount an attack against kernel memory (e.g. via DMA). Signed-off-by: Kees Cook Reviewed-by: Takashi Iwai --- security/capability.c | 6 ++++++ security/security.c | 6 ++++++ 2 files changed, 12 insertions(+) (limited to 'security') diff --git a/security/capability.c b/security/capability.c index e76373de3129..a74fde6a7468 100644 --- a/security/capability.c +++ b/security/capability.c @@ -401,6 +401,11 @@ static int cap_kernel_create_files_as(struct cred *new, struct inode *inode) return 0; } +static int cap_kernel_fw_from_file(struct file *file, char *buf, size_t size) +{ + return 0; +} + static int cap_kernel_module_request(char *kmod_name) { return 0; @@ -1015,6 +1020,7 @@ void __init security_fixup_ops(struct security_operations *ops) set_to_cap_if_null(ops, cred_transfer); set_to_cap_if_null(ops, kernel_act_as); set_to_cap_if_null(ops, kernel_create_files_as); + set_to_cap_if_null(ops, kernel_fw_from_file); set_to_cap_if_null(ops, kernel_module_request); set_to_cap_if_null(ops, kernel_module_from_file); set_to_cap_if_null(ops, task_fix_setuid); diff --git a/security/security.c b/security/security.c index 31614e9e96e5..35d37d0f0d49 100644 --- a/security/security.c +++ b/security/security.c @@ -845,6 +845,12 @@ int security_kernel_create_files_as(struct cred *new, struct inode *inode) return security_ops->kernel_create_files_as(new, inode); } +int security_kernel_fw_from_file(struct file *file, char *buf, size_t size) +{ + return security_ops->kernel_fw_from_file(file, buf, size); +} +EXPORT_SYMBOL_GPL(security_kernel_fw_from_file); + int security_kernel_module_request(char *kmod_name) { return security_ops->kernel_module_request(kmod_name); -- cgit v1.2.3 From 5a9196d715607f76d6b7d96a0970d6065335e62b Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Tue, 22 Jul 2014 10:39:48 -0400 Subject: ima: add support for measuring and appraising firmware The "security: introduce kernel_fw_from_file hook" patch defined a new security hook to evaluate any loaded firmware that wasn't built into the kernel. This patch defines ima_fw_from_file(), which is called from the new security hook, to measure and/or appraise the loaded firmware's integrity. Signed-off-by: Mimi Zohar Signed-off-by: Kees Cook --- security/integrity/ima/ima.h | 3 ++- security/integrity/ima/ima_appraise.c | 8 ++++++++ security/integrity/ima/ima_main.c | 11 +++++++++++ security/integrity/ima/ima_policy.c | 7 +++++++ security/integrity/integrity.h | 9 +++++++-- security/security.c | 7 ++++++- 6 files changed, 41 insertions(+), 4 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index c42056edfc97..57da4bd7ba0c 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -158,7 +158,7 @@ struct integrity_iint_cache *integrity_iint_insert(struct inode *inode); struct integrity_iint_cache *integrity_iint_find(struct inode *inode); /* IMA policy related functions */ -enum ima_hooks { FILE_CHECK = 1, MMAP_CHECK, BPRM_CHECK, MODULE_CHECK, POST_SETATTR }; +enum ima_hooks { FILE_CHECK = 1, MMAP_CHECK, BPRM_CHECK, MODULE_CHECK, FIRMWARE_CHECK, POST_SETATTR }; int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask, int flags); @@ -171,6 +171,7 @@ void ima_delete_rules(void); #define IMA_APPRAISE_ENFORCE 0x01 #define IMA_APPRAISE_FIX 0x02 #define IMA_APPRAISE_MODULES 0x04 +#define IMA_APPRAISE_FIRMWARE 0x08 #ifdef CONFIG_IMA_APPRAISE int ima_appraise_measurement(int func, struct integrity_iint_cache *iint, diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 59ac90275070..86bfd5c5df85 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -75,6 +75,8 @@ enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint, return iint->ima_bprm_status; case MODULE_CHECK: return iint->ima_module_status; + case FIRMWARE_CHECK: + return iint->ima_firmware_status; case FILE_CHECK: default: return iint->ima_file_status; @@ -94,6 +96,9 @@ static void ima_set_cache_status(struct integrity_iint_cache *iint, case MODULE_CHECK: iint->ima_module_status = status; break; + case FIRMWARE_CHECK: + iint->ima_firmware_status = status; + break; case FILE_CHECK: default: iint->ima_file_status = status; @@ -113,6 +118,9 @@ static void ima_cache_flags(struct integrity_iint_cache *iint, int func) case MODULE_CHECK: iint->flags |= (IMA_MODULE_APPRAISED | IMA_APPRAISED); break; + case FIRMWARE_CHECK: + iint->flags |= (IMA_FIRMWARE_APPRAISED | IMA_APPRAISED); + break; case FILE_CHECK: default: iint->flags |= (IMA_FILE_APPRAISED | IMA_APPRAISED); diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 0d696431209c..2917f980bf30 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -319,6 +319,17 @@ int ima_module_check(struct file *file) return process_measurement(file, NULL, MAY_EXEC, MODULE_CHECK); } +int ima_fw_from_file(struct file *file, char *buf, size_t size) +{ + if (!file) { + if ((ima_appraise & IMA_APPRAISE_FIRMWARE) && + (ima_appraise & IMA_APPRAISE_ENFORCE)) + return -EACCES; /* INTEGRITY_UNKNOWN */ + return 0; + } + return process_measurement(file, NULL, MAY_EXEC, FIRMWARE_CHECK); +} + static int __init init_ima(void) { int error; diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index cea84d8bd7be..07099a8bc283 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -84,6 +84,7 @@ static struct ima_rule_entry default_rules[] = { {.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 = FIRMWARE_CHECK, .flags = IMA_FUNC}, }; static struct ima_rule_entry default_appraise_rules[] = { @@ -241,6 +242,8 @@ static int get_subaction(struct ima_rule_entry *rule, int func) return IMA_BPRM_APPRAISE; case MODULE_CHECK: return IMA_MODULE_APPRAISE; + case FIRMWARE_CHECK: + return IMA_FIRMWARE_APPRAISE; case FILE_CHECK: default: return IMA_FILE_APPRAISE; @@ -486,6 +489,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) entry->func = FILE_CHECK; else if (strcmp(args[0].from, "MODULE_CHECK") == 0) entry->func = MODULE_CHECK; + else if (strcmp(args[0].from, "FIRMWARE_CHECK") == 0) + entry->func = FIRMWARE_CHECK; else if ((strcmp(args[0].from, "FILE_MMAP") == 0) || (strcmp(args[0].from, "MMAP_CHECK") == 0)) entry->func = MMAP_CHECK; @@ -636,6 +641,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) result = -EINVAL; else if (entry->func == MODULE_CHECK) ima_appraise |= IMA_APPRAISE_MODULES; + else if (entry->func == FIRMWARE_CHECK) + ima_appraise |= IMA_APPRAISE_FIRMWARE; audit_log_format(ab, "res=%d", !result); audit_log_end(ab); return result; diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index 09c440d9aaee..19b8e314ca96 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -46,10 +46,14 @@ #define IMA_BPRM_APPRAISED 0x00002000 #define IMA_MODULE_APPRAISE 0x00004000 #define IMA_MODULE_APPRAISED 0x00008000 +#define IMA_FIRMWARE_APPRAISE 0x00010000 +#define IMA_FIRMWARE_APPRAISED 0x00020000 #define IMA_APPRAISE_SUBMASK (IMA_FILE_APPRAISE | IMA_MMAP_APPRAISE | \ - IMA_BPRM_APPRAISE | IMA_MODULE_APPRAISE) + IMA_BPRM_APPRAISE | IMA_MODULE_APPRAISE | \ + IMA_FIRMWARE_APPRAISE) #define IMA_APPRAISED_SUBMASK (IMA_FILE_APPRAISED | IMA_MMAP_APPRAISED | \ - IMA_BPRM_APPRAISED | IMA_MODULE_APPRAISED) + IMA_BPRM_APPRAISED | IMA_MODULE_APPRAISED | \ + IMA_FIRMWARE_APPRAISED) enum evm_ima_xattr_type { IMA_XATTR_DIGEST = 0x01, @@ -104,6 +108,7 @@ struct integrity_iint_cache { enum integrity_status ima_mmap_status:4; enum integrity_status ima_bprm_status:4; enum integrity_status ima_module_status:4; + enum integrity_status ima_firmware_status:4; enum integrity_status evm_status:4; struct ima_digest_data *ima_hash; }; diff --git a/security/security.c b/security/security.c index 35d37d0f0d49..e41b1a8d7644 100644 --- a/security/security.c +++ b/security/security.c @@ -847,7 +847,12 @@ int security_kernel_create_files_as(struct cred *new, struct inode *inode) int security_kernel_fw_from_file(struct file *file, char *buf, size_t size) { - return security_ops->kernel_fw_from_file(file, buf, size); + int ret; + + ret = security_ops->kernel_fw_from_file(file, buf, size); + if (ret) + return ret; + return ima_fw_from_file(file, buf, size); } EXPORT_SYMBOL_GPL(security_kernel_fw_from_file); -- cgit v1.2.3 From b64cc5fb85f38ae7ca3c67a8fea9ad8c0d068bfa Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Sat, 26 Jul 2014 23:21:02 -0400 Subject: KEYS: revert encrypted key change Commit fc7c70e "KEYS: struct key_preparsed_payload should have two payload pointers" erroneously modified encrypted-keys. This patch reverts the change to that file. Signed-off-by: Mimi Zohar Signed-off-by: David Howells --- security/keys/encrypted-keys/encrypted.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c index d252c5704f8a..5fe443d120af 100644 --- a/security/keys/encrypted-keys/encrypted.c +++ b/security/keys/encrypted-keys/encrypted.c @@ -811,7 +811,7 @@ static int encrypted_instantiate(struct key *key, goto out; } - prep->payload[0] = epayload; + rcu_assign_keypointer(key, epayload); out: kfree(datablob); return ret; -- cgit v1.2.3 From 2873ead7e46694910ac49c3a8ee0f54956f96e0c Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Mon, 28 Jul 2014 10:42:48 -0400 Subject: Revert "selinux: fix the default socket labeling in sock_graft()" This reverts commit 4da6daf4d3df5a977e4623963f141a627fd2efce. Unfortunately, the commit in question caused problems with Bluetooth devices, specifically it caused them to get caught in the newly created BUG_ON() check. The AF_ALG problem still exists, but will be addressed in a future patch. Cc: stable@vger.kernel.org Signed-off-by: Paul Moore --- security/selinux/hooks.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index b3a6754e932b..336f0a04450e 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -4499,18 +4499,9 @@ static void selinux_sock_graft(struct sock *sk, struct socket *parent) struct inode_security_struct *isec = SOCK_INODE(parent)->i_security; struct sk_security_struct *sksec = sk->sk_security; - switch (sk->sk_family) { - case PF_INET: - case PF_INET6: - case PF_UNIX: + if (sk->sk_family == PF_INET || sk->sk_family == PF_INET6 || + sk->sk_family == PF_UNIX) isec->sid = sksec->sid; - break; - default: - /* by default there is no special labeling mechanism for the - * sksec label so inherit the label from the parent socket */ - BUG_ON(sksec->sid != SECINITSID_UNLABELED); - sksec->sid = isec->sid; - } sksec->sclass = isec->sclass; } -- cgit v1.2.3 From 41c3bd2039e0d7b3dc32313141773f20716ec524 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Fri, 1 Aug 2014 11:17:03 -0400 Subject: netlabel: fix a problem when setting bits below the previously lowest bit The NetLabel category (catmap) functions have a problem in that they assume categories will be set in an increasing manner, e.g. the next category set will always be larger than the last. Unfortunately, this is not a valid assumption and could result in problems when attempting to set categories less than the startbit in the lowest catmap node. In some cases kernel panics and other nasties can result. This patch corrects the problem by checking for this and allocating a new catmap node instance and placing it at the front of the list. Cc: stable@vger.kernel.org Reported-by: Christian Evans Signed-off-by: Paul Moore Tested-by: Casey Schaufler --- security/smack/smack_access.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c index 14293cd9b1e5..9ecf4f4b67a1 100644 --- a/security/smack/smack_access.c +++ b/security/smack/smack_access.c @@ -444,7 +444,7 @@ int smk_netlbl_mls(int level, char *catset, struct netlbl_lsm_secattr *sap, for (m = 0x80; m != 0; m >>= 1, cat++) { if ((m & *cp) == 0) continue; - rc = netlbl_secattr_catmap_setbit(sap->attr.mls.cat, + rc = netlbl_secattr_catmap_setbit(&sap->attr.mls.cat, cat, GFP_ATOMIC); if (rc < 0) { netlbl_secattr_catmap_free(sap->attr.mls.cat); -- cgit v1.2.3 From 4b8feff251da3d7058b5779e21b33a85c686b974 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Fri, 1 Aug 2014 11:17:17 -0400 Subject: netlabel: fix the horribly broken catmap functions The NetLabel secattr catmap functions, and the SELinux import/export glue routines, were broken in many horrible ways and the SELinux glue code fiddled with the NetLabel catmap structures in ways that we probably shouldn't allow. At some point this "worked", but that was likely due to a bit of dumb luck and sub-par testing (both inflicted by yours truly). This patch corrects these problems by basically gutting the code in favor of something less obtuse and restoring the NetLabel abstractions in the SELinux catmap glue code. Everything is working now, and if it decides to break itself in the future this code will be much easier to debug than the code it replaces. One noteworthy side effect of the changes is that it is no longer necessary to allocate a NetLabel catmap before calling one of the NetLabel APIs to set a bit in the catmap. NetLabel will automatically allocate the catmap nodes when needed, resulting in less allocations when the lowest bit is greater than 255 and less code in the LSMs. Cc: stable@vger.kernel.org Reported-by: Christian Evans Signed-off-by: Paul Moore Tested-by: Casey Schaufler --- security/selinux/ss/ebitmap.c | 127 ++++++++++++++++-------------------------- security/smack/smack_access.c | 5 +- 2 files changed, 50 insertions(+), 82 deletions(-) (limited to 'security') diff --git a/security/selinux/ss/ebitmap.c b/security/selinux/ss/ebitmap.c index 820313a04d49..842deca9484d 100644 --- a/security/selinux/ss/ebitmap.c +++ b/security/selinux/ss/ebitmap.c @@ -89,48 +89,33 @@ int ebitmap_netlbl_export(struct ebitmap *ebmap, struct netlbl_lsm_secattr_catmap **catmap) { struct ebitmap_node *e_iter = ebmap->node; - struct netlbl_lsm_secattr_catmap *c_iter; - u32 cmap_idx, cmap_sft; - int i; - - /* NetLabel's NETLBL_CATMAP_MAPTYPE is defined as an array of u64, - * however, it is not always compatible with an array of unsigned long - * in ebitmap_node. - * In addition, you should pay attention the following implementation - * assumes unsigned long has a width equal with or less than 64-bit. - */ + unsigned long e_map; + u32 offset; + unsigned int iter; + int rc; if (e_iter == NULL) { *catmap = NULL; return 0; } - c_iter = netlbl_secattr_catmap_alloc(GFP_ATOMIC); - if (c_iter == NULL) - return -ENOMEM; - *catmap = c_iter; - c_iter->startbit = e_iter->startbit & ~(NETLBL_CATMAP_SIZE - 1); + if (*catmap != NULL) + netlbl_secattr_catmap_free(*catmap); + *catmap = NULL; while (e_iter) { - for (i = 0; i < EBITMAP_UNIT_NUMS; i++) { - unsigned int delta, e_startbit, c_endbit; - - e_startbit = e_iter->startbit + i * EBITMAP_UNIT_SIZE; - c_endbit = c_iter->startbit + NETLBL_CATMAP_SIZE; - if (e_startbit >= c_endbit) { - c_iter->next - = netlbl_secattr_catmap_alloc(GFP_ATOMIC); - if (c_iter->next == NULL) + offset = e_iter->startbit; + for (iter = 0; iter < EBITMAP_UNIT_NUMS; iter++) { + e_map = e_iter->maps[iter]; + if (e_map != 0) { + rc = netlbl_secattr_catmap_setlong(catmap, + offset, + e_map, + GFP_ATOMIC); + if (rc != 0) goto netlbl_export_failure; - c_iter = c_iter->next; - c_iter->startbit - = e_startbit & ~(NETLBL_CATMAP_SIZE - 1); } - delta = e_startbit - c_iter->startbit; - cmap_idx = delta / NETLBL_CATMAP_MAPSIZE; - cmap_sft = delta % NETLBL_CATMAP_MAPSIZE; - c_iter->bitmap[cmap_idx] - |= e_iter->maps[i] << cmap_sft; + offset += EBITMAP_UNIT_SIZE; } e_iter = e_iter->next; } @@ -155,56 +140,42 @@ netlbl_export_failure: int ebitmap_netlbl_import(struct ebitmap *ebmap, struct netlbl_lsm_secattr_catmap *catmap) { + int rc; struct ebitmap_node *e_iter = NULL; - struct ebitmap_node *emap_prev = NULL; - struct netlbl_lsm_secattr_catmap *c_iter = catmap; - u32 c_idx, c_pos, e_idx, e_sft; - - /* NetLabel's NETLBL_CATMAP_MAPTYPE is defined as an array of u64, - * however, it is not always compatible with an array of unsigned long - * in ebitmap_node. - * In addition, you should pay attention the following implementation - * assumes unsigned long has a width equal with or less than 64-bit. - */ - - do { - for (c_idx = 0; c_idx < NETLBL_CATMAP_MAPCNT; c_idx++) { - unsigned int delta; - u64 map = c_iter->bitmap[c_idx]; - - if (!map) - continue; + struct ebitmap_node *e_prev = NULL; + u32 offset = 0, idx; + unsigned long bitmap; + + for (;;) { + rc = netlbl_secattr_catmap_getlong(catmap, &offset, &bitmap); + if (rc < 0) + goto netlbl_import_failure; + if (offset == (u32)-1) + return 0; - c_pos = c_iter->startbit - + c_idx * NETLBL_CATMAP_MAPSIZE; - if (!e_iter - || c_pos >= e_iter->startbit + EBITMAP_SIZE) { - e_iter = kzalloc(sizeof(*e_iter), GFP_ATOMIC); - if (!e_iter) - goto netlbl_import_failure; - e_iter->startbit - = c_pos - (c_pos % EBITMAP_SIZE); - if (emap_prev == NULL) - ebmap->node = e_iter; - else - emap_prev->next = e_iter; - emap_prev = e_iter; - } - delta = c_pos - e_iter->startbit; - e_idx = delta / EBITMAP_UNIT_SIZE; - e_sft = delta % EBITMAP_UNIT_SIZE; - while (map) { - e_iter->maps[e_idx++] |= map & (-1UL); - map = EBITMAP_SHIFT_UNIT_SIZE(map); - } + if (e_iter == NULL || + offset >= e_iter->startbit + EBITMAP_SIZE) { + e_prev = e_iter; + e_iter = kzalloc(sizeof(*e_iter), GFP_ATOMIC); + if (e_iter == NULL) + goto netlbl_import_failure; + e_iter->startbit = offset & ~(EBITMAP_SIZE - 1); + if (e_prev == NULL) + ebmap->node = e_iter; + else + e_prev->next = e_iter; + ebmap->highbit = e_iter->startbit + EBITMAP_SIZE; } - c_iter = c_iter->next; - } while (c_iter); - if (e_iter != NULL) - ebmap->highbit = e_iter->startbit + EBITMAP_SIZE; - else - ebitmap_destroy(ebmap); + /* offset will always be aligned to an unsigned long */ + idx = EBITMAP_NODE_INDEX(e_iter, offset); + e_iter->maps[idx] = bitmap; + + /* next */ + offset += EBITMAP_UNIT_SIZE; + } + + /* NOTE: we should never reach this return */ return 0; netlbl_import_failure: diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c index 9ecf4f4b67a1..ea1bc5055792 100644 --- a/security/smack/smack_access.c +++ b/security/smack/smack_access.c @@ -435,10 +435,7 @@ int smk_netlbl_mls(int level, char *catset, struct netlbl_lsm_secattr *sap, sap->flags |= NETLBL_SECATTR_MLS_CAT; sap->attr.mls.lvl = level; - sap->attr.mls.cat = netlbl_secattr_catmap_alloc(GFP_ATOMIC); - if (!sap->attr.mls.cat) - return -ENOMEM; - sap->attr.mls.cat->startbit = 0; + sap->attr.mls.cat = NULL; for (cat = 1, cp = catset, byte = 0; byte < len; cp++, byte++) for (m = 0x80; m != 0; m >>= 1, cat++) { -- cgit v1.2.3 From 4fbe63d1c773cceef3fe1f6ed0c9c268f4f24760 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Fri, 1 Aug 2014 11:17:37 -0400 Subject: netlabel: shorter names for the NetLabel catmap funcs/structs Historically the NetLabel LSM secattr catmap functions and data structures have had very long names which makes a mess of the NetLabel code and anyone who uses NetLabel. This patch renames the catmap functions and structures from "*_secattr_catmap_*" to just "*_catmap_*" which improves things greatly. There are no substantial code or logic changes in this patch. Signed-off-by: Paul Moore Tested-by: Casey Schaufler --- security/selinux/ss/ebitmap.c | 18 +++++++++--------- security/selinux/ss/ebitmap.h | 8 ++++---- security/smack/smack_access.c | 6 +++--- security/smack/smack_lsm.c | 6 +++--- security/smack/smackfs.c | 14 +++++++------- 5 files changed, 26 insertions(+), 26 deletions(-) (limited to 'security') diff --git a/security/selinux/ss/ebitmap.c b/security/selinux/ss/ebitmap.c index 842deca9484d..afe6a269ec17 100644 --- a/security/selinux/ss/ebitmap.c +++ b/security/selinux/ss/ebitmap.c @@ -86,7 +86,7 @@ int ebitmap_cpy(struct ebitmap *dst, struct ebitmap *src) * */ int ebitmap_netlbl_export(struct ebitmap *ebmap, - struct netlbl_lsm_secattr_catmap **catmap) + struct netlbl_lsm_catmap **catmap) { struct ebitmap_node *e_iter = ebmap->node; unsigned long e_map; @@ -100,7 +100,7 @@ int ebitmap_netlbl_export(struct ebitmap *ebmap, } if (*catmap != NULL) - netlbl_secattr_catmap_free(*catmap); + netlbl_catmap_free(*catmap); *catmap = NULL; while (e_iter) { @@ -108,10 +108,10 @@ int ebitmap_netlbl_export(struct ebitmap *ebmap, for (iter = 0; iter < EBITMAP_UNIT_NUMS; iter++) { e_map = e_iter->maps[iter]; if (e_map != 0) { - rc = netlbl_secattr_catmap_setlong(catmap, - offset, - e_map, - GFP_ATOMIC); + rc = netlbl_catmap_setlong(catmap, + offset, + e_map, + GFP_ATOMIC); if (rc != 0) goto netlbl_export_failure; } @@ -123,7 +123,7 @@ int ebitmap_netlbl_export(struct ebitmap *ebmap, return 0; netlbl_export_failure: - netlbl_secattr_catmap_free(*catmap); + netlbl_catmap_free(*catmap); return -ENOMEM; } @@ -138,7 +138,7 @@ netlbl_export_failure: * */ int ebitmap_netlbl_import(struct ebitmap *ebmap, - struct netlbl_lsm_secattr_catmap *catmap) + struct netlbl_lsm_catmap *catmap) { int rc; struct ebitmap_node *e_iter = NULL; @@ -147,7 +147,7 @@ int ebitmap_netlbl_import(struct ebitmap *ebmap, unsigned long bitmap; for (;;) { - rc = netlbl_secattr_catmap_getlong(catmap, &offset, &bitmap); + rc = netlbl_catmap_getlong(catmap, &offset, &bitmap); if (rc < 0) goto netlbl_import_failure; if (offset == (u32)-1) diff --git a/security/selinux/ss/ebitmap.h b/security/selinux/ss/ebitmap.h index 712c8a7b8e8b..9637b8c71085 100644 --- a/security/selinux/ss/ebitmap.h +++ b/security/selinux/ss/ebitmap.h @@ -132,17 +132,17 @@ int ebitmap_write(struct ebitmap *e, void *fp); #ifdef CONFIG_NETLABEL int ebitmap_netlbl_export(struct ebitmap *ebmap, - struct netlbl_lsm_secattr_catmap **catmap); + struct netlbl_lsm_catmap **catmap); int ebitmap_netlbl_import(struct ebitmap *ebmap, - struct netlbl_lsm_secattr_catmap *catmap); + struct netlbl_lsm_catmap *catmap); #else static inline int ebitmap_netlbl_export(struct ebitmap *ebmap, - struct netlbl_lsm_secattr_catmap **catmap) + struct netlbl_lsm_catmap **catmap) { return -ENOMEM; } static inline int ebitmap_netlbl_import(struct ebitmap *ebmap, - struct netlbl_lsm_secattr_catmap *catmap) + struct netlbl_lsm_catmap *catmap) { return -ENOMEM; } diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c index ea1bc5055792..732df7b91227 100644 --- a/security/smack/smack_access.c +++ b/security/smack/smack_access.c @@ -441,10 +441,10 @@ int smk_netlbl_mls(int level, char *catset, struct netlbl_lsm_secattr *sap, for (m = 0x80; m != 0; m >>= 1, cat++) { if ((m & *cp) == 0) continue; - rc = netlbl_secattr_catmap_setbit(&sap->attr.mls.cat, - cat, GFP_ATOMIC); + rc = netlbl_catmap_setbit(&sap->attr.mls.cat, + cat, GFP_ATOMIC); if (rc < 0) { - netlbl_secattr_catmap_free(sap->attr.mls.cat); + netlbl_catmap_free(sap->attr.mls.cat); return rc; } } diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 14f52be78c75..c32bba566df9 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -3091,9 +3091,9 @@ static struct smack_known *smack_from_secattr(struct netlbl_lsm_secattr *sap, break; } for (acat = -1, kcat = -1; acat == kcat; ) { - acat = netlbl_secattr_catmap_walk( - sap->attr.mls.cat, acat + 1); - kcat = netlbl_secattr_catmap_walk( + acat = netlbl_catmap_walk(sap->attr.mls.cat, + acat + 1); + kcat = netlbl_catmap_walk( skp->smk_netlabel.attr.mls.cat, kcat + 1); if (acat < 0 || kcat < 0) diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c index 3198cfe1dcc6..893b06b93f6d 100644 --- a/security/smack/smackfs.c +++ b/security/smack/smackfs.c @@ -777,7 +777,7 @@ static int cipso_seq_show(struct seq_file *s, void *v) struct list_head *list = v; struct smack_known *skp = list_entry(list, struct smack_known, list); - struct netlbl_lsm_secattr_catmap *cmp = skp->smk_netlabel.attr.mls.cat; + struct netlbl_lsm_catmap *cmp = skp->smk_netlabel.attr.mls.cat; char sep = '/'; int i; @@ -794,8 +794,8 @@ static int cipso_seq_show(struct seq_file *s, void *v) seq_printf(s, "%s %3d", skp->smk_known, skp->smk_netlabel.attr.mls.lvl); - for (i = netlbl_secattr_catmap_walk(cmp, 0); i >= 0; - i = netlbl_secattr_catmap_walk(cmp, i + 1)) { + for (i = netlbl_catmap_walk(cmp, 0); i >= 0; + i = netlbl_catmap_walk(cmp, i + 1)) { seq_printf(s, "%c%d", sep, i); sep = ','; } @@ -916,7 +916,7 @@ static ssize_t smk_set_cipso(struct file *file, const char __user *buf, rc = smk_netlbl_mls(maplevel, mapcatset, &ncats, SMK_CIPSOLEN); if (rc >= 0) { - netlbl_secattr_catmap_free(skp->smk_netlabel.attr.mls.cat); + netlbl_catmap_free(skp->smk_netlabel.attr.mls.cat); skp->smk_netlabel.attr.mls.cat = ncats.attr.mls.cat; skp->smk_netlabel.attr.mls.lvl = ncats.attr.mls.lvl; rc = count; @@ -966,14 +966,14 @@ static int cipso2_seq_show(struct seq_file *s, void *v) struct list_head *list = v; struct smack_known *skp = list_entry(list, struct smack_known, list); - struct netlbl_lsm_secattr_catmap *cmp = skp->smk_netlabel.attr.mls.cat; + struct netlbl_lsm_catmap *cmp = skp->smk_netlabel.attr.mls.cat; char sep = '/'; int i; seq_printf(s, "%s %3d", skp->smk_known, skp->smk_netlabel.attr.mls.lvl); - for (i = netlbl_secattr_catmap_walk(cmp, 0); i >= 0; - i = netlbl_secattr_catmap_walk(cmp, i + 1)) { + for (i = netlbl_catmap_walk(cmp, 0); i >= 0; + i = netlbl_catmap_walk(cmp, i + 1)) { seq_printf(s, "%c%d", sep, i); sep = ','; } -- cgit v1.2.3