linux-cifs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] cifs: fix leak in cifs_smb3_do_mount() ctx
@ 2021-04-22 22:14 David Disseldorp
  2021-04-22 22:16 ` ronnie sahlberg
  2021-04-22 22:35 ` Paulo Alcantara
  0 siblings, 2 replies; 4+ messages in thread
From: David Disseldorp @ 2021-04-22 22:14 UTC (permalink / raw)
  To: linux-cifs; +Cc: David Disseldorp

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] cifs: fix leak in cifs_smb3_do_mount() ctx
  2021-04-22 22:14 [PATCH] cifs: fix leak in cifs_smb3_do_mount() ctx David Disseldorp
@ 2021-04-22 22:16 ` ronnie sahlberg
  2021-04-22 22:35 ` Paulo Alcantara
  1 sibling, 0 replies; 4+ messages in thread
From: ronnie sahlberg @ 2021-04-22 22:16 UTC (permalink / raw)
  To: David Disseldorp; +Cc: linux-cifs

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
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] cifs: fix leak in cifs_smb3_do_mount() ctx
  2021-04-22 22:14 [PATCH] cifs: fix leak in cifs_smb3_do_mount() ctx David Disseldorp
  2021-04-22 22:16 ` ronnie sahlberg
@ 2021-04-22 22:35 ` Paulo Alcantara
  2021-04-23 23:51   ` Steve French
  1 sibling, 1 reply; 4+ messages in thread
From: Paulo Alcantara @ 2021-04-22 22:35 UTC (permalink / raw)
  To: David Disseldorp, linux-cifs; +Cc: David Disseldorp

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>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] cifs: fix leak in cifs_smb3_do_mount() ctx
  2021-04-22 22:35 ` Paulo Alcantara
@ 2021-04-23 23:51   ` Steve French
  0 siblings, 0 replies; 4+ messages in thread
From: Steve French @ 2021-04-23 23:51 UTC (permalink / raw)
  To: Paulo Alcantara; +Cc: David Disseldorp, CIFS

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>



-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-04-23 23:51 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-22 22:14 [PATCH] cifs: fix leak in cifs_smb3_do_mount() ctx David Disseldorp
2021-04-22 22:16 ` ronnie sahlberg
2021-04-22 22:35 ` Paulo Alcantara
2021-04-23 23:51   ` Steve French

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).