Skip to contentSkip to conversation

efi/efi_test: use memdup_user

PATCH Applied Geliang Tang 3 participants 7 messages Apr 29, 2017 – May 10, 2017

Subsystems (2)
EFI EFI test driver
lore
Share & tools
lore

Geliang Tang

author
#0 lore raw

PATCH 1/2 efi/efi_test: use memdup_user

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

Signed-off-by Geliang Tang <[email protected]>
---
 drivers/firmware/efi/test/efi_test.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)
drivers/firmware/efi/test/efi_test.c +3 -8
index 8cd578f..08129b7 100644
@@ -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;
 }
Signature · 1 line
2.9.3
author
#1 lore raw

PATCH 2/2 efi/efi_test: drop useless kfree

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

Signed-off-by Geliang Tang <[email protected]>
---
 drivers/firmware/efi/test/efi_test.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/firmware/efi/test/efi_test.c +1 -3
index 08129b7..00332ae 100644
@@ -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,
Signature · 1 line
2.9.3
maintainer
#2 lore raw

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

Quoted 13 lines · Geliang Tang

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

Signed-off-by Geliang Tang <[email protected]>
---
  drivers/firmware/efi/test/efi_test.c | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)
drivers/firmware/efi/test/efi_test.c +0 -0
index 08129b7..00332ae 100644

drivers/firmware/efi/test/efi_test.c static long efi_runtime_set_variable(unsigned long arg) · line 265

Quoted 5 lines
drivers/firmware/efi/test/efi_test.c +0 -2
@@ -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

Quoted 6 lines
drivers/firmware/efi/test/efi_test.c +1 -1
+	if (IS_ERR(data))
  		return PTR_ERR(data);
-	}
 
  	status = efi.set_variable(name, &vendor_guid,
  				setvariable.attributes,
author
#3 lore raw

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

Quoted 11 lines · ivanhu
Quoted 8 lines · Geliang Tang

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

Signed-off-by Geliang Tang <[email protected]>
---
  drivers/firmware/efi/test/efi_test.c | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)
Quoted 11 lines

drivers/firmware/efi/test/efi_test.c static long efi_runtime_set_variable(unsigned long arg) · line 264

Quoted 9 lines
drivers/firmware/efi/test/efi_test.c +0 -2
index 08129b7..00332ae 100644
@@ -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

Quoted 8 lines

Cheers,
Ivan

Quoted 5 lines
drivers/firmware/efi/test/efi_test.c +1 -1
+	if (IS_ERR(data))
  		return PTR_ERR(data);
-	}
  	status = efi.set_variable(name, &vendor_guid,
  				setvariable.attributes,
maintainer
#5 lore raw
Quoted 10 lines · Matt Fleming
Quoted 6 lines · Geliang Tang

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

Signed-off-by Geliang Tang <[email protected]>
---
 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

Applied
#6 lore raw
Quoted 11 lines · Ivan Hu
Quoted 8 lines · Matt Fleming
Quoted 6 lines · Geliang Tang

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

Signed-off-by Geliang Tang <[email protected]>

---
drivers/firmware/efi/test/efi_test.c | 11 +++--------

1 file changed, 3 insertions(+), 8 deletions(-)
Quoted 4 lines
Quoted 2 lines

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

Looks fine to me too, ACK. Thanks!

Applied, thanks everyone.