[PATCH 1/4] kdb: Replace deprecated strcpy() with memcpy() in kdb_strdup()

[PATCH 1/4] kdb: Replace deprecated strcpy() with memcpy() in kdb_strdup()

[Thorsten Blum]

strcpy() is deprecated; use memcpy() instead.

Link: https://github.com/KSPP/linux/issues/88
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 kernel/debug/kdb/kdb_support.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/debug/kdb/kdb_support.c b/kernel/debug/kdb/kdb_support.c
index 05b137e7dcb9..d36281142fa1 100644
--- a/kernel/debug/kdb/kdb_support.c
+++ b/kernel/debug/kdb/kdb_support.c
@@ -23,6 +23,7 @@
 #include <linux/uaccess.h>
 #include <linux/kdb.h>
 #include <linux/slab.h>
+#include <linux/string.h>
 #include <linux/ctype.h>
 #include "kdb_private.h"
 
@@ -246,11 +247,12 @@ void kdb_symbol_print(unsigned long addr, const kdb_symtab_t *symtab_p,
  */
 char *kdb_strdup(const char *str, gfp_t type)
 {
-	int n = strlen(str)+1;
+	size_t n = strlen(str) + 1;
 	char *s = kmalloc(n, type);
 	if (!s)
 		return NULL;
-	return strcpy(s, str);
+	memcpy(s, str, n);
+	return s;
 }
 
 /*
-- 
2.50.1

[PATCH 2/4] kdb: Replace deprecated strcpy() with memmove() in vkdb_printf()

[Thorsten Blum]

strcpy() is deprecated and its behavior is undefined when the source and
destination buffers overlap. Use memmove() instead to avoid any
undefined behavior.

Adjust comments for clarity.

Link: https://github.com/KSPP/linux/issues/88
Fixes: 5d5314d6795f ("kdb: core for kgdb back end (1 of 2)")
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 kernel/debug/kdb/kdb_io.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c
index 9b11b10b120c..b12b9db75c1d 100644
--- a/kernel/debug/kdb/kdb_io.c
+++ b/kernel/debug/kdb/kdb_io.c
@@ -714,8 +714,8 @@ int vkdb_printf(enum kdb_msgsrc src, const char *fmt, va_list ap)
 		 * it, depending on the results of the search.
 		 */
 		cp++;	 	     /* to byte after the newline */
-		replaced_byte = *cp; /* remember what/where it was */
-		cphold = cp;
+		replaced_byte = *cp; /* remember what it was */
+		cphold = cp;	     /* remember where it was */
 		*cp = '\0';	     /* end the string for our search */
 
 		/*
@@ -732,8 +732,9 @@ int vkdb_printf(enum kdb_msgsrc src, const char *fmt, va_list ap)
 			 * Shift the buffer left.
 			 */
 			*cphold = replaced_byte;
-			strcpy(kdb_buffer, cphold);
-			len = strlen(kdb_buffer);
+			len = strlen(cphold);
+			/* Use memmove() because the buffers overlap */
+			memmove(kdb_buffer, cphold, len + 1);
 			next_avail = kdb_buffer + len;
 			size_avail = sizeof(kdb_buffer) - len;
 			goto kdb_print_out;
@@ -872,8 +873,9 @@ int vkdb_printf(enum kdb_msgsrc src, const char *fmt, va_list ap)
 	 */
 	if (kdb_grepping_flag && !suspend_grep) {
 		*cphold = replaced_byte;
-		strcpy(kdb_buffer, cphold);
-		len = strlen(kdb_buffer);
+		len = strlen(cphold);
+		/* Use memmove() because the buffers overlap */
+		memmove(kdb_buffer, cphold, len + 1);
 		next_avail = kdb_buffer + len;
 		size_avail = sizeof(kdb_buffer) - len;
 	}
-- 
2.50.1

[PATCH 3/4] kdb: Replace deprecated strcpy() with memcpy() in parse_grep()

[Thorsten Blum]

strcpy() is deprecated; use memcpy() instead.

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.

Link: https://github.com/KSPP/linux/issues/88
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 kernel/debug/kdb/kdb_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 7a4d2d4689a5..cdf91976eb7c 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -860,7 +860,7 @@ static void parse_grep(const char *str)
 		kdb_printf("search string too long\n");
 		return;
 	}
