diff mbox

btrfs: fix possible leak in btrfs_ioctl_balance()

Message ID 1445381406-13475-1-git-send-email-cengelma@gmx.at (mailing list archive)
State Accepted
Headers show

Commit Message

Christian Engelmayer Oct. 20, 2015, 10:50 p.m. UTC
Commit 8eb934591f8b ("btrfs: check unsupported filters in balance
arguments") adds a jump to exit label out_bargs in case the argument
check fails. At this point in addition to the bargs memory, the
memory for struct btrfs_balance_control has already been allocated.
Ownership of bctl is passed to btrfs_balance() in the good case,
thus the memory is not freed due to the introduced jump. Make sure
that the memory gets freed in any case as necessary. Detected by
Coverity CID 1328378.

Signed-off-by: Christian Engelmayer <cengelma@gmx.at>
---
The proposed patch is only test compiled.
---
 fs/btrfs/ioctl.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

David Sterba Oct. 21, 2015, 9:46 a.m. UTC | #1
On Wed, Oct 21, 2015 at 12:50:06AM +0200, Christian Engelmayer wrote:
> Commit 8eb934591f8b ("btrfs: check unsupported filters in balance
> arguments") adds a jump to exit label out_bargs in case the argument
> check fails. At this point in addition to the bargs memory, the
> memory for struct btrfs_balance_control has already been allocated.
> Ownership of bctl is passed to btrfs_balance() in the good case,
> thus the memory is not freed due to the introduced jump. Make sure
> that the memory gets freed in any case as necessary. Detected by
> Coverity CID 1328378.
> 
> Signed-off-by: Christian Engelmayer <cengelma@gmx.at>
Reviewed-by: David Sterba <dsterba@suse.com>

Thanks for fixig it.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 3e3e6130637f..8d20f3b1cab0 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -4641,7 +4641,7 @@  locked:
 
 	if (bctl->flags & ~(BTRFS_BALANCE_ARGS_MASK | BTRFS_BALANCE_TYPE_MASK)) {
 		ret = -EINVAL;
-		goto out_bargs;
+		goto out_bctl;
 	}
 
 do_balance:
@@ -4655,12 +4655,15 @@  do_balance:
 	need_unlock = false;
 
 	ret = btrfs_balance(bctl, bargs);
+	bctl = NULL;
 
 	if (arg) {
 		if (copy_to_user(arg, bargs, sizeof(*bargs)))
 			ret = -EFAULT;
 	}
 
+out_bctl:
+	kfree(bctl);
 out_bargs:
 	kfree(bargs);
 out_unlock: