CIFS: Fix undefined behavior when mount fails

Fix double kfree() calls on the same pointers and cleanup mount code.

Reviewed-and-Tested-by: Jeff Layton <jlayton@samba.org>
Signed-off-by: Pavel Shilovsky <piastry@etersoft.ru>
Signed-off-by: Steve French <sfrench@us.ibm.com>

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 131afadce0e02f9cd944d0d601a65f3874691ef6..d1ed7f9946d5ce6b0574843c35b0e824232b5b54 100644 (file)
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -104,29 +104,23 @@ cifs_sb_deactive(struct super_block *sb)
 }
 
 static int
-cifs_read_super(struct super_block *sb, struct cifs_sb_info *cifs_sb,
-               void *data, struct smb_vol *volume_info, const char *devname,
-               int silent)
+cifs_read_super(struct super_block *sb, struct smb_vol *volume_info,
+               const char *devname, int silent)
 {
        struct inode *inode;
+       struct cifs_sb_info *cifs_sb;
        int rc = 0;
 
-       /* BB should we make this contingent on mount parm? */
-       sb->s_flags |= MS_NODIRATIME | MS_NOATIME;
-       sb->s_fs_info = cifs_sb;
+       cifs_sb = CIFS_SB(sb);
 
        spin_lock_init(&cifs_sb->tlink_tree_lock);
        cifs_sb->tlink_tree = RB_ROOT;
 
        rc = bdi_setup_and_register(&cifs_sb->bdi, "cifs", BDI_CAP_MAP_COPY);
-       if (rc) {
-               kfree(cifs_sb);
+       if (rc)
                return rc;
-       }
-       cifs_sb->bdi.ra_pages = default_backing_dev_info.ra_pages;
 
-       if (data)
-               cifs_sb->mountdata = data;
+       cifs_sb->bdi.ra_pages = default_backing_dev_info.ra_pages;
 
        rc = cifs_mount(sb, cifs_sb, volume_info, devname);
 
@@ -179,15 +173,7 @@ out_no_root:
        cifs_umount(sb, cifs_sb);
 
 out_mount_failed:
-       if (cifs_sb) {
-               if (cifs_sb->mountdata) {
-                       kfree(cifs_sb->mountdata);
-                       cifs_sb->mountdata = NULL;
-               }
-               unload_nls(cifs_sb->local_nls);
-               bdi_destroy(&cifs_sb->bdi);
-               kfree(cifs_sb);
-       }
+       bdi_destroy(&cifs_sb->bdi);
        return rc;
 }
 
@@ -553,7 +539,6 @@ cifs_do_mount(struct file_system_type *fs_type,
        struct cifs_sb_info *cifs_sb;
        struct smb_vol *volume_info;
        struct dentry *root;
-       char *copied_data = NULL;
 
        cFYI(1, "Devname: %s flags: %d ", dev_name, flags);
 
@@ -576,20 +561,23 @@ cifs_do_mount(struct file_system_type *fs_type,
                goto out;
        }
 
-       sb->s_flags = flags;
-
        /*
         * Copy mount params for use in submounts. Better to do
         * the copy here and deal with the error before cleanup gets
         * complicated post-mount.
         */
-       copied_data = kstrndup(data, PAGE_SIZE, GFP_KERNEL);
-       if (copied_data == NULL) {
+       cifs_sb->mountdata = kstrndup(data, PAGE_SIZE, GFP_KERNEL);
+       if (cifs_sb->mountdata == NULL) {
                root = ERR_PTR(-ENOMEM);
                goto err_out;
        }
 
-       rc = cifs_read_super(sb, cifs_sb, copied_data, volume_info, dev_name,
+       sb->s_flags = flags;
+       /* BB should we make this contingent on mount parm? */
+       sb->s_flags |= MS_NODIRATIME | MS_NOATIME;
+       sb->s_fs_info = cifs_sb;
+
+       rc = cifs_read_super(sb, volume_info, dev_name,
                             flags & MS_SILENT ? 1 : 0);
        if (rc) {
                root = ERR_PTR(rc);
@@ -604,6 +592,8 @@ out:
        return root;
 
 err_out:
+       kfree(cifs_sb->mountdata);
+       unload_nls(cifs_sb->local_nls);
        kfree(cifs_sb);
        deactivate_locked_super(sb);
        cifs_cleanup_volume_info(&volume_info);