All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] kdb: Replace deprecated strcpy() with strscpy() and memcpy()
@ 2025-08-14 16:32 Thorsten Blum
  2025-08-14 18:29 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 2+ messages in thread
From: Thorsten Blum @ 2025-08-14 16:32 UTC (permalink / raw)
  To: Jason Wessel, Daniel Thompson, Douglas Anderson, Nir Lichtman,
	Thorsten Blum, Yuran Pereira
  Cc: linux-hardening, Daniel Thompson, Greg Kroah-Hartman,
	kgdb-bugreport, linux-kernel

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.

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>
---
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.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..048dbbcd91a4 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);
+	memcpy(kdb_grep_string, cp, len + 1);
 	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


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] kdb: Replace deprecated strcpy() with strscpy() and memcpy()
  2025-08-14 16:32 [PATCH v2] kdb: Replace deprecated strcpy() with strscpy() and memcpy() Thorsten Blum
@ 2025-08-14 18:29 ` Greg Kroah-Hartman
  0 siblings, 0 replies; 2+ messages in thread
From: Greg Kroah-Hartman @ 2025-08-14 18:29 UTC (permalink / raw)
  To: Thorsten Blum
  Cc: Jason Wessel, Daniel Thompson, Douglas Anderson, Nir Lichtman,
	Yuran Pereira, linux-hardening, Daniel Thompson, kgdb-bugreport,
	linux-kernel

On Thu, Aug 14, 2025 at 06:32:34PM +0200, Thorsten Blum 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.
> 
> 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>
> ---
> 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.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..048dbbcd91a4 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);
> +	memcpy(kdb_grep_string, cp, len + 1);
>  	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]);

Again, you are doing two different things in this patch, it should be 2
different patches.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-08-14 18:29 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-14 16:32 [PATCH v2] kdb: Replace deprecated strcpy() with strscpy() and memcpy() Thorsten Blum
2025-08-14 18:29 ` Greg Kroah-Hartman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.