On Thu, Sep 21, 2023 at 05:41:46AM +0000, Justin Stitt wrote:
> `strncpy` is deprecated for use on NUL-terminated destination strings [1].
>
> Let's refactor this kcalloc() + strncpy() into a kmemdup_nul() which has
> more obvious behavior and is less error prone.
>
> To avoid truncating the last byte supply `...length + 1` to
> kmemdup_nul() as `element->string.length` does not account for the
> trailing null as made obvious from it's definition (and associated
> comment):
> | u32 length; /* # of bytes in string, excluding trailing null */
>
> ... this is precisely what the original kcalloc invocation did as well.
>
> Link:
> https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
> [1]
> Link: https://github.com/KSPP/linux/issues/90
> Cc: [email protected]
> Signed-off-by: Justin Stitt <[email protected]>
> ---
> Changes in v3:
> - refactor to use kmemdup_nul() (thanks Thomas and Kees)
> - change commit msg to reflect ^
> - rebase onto 2cf0f71562387282
> - Link to v2:
> https://lore.kernel.org/r/20230919-strncpy-drivers-hwmon-acpi_power_meter-c-v2-1-8348432d6...@google.com
>
> Changes in v2:
> - use memcpy over strscpy (thanks Kees)
> - Link to v1:
> https://lore.kernel.org/r/20230914-strncpy-drivers-hwmon-acpi_power_meter-c-v1-1-905297479...@google.com
> ---
> drivers/hwmon/acpi_power_meter.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hwmon/acpi_power_meter.c
> b/drivers/hwmon/acpi_power_meter.c
> index fa28d447f0df..146c4f09c897 100644
> --- a/drivers/hwmon/acpi_power_meter.c
> +++ b/drivers/hwmon/acpi_power_meter.c
> @@ -796,14 +796,13 @@ static int read_capabilities(struct
> acpi_power_meter_resource *resource)
> goto error;
> }
>
> - *str = kcalloc(element->string.length + 1, sizeof(u8),
> - GFP_KERNEL);
> + *str = kmemdup_nul(element->string.pointer,
> element->string.length + 1,
> + GFP_KERNEL);
kmemdup_nul() already adds the "+ 1", so it's not needed here.
For v4, please improve the Subject along the lines of "...: Replace
open-coded kmemdup_nul"
-Kees
> if (!*str) {
> res = -ENOMEM;
> goto error;
> }
>
> - strncpy(*str, element->string.pointer, element->string.length);
> str++;
> }
>
>
> ---
> base-commit: 2cf0f715623872823a72e451243bbf555d10d032
> change-id: 20230914-strncpy-drivers-hwmon-acpi_power_meter-c-c9f2d8053bef
>
> Best regards,
> --
> Justin Stitt <[email protected]>
>
--
Kees Cook
