From: Justin Stitt <justinstitt@google.com> To: Paul Walmsley <paul.walmsley@sifive.com>, Palmer Dabbelt <palmer@dabbelt.com>, Albert Ou <aou@eecs.berkeley.edu> Cc: linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org, Kees Cook <keescook@chromium.org>, Nick Desaulniers <ndesaulniers@google.com>, linux-hardening@vger.kernel.org, Justin Stitt <justinstitt@google.com> Subject: [PATCH] RISC-V: cpu: refactor deprecated strncpy Date: Tue, 01 Aug 2023 21:14:56 +0000 [thread overview] Message-ID: <20230801-arch-riscv-kernel-v1-1-2b3f2dc0bc61@google.com> (raw) `strncpy` is deprecated for use on NUL-terminated destination strings [1]. A suitable replacement is `strscpy` [2] due to the fact that it guarantees NUL-termination on its destination buffer argument which is _not_ the case for `strncpy`! The `sv_type` buffer is declared with a size of 16 which is then followed by some `strncpy` calls to populate the buffer with one of: "sv32", "sv57", "sv48", "sv39" or "none". Hard-coding the max length as 5 is error-prone and involves counting the number of characters (and hopefully not forgetting to count the NUL-byte) in the raw string. Using a pre-determined max length in combination with `strscpy` provides a cleaner, less error-prone as well as a less ambiguous implementation. `strscpy` guarantees that it's destination buffer is NUL-terminated even if it's source argument exceeds the max length as defined by the third argument. To be clear, there is no bug (i think?) in the current implementation but the current hard-coded values in combination with using a deprecated interface make this a worthwhile change, IMO. [1]: www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [2]: manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html Link: https://github.com/KSPP/linux/issues/90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Justin Stitt <justinstitt@google.com> --- arch/riscv/kernel/cpu.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c index a2fc952318e9..1c576e4ec171 100644 --- a/arch/riscv/kernel/cpu.c +++ b/arch/riscv/kernel/cpu.c @@ -17,6 +17,8 @@ #include <asm/smp.h> #include <asm/pgtable.h> +#define SV_TYPE_MAX_LENGTH 16 + /* * Returns the hart ID of the given device tree node, or -ENODEV if the node * isn't an enabled and valid RISC-V hart node. @@ -271,21 +273,21 @@ static void print_isa(struct seq_file *f, const char *isa) static void print_mmu(struct seq_file *f) { - char sv_type[16]; + char sv_type[SV_TYPE_MAX_LENGTH]; #ifdef CONFIG_MMU #if defined(CONFIG_32BIT) - strncpy(sv_type, "sv32", 5); + strscpy(sv_type, "sv32", SV_TYPE_MAX_LENGTH); #elif defined(CONFIG_64BIT) if (pgtable_l5_enabled) - strncpy(sv_type, "sv57", 5); + strscpy(sv_type, "sv57", SV_TYPE_MAX_LENGTH); else if (pgtable_l4_enabled) - strncpy(sv_type, "sv48", 5); + strscpy(sv_type, "sv48", SV_TYPE_MAX_LENGTH); else - strncpy(sv_type, "sv39", 5); + strscpy(sv_type, "sv39", SV_TYPE_MAX_LENGTH); #endif #else - strncpy(sv_type, "none", 5); + strscpy(sv_type, "none", SV_TYPE_MAX_LENGTH); #endif /* CONFIG_MMU */ seq_printf(f, "mmu\t\t: %s\n", sv_type); } --- base-commit: 5d0c230f1de8c7515b6567d9afba1f196fb4e2f4 change-id: 20230801-arch-riscv-kernel-14a048cc6467 Best regards, -- Justin Stitt <justinstitt@google.com>
WARNING: multiple messages have this Message-ID (diff)
From: Justin Stitt <justinstitt@google.com> To: Paul Walmsley <paul.walmsley@sifive.com>, Palmer Dabbelt <palmer@dabbelt.com>, Albert Ou <aou@eecs.berkeley.edu> Cc: linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org, Kees Cook <keescook@chromium.org>, Nick Desaulniers <ndesaulniers@google.com>, linux-hardening@vger.kernel.org, Justin Stitt <justinstitt@google.com> Subject: [PATCH] RISC-V: cpu: refactor deprecated strncpy Date: Tue, 01 Aug 2023 21:14:56 +0000 [thread overview] Message-ID: <20230801-arch-riscv-kernel-v1-1-2b3f2dc0bc61@google.com> (raw) `strncpy` is deprecated for use on NUL-terminated destination strings [1]. A suitable replacement is `strscpy` [2] due to the fact that it guarantees NUL-termination on its destination buffer argument which is _not_ the case for `strncpy`! The `sv_type` buffer is declared with a size of 16 which is then followed by some `strncpy` calls to populate the buffer with one of: "sv32", "sv57", "sv48", "sv39" or "none". Hard-coding the max length as 5 is error-prone and involves counting the number of characters (and hopefully not forgetting to count the NUL-byte) in the raw string. Using a pre-determined max length in combination with `strscpy` provides a cleaner, less error-prone as well as a less ambiguous implementation. `strscpy` guarantees that it's destination buffer is NUL-terminated even if it's source argument exceeds the max length as defined by the third argument. To be clear, there is no bug (i think?) in the current implementation but the current hard-coded values in combination with using a deprecated interface make this a worthwhile change, IMO. [1]: www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [2]: manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html Link: https://github.com/KSPP/linux/issues/90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Justin Stitt <justinstitt@google.com> --- arch/riscv/kernel/cpu.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c index a2fc952318e9..1c576e4ec171 100644 --- a/arch/riscv/kernel/cpu.c +++ b/arch/riscv/kernel/cpu.c @@ -17,6 +17,8 @@ #include <asm/smp.h> #include <asm/pgtable.h> +#define SV_TYPE_MAX_LENGTH 16 + /* * Returns the hart ID of the given device tree node, or -ENODEV if the node * isn't an enabled and valid RISC-V hart node. @@ -271,21 +273,21 @@ static void print_isa(struct seq_file *f, const char *isa) static void print_mmu(struct seq_file *f) { - char sv_type[16]; + char sv_type[SV_TYPE_MAX_LENGTH]; #ifdef CONFIG_MMU #if defined(CONFIG_32BIT) - strncpy(sv_type, "sv32", 5); + strscpy(sv_type, "sv32", SV_TYPE_MAX_LENGTH); #elif defined(CONFIG_64BIT) if (pgtable_l5_enabled) - strncpy(sv_type, "sv57", 5); + strscpy(sv_type, "sv57", SV_TYPE_MAX_LENGTH); else if (pgtable_l4_enabled) - strncpy(sv_type, "sv48", 5); + strscpy(sv_type, "sv48", SV_TYPE_MAX_LENGTH); else - strncpy(sv_type, "sv39", 5); + strscpy(sv_type, "sv39", SV_TYPE_MAX_LENGTH); #endif #else - strncpy(sv_type, "none", 5); + strscpy(sv_type, "none", SV_TYPE_MAX_LENGTH); #endif /* CONFIG_MMU */ seq_printf(f, "mmu\t\t: %s\n", sv_type); } --- base-commit: 5d0c230f1de8c7515b6567d9afba1f196fb4e2f4 change-id: 20230801-arch-riscv-kernel-14a048cc6467 Best regards, -- Justin Stitt <justinstitt@google.com> _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
next reply other threads:[~2023-08-01 21:15 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-08-01 21:14 Justin Stitt [this message] 2023-08-01 21:14 ` [PATCH] RISC-V: cpu: refactor deprecated strncpy Justin Stitt 2023-08-01 21:26 ` Conor Dooley 2023-08-01 21:26 ` Conor Dooley 2023-08-01 22:22 ` Justin Stitt 2023-08-01 22:22 ` Justin Stitt 2023-08-01 23:02 ` Jessica Clarke 2023-08-01 23:02 ` Jessica Clarke 2023-08-01 23:30 ` Kees Cook 2023-08-01 23:30 ` Kees Cook 2023-08-01 23:33 ` Kees Cook 2023-08-01 23:33 ` Kees Cook
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20230801-arch-riscv-kernel-v1-1-2b3f2dc0bc61@google.com \ --to=justinstitt@google.com \ --cc=aou@eecs.berkeley.edu \ --cc=keescook@chromium.org \ --cc=linux-hardening@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-riscv@lists.infradead.org \ --cc=ndesaulniers@google.com \ --cc=palmer@dabbelt.com \ --cc=paul.walmsley@sifive.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.