On Tue, Feb 6, 2024 at 3:25 AM Steve French <smfrench@xxxxxxxxx> wrote:
>
> Shyam,
> Let me know if any objections to this fix, similar to what was pointed
> out by Dan.
>
> See attached.
>
>
> On Mon, Feb 5, 2024 at 2:52 AM Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> >
> > Hello Shyam Prasad N,
> >
> > This is a semi-automatic email about new static checker warnings.
> >
> > fs/smb/client/sess.c:88 cifs_ses_get_chan_index()
> > warn: variable dereferenced before check 'server' (see line 79)
> >
> > fs/smb/client/sess.c
> > 78 /* if the channel is waiting for termination */
> > 79 if (server->terminate)
> > ^^^^^^^^^^^^^^^^^
> > The patch adds an unchecked dereference
> >
> > 80 return CIFS_INVAL_CHAN_INDEX;
> > 81
> > 82 for (i = 0; i < ses->chan_count; i++) {
> > 83 if (ses->chans[i].server == server)
> > 84 return i;
> > 85 }
> > 86
> > 87 /* If we didn't find the channel, it is likely a bug */
> > 88 if (server)
> > ^^^^^^
> > But the existing code assumed that server could be NULL
> >
> > 89 cifs_dbg(VFS, "unable to get chan index for server: 0x%llx",
> > 90 server->conn_id);
> >
> > regards,
> > dan carpenter
> >
>
Hi Dan,
Thanks for running your analysis on the changes.
>
> --
> Thanks,
>
> Steve
Hi Steve,
With the current code, there should ideally be no case where a NULL
server would be passed to this function. But it is always good to
check explicitly.
Changes look good to me.
--
Regards,
Shyam