-	strcpy(kdb_grep_string, cp);
+	memcpy(kdb_grep_string, cp, len + 1);
 	kdb_grepping_flag++;
 	return;
 }
-- 
2.50.1

Re: [PATCH 3/4] kdb: Replace deprecated strcpy() with memcpy() in parse_grep()

[Doug Anderson]

Hi,

On Mon, Aug 18, 2025 at 11:14 AM Thorsten Blum <thorsten.blum@linux.dev> wrote:
>
> strcpy() is deprecated; use memcpy() instead.
>
> 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.
>
> Link: https://github.com/KSPP/linux/issues/88
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
>  kernel/debug/kdb/kdb_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
> index 7a4d2d4689a5..cdf91976eb7c 100644
> --- a/kernel/debug/kdb/kdb_main.c
> +++ b/kernel/debug/kdb/kdb_main.c
> @@ -860,7 +860,7 @@ static void parse_grep(const char *str)
>                 kdb_printf("search string too long\n");
>                 return;
>         }
> -       strcpy(kdb_grep_string, cp);
> +       memcpy(kdb_grep_string, cp, len + 1);

I'm OK with this:

Reviewed-by: Douglas Anderson <dianders@chromium.org>

A part of me would rather use strscpy() the way it's supposed to be
used to simplify the code a tiny bit... AKA (untested):

-       len = strlen(cp);
+       len = strscpy(kdb_grep_string, cp);
        if (!len)
                return;
-       if (len >= KDB_GREP_STRLEN) {
+       if (len < 0) {
                kdb_printf("search string too long\n");
                return;
        }
-       strcpy(kdb_grep_string, cp);

I guess this does "change" the behavior in that `kdb_grep_string` will
still be set to the empty string in the case that "len" was 0 and will
still be set to a truncated copy in the case that "len" was too big,
but based on my quick analysis of the code that should be fine.

In any case, we're getting pretty far into nitpicks here, so I'm also
OK w/ just leaving it the way you have it. ;-)

[PATCH 4/4] kdb: Replace deprecated strcpy() with helper function in kdb_defcmd()

[Thorsten Blum]

strcpy() is deprecated; use the new helper function kdb_strdup_dequote()
instead. In addition to string duplication similar to kdb_strdup(), it
also trims surrounding quotes from the input string if present.

kdb_strdup_dequote() also checks for a trailing quote in the input
string which was previously not checked.

Link: https://github.com/KSPP/linux/issues/88
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 kernel/debug/kdb/kdb_main.c    | 12 ++----------
 kernel/debug/kdb/kdb_private.h |  1 +
 kernel/debug/kdb/kdb_support.c | 29 +++++++++++++++++++++++++++++
 3 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index cdf91976eb7c..dddf2b5aad57 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -721,20 +721,12 @@ static int kdb_defcmd(int argc, const char **argv)
 	mp->name = kdb_strdup(argv[1], GFP_KDB);
 	if (!mp->name)
 		goto fail_name;
-	mp->usage = kdb_strdup(argv[2], GFP_KDB);
+	mp->usage = kdb_strdup_dequote(argv[2], GFP_KDB);
 	if (!mp->usage)
 		goto fail_usage;
-	mp->help = kdb_strdup(argv[3], GFP_KDB);
+	mp->help = kdb_strdup_dequote(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';
-	}
 
 	INIT_LIST_HEAD(&kdb_macro->statements);
 	defcmd_in_progress = true;
