| Message ID | 20230530162358.984149-1-azeemshaikh38@gmail.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 6a22e017f952ecaf4bb510cd0939259470a27b06 |
| Headers | show |
| Series | nios2: Replace all non-returning strlcpy with strscpy | expand |
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 <azeemshaikh38@gmail.com> Reviewed-by: Kees Cook <keescook@chromium.org>
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 <azeemshaikh38@gmail.com> > > Reviewed-by: Kees Cook <keescook@chromium.org> > Applied! Thanks, Dinh
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 <azeemshaikh38@gmail.com> > > > > Reviewed-by: Kees Cook <keescook@chromium.org> > > > > 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
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 <azeemshaikh38@gmail.com> >>> >>> Reviewed-by: Kees Cook <keescook@chromium.org> >>> >> >> 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
On June 20, 2023 3:27:29 PM PDT, Dinh Nguyen <dinguyen@kernel.org> 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 <azeemshaikh38@gmail.com> >>>> >>>> Reviewed-by: Kees Cook <keescook@chromium.org> >>>> >>> >>> 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
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
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 <azeemshaikh38@gmail.com> --- arch/nios2/kernel/cpuinfo.c | 2 +- arch/nios2/kernel/setup.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)