Messages in this thread | | | Date | Tue, 1 Aug 2023 16:30:48 -0700 | From | Kees Cook <> | Subject | Re: [PATCH] RISC-V: cpu: refactor deprecated strncpy |
| |
On Wed, Aug 02, 2023 at 12:02:11AM +0100, Jessica Clarke wrote: > On 1 Aug 2023, at 22:14, Justin Stitt <justinstitt@google.com> wrote: > > > > `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. > > I would imagine you’d want a BUG_ON() rather than silent truncation if > that ever happened (well, silent if you ignore it then printing the > truncated string). > > Though really you just want a static_strcpy that looks at sizeof* for > source and destination and fails to build if it doesn’t fit; there’s no > reason this needs to be found at run time.
FWIW, under CONFIG_FORTIFY_SOURCE, strscpy() does try to just fold away to a static strcpy when sizes are provably safe, etc.
-- Kees Cook
| |