Re: [PATCH] Btrfs: do not overwrite error return value in the balance ioctl

[Anand Jain <anand.jain@oracle.com>]

On 12/15/2018 03:45 AM, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> If the call to btrfs_balance() failed we would overwrite the error
> returned to user space with -EFAULT if the call to copy_to_user() failed
> as well. Fix that by calling copy_to_user() only if btrfs_balance()
> returned success.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
>   fs/btrfs/ioctl.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index abe45fd97ab5..4ad7288f77d0 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -4707,7 +4707,7 @@ static long btrfs_ioctl_balance(struct file *file, void __user *arg)
>   	ret = btrfs_balance(fs_info, bctl, bargs);
>   	bctl = NULL;
>   
> -	if (arg) {
> +	if (ret == 0 && arg) {
>   		if (copy_to_user(arg, bargs, sizeof(*bargs)))
>   			ret = -EFAULT;
>   	}
> 

This changes the BTRFS_IOC_BALANCE_V2 semantics.

Originally, if BTRFS_IOC_BALANCE_V2 is failed/canceled we do update
bargs [1] and copy to user land, but after this patch we copy only
if the ioctl is successful. I doubt if David is OK with that.

[1]
----------
3717 int btrfs_balance(struct btrfs_fs_info *fs_info,
3718                   struct btrfs_balance_control *bctl,
3719                   struct btrfs_ioctl_balance_args *bargs)
3720 {
::

3855         ret = __btrfs_balance(fs_info);
3856
3857         mutex_lock(&fs_info->balance_mutex);
3858         clear_bit(BTRFS_FS_BALANCE_RUNNING, &fs_info->flags);
3859
3860         if (bargs) {
3861                 memset(bargs, 0, sizeof(*bargs));
3862                 btrfs_update_ioctl_balance_args(fs_info, bargs);
3863         }