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: [email protected]
Suggested-by: Kees Cook <[email protected]>
Signed-off-by: Justin Stitt <[email protected]>
---
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(-)
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;
---
base-commit: 6465e260f48790807eef06b583b38ca9789b6072
change-id: 20230921-strncpy-drivers-input-misc-axp20x-pek-c-3c4708924bbd
Best regards,
--
Justin Stitt <[email protected]>
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: [email protected]
> Suggested-by: Kees Cook <[email protected]>
> Signed-off-by: Justin Stitt <[email protected]>
This looks much cleaner. Thanks!
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook
On Tue, Sep 26, 2023 at 2:00 AM Kees Cook <[email protected]> wrote:
>
> 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: [email protected]
> > Suggested-by: Kees Cook <[email protected]>
> > Signed-off-by: Justin Stitt <[email protected]>
>
> This looks much cleaner. Thanks!
>
> Reviewed-by: Kees Cook <[email protected]>
Reviewed-by: Chen-Yu Tsai <[email protected]>
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: [email protected]
> Suggested-by: Kees Cook <[email protected]>
> Signed-off-by: Justin Stitt <[email protected]>
Applied, thank you.
--
Dmitry