| Message ID | 20230830185428.4109426-1-azeemshaikh38@gmail.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | b149353da471ddfc5d6e698784cf22bc55bff68b |
| Headers | show |
| Series | m68k/atari: Replace strlcpy with strscpy | expand |
On Wed, Aug 30, 2023 at 06:54:28PM +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(). > > Direct replacement is safe here since return value of -errno > is used to check for truncation instead of sizeof(dest). > > [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 Wed, Aug 30, 2023 at 8:54 PM Azeem Shaikh <azeemshaikh38@gmail.com> 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(). > > Direct replacement is safe here since return value of -errno > is used to check for truncation instead of sizeof(dest). > > [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: Geert Uytterhoeven <geert@linux-m68k.org> i.e. will queue in the m68k for-v6.7 branch. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
diff --git a/arch/m68k/emu/natfeat.c b/arch/m68k/emu/natfeat.c index b19dc00026d9..d124c62022c2 100644 --- a/arch/m68k/emu/natfeat.c +++ b/arch/m68k/emu/natfeat.c @@ -42,10 +42,10 @@ long nf_get_id(const char *feature_name) { /* feature_name may be in vmalloc()ed memory, so make a copy */ char name_copy[32]; - size_t n; + ssize_t n; - n = strlcpy(name_copy, feature_name, sizeof(name_copy)); - if (n >= sizeof(name_copy)) + n = strscpy(name_copy, feature_name, sizeof(name_copy)); + if (n < 0) return 0; return nf_get_id_phys(virt_to_phys(name_copy));
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(). Direct replacement is safe here since return value of -errno is used to check for truncation instead of sizeof(dest). [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/m68k/emu/natfeat.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) -- 2.42.0.283.g2d96d420d3-goog