2023-05-30 16:03:54

by Azeem Shaikh

Subject: [PATCH] ARM: Replace all non-returning strlcpy with strscpy

strlcpy() reads the entire source buffer first.
This read may exceed the destination size limit.
This is both inefficient and can lead to linear read
overflows if a source string is not NUL-terminated [1].
In an effort to remove strlcpy() completely [2], replace
strlcpy() here with strscpy().
No return values were used, so direct replacement is safe.

[1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy
[2] https://github.com/KSPP/linux/issues/89

Signed-off-by: Azeem Shaikh <[email protected]>
---
arch/arm/kernel/atags_parse.c | 4 ++--
arch/arm/kernel/setup.c | 2 +-
arch/arm/kernel/vdso.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/kernel/atags_parse.c b/arch/arm/kernel/atags_parse.c
index 373b61f9a4f0..33f6eb5213a5 100644
--- a/arch/arm/kernel/atags_parse.c
+++ b/arch/arm/kernel/atags_parse.c
@@ -127,7 +127,7 @@ static int __init parse_tag_cmdline(const struct tag *tag)
#elif defined(CONFIG_CMDLINE_FORCE)
pr_warn("Ignoring tag cmdline (using the default kernel command line)\n");
#else
- strlcpy(default_command_line, tag->u.cmdline.cmdline,
+ strscpy(default_command_line, tag->u.cmdline.cmdline,
COMMAND_LINE_SIZE);
#endif
return 0;
@@ -224,7 +224,7 @@ setup_machine_tags(void *atags_vaddr, unsigned int machine_nr)
}

/* parse_early_param needs a boot_command_line */
- strlcpy(boot_command_line, from, COMMAND_LINE_SIZE);
+ strscpy(boot_command_line, from, COMMAND_LINE_SIZE);

return mdesc;
}
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 75cd4699e7b3..3048a685ea79 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -1142,7 +1142,7 @@ void __init setup_arch(char **cmdline_p)
setup_initial_init_mm(_text, _etext, _edata, _end);

/* populate cmd_line too for later use, preserving boot_command_line */
- strlcpy(cmd_line, boot_command_line, COMMAND_LINE_SIZE);
+ strscpy(cmd_line, boot_command_line, COMMAND_LINE_SIZE);
*cmdline_p = cmd_line;

early_fixmap_init();
diff --git a/arch/arm/kernel/vdso.c b/arch/arm/kernel/vdso.c
index 3408269d19c7..f297d66a8a76 100644
--- a/arch/arm/kernel/vdso.c
+++ b/arch/arm/kernel/vdso.c
@@ -135,7 +135,7 @@ static Elf32_Sym * __init find_symbol(struct elfinfo *lib, const char *symname)

if (lib->dynsym[i].st_name == 0)
continue;
- strlcpy(name, lib->dynstr + lib->dynsym[i].st_name,
+ strscpy(name, lib->dynstr + lib->dynsym[i].st_name,
MAX_SYMNAME);
c = strchr(name, '@');
if (c)

2023-05-30 23:27:42

by Kees Cook

Subject: Re: [PATCH] ARM: Replace all non-returning strlcpy with strscpy

On Tue, May 30, 2023 at 03:55:01PM +0000, Azeem Shaikh wrote:
> strlcpy() reads the entire source buffer first.
> This read may exceed the destination size limit.
> This is both inefficient and can lead to linear read
> overflows if a source string is not NUL-terminated [1].
> In an effort to remove strlcpy() completely [2], replace
> strlcpy() here with strscpy().
> No return values were used, so direct replacement is safe.
>
> [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy
> [2] https://github.com/KSPP/linux/issues/89
>
> Signed-off-by: Azeem Shaikh <[email protected]>

Reviewed-by: Kees Cook <[email protected]>

--
Kees Cook

2023-05-31 14:11:57

by Linus Walleij

Subject: Re: [PATCH] ARM: Replace all non-returning strlcpy with strscpy

On Tue, May 30, 2023 at 5:55 PM Azeem Shaikh <[email protected]> wrote:

> strlcpy() reads the entire source buffer first.
> This read may exceed the destination size limit.
> This is both inefficient and can lead to linear read
> overflows if a source string is not NUL-terminated [1].
> In an effort to remove strlcpy() completely [2], replace
> strlcpy() here with strscpy().
> No return values were used, so direct replacement is safe.
>
> [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy
> [2] https://github.com/KSPP/linux/issues/89
>
> Signed-off-by: Azeem Shaikh <[email protected]>

Reviewed-by: Linus Walleij <[email protected]>

Please put this into Russell's patch tracker:
https://www.arm.linux.org.uk/developer/patches/

Yours,
Linus Walleij

2023-06-01 09:15:55

by Ard Biesheuvel

Subject: Re: [PATCH] ARM: Replace all non-returning strlcpy with strscpy

On Wed, 31 May 2023 at 15:30, Linus Walleij <[email protected]> wrote:
>
> On Tue, May 30, 2023 at 5:55 PM Azeem Shaikh <[email protected]> wrote:
>
> > strlcpy() reads the entire source buffer first.
> > This read may exceed the destination size limit.
> > This is both inefficient and can lead to linear read
> > overflows if a source string is not NUL-terminated [1].
> > In an effort to remove strlcpy() completely [2], replace
> > strlcpy() here with strscpy().
> > No return values were used, so direct replacement is safe.
> >
> > [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy
> > [2] https://github.com/KSPP/linux/issues/89
> >
> > Signed-off-by: Azeem Shaikh <[email protected]>
>
> Reviewed-by: Linus Walleij <[email protected]>
>
> Please put this into Russell's patch tracker:
> https://www.arm.linux.org.uk/developer/patches/
>

https://www.armlinux.org.uk/developer/patches/viewpatch.php?id=9300/1