Re: memory leak in cap_inode_getsecurity

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
master

On Thu, 3 Oct 2019 at 21:59, syzbot
<syzbot+942d5390db2d9624ced8@xxxxxxxxxxxxxxxxxxxxxxxxx> wrote:
>
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit:    0f1a7b3f timer-of: don't use conditional expression with m..
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=1329640d600000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=9d66badf12ef344c
> dashboard link: https://syzkaller.appspot.com/bug?extid=942d5390db2d9624ced8
> compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=1107b513600000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+942d5390db2d9624ced8@xxxxxxxxxxxxxxxxxxxxxxxxx
>
> 2019/10/03 14:00:37 executed programs: 36
> 2019/10/03 14:00:43 executed programs: 44
> 2019/10/03 14:00:49 executed programs: 63
> BUG: memory leak
> unreferenced object 0xffff8881202cb480 (size 32):
>    comm "syz-executor.0", pid 7246, jiffies 4294946879 (age 14.010s)
>    hex dump (first 32 bytes):
>      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>    backtrace:
>      [<00000000a8379648>] kmemleak_alloc_recursive
> include/linux/kmemleak.h:43 [inline]
>      [<00000000a8379648>] slab_post_alloc_hook mm/slab.h:586 [inline]
>      [<00000000a8379648>] slab_alloc mm/slab.c:3319 [inline]
>      [<00000000a8379648>] __do_kmalloc mm/slab.c:3653 [inline]
>      [<00000000a8379648>] __kmalloc_track_caller+0x165/0x300 mm/slab.c:3670
>      [<000000008858463c>] __do_krealloc mm/slab_common.c:1638 [inline]
>      [<000000008858463c>] krealloc+0x7f/0xb0 mm/slab_common.c:1689
>      [<0000000057f9eb8e>] vfs_getxattr_alloc+0x100/0x180 fs/xattr.c:289
>      [<00000000c2154e30>] cap_inode_getsecurity+0x9c/0x2c0
> security/commoncap.c:389
>      [<00000000b2664a09>] security_inode_getsecurity+0x4c/0x90
> security/security.c:1314
>      [<00000000921624c0>] xattr_getsecurity fs/xattr.c:244 [inline]
>      [<00000000921624c0>] vfs_getxattr+0xf2/0x1a0 fs/xattr.c:332
>      [<000000001ff6977b>] getxattr+0x97/0x240 fs/xattr.c:538
>      [<00000000b945681f>] path_getxattr+0x6b/0xc0 fs/xattr.c:566
>      [<000000001a9d3fce>] __do_sys_getxattr fs/xattr.c:578 [inline]
>      [<000000001a9d3fce>] __se_sys_getxattr fs/xattr.c:575 [inline]
>      [<000000001a9d3fce>] __x64_sys_getxattr+0x28/0x30 fs/xattr.c:575
>      [<000000002e998337>] do_syscall_64+0x73/0x1f0
> arch/x86/entry/common.c:290
>      [<00000000f252aa21>] entry_SYSCALL_64_after_hwframe+0x44/0xa9
>
>
>
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@xxxxxxxxxxxxxxxx.
>
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
> syzbot can test patches for this bug, for details see:
> https://goo.gl/tpsmEJ#testing-patches
From: Miklos Szeredi <mszeredi@xxxxxxxxxx>
Subject: vfs_getxattr_alloc(): don't allocate buf on failure

Some callers of vfs_getxattr_alloc() assume that on failure the allocated
buffer does not need to be freed.

Callers could be fixed, but fixing the semantics of vfs_getxattr_alloc() is
simpler and makes sure that this class of bugs does not occur again.

If this was called in a loop (i.e. xattr_value contains an already
allocated buffer), then caller will still need to clean up after an error.

Reported-and-tested-by: syzbot+942d5390db2d9624ced8@xxxxxxxxxxxxxxxxxxxxxxxxx
Fixes: 1601fbad2b14 ("xattr: define vfs_getxattr_alloc and vfs_xattr_cmp")
Signed-off-by: Miklos Szeredi <mszeredi@xxxxxxxxxx>
---
 fs/xattr.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -383,7 +383,10 @@ vfs_getxattr_alloc(struct user_namespace
 	}
 
 	error = handler->get(handler, dentry, inode, name, value, error);
-	*xattr_value = value;
+	if (error < 0 && *xattr_value == NULL)
+		kfree(value);
+	else
+		*xattr_value = value;
 	return error;
 }
 

[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux