On 11/8/2014 6:48 AM, Andrey Ryabinin wrote:
> From: Andrey Ryabinin <a.ryabi...@samsung.com>
>
> 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 <a.ryabi...@samsung.com>
Applied to git://git.gitorious.org/smack-next/kernel.git#smack-for-3.19
> ---
> 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 5b970ff..ad75ddf 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;
> }
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
