Hi,
On Thu, Aug 14, 2025 at 3:02 PM Thorsten Blum <thorsten.b...@linux.dev> wrote:
>
> strcpy() is deprecated; use strscpy() and memcpy() instead and remove
> several manual NUL-terminations.
>
> In parse_grep(), we can safely use memcpy() because we already know the
> length of the source string 'cp' and that it is guaranteed to be
> NUL-terminated within the first KDB_GREP_STRLEN bytes.
>
> No functional changes intended.
>
> Link: https://github.com/KSPP/linux/issues/88
> Signed-off-by: Thorsten Blum <thorsten.b...@linux.dev>
> ---
> Changes in v3:
> - Extract the strscpy() changes into a separate patch and focus on
> replacing the deprecated strcpy() calls as suggested by Greg
> - Link to v2:
> https://lore.kernel.org/lkml/20250814163237.229544-2-thorsten.b...@linux.dev/
>
> Changes in v2:
> - Use memcpy() instead of strscpy() in parse_grep() as suggested by Greg
> - Compile-tested only so far
> - Link to v1:
> https://lore.kernel.org/lkml/20250814120338.219585-2-thorsten.b...@linux.dev/
> ---
> kernel/debug/kdb/kdb_main.c | 14 +++++---------
> 1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
> index 7a4d2d4689a5..40de0ece724b 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);
> + if (mp->help[0] == '"')
> + strscpy(mp->help, argv[3] + 1, strlen(argv[3]) - 1);
Let's think about some test cases...
Old code:
mp->usage = kdb_strdup(argv[2], GFP_KDB);
if (mp->usage[0] == '"') {
strcpy(mp->usage, argv[2]+1);
mp->usage[strlen(mp->usage)-1] = '\0';
}
New code:
mp->usage = kdb_strdup(argv[2], GFP_KDB);
if (mp->usage[0] == '"')
strscpy(mp->usage, argv[2] + 1, strlen(argv[2]) - 1);
Example string: argv[2] = "\"xyz\""
Old:
mp->usage = strdup("\"xyz\"")
mp->usage becomes "xyz\""
mp->usage becomes "xyz"
New:
mp->usage = strdup("\"xyz\"")
mp->usage becomes "xyz\""
mp->usage doesn't change (!)
To match old behavior, I think you'd need "strlen(argv[2]) - 2", right?
I'll also note that with a different (malformed) example string, the
old code would have also been broken.
Example string: argv[2] = "\""
Old:
mp->usage = strdup("\"")
mp->usage becomes ""
mp->usage[-1] = '\0'; // BAD!
That should probably be fixed too. Luckily this command can't be run
by a user in kdb and it just runs stuff at init time...
Maybe a right fix is something like this (untested). You could even
put it in a small helper so it doesn't need to be duplicated for both
help and usage:
len = strlen(to_copy);
if (to_copy[0] == '"') {
to_copy++;
len--;
if (to_copy[len-1] == '"')
len--;
}
dest = kstrndup(to_copy, len, GFP_KDB);
...of course, that stops using kdb_strdup(). I don't really see why
that exists? The comments make no sense...
-Doug
