[PATCH] cifs: fix error handling in mount-time DFS referral chasing code

[Jeff Layton-2]

If the referral is malformed or the hostname can't be resolved, then
the current code generates an oops. Fix it to handle these errors
gracefully.

Reported-by: Sandro Mathys <sm@...>
Signed-off-by: Jeff Layton <jlayton@...>
---
 fs/cifs/cifs_dfs_ref.c |   12 +++++++++---
 fs/cifs/connect.c      |   13 ++++++++++---
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/fs/cifs/cifs_dfs_ref.c b/fs/cifs/cifs_dfs_ref.c
index 3bb11be..606912d 100644
--- a/fs/cifs/cifs_dfs_ref.c
+++ b/fs/cifs/cifs_dfs_ref.c
@@ -55,7 +55,7 @@ void cifs_dfs_release_automount_timer(void)
  * i.e. strips from UNC trailing path that is not part of share
  * name and fixup missing '\' in the begining of DFS node refferal
  * if neccessary.
- * Returns pointer to share name on success or NULL on error.
+ * Returns pointer to share name on success or ERR_PTR on error.
  * Caller is responsible for freeing returned string.
  */
 static char *cifs_get_share_name(const char *node_name)
@@ -68,7 +68,7 @@ static char *cifs_get_share_name(const char *node_name)
  UNC = kmalloc(len+2 /*for term null and additional \ if it's missed */,
  GFP_KERNEL);
  if (!UNC)
- return NULL;
+ return ERR_PTR(-ENOMEM);
 
  /* get share name and server name */
  if (node_name[1] != '\\') {
@@ -87,7 +87,7 @@ static char *cifs_get_share_name(const char *node_name)
  cERROR(1, ("%s: no server name end in node name: %s",
  __func__, node_name));
  kfree(UNC);
- return NULL;
+ return ERR_PTR(-EINVAL);
  }
 
  /* find sharename end */
@@ -133,6 +133,12 @@ char *cifs_compose_mount_options(const char *sb_mountdata,
  return ERR_PTR(-EINVAL);
 
  *devname = cifs_get_share_name(ref->node_name);
+ if (IS_ERR(*devname)) {
+ rc = PTR_ERR(*devname);
+ *devname = NULL;
+ goto compose_mount_options_err;
+ }
+
  rc = dns_resolve_server_name_to_ip(*devname, &srvIP);
  if (rc != 0) {
  cERROR(1, ("%s: Failed to resolve server part of %s to IP: %d",
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index fc44d31..f248688 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -2544,11 +2544,20 @@ remote_path_check:
 
  if (mount_data != mount_data_global)
  kfree(mount_data);
+
  mount_data = cifs_compose_mount_options(
  cifs_sb->mountdata, full_path + 1,
  referrals, &fake_devname);
- kfree(fake_devname);
+
  free_dfs_info_array(referrals, num_referrals);
+ kfree(fake_devname);
+ kfree(full_path);
+
+ if (IS_ERR(mount_data)) {
+ rc = PTR_ERR(mount_data);
+ mount_data = NULL;
+ goto mount_fail_check;
+ }
 
  if (tcon)
  cifs_put_tcon(tcon);
@@ -2556,8 +2565,6 @@ remote_path_check:
  cifs_put_smb_ses(pSesInfo);
 
  cleanup_volume_info(&volume_info);
- FreeXid(xid);
- kfree(full_path);
  referral_walks_count++;
  goto try_mount_again;
  }
--
1.5.5.6

_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@...
https://lists.samba.org/mailman/listinfo/linux-cifs-client

[Steve French-2]

On Mon, Jul 27, 2009 at 4:44 AM, Igor Mammedov <niallain@...> wrote:

Looks good to me:

Acked-by: Igor Mammedov <niallain@...>

Jeff Layton wrote:
> If the referral is malformed or the hostname can't be resolved, then
> the current code generates an oops. Fix it to handle these errors
> gracefully.

This is in the cifs-2.6.git tree now.   Will give it a few days before requesting merge upstream.

--
Thanks,

Steve

_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@...
https://lists.samba.org/mailman/listinfo/linux-cifs-client