| Message ID | 20210422221403.13617-1-ddiss@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cifs: fix leak in cifs_smb3_do_mount() ctx | expand |
Acked-by me On Fri, Apr 23, 2021 at 8:14 AM David Disseldorp <ddiss@suse.de> wrote: > > cifs_smb3_do_mount() calls smb3_fs_context_dup() and then > cifs_setup_volume_info(). The latter's subsequent smb3_parse_devname() > call overwrites the cifs_sb->ctx->UNC string already dup'ed by > smb3_fs_context_dup(), resulting in a leak. E.g. > > unreferenced object 0xffff888002980420 (size 32): > comm "mount", pid 160, jiffies 4294892541 (age 30.416s) > hex dump (first 32 bytes): > 5c 5c 31 39 32 2e 31 36 38 2e 31 37 34 2e 31 30 \\192.168.174.10 > 34 5c 72 61 70 69 64 6f 2d 73 68 61 72 65 00 00 4\rapido-share.. > backtrace: > [<00000000069e12f6>] kstrdup+0x28/0x50 > [<00000000b61f4032>] smb3_fs_context_dup+0x127/0x1d0 [cifs] > [<00000000c6e3e3bf>] cifs_smb3_do_mount+0x77/0x660 [cifs] > [<0000000063467a6b>] smb3_get_tree+0xdf/0x220 [cifs] > [<00000000716f731e>] vfs_get_tree+0x1b/0x90 > [<00000000491d3892>] path_mount+0x62a/0x910 > [<0000000046b2e774>] do_mount+0x50/0x70 > [<00000000ca7b64dd>] __x64_sys_mount+0x81/0xd0 > [<00000000b5122496>] do_syscall_64+0x33/0x40 > [<000000002dd397af>] entry_SYSCALL_64_after_hwframe+0x44/0xae > > This change is a bandaid until the cifs_setup_volume_info() TODO and > error handling issues are resolved. > > Signed-off-by: David Disseldorp <ddiss@suse.de> > --- > fs/cifs/cifsfs.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c > index 5ddd20b62484..34c125798ad3 100644 > --- a/fs/cifs/cifsfs.c > +++ b/fs/cifs/cifsfs.c > @@ -834,6 +834,12 @@ cifs_smb3_do_mount(struct file_system_type *fs_type, > goto out; > } > > + /* cifs_setup_volume_info->smb3_parse_devname() redups UNC & prepath */ > + kfree(cifs_sb->ctx->UNC); > + cifs_sb->ctx->UNC = NULL; > + kfree(cifs_sb->ctx->prepath); > + cifs_sb->ctx->prepath = NULL; > + > rc = cifs_setup_volume_info(cifs_sb->ctx, NULL, old_ctx->UNC); > if (rc) { > root = ERR_PTR(rc); > -- > 2.26.2 >
David Disseldorp <ddiss@suse.de> writes: > cifs_smb3_do_mount() calls smb3_fs_context_dup() and then > cifs_setup_volume_info(). The latter's subsequent smb3_parse_devname() > call overwrites the cifs_sb->ctx->UNC string already dup'ed by > smb3_fs_context_dup(), resulting in a leak. E.g. > > unreferenced object 0xffff888002980420 (size 32): > comm "mount", pid 160, jiffies 4294892541 (age 30.416s) > hex dump (first 32 bytes): > 5c 5c 31 39 32 2e 31 36 38 2e 31 37 34 2e 31 30 \\192.168.174.10 > 34 5c 72 61 70 69 64 6f 2d 73 68 61 72 65 00 00 4\rapido-share.. > backtrace: > [<00000000069e12f6>] kstrdup+0x28/0x50 > [<00000000b61f4032>] smb3_fs_context_dup+0x127/0x1d0 [cifs] > [<00000000c6e3e3bf>] cifs_smb3_do_mount+0x77/0x660 [cifs] > [<0000000063467a6b>] smb3_get_tree+0xdf/0x220 [cifs] > [<00000000716f731e>] vfs_get_tree+0x1b/0x90 > [<00000000491d3892>] path_mount+0x62a/0x910 > [<0000000046b2e774>] do_mount+0x50/0x70 > [<00000000ca7b64dd>] __x64_sys_mount+0x81/0xd0 > [<00000000b5122496>] do_syscall_64+0x33/0x40 > [<000000002dd397af>] entry_SYSCALL_64_after_hwframe+0x44/0xae > > This change is a bandaid until the cifs_setup_volume_info() TODO and > error handling issues are resolved. > > Signed-off-by: David Disseldorp <ddiss@suse.de> > --- > fs/cifs/cifsfs.c | 6 ++++++ > 1 file changed, 6 insertions(+) Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
merged into cifs-2.6.git for-next and kicking off (buildbot) testing run with it now On Thu, Apr 22, 2021 at 5:36 PM Paulo Alcantara <pc@cjr.nz> wrote: > > David Disseldorp <ddiss@suse.de> writes: > > > cifs_smb3_do_mount() calls smb3_fs_context_dup() and then > > cifs_setup_volume_info(). The latter's subsequent smb3_parse_devname() > > call overwrites the cifs_sb->ctx->UNC string already dup'ed by > > smb3_fs_context_dup(), resulting in a leak. E.g. > > > > unreferenced object 0xffff888002980420 (size 32): > > comm "mount", pid 160, jiffies 4294892541 (age 30.416s) > > hex dump (first 32 bytes): > > 5c 5c 31 39 32 2e 31 36 38 2e 31 37 34 2e 31 30 \\192.168.174.10 > > 34 5c 72 61 70 69 64 6f 2d 73 68 61 72 65 00 00 4\rapido-share.. > > backtrace: > > [<00000000069e12f6>] kstrdup+0x28/0x50 > > [<00000000b61f4032>] smb3_fs_context_dup+0x127/0x1d0 [cifs] > > [<00000000c6e3e3bf>] cifs_smb3_do_mount+0x77/0x660 [cifs] > > [<0000000063467a6b>] smb3_get_tree+0xdf/0x220 [cifs] > > [<00000000716f731e>] vfs_get_tree+0x1b/0x90 > > [<00000000491d3892>] path_mount+0x62a/0x910 > > [<0000000046b2e774>] do_mount+0x50/0x70 > > [<00000000ca7b64dd>] __x64_sys_mount+0x81/0xd0 > > [<00000000b5122496>] do_syscall_64+0x33/0x40 > > [<000000002dd397af>] entry_SYSCALL_64_after_hwframe+0x44/0xae > > > > This change is a bandaid until the cifs_setup_volume_info() TODO and > > error handling issues are resolved. > > > > Signed-off-by: David Disseldorp <ddiss@suse.de> > > --- > > fs/cifs/cifsfs.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c index 5ddd20b62484..34c125798ad3 100644 --- a/fs/cifs/cifsfs.c +++ b/fs/cifs/cifsfs.c @@ -834,6 +834,12 @@ cifs_smb3_do_mount(struct file_system_type *fs_type, goto out; } + /* cifs_setup_volume_info->smb3_parse_devname() redups UNC & prepath */ + kfree(cifs_sb->ctx->UNC); + cifs_sb->ctx->UNC = NULL; + kfree(cifs_sb->ctx->prepath); + cifs_sb->ctx->prepath = NULL; + rc = cifs_setup_volume_info(cifs_sb->ctx, NULL, old_ctx->UNC); if (rc) { root = ERR_PTR(rc);
cifs_smb3_do_mount() calls smb3_fs_context_dup() and then cifs_setup_volume_info(). The latter's subsequent smb3_parse_devname() call overwrites the cifs_sb->ctx->UNC string already dup'ed by smb3_fs_context_dup(), resulting in a leak. E.g. unreferenced object 0xffff888002980420 (size 32): comm "mount", pid 160, jiffies 4294892541 (age 30.416s) hex dump (first 32 bytes): 5c 5c 31 39 32 2e 31 36 38 2e 31 37 34 2e 31 30 \\192.168.174.10 34 5c 72 61 70 69 64 6f 2d 73 68 61 72 65 00 00 4\rapido-share.. backtrace: [<00000000069e12f6>] kstrdup+0x28/0x50 [<00000000b61f4032>] smb3_fs_context_dup+0x127/0x1d0 [cifs] [<00000000c6e3e3bf>] cifs_smb3_do_mount+0x77/0x660 [cifs] [<0000000063467a6b>] smb3_get_tree+0xdf/0x220 [cifs] [<00000000716f731e>] vfs_get_tree+0x1b/0x90 [<00000000491d3892>] path_mount+0x62a/0x910 [<0000000046b2e774>] do_mount+0x50/0x70 [<00000000ca7b64dd>] __x64_sys_mount+0x81/0xd0 [<00000000b5122496>] do_syscall_64+0x33/0x40 [<000000002dd397af>] entry_SYSCALL_64_after_hwframe+0x44/0xae This change is a bandaid until the cifs_setup_volume_info() TODO and error handling issues are resolved. Signed-off-by: David Disseldorp <ddiss@suse.de> --- fs/cifs/cifsfs.c | 6 ++++++ 1 file changed, 6 insertions(+)