diff mbox

efi_runtime: use memdup_user() as a cleanup

Message ID 1473838295-12528-1-git-send-email-ivan.hu@canonical.com
State Accepted
Headers show

Commit Message

Ivan Hu Sept. 14, 2016, 7:31 a.m. UTC
Got warning when upstreaming efi_runtime driver

drivers/firmware/efi/test/efi_test.c:269:8-15: WARNING opportunity for
memdup_user

Use memdup_user rather than duplicating its implementation
This is a little bit restricted to reduce false positives

Generated by: coccinelle/api/memdup_user.coc

Signed-off-by: Ivan Hu <ivan.hu@canonical.com>
---
 efi_runtime/efi_runtime.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

Comments

Colin Ian King Sept. 14, 2016, 7:32 a.m. UTC | #1
On 14/09/16 08:31, Ivan Hu wrote:
> Got warning when upstreaming efi_runtime driver
> 
> drivers/firmware/efi/test/efi_test.c:269:8-15: WARNING opportunity for
> memdup_user
> 
> Use memdup_user rather than duplicating its implementation
> This is a little bit restricted to reduce false positives
> 
> Generated by: coccinelle/api/memdup_user.coc
> 
> Signed-off-by: Ivan Hu <ivan.hu@canonical.com>
> ---
>  efi_runtime/efi_runtime.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/efi_runtime/efi_runtime.c b/efi_runtime/efi_runtime.c
> index 594eccd..19f8103 100644
> --- a/efi_runtime/efi_runtime.c
> +++ b/efi_runtime/efi_runtime.c
> @@ -284,14 +284,10 @@ static long efi_runtime_set_variable(unsigned long arg)
>  			return rv;
>  	}
>  
> -	data = kmalloc(setvariable.data_size, GFP_KERNEL);
> -	if (!data) {
> +	data = memdup_user(setvariable.data, setvariable.data_size);
> +	if (IS_ERR(data)) {
>  		kfree(name);
> -		return -ENOMEM;
> -	}
> -	if (copy_from_user(data, setvariable.data, setvariable.data_size)) {
> -		rv = -EFAULT;
> -		goto out;
> +		return PTR_ERR(data);
>  	}
>  
>  	status = efi.set_variable(name, &vendor_guid,
> 
memdup_user was added to the kernel in 2009, so I guess this works fine
for all our supported releases.

Acked-by: Colin Ian King <colin.king@canonical.com>
Alex Hung Sept. 14, 2016, 4:42 p.m. UTC | #2
On 2016-09-14 12:31 AM, Ivan Hu wrote:
> Got warning when upstreaming efi_runtime driver
>
> drivers/firmware/efi/test/efi_test.c:269:8-15: WARNING opportunity for
> memdup_user
>
> Use memdup_user rather than duplicating its implementation
> This is a little bit restricted to reduce false positives
>
> Generated by: coccinelle/api/memdup_user.coc
>
> Signed-off-by: Ivan Hu <ivan.hu@canonical.com>
> ---
>  efi_runtime/efi_runtime.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/efi_runtime/efi_runtime.c b/efi_runtime/efi_runtime.c
> index 594eccd..19f8103 100644
> --- a/efi_runtime/efi_runtime.c
> +++ b/efi_runtime/efi_runtime.c
> @@ -284,14 +284,10 @@ static long efi_runtime_set_variable(unsigned long arg)
>  			return rv;
>  	}
>
> -	data = kmalloc(setvariable.data_size, GFP_KERNEL);
> -	if (!data) {
> +	data = memdup_user(setvariable.data, setvariable.data_size);
> +	if (IS_ERR(data)) {
>  		kfree(name);
> -		return -ENOMEM;
> -	}
> -	if (copy_from_user(data, setvariable.data, setvariable.data_size)) {
> -		rv = -EFAULT;
> -		goto out;
> +		return PTR_ERR(data);
>  	}
>
>  	status = efi.set_variable(name, &vendor_guid,
>
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 594eccd..19f8103 100644
--- a/efi_runtime/efi_runtime.c
+++ b/efi_runtime/efi_runtime.c
@@ -284,14 +284,10 @@  static long efi_runtime_set_variable(unsigned long arg)
 			return rv;
 	}
 
-	data = kmalloc(setvariable.data_size, GFP_KERNEL);
-	if (!data) {
+	data = memdup_user(setvariable.data, setvariable.data_size);
+	if (IS_ERR(data)) {
 		kfree(name);
-		return -ENOMEM;
-	}
-	if (copy_from_user(data, setvariable.data, setvariable.data_size)) {
-		rv = -EFAULT;
-		goto out;
+		return PTR_ERR(data);
 	}
 
 	status = efi.set_variable(name, &vendor_guid,