diff mbox

[2/4] efi_runtime: fix memory leak of getvariable funtion

Message ID 1471856920-25084-2-git-send-email-ivan.hu@canonical.com
State Accepted
Headers show

Commit Message

Ivan Hu Aug. 22, 2016, 9:08 a.m. UTC
Signed-off-by: Ivan Hu <ivan.hu@canonical.com>
---
 efi_runtime/efi_runtime.c | 43 ++++++++++++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 15 deletions(-)

Comments

Colin Ian King Aug. 22, 2016, 10:02 a.m. UTC | #1
On 22/08/16 10:08, Ivan Hu wrote:
> Signed-off-by: Ivan Hu <ivan.hu@canonical.com>
> ---
>  efi_runtime/efi_runtime.c | 43 ++++++++++++++++++++++++++++---------------
>  1 file changed, 28 insertions(+), 15 deletions(-)
> 
> diff --git a/efi_runtime/efi_runtime.c b/efi_runtime/efi_runtime.c
> index c60a6d8..ee159ff 100644
> --- a/efi_runtime/efi_runtime.c
> +++ b/efi_runtime/efi_runtime.c
> @@ -218,33 +218,46 @@ static long efi_runtime_get_variable(unsigned long arg)
>  	status = efi.get_variable(name, vd, at, dz, data);
>  	kfree(name);
>  
> -	if (put_user(status, getvariable.status))
> -		return -EFAULT;
> +	if (put_user(status, getvariable.status)) {
> +		rv = -EFAULT;
> +		goto out;
> +	}
>  
>  	if (status != EFI_SUCCESS) {
>  		if (status == EFI_BUFFER_TOO_SMALL) {
> -			if (dz && put_user(datasize, getvariable.data_size))
> -				return -EFAULT;
> +			if (dz && put_user(datasize, getvariable.data_size)) {
> +				rv = -EFAULT;
> +				goto out;
> +			}
>  		}
> -		return -EINVAL;
> +		rv = -EINVAL;
> +		goto out;
>  	}
>  
> -	if (prev_datasize < datasize)
> -		return -EINVAL;
> +	if (prev_datasize < datasize) {
> +		rv = -EINVAL;
> +		goto out;
> +	}
>  
>  	if (data) {
> -		rv = copy_to_user(getvariable.data, data, datasize);
> -		kfree(data);
> -		if (rv)
> -			return rv;
> +		if (copy_to_user(getvariable.data, data, datasize)) {
> +			rv = -EFAULT;
> +			goto out;
> +		}
> +	}
> +
> +	if (at && put_user(attr, getvariable.attributes)) {
> +		rv = -EFAULT;
> +		goto out;
>  	}
>  
> -	if (at && put_user(attr, getvariable.attributes))
> -		return -EFAULT;
>  	if (dz && put_user(datasize, getvariable.data_size))
> -		return -EFAULT;
> +		rv = -EFAULT;
> +
> +out:
> +	kfree(data);
> +	return rv;
>  
> -	return 0;
>  }
>  
>  static long efi_runtime_set_variable(unsigned long arg)
> 
Acked-by: Colin Ian King <colin.king@canonical.com>
Alex Hung Aug. 23, 2016, 2:06 a.m. UTC | #2
On 2016-08-22 05:08 PM, Ivan Hu wrote:
> Signed-off-by: Ivan Hu <ivan.hu@canonical.com>
> ---
>  efi_runtime/efi_runtime.c | 43 ++++++++++++++++++++++++++++---------------
>  1 file changed, 28 insertions(+), 15 deletions(-)
>
> diff --git a/efi_runtime/efi_runtime.c b/efi_runtime/efi_runtime.c
> index c60a6d8..ee159ff 100644
> --- a/efi_runtime/efi_runtime.c
> +++ b/efi_runtime/efi_runtime.c
> @@ -218,33 +218,46 @@ static long efi_runtime_get_variable(unsigned long arg)
>  	status = efi.get_variable(name, vd, at, dz, data);
>  	kfree(name);
>
> -	if (put_user(status, getvariable.status))
> -		return -EFAULT;
> +	if (put_user(status, getvariable.status)) {
> +		rv = -EFAULT;
> +		goto out;
> +	}
>
>  	if (status != EFI_SUCCESS) {
>  		if (status == EFI_BUFFER_TOO_SMALL) {
> -			if (dz && put_user(datasize, getvariable.data_size))
> -				return -EFAULT;
> +			if (dz && put_user(datasize, getvariable.data_size)) {
> +				rv = -EFAULT;
> +				goto out;
> +			}
>  		}
> -		return -EINVAL;
> +		rv = -EINVAL;
> +		goto out;
>  	}
>
> -	if (prev_datasize < datasize)
> -		return -EINVAL;
> +	if (prev_datasize < datasize) {
> +		rv = -EINVAL;
> +		goto out;
> +	}
>
>  	if (data) {
> -		rv = copy_to_user(getvariable.data, data, datasize);
> -		kfree(data);
> -		if (rv)
> -			return rv;
> +		if (copy_to_user(getvariable.data, data, datasize)) {
> +			rv = -EFAULT;
> +			goto out;
> +		}
> +	}
> +
> +	if (at && put_user(attr, getvariable.attributes)) {
> +		rv = -EFAULT;
> +		goto out;
>  	}
>
> -	if (at && put_user(attr, getvariable.attributes))
> -		return -EFAULT;
>  	if (dz && put_user(datasize, getvariable.data_size))
> -		return -EFAULT;
> +		rv = -EFAULT;
> +
> +out:
> +	kfree(data);
> +	return rv;
>
> -	return 0;
>  }
>
>  static long efi_runtime_set_variable(unsigned long arg)
>

