[PATCH] kdb: Replace deprecated strcpy() with strscpy()

[PATCH] kdb: Replace deprecated strcpy() with strscpy()

[Thorsten Blum]

strcpy() is deprecated; use strscpy() instead and remove several manual
NUL-terminations.

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.

No functional changes intended.

Link: https://github.com/KSPP/linux/issues/88
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 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);
+	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);
 	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]);
 		return 1;
 	case CTRL_N:
 		if (cmdptr != cmd_head)
 			cmdptr = (cmdptr+1) % KDB_CMD_HISTORY_COUNT;
-		strscpy(cmd_cur, cmd_hist[cmdptr], CMD_BUFLEN);
+		strscpy(cmd_cur, cmd_hist[cmdptr]);
 		return 1;
 	}
 	return 0;
@@ -1285,19 +1281,19 @@ static int kdb_local(kdb_reason_t reason, int error, struct pt_regs *regs,
 		cmdbuf = kdb_getstr(cmdbuf, CMD_BUFLEN, kdb_prompt_str);
 		if (*cmdbuf != '\n') {
 			if (*cmdbuf < 32) {
-				if (cmdptr == cmd_head) {
+				if (cmdptr == cmd_head)
+					/* Copy the current command to the
+					 * history and let strscpy() replace the
+					 * last character with a NUL terminator.
+					 */
 					strscpy(cmd_hist[cmd_head], cmd_cur,
-						CMD_BUFLEN);
-					*(cmd_hist[cmd_head] +
-					  strlen(cmd_hist[cmd_head])-1) = '\0';
-				}
+						strlen(cmd_cur));
 				if (!handle_ctrl_cmd(cmdbuf))
 					*(cmd_cur+strlen(cmd_cur)-1) = '\0';
 				cmdbuf = cmd_cur;
 				goto do_full_getstr;
 			} else {
-				strscpy(cmd_hist[cmd_head], cmd_cur,
-					CMD_BUFLEN);
+				strscpy(cmd_hist[cmd_head], cmd_cur);
 			}
 
 			cmd_head = (cmd_head+1) % KDB_CMD_HISTORY_COUNT;
-- 
2.50.1

Re: [PATCH] kdb: Replace deprecated strcpy() with strscpy()

[Greg Kroah-Hartman]

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 <thorsten.blum@linux.dev>
> ---
>  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?


>  	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...

thanks,

greg k-h

Re: [PATCH] kdb: Replace deprecated strcpy() with strscpy()

[Greg Kroah-Hartman]

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 <thorsten.blum@linux.dev>
> > ---
> >  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

Re: [PATCH] kdb: Replace deprecated strcpy() with strscpy()

[Thorsten Blum]

Hi Greg,

On 14. Aug 2025, at 16:35, Greg Kroah-Hartman wrote:
> 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(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?

Yes, this could just be

	memcpy(kdb_grep_string, cp, len + 1);

because we already know the length which strscpy() would just recompute
before calling memcpy() internally. I'll submit a v2.

>>> -		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?

Yes, it's the same because sizeof(cmd_cur) equals CMD_BUFLEN.

Thanks,
Thorsten

[PATCH] kdb: Replace deprecated strcpy() with strscpy()

[Thorsten Blum]

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

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

diff --git a/kernel/debug/kdb/kdb_support.c b/kernel/debug/kdb/kdb_support.c
index 05b137e7dcb9..158430a890b6 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"
 
@@ -250,7 +251,8 @@ char *kdb_strdup(const char *str, gfp_t type)
 	char *s = kmalloc(n, type);
 	if (!s)
 		return NULL;
-	return strcpy(s, str);
+	strscpy(s, str, n);
+	return s;
 }
 
 /*
-- 
2.50.1

Re: [PATCH] kdb: Replace deprecated strcpy() with strscpy()

[Doug Anderson]

Hi,

On Fri, Jul 18, 2025 at 2:40 PM Thorsten Blum <thorsten.blum@linux.dev> wrote:
>
> strcpy() is deprecated; use strscpy() instead.
>
> Link: https://github.com/KSPP/linux/issues/88
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
>  kernel/debug/kdb/kdb_support.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

nit: Since this only covers things in the file `kdb_support.c` and not
everything in kernel/debug/kdb, perhaps that should be in the subject
line? Maybe "kdb: Replace deprecated strcpy() with strscpy() in
kdb_strdup()"?

Other than that, this looks fine to me.

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

Re: [PATCH] kdb: Replace deprecated strcpy() with strscpy()

[Thorsten Blum]

Hi Doug,

On 19. Jul 2025, at 00:48, Doug Anderson wrote:
> On Fri, Jul 18, 2025 at 2:40 PM Thorsten Blum wrote:
>> 
>> strcpy() is deprecated; use strscpy() instead.
>> 
>> Link: https://github.com/KSPP/linux/issues/88
>> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
>> ---
>> kernel/debug/kdb/kdb_support.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
> 
> nit: Since this only covers things in the file `kdb_support.c` and not
> everything in kernel/debug/kdb, perhaps that should be in the subject
> line? Maybe "kdb: Replace deprecated strcpy() with strscpy() in
> kdb_strdup()"?
> 
> Other than that, this looks fine to me.
> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>

I'm preparing a patch series for Daniel with my kdb changes.

For this one here, I initially used:

	strscpy(s, str, n);
	return s;

to replace 'return strcpy(s, str);', but now prefer:

	memcpy(s, str, n);
	return s;

because we already know the string length 'n'.

Can I keep your Reviewed-by: tag when making this change and submitting
it as part of a patch series?

Thanks,
Thorsten

Re: [PATCH] kdb: Replace deprecated strcpy() with strscpy()

[Doug Anderson]

Hi,

On Mon, Aug 18, 2025 at 4:03 AM Thorsten Blum <thorsten.blum@linux.dev> wrote:
>
> Hi Doug,
>
> On 19. Jul 2025, at 00:48, Doug Anderson wrote:
> > On Fri, Jul 18, 2025 at 2:40 PM Thorsten Blum wrote:
> >>
> >> strcpy() is deprecated; use strscpy() instead.
> >>
> >> Link: https://github.com/KSPP/linux/issues/88
> >> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> >> ---
> >> kernel/debug/kdb/kdb_support.c | 4 +++-
> >> 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > nit: Since this only covers things in the file `kdb_support.c` and not
> > everything in kernel/debug/kdb, perhaps that should be in the subject
> > line? Maybe "kdb: Replace deprecated strcpy() with strscpy() in
> > kdb_strdup()"?
> >
> > Other than that, this looks fine to me.
> >
> > Reviewed-by: Douglas Anderson <dianders@chromium.org>
>
> I'm preparing a patch series for Daniel with my kdb changes.
>
> For this one here, I initially used:
>
>         strscpy(s, str, n);
>         return s;
>
> to replace 'return strcpy(s, str);', but now prefer:
>
>         memcpy(s, str, n);
>         return s;
>
> because we already know the string length 'n'.
>
> Can I keep your Reviewed-by: tag when making this change and submitting
> it as part of a patch series?

Sure.