cifs: client: fix memory leak in smb3_fs_context_parse_param

[PATCH] cifs: client: fix memory leak in smb3_fs_context_parse_param

Posted by Edward Adam Davis 3 months ago

The user calls fsconfig twice, but when the program exits, free() only
frees ctx->source for the second fsconfig, not the first.
Regarding fc->source, there is no code in the fs context related to its
memory reclamation.

To fix this memory leak, release the source memory corresponding to ctx
or fc before each parsing.

syzbot reported:
BUG: memory leak
unreferenced object 0xffff888128afa360 (size 96):
  backtrace (crc 79c9c7ba):
    kstrdup+0x3c/0x80 mm/util.c:84
    smb3_fs_context_parse_param+0x229b/0x36c0 fs/smb/client/fs_context.c:1444

BUG: memory leak
unreferenced object 0xffff888112c7d900 (size 96):
  backtrace (crc 79c9c7ba):
    smb3_fs_context_fullpath+0x70/0x1b0 fs/smb/client/fs_context.c:629
    smb3_fs_context_parse_param+0x2266/0x36c0 fs/smb/client/fs_context.c:1438

Reported-by: syzbot+72afd4c236e6bc3f4bac@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=72afd4c236e6bc3f4bac
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
 fs/smb/client/fs_context.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c
index e60927b2a7c8..0e1949bcd6ea 100644
--- a/fs/smb/client/fs_context.c
+++ b/fs/smb/client/fs_context.c
@@ -1435,12 +1435,14 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
 			cifs_errorf(fc, "Unknown error parsing devname\n");
 			goto cifs_parse_mount_err;
 		}
+		kfree(ctx->source);
 		ctx->source = smb3_fs_context_fullpath(ctx, '/');
 		if (IS_ERR(ctx->source)) {
 			ctx->source = NULL;
 			cifs_errorf(fc, "OOM when copying UNC string\n");
 			goto cifs_parse_mount_err;
 		}
+		kfree(fc->source);
 		fc->source = kstrdup(ctx->source, GFP_KERNEL);
 		if (fc->source == NULL) {
 			cifs_errorf(fc, "OOM when copying UNC string\n");
-- 
2.43.0

Re: [PATCH] cifs: client: fix memory leak in smb3_fs_context_parse_param

Posted by Paulo Alcantara 3 months ago

Edward Adam Davis <eadavis@qq.com> writes:

> The user calls fsconfig twice, but when the program exits, free() only
> frees ctx->source for the second fsconfig, not the first.
> Regarding fc->source, there is no code in the fs context related to its
> memory reclamation.
>
> To fix this memory leak, release the source memory corresponding to ctx
> or fc before each parsing.
>
> syzbot reported:
> BUG: memory leak
> unreferenced object 0xffff888128afa360 (size 96):
>   backtrace (crc 79c9c7ba):
>     kstrdup+0x3c/0x80 mm/util.c:84
>     smb3_fs_context_parse_param+0x229b/0x36c0 fs/smb/client/fs_context.c:1444
>
> BUG: memory leak
> unreferenced object 0xffff888112c7d900 (size 96):
>   backtrace (crc 79c9c7ba):
>     smb3_fs_context_fullpath+0x70/0x1b0 fs/smb/client/fs_context.c:629
>     smb3_fs_context_parse_param+0x2266/0x36c0 fs/smb/client/fs_context.c:1438
>
> Reported-by: syzbot+72afd4c236e6bc3f4bac@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=72afd4c236e6bc3f4bac
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
>  fs/smb/client/fs_context.c | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>

Re: [PATCH] cifs: client: fix memory leak in smb3_fs_context_parse_param

Posted by Steve French 3 months ago

merged into cifs-2.6.git for-next pending additional testing.

Also added Cc:stable tag

On Fri, Nov 7, 2025 at 8:52 AM Paulo Alcantara via samba-technical
<samba-technical@lists.samba.org> wrote:
>
> Edward Adam Davis <eadavis@qq.com> writes:
>
> > The user calls fsconfig twice, but when the program exits, free() only
> > frees ctx->source for the second fsconfig, not the first.
> > Regarding fc->source, there is no code in the fs context related to its
> > memory reclamation.
> >
> > To fix this memory leak, release the source memory corresponding to ctx
> > or fc before each parsing.
> >
> > syzbot reported:
> > BUG: memory leak
> > unreferenced object 0xffff888128afa360 (size 96):
> >   backtrace (crc 79c9c7ba):
> >     kstrdup+0x3c/0x80 mm/util.c:84
> >     smb3_fs_context_parse_param+0x229b/0x36c0 fs/smb/client/fs_context.c:1444
> >
> > BUG: memory leak
> > unreferenced object 0xffff888112c7d900 (size 96):
> >   backtrace (crc 79c9c7ba):
> >     smb3_fs_context_fullpath+0x70/0x1b0 fs/smb/client/fs_context.c:629
> >     smb3_fs_context_parse_param+0x2266/0x36c0 fs/smb/client/fs_context.c:1438
> >
> > Reported-by: syzbot+72afd4c236e6bc3f4bac@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=72afd4c236e6bc3f4bac
> > Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> > ---
> >  fs/smb/client/fs_context.c | 2 ++
> >  1 file changed, 2 insertions(+)
>
> Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
>


-- 
Thanks,

Steve

Re: [PATCH] cifs: client: fix memory leak in smb3_fs_context_parse_param

Posted by Shyam Prasad N 3 months ago

On Sat, Nov 8, 2025 at 4:52 AM Steve French <smfrench@gmail.com> wrote:
>
> merged into cifs-2.6.git for-next pending additional testing.
>
> Also added Cc:stable tag
>
> On Fri, Nov 7, 2025 at 8:52 AM Paulo Alcantara via samba-technical
> <samba-technical@lists.samba.org> wrote:
> >
> > Edward Adam Davis <eadavis@qq.com> writes:
> >
> > > The user calls fsconfig twice, but when the program exits, free() only
> > > frees ctx->source for the second fsconfig, not the first.
> > > Regarding fc->source, there is no code in the fs context related to its
> > > memory reclamation.
> > >
> > > To fix this memory leak, release the source memory corresponding to ctx
> > > or fc before each parsing.
> > >
> > > syzbot reported:
> > > BUG: memory leak
> > > unreferenced object 0xffff888128afa360 (size 96):
> > >   backtrace (crc 79c9c7ba):
> > >     kstrdup+0x3c/0x80 mm/util.c:84
> > >     smb3_fs_context_parse_param+0x229b/0x36c0 fs/smb/client/fs_context.c:1444
> > >
> > > BUG: memory leak
> > > unreferenced object 0xffff888112c7d900 (size 96):
> > >   backtrace (crc 79c9c7ba):
> > >     smb3_fs_context_fullpath+0x70/0x1b0 fs/smb/client/fs_context.c:629
> > >     smb3_fs_context_parse_param+0x2266/0x36c0 fs/smb/client/fs_context.c:1438
> > >
> > > Reported-by: syzbot+72afd4c236e6bc3f4bac@syzkaller.appspotmail.com
> > > Closes: https://syzkaller.appspot.com/bug?extid=72afd4c236e6bc3f4bac
> > > Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> > > ---
> > >  fs/smb/client/fs_context.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> >
> > Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
> >
>
>
> --
> Thanks,
>
> Steve
>

Looks good to me.
Steve, this is one of the mount options that is missing in the man page.
Also, I don't understand this option. Why is it needed? Don't we
specify the source in the mount command as devname anyway?

-- 
Regards,
Shyam