Acked-by me
On Fri, Apr 23, 2021 at 8:14 AM David Disseldorp <ddiss@xxxxxxx> 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@xxxxxxx>
> ---
> 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
>
