From 6c892df2686c5611979792aaa4ddea9ee9f18749 Mon Sep 17 00:00:00 2001 From: Casey Schaufler Date: Thu, 9 Oct 2014 16:18:55 -0700 Subject: Smack: Lock mode for the floor and hat labels The lock access mode allows setting a read lock on a file for with the process has only read access. The floor label is defined to make it easy to have the basic system installed such that everyone can read it. Once there's a desire to read lock (rationally or otherwise) a floor file a rule needs to get set. This happens all the time, so make the floor label a little bit more special and allow everyone lock access, too. By implication, give processes with the hat label (hat can read everything) lock access as well. This reduces clutter in the Smack rule set. Signed-off-by: Casey Schaufler --- security/smack/smack_access.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c index 5b970ffde024..999224fe8593 100644 --- a/security/smack/smack_access.c +++ b/security/smack/smack_access.c @@ -142,8 +142,7 @@ int smk_access(struct smack_known *subject, struct smack_known *object, * Tasks cannot be assigned the internet label. * An internet subject can access any object. */ - if (object == &smack_known_web || - subject == &smack_known_web) + if (object == &smack_known_web || subject == &smack_known_web) goto out_audit; /* * A star object can be accessed by any subject. @@ -157,10 +156,11 @@ int smk_access(struct smack_known *subject, struct smack_known *object, if (subject->smk_known == object->smk_known) goto out_audit; /* - * A hat subject can read any object. - * A floor object can be read by any subject. + * A hat subject can read or lock any object. + * A floor object can be read or locked by any subject. */ - if ((request & MAY_ANYREAD) == request) { + if ((request & MAY_ANYREAD) == request || + (request & MAY_LOCK) == request) { if (object == &smack_known_floor) goto out_audit; if (subject == &smack_known_hat) -- cgit v1.2.3 From 1a5b472bde752783e0a31b59c61c9ff5b37a0983 Mon Sep 17 00:00:00 2001 From: Rohit Date: Wed, 15 Oct 2014 17:40:41 +0530 Subject: Security: smack: replace kzalloc with kmem_cache for inode_smack The patch use kmem_cache to allocate/free inode_smack since they are alloced in high volumes making it a perfect case for kmem_cache. As per analysis, 24 bytes of memory is wasted per allocation due to internal fragmentation. With kmem_cache, this can be avoided. Accounting of memory allocation is below : total slack net count-alloc/free caller Before (with kzalloc) 1919872 719952 1919872 29998/0 new_inode_smack+0x14 After (with kmem_cache) 1201680 0 1201680 30042/0 new_inode_smack+0x18 >From above data, we found that 719952 bytes(~700 KB) of memory is saved on allocation of 29998 smack inodes. Signed-off-by: Rohit --- security/smack/smack_lsm.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 93dc876734a4..2717cdd7872c 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -53,6 +53,7 @@ #define SMK_SENDING 2 LIST_HEAD(smk_ipv6_port_list); +static struct kmem_cache *smack_inode_cache; #ifdef CONFIG_SECURITY_SMACK_BRINGUP static void smk_bu_mode(int mode, char *s) @@ -240,7 +241,7 @@ struct inode_smack *new_inode_smack(struct smack_known *skp) { struct inode_smack *isp; - isp = kzalloc(sizeof(struct inode_smack), GFP_NOFS); + isp = kmem_cache_zalloc(smack_inode_cache, GFP_NOFS); if (isp == NULL) return NULL; @@ -767,7 +768,7 @@ static int smack_inode_alloc_security(struct inode *inode) */ static void smack_inode_free_security(struct inode *inode) { - kfree(inode->i_security); + kmem_cache_free(smack_inode_cache, inode->i_security); inode->i_security = NULL; } @@ -4265,10 +4266,16 @@ static __init int smack_init(void) if (!security_module_enable(&smack_ops)) return 0; + smack_inode_cache = KMEM_CACHE(inode_smack, 0); + if (!smack_inode_cache) + return -ENOMEM; + tsp = new_task_smack(&smack_known_floor, &smack_known_floor, GFP_KERNEL); - if (tsp == NULL) + if (tsp == NULL) { + kmem_cache_destroy(smack_inode_cache); return -ENOMEM; + } printk(KERN_INFO "Smack: Initializing.\n"); -- cgit v1.2.3 From 5c1b66240b7f4abc29c618a768121d6a00f4c95a Mon Sep 17 00:00:00 2001 From: Andrey Ryabinin Date: Sat, 8 Nov 2014 17:48:05 +0300 Subject: security: smack: fix out-of-bounds access in smk_parse_smack() Setting smack label on file (e.g. 'attr -S -s SMACK64 -V "test" test') triggered following spew on the kernel with KASan applied: ================================================================== BUG: AddressSanitizer: out of bounds access in strncpy+0x28/0x60 at addr ffff8800059ad064 ============================================================================= BUG kmalloc-8 (Not tainted): kasan error ----------------------------------------------------------------------------- Disabling lock debugging due to kernel taint INFO: Slab 0xffffea0000166b40 objects=128 used=7 fp=0xffff8800059ad080 flags=0x4000000000000080 INFO: Object 0xffff8800059ad060 @offset=96 fp=0xffff8800059ad080 Bytes b4 ffff8800059ad050: a0 df 9a 05 00 88 ff ff 5a 5a 5a 5a 5a 5a 5a 5a ........ZZZZZZZZ Object ffff8800059ad060: 74 65 73 74 6b 6b 6b a5 testkkk. Redzone ffff8800059ad068: cc cc cc cc cc cc cc cc ........ Padding ffff8800059ad078: 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZ CPU: 0 PID: 528 Comm: attr Tainted: G B 3.18.0-rc1-mm1+ #5 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 0000000000000000 ffff8800059ad064 ffffffff81534cf2 ffff880005a5bc40 ffffffff8112fe1a 0000000100800006 0000000f059ad060 ffff880006000f90 0000000000000296 ffffea0000166b40 ffffffff8107ca97 ffff880005891060 Call Trace: ? dump_stack (lib/dump_stack.c:52) ? kasan_report_error (mm/kasan/report.c:102 mm/kasan/report.c:178) ? preempt_count_sub (kernel/sched/core.c:2651) ? __asan_load1 (mm/kasan/kasan.h:50 mm/kasan/kasan.c:248 mm/kasan/kasan.c:358) ? strncpy (lib/string.c:121) ? strncpy (lib/string.c:121) ? smk_parse_smack (security/smack/smack_access.c:457) ? setxattr (fs/xattr.c:343) ? smk_import_entry (security/smack/smack_access.c:514) ? smack_inode_setxattr (security/smack/smack_lsm.c:1093 (discriminator 1)) ? security_inode_setxattr (security/security.c:602) ? vfs_setxattr (fs/xattr.c:134) ? setxattr (fs/xattr.c:343) ? setxattr (fs/xattr.c:360) ? get_parent_ip (kernel/sched/core.c:2606) ? preempt_count_sub (kernel/sched/core.c:2651) ? __percpu_counter_add (arch/x86/include/asm/preempt.h:98 lib/percpu_counter.c:90) ? get_parent_ip (kernel/sched/core.c:2606) ? preempt_count_sub (kernel/sched/core.c:2651) ? __mnt_want_write (arch/x86/include/asm/preempt.h:98 fs/namespace.c:359) ? path_setxattr (fs/xattr.c:380) ? SyS_lsetxattr (fs/xattr.c:397) ? system_call_fastpath (arch/x86/kernel/entry_64.S:423) Read of size 1 by task attr: Memory state around the buggy address: ffff8800059ace80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ffff8800059acf00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ffff8800059acf80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >ffff8800059ad000: 00 fc fc fc 00 fc fc fc 05 fc fc fc 04 fc fc fc ^ ffff8800059ad080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb ffff8800059ad100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb ffff8800059ad180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb ================================================================== strncpy() copies one byte more than the source string has. Fix this by passing the correct length to strncpy(). Now we can remove initialization of the last byte in 'smack' string because kzalloc() already did this for us. Signed-off-by: Andrey Ryabinin --- security/smack/smack_access.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c index 999224fe8593..1158430f5bb9 100644 --- a/security/smack/smack_access.c +++ b/security/smack/smack_access.c @@ -452,10 +452,9 @@ char *smk_parse_smack(const char *string, int len) return NULL; smack = kzalloc(i + 1, GFP_KERNEL); - if (smack != NULL) { - strncpy(smack, string, i + 1); - smack[i] = '\0'; - } + if (smack != NULL) + strncpy(smack, string, i); + return smack; } -- cgit v1.2.3