lkml.org 
[lkml]   [2022]   [Aug]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Date
SubjectRe: memory leak in cap_inode_getsecurity
#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@syzkaller.appspotmail.com> 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@syzkaller.appspotmail.com
>
> 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@googlegroups.com.
>
> 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@redhat.com>
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@syzkaller.appspotmail.com
Fixes: 1601fbad2b14 ("xattr: define vfs_getxattr_alloc and vfs_xattr_cmp")
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
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;
}
\
 
 \ /
  Last update: 2022-08-03 06:37    [W:0.089 / U:0.312 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site