* Justin Stitt <justinstitt@xxxxxxxxxx> wrote: > 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> > arch/x86/platform/uv/uv_nmi.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) Note that this commit is already upstream: 1e6f01f72855 ("x86/platform/uv: Refactor code using deprecated strcpy()/strncpy() interfaces to use strscpy()") Below is the delta your v3 patch has compared to what is upstream - is it really necessary to open code it, instead of using strnchrnul() as your original patch did? Am I missing anything here? Thanks, Ingo --- a/arch/x86/platform/uv/uv_nmi.c +++ b/arch/x86/platform/uv/uv_nmi.c @@ -202,10 +202,13 @@ static int param_set_action(const char *val, const struct kernel_param *kp) { int i; int n = ARRAY_SIZE(valid_acts); - char arg[ACTION_LEN]; + char arg[ACTION_LEN], *p; /* (remove possible '\n') */ - strscpy(arg, val, strnchrnul(val, sizeof(arg)-1, '\n') - val + 1); + strscpy(arg, val, sizeof(arg)); + p = strchr(arg, '\n'); + if (p) + *p = '\0'; for (i = 0; i < n; i++) if (!strcmp(arg, valid_acts[i].action))
