On 11/13/20 9:17 AM, syzbot wrote: > Hello, > > syzbot found the following issue on: > > HEAD commit: af5043c8 Merge tag 'acpi-5.10-rc4' of git://git.kernel.org.. > git tree: upstream > console output: https://syzkaller.appspot.com/x/log.txt?x=13e8c906500000 > kernel config: https://syzkaller.appspot.com/x/.config?x=a3f13716fa0212fd > dashboard link: https://syzkaller.appspot.com/bug?extid=86dc6632faaca40133ab > compiler: gcc (GCC) 10.1.0-syz 20200507 > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=102a57dc500000 > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=129ca3d6500000 > > IMPORTANT: if you fix the issue, please add the following tag to the commit: > Reported-by: syzbot+86dc6632faaca40133ab@xxxxxxxxxxxxxxxxxxxxxxxxx > > Warning: Permanently added '10.128.0.84' (ECDSA) to the list of known hosts. > executing program > executing program > BUG: memory leak > unreferenced object 0xffff888111f15a80 (size 32): > comm "syz-executor841", pid 8507, jiffies 4294942125 (age 14.070s) > hex dump (first 32 bytes): > 25 5e 5d 24 5b 2b 25 5d 28 24 7b 3a 0f 6b 5b 29 %^]$[+%](${:.k[) > 2d 3a 00 00 00 00 00 00 00 00 00 00 00 00 00 00 -:.............. > backtrace: > [<000000005c6f565d>] kmemdup_nul+0x2d/0x70 mm/util.c:151 > [<0000000054985c27>] vfs_parse_fs_string+0x6e/0xd0 fs/fs_context.c:155 > [<0000000077ef66e4>] generic_parse_monolithic+0xe0/0x130 fs/fs_context.c:201 > [<00000000d4d4a652>] do_new_mount fs/namespace.c:2871 [inline] > [<00000000d4d4a652>] path_mount+0xbbb/0x1170 fs/namespace.c:3205 > [<00000000f43f0071>] do_mount fs/namespace.c:3218 [inline] > [<00000000f43f0071>] __do_sys_mount fs/namespace.c:3426 [inline] > [<00000000f43f0071>] __se_sys_mount fs/namespace.c:3403 [inline] > [<00000000f43f0071>] __x64_sys_mount+0x18e/0x1d0 fs/namespace.c:3403 > [<00000000dc5fffd5>] do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46 > [<000000004e665669>] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > Hi David, Is this a false positive, maybe having to do with this comment from fs/fsopen.c: ? /* * Check the state and apply the configuration. Note that this function is * allowed to 'steal' the value by setting param->xxx to NULL before returning. */ static int vfs_fsconfig_locked(struct fs_context *fc, int cmd, struct fs_parameter *param) { Otherwise please look at the patch below. Thanks. > --- > This report 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 issue. See: > https://goo.gl/tpsmEJ#status for how to communicate with syzbot. > syzbot can test patches for this issue, for details see: > https://goo.gl/tpsmEJ#testing-patches --- From: Randy Dunlap <rdunlap@xxxxxxxxxxxxx> Callers to vfs_parse_fs_param() should be responsible for freeing param.string. Fixes: ecdab150fddb ("vfs: syscall: Add fsconfig() for configuring and managing a context") Signed-off-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx> Reported-by: syzbot+86dc6632faaca40133ab@xxxxxxxxxxxxxxxxxxxxxxxxx Cc: David Howells <dhowells@xxxxxxxxxx> Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx> --- This looks promising to me but I haven't fully tested it yet because my build/test machine just started acting flaky, like it is having memory or disk errors. OTOH, it could have ramifications in other places. fs/fs_context.c | 1 - fs/fsopen.c | 4 +++- 2 files changed, 3 insertions(+), 2 deletions(-) --- linux-next-20201204.orig/fs/fs_context.c +++ linux-next-20201204/fs/fs_context.c @@ -128,7 +128,6 @@ int vfs_parse_fs_param(struct fs_context if (fc->source) return invalf(fc, "VFS: Multiple sources"); fc->source = param->string; - param->string = NULL; return 0; } --- linux-next-20201204.orig/fs/fsopen.c +++ linux-next-20201204/fs/fsopen.c @@ -262,7 +262,9 @@ static int vfs_fsconfig_locked(struct fs fc->phase != FS_CONTEXT_RECONF_PARAMS) return -EBUSY; - return vfs_parse_fs_param(fc, param); + ret = vfs_parse_fs_param(fc, param); + kfree(param->string); + return ret; } fc->phase = FS_CONTEXT_FAILED; return ret;