From: Kees Cook <keescook@chromium.org>
To: Justin Stitt <justinstitt@google.com>
Cc: Tyler Hicks <code@tyhicks.com>,
ecryptfs@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-hardening@vger.kernel.org
Subject: Re: [PATCH] fs: ecryptfs: replace deprecated strncpy with strscpy
Date: Thu, 28 Mar 2024 21:08:58 -0700 [thread overview]
Message-ID: <202403282108.7FF7CE596C@keescook> (raw)
In-Reply-To: <20240321-strncpy-fs-ecryptfs-crypto-c-v1-1-d78b74c214ac@google.com>
On Thu, Mar 21, 2024 at 12:38:54AM +0000, Justin Stitt wrote:
> strncpy() is deprecated for use on NUL-terminated destination strings
> [1] and as such we should prefer more robust and less ambiguous string
> interfaces. A good alternative is strscpy() as it guarantees
> NUL-termination on the destination buffer.
>
> In crypto.c:
> We expect cipher_name to be NUL-terminated based on its use with
> the C-string format specifier %s and with other string apis like
> strlen():
> | printk(KERN_ERR "Error attempting to initialize key TFM "
> | "cipher with name = [%s]; rc = [%d]\n",
> | tmp_tfm->cipher_name, rc);
> and
> | int cipher_name_len = strlen(cipher_name);
>
> In main.c:
> We can remove the manual NUL-byte assignments as well as the pointers to
> destinations (which I assume only existed to trim down on line length?)
> in favor of directly using the destination buffer which allows the
> compiler to get size information -- enabling the usage of the new
> 2-argument strscpy().
>
> Note that this patch relies on the _new_ 2-argument versions of
> strscpy() and strscpy_pad() introduced in Commit e6584c3964f2f ("string:
> Allow 2-argument strscpy()").
>
> 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>
Looks correct. I don't see any need for padding.
Reviewed-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
next prev parent reply other threads:[~2024-03-29 4:09 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-21 0:38 [PATCH] fs: ecryptfs: replace deprecated strncpy with strscpy Justin Stitt
2024-03-29 4:08 ` Kees Cook [this message]
2024-04-24 23:57 ` 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=202403282108.7FF7CE596C@keescook \
--to=keescook@chromium.org \
--cc=code@tyhicks.com \
--cc=ecryptfs@vger.kernel.org \
--cc=justinstitt@google.com \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/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: link
Be 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.