Acked-by: Alex Hung <alex.hung@canonical.com>
diff mbox

Patch

diff --git a/efi_runtime/efi_runtime.c b/efi_runtime/efi_runtime.c
index c60a6d8..ee159ff 100644
--- a/efi_runtime/efi_runtime.c
+++ b/efi_runtime/efi_runtime.c
@@ -218,33 +218,46 @@  static long efi_runtime_get_variable(unsigned long arg)
 	status = efi.get_variable(name, vd, at, dz, data);
 	kfree(name);
 
-	if (put_user(status, getvariable.status))
-		return -EFAULT;
+	if (put_user(status, getvariable.status)) {
+		rv = -EFAULT;
+		goto out;
+	}
 
 	if (status != EFI_SUCCESS) {
 		if (status == EFI_BUFFER_TOO_SMALL) {
-			if (dz && put_user(datasize, getvariable.data_size))
-				return -EFAULT;
+			if (dz && put_user(datasize, getvariable.data_size)) {
+				rv = -EFAULT;
+				goto out;
+			}
 		}
-		return -EINVAL;
+		rv = -EINVAL;
+		goto out;
 	}
 
-	if (prev_datasize < datasize)
-		return -EINVAL;
+	if (prev_datasize < datasize) {
+		rv = -EINVAL;
+		goto out;
+	}
 
 	if (data) {
-		rv = copy_to_user(getvariable.data, data, datasize);
-		kfree(data);
-		if (rv)
-			return rv;
+		if (copy_to_user(getvariable.data, data, datasize)) {
+			rv = -EFAULT;
+			goto out;
+		}
+	}
+
+	if (at && put_user(attr, getvariable.attributes)) {
+		rv = -EFAULT;
+		goto out;
 	}
 
-	if (at && put_user(attr, getvariable.attributes))
-		return -EFAULT;
 	if (dz && put_user(datasize, getvariable.data_size))
-		return -EFAULT;
+		rv = -EFAULT;
+
+out:
+	kfree(data);
+	return rv;
 
-	return 0;
 }
 
 static long efi_runtime_set_variable(unsigned long arg)