[PATCH RESEND] alpha: Replace strcpy() with strscpy() in setup_arch()

Thorsten Blum posted 1 patch 1 month, 2 weeks ago
arch/alpha/kernel/setup.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
[PATCH RESEND] alpha: Replace strcpy() with strscpy() in setup_arch()
Posted by Thorsten Blum 1 month, 2 weeks ago
strcpy() is deprecated; use strscpy() instead.

Since the destination buffer 'command_line' has a fixed length,
strscpy() automatically determines its size using sizeof() when the size
argument is omitted. This makes the explicit size argument for the
existing strscpy() call unnecessary - remove it.

No functional changes intended.

Link: https://github.com/KSPP/linux/issues/88
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 arch/alpha/kernel/setup.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/alpha/kernel/setup.c b/arch/alpha/kernel/setup.c
index bebdffafaee8..8b51e6ca83d6 100644
--- a/arch/alpha/kernel/setup.c
+++ b/arch/alpha/kernel/setup.c
@@ -468,8 +468,8 @@ setup_arch(char **cmdline_p)
 	/* 
 	 * Locate the command line.
 	 */
-	strscpy(command_line, COMMAND_LINE, sizeof(command_line));
-	strcpy(boot_command_line, command_line);
+	strscpy(command_line, COMMAND_LINE);
+	strscpy(boot_command_line, command_line, COMMAND_LINE_SIZE);
 	*cmdline_p = command_line;
 
 	/* 
@@ -511,7 +511,7 @@ setup_arch(char **cmdline_p)
 	}
 
 	/* Replace the command line, now that we've killed it with strsep.  */
-	strcpy(command_line, boot_command_line);
+	strscpy(command_line, boot_command_line);
 
 	/* If we want SRM console printk echoing early, do it now. */
 	if (alpha_using_srm && srmcons_output) {
-- 
2.50.1
Re: [PATCH RESEND] alpha: Replace strcpy() with strscpy() in setup_arch()
Posted by Magnus Lindholm 1 month ago
Hi,

It can be a bit controversial to replace the platform specific string
manipulation functions with a generic interface. On Alpha, there is nothing
(at least to my knowledge) that indicates that strcpy is broken or used in
an unsafe way and hence this patch doesn't really fix anything.
In my opinion, I think this should be a NAK.

Linus wrote some relevant stuff on this topic

Link:
https://github.com/torvalds/linux/commit/30c44659f4a3e7e1f9f47e895591b4b40bf62671

like for example:

"Use this in places where it makes sense, but don't do trivial patches
to fix things that aren't actually known to be broken."


Regards

Magnus Lindholm

On Mon, Aug 18, 2025 at 1:23 AM Thorsten Blum <thorsten.blum@linux.dev> wrote:
>
> strcpy() is deprecated; use strscpy() instead.
>
> Since the destination buffer 'command_line' has a fixed length,
> strscpy() automatically determines its size using sizeof() when the size
> argument is omitted. This makes the explicit size argument for the
> existing strscpy() call unnecessary - remove it.
>
> No functional changes intended.
>
> Link: https://github.com/KSPP/linux/issues/88
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
>  arch/alpha/kernel/setup.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/alpha/kernel/setup.c b/arch/alpha/kernel/setup.c
> index bebdffafaee8..8b51e6ca83d6 100644
> --- a/arch/alpha/kernel/setup.c
> +++ b/arch/alpha/kernel/setup.c
> @@ -468,8 +468,8 @@ setup_arch(char **cmdline_p)
>         /*
>          * Locate the command line.
>          */
> -       strscpy(command_line, COMMAND_LINE, sizeof(command_line));
> -       strcpy(boot_command_line, command_line);
> +       strscpy(command_line, COMMAND_LINE);
> +       strscpy(boot_command_line, command_line, COMMAND_LINE_SIZE);
>         *cmdline_p = command_line;
>
>         /*
> @@ -511,7 +511,7 @@ setup_arch(char **cmdline_p)
>         }
>
>         /* Replace the command line, now that we've killed it with strsep.  */
> -       strcpy(command_line, boot_command_line);
> +       strscpy(command_line, boot_command_line);
>
>         /* If we want SRM console printk echoing early, do it now. */
>         if (alpha_using_srm && srmcons_output) {
> --
> 2.50.1
>
>
Re: [PATCH RESEND] alpha: Replace strcpy() with strscpy() in setup_arch()
Posted by Thorsten Blum 3 weeks, 4 days ago
Hi Magnus,

On 30. Aug 2025, at 21:11, Magnus Lindholm wrote:
> It can be a bit controversial to replace the platform specific string
> manipulation functions with a generic interface. On Alpha, there is nothing
> (at least to my knowledge) that indicates that strcpy is broken or used in
> an unsafe way and hence this patch doesn't really fix anything.
> In my opinion, I think this should be a NAK.

My patch doesn't fix anything, it simply replaces the deprecated
strcpy() with the safer strscpy(), without introducing any functional
changes.

I'm not sure whether the platform-specific strcpy() has any performance
benefits over the generic strscpy(), but setup_arch() is already using
strscpy() (converted from strlcpy() [1]) to copy the COMMAND_LINE string
to 'command_line'.

The current code feels unnecessarily inconsistent, with one line using
strscpy() and the next using the platform-specific strcpy().

Thanks,
Thorsten

[1] https://lore.kernel.org/all/20220818205936.6144-1-wsa+renesas@sang-engineering.com/
Re: [PATCH RESEND] alpha: Replace strcpy() with strscpy() in setup_arch()
Posted by Magnus Lindholm 3 weeks, 4 days ago
>
> The current code feels unnecessarily inconsistent, with one line using
> strscpy() and the next using the platform-specific strcpy().
>
I guess you have a point there, I believe that this already existing instance
of strscpy came from the patch you referred to as an attempt to replace strlcpy.
strlcpy does not have an Alpha specific implementation.


Regards

Magnus