On Thu, Aug 14, 2025 at 02:35:56PM +0200, Greg Kroah-Hartman wrote: > On Thu, Aug 14, 2025 at 02:03:37PM +0200, Thorsten Blum wrote: > > strcpy() is deprecated; use strscpy() instead and remove several manual > > NUL-terminations. > > Manual NULL terminations are good, why get rid of that? > > > Since the destination buffers 'cmd_cur' and 'cmd_hist[cmd_head]' have > > the fixed length CMD_BUFLEN, strscpy() automatically determines their > > size using sizeof() when the size argument is omitted. This makes the > > explicit size arguments for the existing strscpy() calls unnecessary, > > remove them. > > But now you are dynamically calculating this? > > > No functional changes intended. > > How did you test this? Many of these types of changes are wrong, so you > really really need to prove it is correct. > > > > > Link: https://github.com/KSPP/linux/issues/88 > > Signed-off-by: Thorsten Blum <[email protected]> > > --- > > kernel/debug/kdb/kdb_main.c | 32 ++++++++++++++------------------ > > 1 file changed, 14 insertions(+), 18 deletions(-) > > > > diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c > > index 7a4d2d4689a5..ea7dc2540e40 100644 > > --- a/kernel/debug/kdb/kdb_main.c > > +++ b/kernel/debug/kdb/kdb_main.c > > @@ -727,14 +727,10 @@ static int kdb_defcmd(int argc, const char **argv) > > mp->help = kdb_strdup(argv[3], GFP_KDB); > > if (!mp->help) > > goto fail_help; > > - if (mp->usage[0] == '"') { > > - strcpy(mp->usage, argv[2]+1); > > - mp->usage[strlen(mp->usage)-1] = '\0'; > > - } > > - if (mp->help[0] == '"') { > > - strcpy(mp->help, argv[3]+1); > > - mp->help[strlen(mp->help)-1] = '\0'; > > - } > > + if (mp->usage[0] == '"') > > + strscpy(mp->usage, argv[2] + 1, strlen(argv[2]) - 1); > > Now you are manually testing the length of argv[2], are you sure that's > ok? > > > + if (mp->help[0] == '"') > > + strscpy(mp->help, argv[3] + 1, strlen(argv[3]) - 1); > > > > INIT_LIST_HEAD(&kdb_macro->statements); > > defcmd_in_progress = true; > > @@ -860,7 +856,7 @@ static void parse_grep(const char *str) > > kdb_printf("search string too long\n"); > > return; > > } > > - strcpy(kdb_grep_string, cp); > > + strscpy(kdb_grep_string, cp); > > If this was just a search/replace, it would have been done already, so > why is this ok?
I missed that strscpy() can now handle 2 arguments like this, so yes, this should be ok. BUT, you just checked the length above this line, which now isn't needed, right? So this function can get simpler? > > > > kdb_grepping_flag++; > > return; > > } > > @@ -1076,12 +1072,12 @@ static int handle_ctrl_cmd(char *cmd) > > if (cmdptr != cmd_tail) > > cmdptr = (cmdptr + KDB_CMD_HISTORY_COUNT - 1) % > > KDB_CMD_HISTORY_COUNT; > > - strscpy(cmd_cur, cmd_hist[cmdptr], CMD_BUFLEN); > > + strscpy(cmd_cur, cmd_hist[cmdptr]); > > Same here. And other places... Sorry, this should also be ok, BUT it's really just doing the same exact thing, right? And, it's a different thing, so it should be a different patch (i.e. do not mix different logical things in the same patch, it confuses everyone. Well, me at least...) thanks, greg k-h
