Both `strncpy` and `strcpy` are deprecated for use on NUL-terminated destination strings [1]. We can see that `arg` and `uv_nmi_action` are expected to be NUL-terminated strings due to their use within `strcmp()` and format strings respectively. With this in mind, a suitable replacement is `strscpy` [2] due to the fact that it guarantees NUL-termination on its destination buffer argument which is _not_ the case for `strncpy` or `strcpy`! In this case, we can drop both the forced NUL-termination and the `... -1` from: | strncpy(arg, val, ACTION_LEN - 1); as `strscpy` implicitly has this behavior. Link: www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings[1] Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] Link: https://github.com/KSPP/linux/issues/90 Cc: linux-hardening@xxxxxxxxxxxxxxx Signed-off-by: Justin Stitt <justinstitt@xxxxxxxxxx> --- Changes in v3: - Use sizeof instead of strlen (thanks Andy and Dimitri) - Drop unrelated changes regarding strnchrnul (thanks Hans) - Link to v2: https://lore.kernel.org/r/20230824-strncpy-arch-x86-platform-uv-uv_nmi-v2-1-e16d9a3ec570@xxxxxxxxxx Changes in v2: - use `sizeof` on destination string instead of `strlen` (thanks Andy, Kees and Dimitri) - refactor code to remove potential new-line chars (thanks Yang Yang and Andy) - Link to v1: https://lore.kernel.org/r/20230822-strncpy-arch-x86-platform-uv-uv_nmi-v1-1-931f2943de0d@xxxxxxxxxx --- Note: build-tested only --- arch/x86/platform/uv/uv_nmi.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/arch/x86/platform/uv/uv_nmi.c b/arch/x86/platform/uv/uv_nmi.c index a60af0230e27..dd30fb2baf6c 100644 --- a/arch/x86/platform/uv/uv_nmi.c +++ b/arch/x86/platform/uv/uv_nmi.c @@ -205,8 +205,7 @@ static int param_set_action(const char *val, const struct kernel_param *kp) char arg[ACTION_LEN], *p; /* (remove possible '\n') */ - strncpy(arg, val, ACTION_LEN - 1); - arg[ACTION_LEN - 1] = '\0'; + strscpy(arg, val, sizeof(arg)); p = strchr(arg, '\n'); if (p) *p = '\0'; @@ -216,7 +215,7 @@ static int param_set_action(const char *val, const struct kernel_param *kp) break; if (i < n) { - strcpy(uv_nmi_action, arg); + strscpy(uv_nmi_action, arg, sizeof(uv_nmi_action)); pr_info("UV: New NMI action:%s\n", uv_nmi_action); return 0; } @@ -959,7 +958,7 @@ static int uv_handle_nmi(unsigned int reason, struct pt_regs *regs) /* Unexpected return, revert action to "dump" */ if (master) - strncpy(uv_nmi_action, "dump", strlen(uv_nmi_action)); + strscpy(uv_nmi_action, "dump", sizeof(uv_nmi_action)); } /* Pause as all CPU's enter the NMI handler */ --- base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c change-id: 20230822-strncpy-arch-x86-platform-uv-uv_nmi-474e5295c2c1 Best regards, -- Justin Stitt <justinstitt@xxxxxxxxxx>
