2023-05-30 16:32:02

by Azeem Shaikh

[permalink] [raw]
Subject: [PATCH] nios2: 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/nios2/kernel/cpuinfo.c | 2 +-
arch/nios2/kernel/setup.c | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/nios2/kernel/cpuinfo.c b/arch/nios2/kernel/cpuinfo.c
index 203870c4b86d..338849c430a5 100644
--- a/arch/nios2/kernel/cpuinfo.c
+++ b/arch/nios2/kernel/cpuinfo.c
@@ -47,7 +47,7 @@ void __init setup_cpuinfo(void)

str = of_get_property(cpu, "altr,implementation", &len);
if (str)
- strlcpy(cpuinfo.cpu_impl, str, sizeof(cpuinfo.cpu_impl));
+ strscpy(cpuinfo.cpu_impl, str, sizeof(cpuinfo.cpu_impl));
else
strcpy(cpuinfo.cpu_impl, "<unknown>");

diff --git a/arch/nios2/kernel/setup.c b/arch/nios2/kernel/setup.c
index 40bc8fb75e0b..8582ed965844 100644
--- a/arch/nios2/kernel/setup.c
+++ b/arch/nios2/kernel/setup.c
@@ -121,7 +121,7 @@ asmlinkage void __init nios2_boot_init(unsigned r4, unsigned r5, unsigned r6,
dtb_passed = r6;

if (r7)
- strlcpy(cmdline_passed, (char *)r7, COMMAND_LINE_SIZE);
+ strscpy(cmdline_passed, (char *)r7, COMMAND_LINE_SIZE);
}
#endif

@@ -129,10 +129,10 @@ asmlinkage void __init nios2_boot_init(unsigned r4, unsigned r5, unsigned r6,

#ifndef CONFIG_CMDLINE_FORCE
if (cmdline_passed[0])
- strlcpy(boot_command_line, cmdline_passed, COMMAND_LINE_SIZE);
+ strscpy(boot_command_line, cmdline_passed, COMMAND_LINE_SIZE);
#ifdef CONFIG_NIOS2_CMDLINE_IGNORE_DTB
else
- strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
+ strscpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
#endif
#endif




2023-05-30 23:23:44

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH] nios2: Replace all non-returning strlcpy with strscpy

On Tue, May 30, 2023 at 04:23:58PM +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-06-13 23:06:29

by Dinh Nguyen

[permalink] [raw]
Subject: Re: [PATCH] nios2: Replace all non-returning strlcpy with strscpy



On 5/30/23 18:20, Kees Cook wrote:
> On Tue, May 30, 2023 at 04:23:58PM +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]>
>

Applied!

Thanks,
Dinh

2023-06-20 20:33:35

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH] nios2: Replace all non-returning strlcpy with strscpy

On Tue, Jun 13, 2023 at 05:15:41PM -0500, Dinh Nguyen wrote:
>
>
> On 5/30/23 18:20, Kees Cook wrote:
> > On Tue, May 30, 2023 at 04:23:58PM +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]>
> >
>
> Applied!

Thanks for taking this patch! I just wanted to double-check, though; I
haven't seen it show up in -next yet. Is this still queued?

Thanks!

-Kees

--
Kees Cook

2023-06-20 23:23:12

by Dinh Nguyen

[permalink] [raw]
Subject: Re: [PATCH] nios2: Replace all non-returning strlcpy with strscpy



On 6/20/23 15:15, Kees Cook wrote:
> On Tue, Jun 13, 2023 at 05:15:41PM -0500, Dinh Nguyen wrote:
>>
>>
>> On 5/30/23 18:20, Kees Cook wrote:
>>> On Tue, May 30, 2023 at 04:23:58PM +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]>
>>>
>>
>> Applied!
>
> Thanks for taking this patch! I just wanted to double-check, though; I
> haven't seen it show up in -next yet. Is this still queued?
>
> Thanks!

I've queued it for v6.5. Do you need it in v6.4?

Dinh

2023-06-21 01:05:14

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH] nios2: Replace all non-returning strlcpy with strscpy

On June 20, 2023 3:27:29 PM PDT, Dinh Nguyen <[email protected]> wrote:
>
>
>On 6/20/23 15:15, Kees Cook wrote:
>> On Tue, Jun 13, 2023 at 05:15:41PM -0500, Dinh Nguyen wrote:
>>>
>>>
>>> On 5/30/23 18:20, Kees Cook wrote:
>>>> On Tue, May 30, 2023 at 04:23:58PM +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]>
>>>>
>>>
>>> Applied!
>>
>> Thanks for taking this patch! I just wanted to double-check, though; I
>> haven't seen it show up in -next yet. Is this still queued?
>>
>> Thanks!
>
>I've queued it for v6.5. Do you need it in v6.4?

6.5 is fine, yeah. I just wanted to make sure it didn't get lost. :) (I didn't see it in sfr's linux-next merges tree.)

Thanks!

-Kees



--
Kees Cook