diff mbox series

[v2] Input: axp20x-pek - avoid needless newline removal

Message ID 20230925-strncpy-drivers-input-misc-axp20x-pek-c-v2-1-ff7abe8498d6@google.com
State Accepted
Commit 68ede283a1d8fe0813b218aeb498faf3b0fc0a7b
Headers show
Series [v2] Input: axp20x-pek - avoid needless newline removal | expand

Commit Message

Justin Stitt Sept. 25, 2023, 4:31 a.m. UTC
This code is doing more work than it needs to.

Before handing off `val_str` to `kstrtouint()` we are eagerly removing
any trailing newline which requires copying `buf`, validating it's
length and checking/replacing any potential newlines.

kstrtouint() handles this implicitly:
kstrtouint ->
  kstrotoull -> (documentation)
|   /**
|    * kstrtoull - convert a string to an unsigned long long
|    * @s: The start of the string. The string must be null-terminated, and may also
|    *  include a single newline before its terminating null. The first character
|    ...

Let's remove the redundant functionality and let kstrtouint handle it.

Link: https://github.com/KSPP/linux/issues/90
Cc: linux-hardening@vger.kernel.org
Suggested-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Justin Stitt <justinstitt@google.com>
---
Changes in v2:
- remove eager newline removal (thanks Kees)
- use better subject line (thanks Kees)
- rebase on 6465e260f4879080
- Link to v1: https://lore.kernel.org/r/20230921-strncpy-drivers-input-misc-axp20x-pek-c-v1-1-f7f6f4a5cf81@google.com
---
Note: build-tested only.
---
 drivers/input/misc/axp20x-pek.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)


---
base-commit: 6465e260f48790807eef06b583b38ca9789b6072
change-id: 20230921-strncpy-drivers-input-misc-axp20x-pek-c-3c4708924bbd

Best regards,
--
Justin Stitt <justinstitt@google.com>

Comments

Dmitry Torokhov Sept. 30, 2023, 4:34 p.m. UTC | #1
On Mon, Sep 25, 2023 at 04:31:05AM +0000, Justin Stitt wrote:
> This code is doing more work than it needs to.
> 
> Before handing off `val_str` to `kstrtouint()` we are eagerly removing
> any trailing newline which requires copying `buf`, validating it's
> length and checking/replacing any potential newlines.
> 
> kstrtouint() handles this implicitly:
> kstrtouint ->
>   kstrotoull -> (documentation)
> |   /**
> |    * kstrtoull - convert a string to an unsigned long long
> |    * @s: The start of the string. The string must be null-terminated, and may also
> |    *  include a single newline before its terminating null. The first character
> |    ...
> 
> Let's remove the redundant functionality and let kstrtouint handle it.
> 
> Link: https://github.com/KSPP/linux/issues/90
> Cc: linux-hardening@vger.kernel.org
> Suggested-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Justin Stitt <justinstitt@google.com>

Applied, thank you.
diff mbox series

Patch

diff --git a/drivers/input/misc/axp20x-pek.c b/drivers/input/misc/axp20x-pek.c
index 4581606a28d6..24f9e9d893de 100644
--- a/drivers/input/misc/axp20x-pek.c
+++ b/drivers/input/misc/axp20x-pek.c
@@ -133,20 +133,11 @@  static ssize_t axp20x_store_attr(struct device *dev,
 				 size_t count)
 {
 	struct axp20x_pek *axp20x_pek = dev_get_drvdata(dev);
-	char val_str[20];
-	size_t len;
 	int ret, i;
 	unsigned int val, idx = 0;
 	unsigned int best_err = UINT_MAX;
 
-	val_str[sizeof(val_str) - 1] = '\0';
-	strncpy(val_str, buf, sizeof(val_str) - 1);
-	len = strlen(val_str);
-
-	if (len && val_str[len - 1] == '\n')
-		val_str[len - 1] = '\0';
-
-	ret = kstrtouint(val_str, 10, &val);
+	ret = kstrtouint(buf, 10, &val);
 	if (ret)
 		return ret;