| Message ID | 20230925-strncpy-drivers-input-misc-axp20x-pek-c-v2-1-ff7abe8498d6@google.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 68ede283a1d8fe0813b218aeb498faf3b0fc0a7b |
| Headers | show |
| Series | [v2] Input: axp20x-pek - avoid needless newline removal | expand |
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> This looks much cleaner. Thanks! Reviewed-by: Kees Cook <keescook@chromium.org>
On Tue, Sep 26, 2023 at 2:00 AM Kees Cook <keescook@chromium.org> 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: linux-hardening@vger.kernel.org > > Suggested-by: Kees Cook <keescook@chromium.org> > > Signed-off-by: Justin Stitt <justinstitt@google.com> > > This looks much cleaner. Thanks! > > Reviewed-by: Kees Cook <keescook@chromium.org> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
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 --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;
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>