diff --git a/kernel/debug/kdb/kdb_private.h b/kernel/debug/kdb/kdb_private.h
index d2520d72b1f5..a2fc7d2bc9fc 100644
--- a/kernel/debug/kdb/kdb_private.h
+++ b/kernel/debug/kdb/kdb_private.h
@@ -110,6 +110,7 @@ extern int kdbgetaddrarg(int, const char **, int*, unsigned long *,
 extern int kdbgetsymval(const char *, kdb_symtab_t *);
 extern int kdbnearsym(unsigned long, kdb_symtab_t *);
 extern char *kdb_strdup(const char *str, gfp_t type);
+extern char *kdb_strdup_dequote(const char *str, gfp_t type);
 extern void kdb_symbol_print(unsigned long, const kdb_symtab_t *, unsigned int);
 
 /* Routine for debugging the debugger state. */
diff --git a/kernel/debug/kdb/kdb_support.c b/kernel/debug/kdb/kdb_support.c
index d36281142fa1..b42dc54f5a9a 100644
--- a/kernel/debug/kdb/kdb_support.c
+++ b/kernel/debug/kdb/kdb_support.c
@@ -255,6 +255,35 @@ char *kdb_strdup(const char *str, gfp_t type)
 	return s;
 }
 
+/*
+ * kdb_strdup_dequote - same as kdb_strdup(), but trims surrounding quotes from
+ *			the input string if present.
+ * Remarks:
+ *	Quotes are only removed if there is both a leading and a trailing quote.
+ */
+char *kdb_strdup_dequote(const char *str, gfp_t type)
+{
+	size_t len = strlen(str);
+	char *s;
+
+	if (str[0] == '"' && len > 1 && str[len - 1] == '"') {
+		/* trim both leading and trailing quotes */
+		str++;
+		len -= 2;
+	}
+
+	len++; /* add space for NUL terminator */
+
+	s = kmalloc(len, type);
+	if (!s)
+		return NULL;
+
+	memcpy(s, str, len);
+	s[len - 1] = '\0';
+
+	return s;
+}
+
 /*
  * kdb_getarea_size - Read an area of data.  The kdb equivalent of
  *	copy_from_user, with kdb messages for invalid addresses.
-- 
2.50.1

Re: [PATCH 4/4] kdb: Replace deprecated strcpy() with helper function in kdb_defcmd()

[Doug Anderson]

Hi,

On Mon, Aug 18, 2025 at 11:13 AM Thorsten Blum <thorsten.blum@linux.dev> wrote:
>
> +/*
> + * kdb_strdup_dequote - same as kdb_strdup(), but trims surrounding quotes from
> + *                     the input string if present.
> + * Remarks:
> + *     Quotes are only removed if there is both a leading and a trailing quote.
> + */
> +char *kdb_strdup_dequote(const char *str, gfp_t type)
> +{
> +       size_t len = strlen(str);
> +       char *s;
> +
> +       if (str[0] == '"' && len > 1 && str[len - 1] == '"') {
> +               /* trim both leading and trailing quotes */
> +               str++;
> +               len -= 2;
> +       }
> +
> +       len++; /* add space for NUL terminator */
> +
> +       s = kmalloc(len, type);
> +       if (!s)
> +               return NULL;
> +
> +       memcpy(s, str, len);
> +       s[len - 1] = '\0';

Very nitty, but technically the above memcpy() could pass "len - 1", right?

It doesn't really matter other than the wasteful copy of 1-byte, so:

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Re: [PATCH 4/4] kdb: Replace deprecated strcpy() with helper function in kdb_defcmd()

[Thorsten Blum]

On 18. Aug 2025, at 22:42, Doug Anderson wrote:
>> +	memcpy(s, str, len);
>> +	s[len - 1] = '\0';
> 
> Very nitty, but technically the above memcpy() could pass "len - 1", right?

Ah yes, I missed this after adding the manual NUL-termination.

I'll send a v2 tomorrow.

> It doesn't really matter other than the wasteful copy of 1-byte, so:
> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>

Thanks,
Thorsten