On Wed, Jul 14, 2021 at 6:19 AM Steve French <smfrench@xxxxxxxxx> wrote: > > Although it is unlikely to be have ended up with a null > session pointer calling cifs_try_adding_channels in cifs_mount. > Coverity correctly notes that we are already checking for > it earlier (when we return from do_dfs_failover), so at > a minimum to clarify the code we should make sure we also > check for it when we exit the loop so we don't end up calling > cifs_try_adding_channels or mount_setup_tlink with a null > ses pointer. > > Addresses-Coverity: 1505608 ("Derefernce after null check") > Reviewed-off-by: Paulo Alcantara (SUSE) <pc@xxxxxx> > Signed-off-by: Steve French <stfrench@xxxxxxxxxxxxx> > --- > fs/cifs/connect.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c > index db6c607269f5..463cae116c12 100644 > --- a/fs/cifs/connect.c > +++ b/fs/cifs/connect.c > @@ -3577,7 +3577,7 @@ int cifs_mount(struct cifs_sb_info *cifs_sb, > struct smb3_fs_context *ctx) > rc = -ELOOP; > } while (rc == -EREMOTE); > > - if (rc || !tcon) > + if (rc || !tcon || !ses) > goto error; > > kfree(ref_path); > > -- > Thanks, > > Steve Hi Paulo, Doesn't it make sense to check rc, tcon and ses values right after mount_get_conns call? rc = mount_get_conns(ctx, cifs_sb, &xid, &server, &ses, &tcon); <<<<<<<<<<<<<<<<<<< /* * If called with 'nodfs' mount option, then skip DFS resolving. Otherwise unconditionally * try to get an DFS referral (even cached) to determine whether it is an DFS mount. * * Skip prefix path to provide support for DFS referrals from w2k8 servers which don't seem * to respond with PATH_NOT_COVERED to requests that include the prefix. */ if ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_DFS) || dfs_cache_find(xid, ses, cifs_sb->local_nls, cifs_remap(cifs_sb), ctx->UNC + 1, NULL, NULL)) { if (rc) goto error; /* Check if it is fully accessible and then mount it */ rc = is_path_remote(cifs_sb, ctx, xid, server, tcon); if (!rc) goto out; if (rc != -EREMOTE) goto error; } Why don't we check for all rc values that we don't expect, and call dfs_cache_find only when it's an expected error? -- Regards, Shyam