From: Kees Cook <keescook@chromium.org>
To: "Martin K . Petersen" <martin.petersen@oracle.com>
Cc: Justin Stitt <justinstitt@google.com>,
Andy Shevchenko <andy@kernel.org>,
linux-hardening@vger.kernel.org,
Charles Bertsch <cbertsch@cox.net>,
Bart Van Assche <bvanassche@acm.org>,
Sathya Prakash <sathya.prakash@broadcom.com>,
Sreekanth Reddy <sreekanth.reddy@broadcom.com>,
Suganath Prabu Subramani <suganath-prabu.subramani@broadcom.com>,
"James E.J. Bottomley" <jejb@linux.ibm.com>,
Kashyap Desai <kashyap.desai@broadcom.com>,
Sumit Saxena <sumit.saxena@broadcom.com>,
Nilesh Javali <njavali@marvell.com>,
Andrew Morton <akpm@linux-foundation.org>,
Himanshu Madhani <himanshu.madhani@oracle.com>,
linux-kernel@vger.kernel.org, MPT-FusionLinux.pdl@broadcom.com,
linux-scsi@vger.kernel.org, mpi3mr-linuxdrv.pdl@broadcom.com,
GR-QLogic-Storage-Upstream@marvell.com
Subject: Re: [PATCH 1/5] string.h: Introduce memtostr() and memtostr_pad()
Date: Wed, 24 Apr 2024 08:59:33 -0700 [thread overview]
Message-ID: <202404240858.0FDD390@keescook> (raw)
In-Reply-To: <20240410023155.2100422-1-keescook@chromium.org>
On Tue, Apr 09, 2024 at 07:31:50PM -0700, Kees Cook wrote:
> Another ambiguous use of strncpy() is to copy from strings that may not
> be NUL-terminated. These cases depend on having the destination buffer
> be explicitly larger than the source buffer's maximum size, having
> the size of the copy exactly match the source buffer's maximum size,
> and for the destination buffer to get explicitly NUL terminated.
>
> This usually happens when parsing protocols or hardware character arrays
> that are not guaranteed to be NUL-terminated. The code pattern is
> effectively this:
>
> char dest[sizeof(src) + 1];
>
> strncpy(dest, src, sizeof(src));
> dest[sizeof(dest) - 1] = '\0';
>
> In practice it usually looks like:
>
> struct from_hardware {
> ...
> char name[HW_NAME_SIZE] __nonstring;
> ...
> };
>
> struct from_hardware *p = ...;
> char name[HW_NAME_SIZE + 1];
>
> strncpy(name, p->name, HW_NAME_SIZE);
> name[NW_NAME_SIZE] = '\0';
>
> This cannot be replaced with:
>
> strscpy(name, p->name, sizeof(name));
>
> because p->name is smaller and not NUL-terminated, so FORTIFY will
> trigger when strnlen(p->name, sizeof(name)) is used. And it cannot be
> replaced with:
>
> strscpy(name, p->name, sizeof(p->name));
>
> because then "name" may contain a 1 character early truncation of
> p->name.
>
> Provide an unambiguous interface for converting a maybe not-NUL-terminated
> string to a NUL-terminated string, with compile-time buffer size checking
> so that it can never fail at runtime: memtostr() and memtostr_pad(). Also
> add KUnit tests for both.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
FYI,
As the string KUnit tests have seen some refactoring, I'm taking this
patch and refactoring it onto my tree. Once the SCSI fixes are reviewed, if
we want to land them in -next, it's probably easiest for them to go via
my tree.
-Kees
--
Kees Cook
next prev parent reply other threads:[~2024-04-24 15:59 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-10 2:31 [PATCH 0/5] scsi: Avoid possible run-time warning with long manufacturer strings Kees Cook
2024-04-10 2:31 ` [PATCH 1/5] string.h: Introduce memtostr() and memtostr_pad() Kees Cook
2024-04-10 4:08 ` Andy Shevchenko
2024-04-10 18:33 ` Kees Cook
2024-04-24 15:59 ` Kees Cook [this message]
2024-04-25 1:42 ` Martin K. Petersen
2024-04-10 2:31 ` [PATCH 2/5] scsi: mptfusion: Avoid possible run-time warning with long manufacturer strings Kees Cook
2024-04-10 2:31 ` [PATCH 3/5] scsi: mpt3sas: " Kees Cook
2024-04-10 2:31 ` [PATCH 4/5] scsi: mpi3mr: " Kees Cook
2024-04-10 2:31 ` [PATCH 5/5] scsi: qla2xxx: Avoid possible run-time warning with long model_num Kees Cook
2024-04-17 17:35 ` [PATCH 0/5] scsi: Avoid possible run-time warning with long manufacturer strings Kees Cook
2024-04-18 0:35 ` Martin K. Petersen
2024-04-18 17:40 ` Kees Cook
2024-04-25 1:55 ` Martin K. Petersen
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=202404240858.0FDD390@keescook \
--to=keescook@chromium.org \
--cc=GR-QLogic-Storage-Upstream@marvell.com \
--cc=MPT-FusionLinux.pdl@broadcom.com \
--cc=akpm@linux-foundation.org \
--cc=andy@kernel.org \
--cc=bvanassche@acm.org \
--cc=cbertsch@cox.net \
--cc=himanshu.madhani@oracle.com \
--cc=jejb@linux.ibm.com \
--cc=justinstitt@google.com \
--cc=kashyap.desai@broadcom.com \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=mpi3mr-linuxdrv.pdl@broadcom.com \
--cc=njavali@marvell.com \
--cc=sathya.prakash@broadcom.com \
--cc=sreekanth.reddy@broadcom.com \
--cc=suganath-prabu.subramani@broadcom.com \
--cc=sumit.saxena@broadcom.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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).