[bug report] cifs: Simplify reconnect code when dfs upcall is enabled

[bug report] cifs: Simplify reconnect code when dfs upcall is enabled

[Dan Carpenter]

Hello Samuel Cabrero,

The patch 7d6535b72042: "cifs: Simplify reconnect code when dfs
upcall is enabled" from Nov 30, 2020, leads to the following static
checker warning:

	fs/cifs/connect.c:160 reconn_set_next_dfs_target()
	error: 'server->hostname' dereferencing possible ERR_PTR()

fs/cifs/connect.c
   128  static void reconn_set_next_dfs_target(struct TCP_Server_Info *server,
   129                                         struct cifs_sb_info *cifs_sb,
   130                                         struct dfs_cache_tgt_list *tgt_list,
   131                                         struct dfs_cache_tgt_iterator **tgt_it)
   132  {
   133          const char *name;
   134          int rc;
   135  
   136          if (!cifs_sb || !cifs_sb->origin_fullpath)
   137                  return;
   138  
   139          if (!*tgt_it) {
   140                  *tgt_it = dfs_cache_get_tgt_iterator(tgt_list);
   141          } else {
   142                  *tgt_it = dfs_cache_get_next_tgt(tgt_list, *tgt_it);
   143                  if (!*tgt_it)
   144                          *tgt_it = dfs_cache_get_tgt_iterator(tgt_list);
   145          }
   146  
   147          cifs_dbg(FYI, "%s: UNC: %s\n", __func__, cifs_sb->origin_fullpath);
   148  
   149          name = dfs_cache_get_tgt_name(*tgt_it);
   150  
   151          kfree(server->hostname);
   152  
   153          server->hostname = extract_hostname(name);
   154          if (IS_ERR(server->hostname)) {
                           ^^^^^^^^^^^^^^^^

   155                  cifs_dbg(FYI,
   156                           "%s: failed to extract hostname from target: %ld\n",
   157                           __func__, PTR_ERR(server->hostname));

This should probably just return here.  I don't totally understand why
this is a void function...

   158          }
   159  
   160          rc = reconn_set_ipaddr_from_hostname(server);
                                                     ^^^^^^
"server->hostname" is dereferenced inside the function.

   161          if (rc) {
   162                  cifs_dbg(FYI, "%s: failed to resolve hostname: %d\n",
   163                           __func__, rc);
   164          }
   165  }

regards,
dan carpenter

[PATCH] cifs: Avoid error pointer dereference

[Samuel Cabrero]

The patch 7d6535b72042: "cifs: Simplify reconnect code when dfs
upcall is enabled" leads to the following static checker warning:

	fs/cifs/connect.c:160 reconn_set_next_dfs_target()
	error: 'server->hostname' dereferencing possible ERR_PTR()

Avoid dereferencing the error pointer by early returning on error
condition.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Samuel Cabrero <scabrero@suse.de>
---
 fs/cifs/connect.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 509a41ff56b8..b9df85506938 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -155,6 +155,7 @@ static void reconn_set_next_dfs_target(struct TCP_Server_Info *server,
 		cifs_dbg(FYI,
 			 "%s: failed to extract hostname from target: %ld\n",
 			 __func__, PTR_ERR(server->hostname));
+		return;
 	}
 
 	rc = reconn_set_ipaddr_from_hostname(server);
-- 
2.29.2

Re: [PATCH] cifs: Avoid error pointer dereference

[Steve French]

tentatively merged into cifs-2.6.git for-next pending more testing

On Fri, Dec 18, 2020 at 3:48 AM Samuel Cabrero <scabrero@suse.de> wrote:
>
> The patch 7d6535b72042: "cifs: Simplify reconnect code when dfs
> upcall is enabled" leads to the following static checker warning:
>
>         fs/cifs/connect.c:160 reconn_set_next_dfs_target()
>         error: 'server->hostname' dereferencing possible ERR_PTR()
>
> Avoid dereferencing the error pointer by early returning on error
> condition.
>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Samuel Cabrero <scabrero@suse.de>
> ---
>  fs/cifs/connect.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 509a41ff56b8..b9df85506938 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -155,6 +155,7 @@ static void reconn_set_next_dfs_target(struct TCP_Server_Info *server,
>                 cifs_dbg(FYI,
>                          "%s: failed to extract hostname from target: %ld\n",
>                          __func__, PTR_ERR(server->hostname));
> +               return;
>         }
>
>         rc = reconn_set_ipaddr_from_hostname(server);
> --
> 2.29.2
>


-- 
Thanks,

Steve