ACK: [PATCH 3/4] efi_runtime: move the checking status forward

[Alex Hung]

On 2016-08-22 05:08 PM, Ivan Hu wrote:
> Move the checking status forward, no need to copy the invalid data to user,
> when firmware return not EFI_SUCCESS.
>
> Signed-off-by: Ivan Hu <ivan.hu at canonical.com>
> ---
>  efi_runtime/efi_runtime.c | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/efi_runtime/efi_runtime.c b/efi_runtime/efi_runtime.c
> index ee159ff..563f782 100644
> --- a/efi_runtime/efi_runtime.c
> +++ b/efi_runtime/efi_runtime.c
> @@ -395,8 +395,10 @@ static long efi_runtime_get_waketime(unsigned long arg)
>
>  	if (put_user(status, getwakeuptime.status))
>  		return -EFAULT;
> +
>  	if (status != EFI_SUCCESS)
>  		return -EINVAL;
> +
>  	if (getwakeuptime.enabled && put_user(enabled,
>  						getwakeuptime.enabled))
>  		return -EFAULT;
> @@ -565,13 +567,13 @@ static long efi_runtime_get_nexthighmonocount(unsigned long arg)
>  	if (put_user(status, getnexthighmonocount.status))
>  		return -EFAULT;
>
> +	if (status != EFI_SUCCESS)
> +		return -EINVAL;
> +
>  	if (getnexthighmonocount.high_count &&
>  	    put_user(count, getnexthighmonocount.high_count))
>  		return -EFAULT;
>
> -	if (status != EFI_SUCCESS)
> -		return -EINVAL;
> -
>  	return 0;
>  }
>
> @@ -593,6 +595,12 @@ static long efi_runtime_query_variableinfo(unsigned long arg)
>  	status = efi.query_variable_info(queryvariableinfo.attributes,
>  					 &max_storage, &remaining, &max_size);
>
> +	if (put_user(status, queryvariableinfo.status))
> +		return -EFAULT;
> +
> +	if (status != EFI_SUCCESS)
> +		return -EINVAL;
> +
>  	if (put_user(max_storage,
>  		     queryvariableinfo.maximum_variable_storage_size))
>  		return -EFAULT;
> @@ -604,11 +612,6 @@ static long efi_runtime_query_variableinfo(unsigned long arg)
>  	if (put_user(max_size, queryvariableinfo.maximum_variable_size))
>  		return -EFAULT;
>
> -	if (put_user(status, queryvariableinfo.status))
> -		return -EFAULT;
> -	if (status != EFI_SUCCESS)
> -		return -EINVAL;
> -
>  	return 0;
>  }
>
> @@ -662,20 +665,18 @@ static long efi_runtime_query_capsulecaps(unsigned long arg)
>  		goto out;
>  	}
>
> -	if (put_user(max_size, qcaps.maximum_capsule_size)) {
> -		rv = -EFAULT;
> +	if (status != EFI_SUCCESS) {
> +		rv = -EINVAL;
>  		goto out;
>  	}
>
> -	if (put_user(reset_type, qcaps.reset_type)) {
> +	if (put_user(max_size, qcaps.maximum_capsule_size)) {
>  		rv = -EFAULT;
>  		goto out;
>  	}
>
> -	if (status != EFI_SUCCESS) {
> -		rv = -EINVAL;
> -		goto out;
> -	}
> +	if (put_user(reset_type, qcaps.reset_type))
> +		rv = -EFAULT;
>
>  out:
>  	kfree(capsules);
>


Acked-by: Alex Hung <alex.hung at canonical.com>