diff mbox

cifs: fix double-free of "string" in cifs_parse_mount_options

Message ID 1355396571-19510-1-git-send-email-jlayton@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jeff Layton Dec. 13, 2012, 11:02 a.m. UTC
Dan reported the following regression in commit d387a5c5:

    + fs/cifs/connect.c:1903 cifs_parse_mount_options() error: double free of 'string'

That patch has some of the new option parsing code free "string" without
setting the variable to NULL afterward. Since "string" is automatically
freed in an error condition, fix the code to just rely on that instead
of freeing it explicitly.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/cifs/connect.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

Comments

Steve French Dec. 13, 2012, 2:51 p.m. UTC | #1
I will merge this ASAP

On Thu, Dec 13, 2012 at 5:02 AM, Jeff Layton <jlayton@redhat.com> wrote:
> Dan reported the following regression in commit d387a5c5:
>
>     + fs/cifs/connect.c:1903 cifs_parse_mount_options() error: double free of 'string'
>
> That patch has some of the new option parsing code free "string" without
> setting the variable to NULL afterward. Since "string" is automatically
> freed in an error condition, fix the code to just rely on that instead
> of freeing it explicitly.
>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
>  fs/cifs/connect.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 7635b5d..17c3643 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -1624,14 +1624,11 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>                 case Opt_unc:
>                         string = vol->UNC;
>                         vol->UNC = match_strdup(args);
> -                       if (vol->UNC == NULL) {
> -                               kfree(string);
> +                       if (vol->UNC == NULL)
>                                 goto out_nomem;
> -                       }
>
>                         convert_delimiter(vol->UNC, '\\');
>                         if (vol->UNC[0] != '\\' || vol->UNC[1] != '\\') {
> -                               kfree(string);
>                                 printk(KERN_ERR "CIFS: UNC Path does not "
>                                                 "begin with // or \\\\\n");
>                                 goto cifs_parse_mount_err;
> @@ -1687,10 +1684,8 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>
>                         string = vol->prepath;
>                         vol->prepath = match_strdup(args);
> -                       if (vol->prepath == NULL) {
> -                               kfree(string);
> +                       if (vol->prepath == NULL)
>                                 goto out_nomem;
> -                       }
>                         /* Compare old prefixpath= option to new one */
>                         if (!string || strcmp(string, vol->prepath))
>                                 printk(KERN_WARNING "CIFS: the value of the "
> --
> 1.7.11.7
>
diff mbox

Patch

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 7635b5d..17c3643 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -1624,14 +1624,11 @@  cifs_parse_mount_options(const char *mountdata, const char *devname,
 		case Opt_unc:
 			string = vol->UNC;
 			vol->UNC = match_strdup(args);
-			if (vol->UNC == NULL) {
-				kfree(string);
+			if (vol->UNC == NULL)
 				goto out_nomem;
-			}
 
 			convert_delimiter(vol->UNC, '\\');
 			if (vol->UNC[0] != '\\' || vol->UNC[1] != '\\') {
-				kfree(string);
 				printk(KERN_ERR "CIFS: UNC Path does not "
 						"begin with // or \\\\\n");
 				goto cifs_parse_mount_err;
@@ -1687,10 +1684,8 @@  cifs_parse_mount_options(const char *mountdata, const char *devname,
 
 			string = vol->prepath;
 			vol->prepath = match_strdup(args);
-			if (vol->prepath == NULL) {
-				kfree(string);
+			if (vol->prepath == NULL)
 				goto out_nomem;
-			}
 			/* Compare old prefixpath= option to new one */
 			if (!string || strcmp(string, vol->prepath))
 				printk(KERN_WARNING "CIFS: the value of the "