diff options
| author | Dan Carpenter <dan.carpenter@oracle.com> | 2014-09-04 14:09:15 +0300 | 
|---|---|---|
| committer | Chris Mason <clm@fb.com> | 2014-09-08 13:56:42 -0700 | 
| commit | c47ca32d3aadb234f73389a34c97574085bc9eda (patch) | |
| tree | c53ea33e0067560a26f0122b0b606cc5743b8acc | |
| parent | dac5705cad20070a70bb028ca52e1f0bc157b42d (diff) | |
Btrfs: kfree()ing ERR_PTRs
The "inherit" in btrfs_ioctl_snap_create_v2() and "vol_args" in
btrfs_ioctl_rm_dev() are ERR_PTRs so we can't call kfree() on them.
These kind of bugs are "One Err Bugs" where there is just one error
label that does everything.  I could set the "inherit = NULL" and keep
the single out label but it ends up being more complicated that way.  It
makes the code simpler to re-order the unwind so it's in the mirror
order of the allocation and introduce some new error labels.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Chris Mason <clm@fb.com>
| -rw-r--r-- | fs/btrfs/ioctl.c | 25 | 
1 files changed, 15 insertions, 10 deletions
| diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index a018ea484d39..8a8e29878c34 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -1703,7 +1703,7 @@ static noinline int btrfs_ioctl_snap_create_v2(struct file *file,  	    ~(BTRFS_SUBVOL_CREATE_ASYNC | BTRFS_SUBVOL_RDONLY |  	      BTRFS_SUBVOL_QGROUP_INHERIT)) {  		ret = -EOPNOTSUPP; -		goto out; +		goto free_args;  	}  	if (vol_args->flags & BTRFS_SUBVOL_CREATE_ASYNC) @@ -1713,27 +1713,31 @@ static noinline int btrfs_ioctl_snap_create_v2(struct file *file,  	if (vol_args->flags & BTRFS_SUBVOL_QGROUP_INHERIT) {  		if (vol_args->size > PAGE_CACHE_SIZE) {  			ret = -EINVAL; -			goto out; +			goto free_args;  		}  		inherit = memdup_user(vol_args->qgroup_inherit, vol_args->size);  		if (IS_ERR(inherit)) {  			ret = PTR_ERR(inherit); -			goto out; +			goto free_args;  		}  	}  	ret = btrfs_ioctl_snap_create_transid(file, vol_args->name,  					      vol_args->fd, subvol, ptr,  					      readonly, inherit); +	if (ret) +		goto free_inherit; -	if (ret == 0 && ptr && -	    copy_to_user(arg + -			 offsetof(struct btrfs_ioctl_vol_args_v2, -				  transid), ptr, sizeof(*ptr))) +	if (ptr && copy_to_user(arg + +				offsetof(struct btrfs_ioctl_vol_args_v2, +					transid), +				ptr, sizeof(*ptr)))  		ret = -EFAULT; -out: -	kfree(vol_args); + +free_inherit:  	kfree(inherit); +free_args: +	kfree(vol_args);  	return ret;  } @@ -2653,7 +2657,7 @@ static long btrfs_ioctl_rm_dev(struct file *file, void __user *arg)  	vol_args = memdup_user(arg, sizeof(*vol_args));  	if (IS_ERR(vol_args)) {  		ret = PTR_ERR(vol_args); -		goto out; +		goto err_drop;  	}  	vol_args->name[BTRFS_PATH_NAME_MAX] = '\0'; @@ -2671,6 +2675,7 @@ static long btrfs_ioctl_rm_dev(struct file *file, void __user *arg)  out:  	kfree(vol_args); +err_drop:  	mnt_drop_write_file(file);  	return ret;  } | 
