linux-kernel - Re: [PATCH next] smb: client: Fix NULL vs ERR_PTR() returns in cifs_get_tcon

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAH2r5muVjS+Y_NFSWwYoisPGfynyTkmynjpQHi2_Kk6Z8AiG0w@mail.gmail.com>
Date: Mon, 18 Aug 2025 16:41:52 -0500
From: Steve French <smfrench@...il.com>
To: Wang Zhaolong <wangzhaolong@...weicloud.com>
Cc: Dan Carpenter <dan.carpenter@...aro.org>, Steve French <sfrench@...ba.org>, 
	Paulo Alcantara <pc@...guebit.org>, Ronnie Sahlberg <ronniesahlberg@...il.com>, 
	Shyam Prasad N <sprasad@...rosoft.com>, Tom Talpey <tom@...pey.com>, 
	Bharath SM <bharathsm@...rosoft.com>, linux-cifs@...r.kernel.org, 
	samba-technical@...ts.samba.org, linux-kernel@...r.kernel.org, 
	kernel-janitors@...r.kernel.org
Subject: Re: [PATCH next] smb: client: Fix NULL vs ERR_PTR() returns in cifs_get_tcon_super()

Since Paulo pointed out a problem with v4 of this patch, an obvious
question is Dan's patch "independent enough" to take or would it make
the v5 of your patch harder.  Let me know when there is a v5 of the
patch so we can do more testing and review

On Mon, Aug 18, 2025 at 8:30 AM Wang Zhaolong
<wangzhaolong@...weicloud.com> wrote:
>
>
>
> > The cifs_get_tcon_super() function returns NULL on error but the caller
> > expect it to return error pointers instead.  Change it to return error
> > pointers.  Otherwise it results in a NULL pointer dereference.
> >
> > Fixes: 0938b093b1ae ("smb: client: Fix mount deadlock by avoiding super block iteration in DFS reconnect")
> > Signed-off-by: Dan Carpenter <dan.carpenter@...aro.org>
>
> Hi Dan,
>
> Thank you for your patch and for taking the time to address this issue.
>
> I would like to mention that I have recently sent out the V4 version of
> the patch series, which addresses the issues related to `cifs_get_tcon_super()`.
> In the latest version, the issue of NULL pointer dereference has already
> been resolved.
>
> https://lore.kernel.org/all/CAH2r5msLMNdqdo6EBuTvrQ0hwrqSRC-LSZuN2WpwV+PkDwsCOw@mail.gmail.com/
>
> I avoid null pointer dereferencing by performing a null pointer check on
> the return value of cifs_get_dfs_tcon_super().
>
>
> > ---
> >   fs/smb/client/misc.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/smb/client/misc.c b/fs/smb/client/misc.c
> > index 3b6920a52daa..d73c36862e97 100644
> > --- a/fs/smb/client/misc.c
> > +++ b/fs/smb/client/misc.c
> > @@ -1116,7 +1116,7 @@ static struct super_block *cifs_get_tcon_super(struct cifs_tcon *tcon)
> >       struct cifs_sb_info *cifs_sb;
> >
> >       if (!tcon)
> > -             return NULL;
> > +             return ERR_PTR(-EINVAL);
> >
> >       spin_lock(&tcon->sb_list_lock);
> >       list_for_each_entry(cifs_sb, &tcon->cifs_sb_list, tcon_sb_link) {
> > @@ -1141,7 +1141,7 @@ static struct super_block *cifs_get_tcon_super(struct cifs_tcon *tcon)
> >       }
> >       spin_unlock(&tcon->sb_list_lock);
> >
> > -     return NULL;
> > +     return ERR_PTR(-ENOENT);
> >   }
> >
> >   struct super_block *cifs_get_dfs_tcon_super(struct cifs_tcon *tcon)
>
> Additionally, I think it somewhat peculiar that in the current
> implementation, cifs_get_tcon_super() returns -EINVAL.
>
> I would greatly appreciate it if you could review my latest patch series to
> confirm if it resolves the concerns. If there are any additional improvements, I
> would be happy to collaborate further to ensure the best possible solution.
>
> Best regards,
> Wang Zhaolong
>
>


-- 
Thanks,

Steve