[PATCH 1/2] efi/efi_test: use memdup_user

[PATCH 1/2] efi/efi_test: use memdup_user

[Geliang Tang]

Use memdup_user() helper instead of open-coding to simplify the code.

Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
 drivers/firmware/efi/test/efi_test.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/firmware/efi/test/efi_test.c b/drivers/firmware/efi/test/efi_test.c
index 8cd578f..08129b7 100644
--- a/drivers/firmware/efi/test/efi_test.c
+++ b/drivers/firmware/efi/test/efi_test.c
@@ -71,18 +71,13 @@ copy_ucs2_from_user_len(efi_char16_t **dst, efi_char16_t __user *src,
 	if (!access_ok(VERIFY_READ, src, 1))
 		return -EFAULT;
 
-	buf = kmalloc(len, GFP_KERNEL);
-	if (!buf) {
+	buf = memdup_user(src, len);
+	if (IS_ERR(buf)) {
 		*dst = NULL;
-		return -ENOMEM;
+		return PTR_ERR(buf);
 	}
 	*dst = buf;
 
-	if (copy_from_user(*dst, src, len)) {
-		kfree(buf);
-		return -EFAULT;
-	}
-
 	return 0;
 }
 
-- 
2.9.3

[PATCH 2/2] efi/efi_test: drop useless kfree

[Geliang Tang]

Drop useless kfree when memdup_user() failed, since we have already
called kfree in memdup_user().

Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
 drivers/firmware/efi/test/efi_test.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/firmware/efi/test/efi_test.c b/drivers/firmware/efi/test/efi_test.c
index 08129b7..00332ae 100644
--- a/drivers/firmware/efi/test/efi_test.c
+++ b/drivers/firmware/efi/test/efi_test.c
@@ -261,10 +261,8 @@ static long efi_runtime_set_variable(unsigned long arg)
 	}
 
 	data = memdup_user(setvariable.data, setvariable.data_size);
-	if (IS_ERR(data)) {
-		kfree(name);
+	if (IS_ERR(data))
 		return PTR_ERR(data);
-	}
 
 	status = efi.set_variable(name, &vendor_guid,
 				setvariable.attributes,
-- 
2.9.3

Re: [PATCH 2/2] efi/efi_test: drop useless kfree

[ivanhu]

On 04/29/2017 09:42 AM, Geliang Tang wrote:
> Drop useless kfree when memdup_user() failed, since we have already
> called kfree in memdup_user().
>
> Signed-off-by: Geliang Tang <geliangtang@gmail.com>
> ---
>   drivers/firmware/efi/test/efi_test.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/firmware/efi/test/efi_test.c b/drivers/firmware/efi/test/efi_test.c
> index 08129b7..00332ae 100644
> --- a/drivers/firmware/efi/test/efi_test.c
> +++ b/drivers/firmware/efi/test/efi_test.c
> @@ -261,10 +261,8 @@ static long efi_runtime_set_variable(unsigned long arg)
>   	}
>   
>   	data = memdup_user(setvariable.data, setvariable.data_size);
> -	if (IS_ERR(data)) {
> -		kfree(name);
You mean "name" or "data"?

Cheers,
Ivan

> +	if (IS_ERR(data))
>   		return PTR_ERR(data);
> -	}
>   
>   	status = efi.set_variable(name, &vendor_guid,
>   				setvariable.attributes,

Re: [PATCH 2/2] efi/efi_test: drop useless kfree

[Geliang Tang]

On Tue, May 02, 2017 at 03:11:45PM +0800, ivanhu wrote:
> 
> 
> On 04/29/2017 09:42 AM, Geliang Tang wrote:
> > Drop useless kfree when memdup_user() failed, since we have already
> > called kfree in memdup_user().
> > 
> > Signed-off-by: Geliang Tang <geliangtang@gmail.com>
> > ---
> >   drivers/firmware/efi/test/efi_test.c | 4 +---
> >   1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/drivers/firmware/efi/test/efi_test.c b/drivers/firmware/efi/test/efi_test.c
> > index 08129b7..00332ae 100644
> > --- a/drivers/firmware/efi/test/efi_test.c
> > +++ b/drivers/firmware/efi/test/efi_test.c
> > @@ -261,10 +261,8 @@ static long efi_runtime_set_variable(unsigned long arg)
> >   	}
> >   	data = memdup_user(setvariable.data, setvariable.data_size);
> > -	if (IS_ERR(data)) {
> > -		kfree(name);
> You mean "name" or "data"?
> 

Sorry, it's my mistake. The original code is correct. Please ignore this
patch.

-Geliang

> Cheers,
> Ivan
> 
> > +	if (IS_ERR(data))
> >   		return PTR_ERR(data);
> > -	}
> >   	status = efi.set_variable(name, &vendor_guid,
> >   				setvariable.attributes,

Re: [PATCH 1/2] efi/efi_test: use memdup_user

[Matt Fleming]

On Sat, 29 Apr, at 09:42:52AM, Geliang Tang wrote:
> Use memdup_user() helper instead of open-coding to simplify the code.
> 
> Signed-off-by: Geliang Tang <geliangtang@gmail.com>
> ---
>  drivers/firmware/efi/test/efi_test.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)

This one looks fine. Ivan, do you want to ACK it?

Re: [PATCH 1/2] efi/efi_test: use memdup_user

[ivanhu]

On 05/06/2017 04:53 AM, Matt Fleming wrote:
> On Sat, 29 Apr, at 09:42:52AM, Geliang Tang wrote:
>> Use memdup_user() helper instead of open-coding to simplify the code.
>>
>> Signed-off-by: Geliang Tang <geliangtang@gmail.com>
>> ---
>>  drivers/firmware/efi/test/efi_test.c | 11 +++--------
>>  1 file changed, 3 insertions(+), 8 deletions(-)
>
> This one looks fine. Ivan, do you want to ACK it?
>

Looks fine to me too, ACK. Thanks!

Ivan

Re: [PATCH 1/2] efi/efi_test: use memdup_user

[Matt Fleming]

On Mon, 08 May, at 04:18:30PM, Ivan Hu wrote:
> 
> 
> On 05/06/2017 04:53 AM, Matt Fleming wrote:
> >On Sat, 29 Apr, at 09:42:52AM, Geliang Tang wrote:
> >>Use memdup_user() helper instead of open-coding to simplify the code.
> >>
> >>Signed-off-by: Geliang Tang <geliangtang@gmail.com>
> >>---
> >> drivers/firmware/efi/test/efi_test.c | 11 +++--------
> >> 1 file changed, 3 insertions(+), 8 deletions(-)
> >
> >This one looks fine. Ivan, do you want to ACK it?
> >
> 
> Looks fine to me too, ACK. Thanks!

Applied, thanks everyone.