Messages in this thread | | | Date | Thu, 19 Oct 2023 17:18:41 -0700 | From | Kees Cook <> | Subject | Re: [PATCH v2] nvdimm/btt: replace deprecated strncpy with strscpy |
| |
On Thu, Oct 19, 2023 at 05:54:15PM +0000, Justin Stitt wrote: > Found with grep. > > strncpy() is deprecated for use on NUL-terminated destination strings > [1] and as such we should prefer more robust and less ambiguous string > interfaces. > > We expect super->signature to be NUL-terminated based on its usage with > memcmp against a NUL-term'd buffer: > btt_devs.c: > 253 | if (memcmp(super->signature, BTT_SIG, BTT_SIG_LEN) != 0) > btt.h: > 13 | #define BTT_SIG "BTT_ARENA_INFO\0" > > NUL-padding is not required as `super` is already zero-allocated: > btt.c: > 985 | super = kzalloc(sizeof(struct btt_sb), GFP_NOIO); > ... rendering any additional NUL-padding superfluous. > > Considering the above, a suitable replacement is `strscpy` [2] due to > the fact that it guarantees NUL-termination on the destination buffer > without unnecessarily NUL-padding. > > Let's also use the more idiomatic strscpy usage of (dest, src, > sizeof(dest)) instead of (dest, src, XYZ_LEN) for buffers that the > compiler can determine the size of. This more tightly correlates the > destination buffer to the amount of bytes copied. > > Side note, this pattern of memcmp() on two NUL-terminated strings should > really be changed to just a strncmp(), if i'm not mistaken? I see > multiple instances of this pattern in this system: > > | if (memcmp(super->signature, BTT_SIG, BTT_SIG_LEN) != 0) > | return false; > > where BIT_SIG is defined (weirdly) as a double NUL-terminated string: > > | #define BTT_SIG "BTT_ARENA_INFO\0" > > Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] > Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] > Link: https://github.com/KSPP/linux/issues/90 > Cc: linux-hardening@vger.kernel.org > Signed-off-by: Justin Stitt <justinstitt@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
-- Kees Cook
| |