On Mon, Feb 5, 2024 at 11:51 PM Shyam Prasad N <nspmangalore@xxxxxxxxx> wrote:
>
> 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
Added the Reviewed-by and added to cifs-2.6.git for-next
--
Thanks,
Steve
